Subject: RE: [PATCH 1/2] Support for setitimer() on platforms lacking it

From: "Joachim Schmitz" <[email-masked]>
Date: Wed, 5 Sep 2012 11:59:37 +0200

> From: Junio C Hamano [mailto:[email-masked]]
> Sent: Wednesday, September 05, 2012 12:45 AM
> To: Joachim Schmitz
> Cc: [email-masked]; 'Johannes Sixt'
> Subject: Re: [PATCH 1/2] Support for setitimer() on platforms lacking it
> 
> "Joachim Schmitz" <[email-masked]> writes:
> 
> >> From: Junio C Hamano [mailto:[email-masked]]
> >> Sent: Tuesday, September 04, 2012 8:47 PM
> >> To: Joachim Schmitz
> >> Cc: [email-masked]; 'Johannes Sixt'
> >> Subject: Re: [PATCH 1/2] Support for setitimer() on platforms lacking it
> >>
> >> Junio C Hamano <[email-masked]> writes:
> >>
> >> > "Joachim Schmitz" <[email-masked]> writes:
> >> >
> >> >>> Only with the observation of "clone", I cannot tell if your timer is
> >> >>> working.  You can try repacking the test repository you created by
> >> >>> your earlier "git clone" with "git repack -a -d -f" and see what
> >> >>> happens.
> >> >>
> >> >> It does update the counter too.
> >> >
> >> > Yeah, that was not a very good way to diagnose it.
> >> >
> >> > You see the progress from pack-objects (which is the underlying
> >> > machinery "git repack" uses) only because it knows how many objects
> >> > it is going to pack, and it updates the progress meter for every
> >> > per-cent progress it makes, without any help from the timer
> >> > interrupt.
> >>
> >> I think the "Counting objects: $number" phase is purely driven by
> >> the timer, as there is no way to say "we are done X per-cent so
> >> far".
> >>
> >> Doesn't your repack show "Counting objects: " with a number once,
> >> pause forever and then show "Counting objects: $number, done."?
> >
> > Yes, only once, when it is done
> > $ ./git repack -a -d -f
> > warning: no threads support, ignoring --threads
> > Counting objects: 140302, done.
> > Compressing objects:   1% (1385/138407)
> 
> So this strongly suggests that (1) your "poor-man's" is not a real
> substitute for recurring itimer, and (2) users could live with the
> progress.c code without any itimer firing.

OK

> Perhaps a no-op macro would work equally well?

Like the following:

diff --git a/git-compat-util.h b/git-compat-util.h
index 18089f0..55b9421 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h@@ -163,6 +163,10 @@
 #define probe_utf8_pathname_composition(a,b)
 #endif

+#ifdef NO_SETITIMER
+#define setitimer(w,v,o) /* NOP */
+#endif
+
 #ifdef MKDIR_WO_TRAILING_SLASH
 #define mkdir(a,b) compat_mkdir_wo_trailing_slash((a),(b))
 extern int compat_mkdir_wo_trailing_slash(const char*, mode_t);

Does work for me and does not seem to make any difference, not in those test cases at least

Does the inability to re-arm the timer depend on SA_RESTART, possibly?
If so we may instead want 
#if SA_RSTART == 0  && defined(NO_SETITIMER)

Bye, Jojo