{"thread":{"id":"33234","subject":"USE_NSEC bug?","startedAt":"2013-03-20T06:36:32Z","lastAt":"2013-03-20T19:00:53Z","messageCount":9,"participants":["Andrew Rodland","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"211724","messageId":"CABFQKmP6=Thhph4Ug+4LOBmzsVK42qBQYW2QHw87QD3019n+Kw@mail.gmail.com","threadId":"33234","inReplyTo":null,"subject":"USE_NSEC bug?","fromName":"Andrew Rodland","fromEmail":"andrew@cleverdomain.org","sentAt":"2013-03-20T06:36:32Z","receivedAt":"2013-03-20T06:36:32Z","isPatch":false,"sender":{"key":"andrew@cleverdomain.org","avatar":null},"body":"While investigating this StackOverflow question:\nhttp://stackoverflow.com/questions/15516168/how-to-cross-compile-git-for-arm\nI found that fetch-pack.c uses ST_MTIME_NSEC outside of the protection\nof #ifdef USE_NSEC. This results in a broken build if\n!defined(USE_NSEC) && !defined(NO_NSEC) and the target system doesn't\nhappen to be recent glibc.\n\nI'm not sure where to pin the bug exactly — on fetch-pack.c, on\ngit-compat-util.h, on configure for not seeing if one of those fields\nactually exists and setting NO_NSEC otherwise, or elsewhere, but I\nwould be tempted to fix it in one of the two below ways, which should\nalways be foolproof when !defined(USE_NSEC). That seems right to me,\nsince USE_NSEC is an experimental-ish feature.\n\nPlease Cc any replies as I'm not subscribed.\n\nWay one (smaller diff for an uglier result):\n\n--\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 90e0372..222caaa 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -638,10 +638,13 @@ void git_qsort(void *base, size_t nmemb, size_t size,\n # define FORCE_DIR_SET_GID 0\n #endif\n\n #ifdef NO_NSEC\n #undef USE_NSEC\n+#endif\n+\n+#ifndef USE_NSEC\n #define ST_CTIME_NSEC(st) 0\n #define ST_MTIME_NSEC(st) 0\n #else\n #ifdef USE_ST_TIMESPEC\n #define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctimespec.tv_nsec))\n\n--\n\nWay two (bigger diff for a prettier result):\n\ndiff --git a/git-compat-util.h b/git-compat-util.h\nindex 90e0372..0a15b1a 100644\n--- a/git-compat-util.h\n+++ b/git-compat-util.h\n@@ -638,20 +638,23 @@ void git_qsort(void *base, size_t nmemb, size_t size,\n # define FORCE_DIR_SET_GID 0\n #endif\n\n #ifdef NO_NSEC\n #undef USE_NSEC\n-#define ST_CTIME_NSEC(st) 0\n-#define ST_MTIME_NSEC(st) 0\n-#else\n-#ifdef USE_ST_TIMESPEC\n-#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctimespec.tv_nsec))\n-#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtimespec.tv_nsec))\n-#else\n-#define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctim.tv_nsec))\n-#define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtim.tv_nsec))\n #endif\n+\n+#ifdef USE_NSEC\n+# ifdef USE_ST_TIMESPEC\n+#  define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctimespec.tv_nsec))\n+#  define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtimespec.tv_nsec))\n+# else\n+#  define ST_CTIME_NSEC(st) ((unsigned int)((st).st_ctim.tv_nsec))\n+#  define ST_MTIME_NSEC(st) ((unsigned int)((st).st_mtim.tv_nsec))\n+# endif\n+#else /* !USE_NSEC */\n+# define ST_CTIME_NSEC(st) 0\n+# define ST_MTIME_NSEC(st) 0\n #endif\n\n #ifdef UNRELIABLE_FSTAT\n #define fstat_is_reliable() 0\n #else\n"},{"id":"211725","messageId":"20130320075344.GA8159@sigill.intra.peff.net","threadId":"33234","inReplyTo":"CABFQKmP6=Thhph4Ug+4LOBmzsVK42qBQYW2QHw87QD3019n+Kw@mail.gmail.com","subject":"Re: USE_NSEC bug?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-20T07:53:44Z","receivedAt":"2013-03-20T07:53:44Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 20, 2013 at 02:36:32AM -0400, Andrew Rodland wrote:\n\n> While investigating this StackOverflow question:\n> http://stackoverflow.com/questions/15516168/how-to-cross-compile-git-for-arm\n> I found that fetch-pack.c uses ST_MTIME_NSEC outside of the protection\n> of #ifdef USE_NSEC. This results in a broken build if\n> !defined(USE_NSEC) && !defined(NO_NSEC) and the target system doesn't\n> happen to be recent glibc.\n\nRight; the point of NO_NSEC is to tell git that your libc does not have\nthose fields. If it's not set, then it is a bug in your config.mak (or\nin the autoconf script, if you are using it).\n\nThat being said, I really don't see the point of having both USE_NSEC\nand NO_NSEC. If you do not set USE_NSEC (which most people do not; it is\noff by default), what is the point of not setting NO_NSEC? As far as I\ncan tell, it means we will copy stat information out \"struct st\" into\nthe index, but we will not actually _use_ it for anything.\n\nShould there just be a single option USE_NSEC that defaults to off? And\nif it is not set, we do not even bother caring whether libc provides\nnsec fields, since we are not going to use them for anything (i.e., we\njust do the equivalent of NO_NSEC now).\n\nWhich I think is the direction you are proposing with your patches\nbelow, but the rationale should be \"why bother requiring users to use\nthe NO_NSEC knob explicitly when we know we do not care about its value\nanyway\".\n\nBut maybe there is some subtle reason I'm missing for having the two\noptions separate. Or maybe it's just a historical accident that we ended\nup here.\n\n-Peff\n"},{"id":"211764","messageId":"7vppyuf1f5.fsf@alter.siamese.dyndns.org","threadId":"33234","inReplyTo":"20130320075344.GA8159@sigill.intra.peff.net","subject":"Re: USE_NSEC bug?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-20T17:04:14Z","receivedAt":"2013-03-20T17:04:14Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> But maybe there is some subtle reason I'm missing for having the two\n> options separate.\n\nThe closest I found was c06ff4908bf9 (Record ns-timestamps if\npossible, but do not use it without USE_NSEC, 2009-03-04).\n\ncommit c06ff4908bf9ad8bf2448439a3574321c9399b17\nAuthor: Kjetil Barvik <barvik@broadpark.no>\nDate:   Wed Mar 4 18:47:40 2009 +0100\n\n    Record ns-timestamps if possible, but do not use it without USE_NSEC\n    \n    Traditionally, the lack of USE_NSEC meant \"do not record nor use the\n    nanosecond resolution part of the file timestamps\".  To avoid problems on\n    filesystems that lose the ns part when the metadata is flushed to the disk\n    and then later read back in, disabling USE_NSEC has been a good idea in\n    general.\n    \n    If you are on a filesystem without such an issue, it does not hurt to read\n    and store them in the cached stat data in the index entries even if your\n    git is compiled without USE_NSEC.  The index left with such a version of\n    git can be read by git compiled with USE_NSEC and it can make use of the\n    nanosecond part to optimize the check to see if the path on the filesystem\n    hsa been modified since we last looked at.\n"},{"id":"211766","messageId":"20130320170957.GA21350@sigill.intra.peff.net","threadId":"33234","inReplyTo":"7vppyuf1f5.fsf@alter.siamese.dyndns.org","subject":"Re: USE_NSEC bug?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-20T17:09:57Z","receivedAt":"2013-03-20T17:09:57Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 20, 2013 at 10:04:14AM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > But maybe there is some subtle reason I'm missing for having the two\n> > options separate.\n> \n> The closest I found was c06ff4908bf9 (Record ns-timestamps if\n> possible, but do not use it without USE_NSEC, 2009-03-04).\n> \n> commit c06ff4908bf9ad8bf2448439a3574321c9399b17\n> Author: Kjetil Barvik <barvik@broadpark.no>\n> Date:   Wed Mar 4 18:47:40 2009 +0100\n> \n>     Record ns-timestamps if possible, but do not use it without USE_NSEC\n>     \n>     Traditionally, the lack of USE_NSEC meant \"do not record nor use the\n>     nanosecond resolution part of the file timestamps\".  To avoid problems on\n>     filesystems that lose the ns part when the metadata is flushed to the disk\n>     and then later read back in, disabling USE_NSEC has been a good idea in\n>     general.\n>     \n>     If you are on a filesystem without such an issue, it does not hurt to read\n>     and store them in the cached stat data in the index entries even if your\n>     git is compiled without USE_NSEC.  The index left with such a version of\n>     git can be read by git compiled with USE_NSEC and it can make use of the\n>     nanosecond part to optimize the check to see if the path on the filesystem\n>     hsa been modified since we last looked at.\n\nThanks, I suspected that might be the reason. IMHO it is not a big win,\nas you would just refresh the index when using the version compiled with\nUSE_NSEC (so it really only helps you if you are frequently switching\nbetween the two versions, which seems silly).\n\nAnd the cost is that we have another Makefile knob people need to tweak\nthat would not otherwise need to be there. Which can be annoying, but is\nalso not that huge a cost to deal with (we might want to improve the\nconfigure script or something, though).\n\nI admit I don't care too much either way.\n\n-Peff\n"},{"id":"211772","messageId":"7v620mf05l.fsf@alter.siamese.dyndns.org","threadId":"33234","inReplyTo":"20130320170957.GA21350@sigill.intra.peff.net","subject":"Re: USE_NSEC bug?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-20T17:31:34Z","receivedAt":"2013-03-20T17:31:34Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> And the cost is that we have another Makefile knob people need to tweak\n> that would not otherwise need to be there. Which can be annoying, but is\n> also not that huge a cost to deal with (we might want to improve the\n> configure script or something, though).\n>\n> I admit I don't care too much either way.\n\nI don't, either ;-)\n"},{"id":"211784","messageId":"CABFQKmONh2dQ+dkt3MsSXtFyKh5mnfK1fQVyw6Ez6aWajo=BDg@mail.gmail.com","threadId":"33234","inReplyTo":"7v620mf05l.fsf@alter.siamese.dyndns.org","subject":"Re: USE_NSEC bug?","fromName":"Andrew Rodland","fromEmail":"andrew@cleverdomain.org","sentAt":"2013-03-20T18:22:27Z","receivedAt":"2013-03-20T18:22:27Z","isPatch":false,"sender":{"key":"andrew@cleverdomain.org","avatar":null},"body":"On Wed, Mar 20, 2013 at 1:31 PM, Junio C Hamano <gitster@pobox.com> wrote:\n> Jeff King <peff@peff.net> writes:\n>\n>> And the cost is that we have another Makefile knob people need to tweak\n>> that would not otherwise need to be there. Which can be annoying, but is\n>> also not that huge a cost to deal with (we might want to improve the\n>> configure script or something, though).\n>>\n>> I admit I don't care too much either way.\n>\n> I don't, either ;-)\n>\n\nFor what it's worth, I'm not really invested either; I just figured\nthat raising the issue was better than not, since I bumped into it.\n\nI think it would be good if NO_NSEC and USE_ST_TIMESPEC were\ncontrolled by configure instead of config.mak, and it doesn't seem\nlike too tall of an order.\n\nAndrew\n"},{"id":"211787","messageId":"20130320183049.GB29609@sigill.intra.peff.net","threadId":"33234","inReplyTo":"CABFQKmONh2dQ+dkt3MsSXtFyKh5mnfK1fQVyw6Ez6aWajo=BDg@mail.gmail.com","subject":"Re: USE_NSEC bug?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-20T18:30:50Z","receivedAt":"2013-03-20T18:30:50Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 20, 2013 at 02:22:27PM -0400, Andrew Rodland wrote:\n\n> I think it would be good if NO_NSEC and USE_ST_TIMESPEC were\n> controlled by configure instead of config.mak, and it doesn't seem\n> like too tall of an order.\n\nThere is no \"instead of\" here. The Makefile provides knobs which you can\ntweak on the command-line or via config.mak, and the configure script is\nexpected to set any of those knobs it wants. But as most of the regular\ndevelopers do not use the configure script themselves, it is often\nmissing support for many of the knobs.\n\nSo I think your real complaint is \"configure does not set NO_NSEC\nproperly\", which is something worth fixing.\n\n-Peff\n"},{"id":"211799","messageId":"7vobedewer.fsf@alter.siamese.dyndns.org","threadId":"33234","inReplyTo":"20130320183049.GB29609@sigill.intra.peff.net","subject":"Re: USE_NSEC bug?","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-03-20T18:52:28Z","receivedAt":"2013-03-20T18:52:28Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> So I think your real complaint is \"configure does not set NO_NSEC\n> properly\", which is something worth fixing.\n\nYes.\n\nI think our \"Meh\" was about \"do we want to unify USE_NSEC and\nNO_NSEC?\" and nothing else.\n"},{"id":"211803","messageId":"20130320190053.GC30165@sigill.intra.peff.net","threadId":"33234","inReplyTo":"7vobedewer.fsf@alter.siamese.dyndns.org","subject":"Re: USE_NSEC bug?","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-03-20T19:00:53Z","receivedAt":"2013-03-20T19:00:53Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Mar 20, 2013 at 11:52:28AM -0700, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > So I think your real complaint is \"configure does not set NO_NSEC\n> > properly\", which is something worth fixing.\n> \n> Yes.\n> \n> I think our \"Meh\" was about \"do we want to unify USE_NSEC and\n> NO_NSEC?\" and nothing else.\n\nExactly. Now I am hoping Andrew will volunteer to fix configure.ac, as I\nwill be happy to go to my grave never having had to figure out autoconf\nm4 syntax ever again. :)\n\n-Peff\n"}]}