{"thread":{"id":"47432","subject":"[PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak","startedAt":"2017-12-12T14:06:10Z","lastAt":"2017-12-12T23:21:43Z","messageCount":8,"participants":["Ramsay Jones","Junio C Hamano","Ævar Arnfjörð Bjarmason","Jonathan Tan"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"334676","messageId":"7272c221-c1b6-ca41-3be4-329275e94b8a@ramsayjones.plus.com","threadId":"47432","inReplyTo":null,"subject":"[PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak","fromName":"Ramsay Jones","fromEmail":"ramsay@ramsayjones.plus.com","sentAt":"2017-12-12T12:47:25Z","receivedAt":"2017-12-12T14:06:10Z","isPatch":true,"sender":{"key":"ramsay@ramsayjones.plus.com","avatar":"https://avatars.githubusercontent.com/u/33702710?v=4"},"body":"\nSigned-off-by: Ramsay Jones \n---\n\nHi Junio,\n\nCould you please add (or squash) this on top of the 'ab/sha1dc-build'\nbranch, so that I can build with NO_DC_SHA1_SUBMODULE=NoThanks in my\nconfig.mak.\n\n[If I were to get a vote, I would vote no to the submodule. ;-) ]\n\nThanks!\n\nATB,\nRamsay Jones\n\n Makefile | 8 ++++----\n 1 file changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 929b49b04..91bbb0ed8 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1042,6 +1042,10 @@ EXTLIBS =\n \n GIT_USER_AGENT = git/$(GIT_VERSION)\n \n+include config.mak.uname\n+-include config.mak.autogen\n+-include config.mak\n+\n ifndef NO_DC_SHA1_SUBMODULE\n \tifndef DC_SHA1_EXTERNAL\n \t\tifneq ($(wildcard sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h)\n@@ -1053,10 +1057,6 @@ whatever reason define NO_DC_SHA1_SUBMODULE=NoThanks)\n \tendif\n endif\n \n-include config.mak.uname\n--include config.mak.autogen\n--include config.mak\n-\n ifdef DEVELOPER\n CFLAGS += $(DEVELOPER_CFLAGS)\n endif\n-- \n2.15.0\n"},{"id":"334698","messageId":"xmqqk1xr204t.fsf@gitster.mtv.corp.google.com","threadId":"47432","inReplyTo":"7272c221-c1b6-ca41-3be4-329275e94b8a@ramsayjones.plus.com","subject":"Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-12T19:53:54Z","receivedAt":"2017-12-12T19:54:01Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ramsay Jones writes:\n\n> Signed-off-by: Ramsay Jones \n> ---\n>\n> Hi Junio,\n>\n> Could you please add (or squash) this on top of the 'ab/sha1dc-build'\n> branch, so that I can build with NO_DC_SHA1_SUBMODULE=NoThanks in my\n> config.mak.\n\nMakes sense. The patch looks scary by appearing to move the\nincludes far to the front of the Makefile, but it in fact is moving\nthe NO_DC_SHA1_SUBMODULE block slightly down and it is a sensible\nand safe move.\n\nI actually think that the block can go even further down, perhaps\nclose to the run of choices \"what variant are we building?\" we make\nat around we have \"ifdef NO_CURL\".\n\nÆvar?\n\n> diff --git a/Makefile b/Makefile\n> index 929b49b04..91bbb0ed8 100644\n> --- a/Makefile\n> +++ b/Makefile\n> @@ -1042,6 +1042,10 @@ EXTLIBS =\n> \n> GIT_USER_AGENT = git/$(GIT_VERSION)\n> \n> +include config.mak.uname\n> +-include config.mak.autogen\n> +-include config.mak\n> +\n> ifndef NO_DC_SHA1_SUBMODULE\n> \tifndef DC_SHA1_EXTERNAL\n> \t\tifneq ($(wildcard sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h)\n> @@ -1053,10 +1057,6 @@ whatever reason define NO_DC_SHA1_SUBMODULE=NoThanks)\n> \tendif\n> endif\n> \n> -include config.mak.uname\n> --include config.mak.autogen\n> --include config.mak\n> -\n> ifdef DEVELOPER\n> CFLAGS += $(DEVELOPER_CFLAGS)\n> endif\n"},{"id":"334703","messageId":"xmqqfu8f1zyv.fsf@gitster.mtv.corp.google.com","threadId":"47432","inReplyTo":"xmqqk1xr204t.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-12T19:57:28Z","receivedAt":"2017-12-12T19:57:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Makes sense. The patch looks scary by appearing to move the\n> includes far to the front of the Makefile, but it in fact is moving\n> the NO_DC_SHA1_SUBMODULE block slightly down and it is a sensible\n> and safe move.\n\nA completely unrelated tangent. This was a good change to try your\nanchored diff on. Viewing this change with\n\n $ git show --anchored='include config.mak'\n\nlooks a lot less scary than the way it is shown by default (which\nmatches what was posted on the list).\n\nGood job.\n"},{"id":"334714","messageId":"CACBZZX4DFXYNZyU-QPrun2g4zTpt8u6yE6DsEGsQvOEhUNRd8Q@mail.gmail.com","threadId":"47432","inReplyTo":"xmqqk1xr204t.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-12-12T21:06:41Z","receivedAt":"2017-12-12T21:07:07Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Tue, Dec 12, 2017 at 8:53 PM, Junio C Hamano wrote:\n> Ramsay Jones writes:\n>\n>> Signed-off-by: Ramsay Jones \n>> ---\n>>\n>> Hi Junio,\n>>\n>> Could you please add (or squash) this on top of the 'ab/sha1dc-build'\n>> branch, so that I can build with NO_DC_SHA1_SUBMODULE=NoThanks in my\n>> config.mak.\n>\n> Makes sense. The patch looks scary by appearing to move the\n> includes far to the front of the Makefile, but it in fact is moving\n> the NO_DC_SHA1_SUBMODULE block slightly down and it is a sensible\n> and safe move.\n>\n> I actually think that the block can go even further down, perhaps\n> close to the run of choices \"what variant are we building?\" we make\n> at around we have \"ifdef NO_CURL\".\n>\n> Ævar?\n\nMakes sense to me, do you want to squash this + your proposed edit &\nI'll pick it up if there's another version, or I can re-submit.\n\n>> diff --git a/Makefile b/Makefile\n>> index 929b49b04..91bbb0ed8 100644\n>> --- a/Makefile\n>> +++ b/Makefile\n>> @@ -1042,6 +1042,10 @@ EXTLIBS =\n>>\n>> GIT_USER_AGENT = git/$(GIT_VERSION)\n>>\n>> +include config.mak.uname\n>> +-include config.mak.autogen\n>> +-include config.mak\n>> +\n>> ifndef NO_DC_SHA1_SUBMODULE\n>> ifndef DC_SHA1_EXTERNAL\n>> ifneq ($(wildcard sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h)\n>> @@ -1053,10 +1057,6 @@ whatever reason define NO_DC_SHA1_SUBMODULE=NoThanks)\n>> endif\n>> endif\n>>\n>> -include config.mak.uname\n>> --include config.mak.autogen\n>> --include config.mak\n>> -\n>> ifdef DEVELOPER\n>> CFLAGS += $(DEVELOPER_CFLAGS)\n>> endif\n"},{"id":"334715","messageId":"20171212131158.ec87dcde2a74dd879068cd95@google.com","threadId":"47432","inReplyTo":"xmqqfu8f1zyv.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-12-12T21:11:58Z","receivedAt":"2017-12-12T21:12:06Z","isPatch":true,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"On Tue, 12 Dec 2017 11:57:28 -0800\nJunio C Hamano wrote:\n\n> Junio C Hamano writes:\n> \n> > Makes sense. The patch looks scary by appearing to move the\n> > includes far to the front of the Makefile, but it in fact is moving\n> > the NO_DC_SHA1_SUBMODULE block slightly down and it is a sensible\n> > and safe move.\n> \n> A completely unrelated tangent. This was a good change to try your\n> anchored diff on. Viewing this change with\n> \n> $ git show --anchored='include config.mak'\n> \n> looks a lot less scary than the way it is shown by default (which\n> matches what was posted on the list).\n> \n> Good job.\n\nThanks, glad that it helped.\n"},{"id":"334720","messageId":"xmqq609bziz5.fsf@gitster.mtv.corp.google.com","threadId":"47432","inReplyTo":"CACBZZX4DFXYNZyU-QPrun2g4zTpt8u6yE6DsEGsQvOEhUNRd8Q@mail.gmail.com","subject":"Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-12T22:20:30Z","receivedAt":"2017-12-12T22:20:37Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason writes:\n\n>> I actually think that the block can go even further down, perhaps\n>> close to the run of choices \"what variant are we building?\" we make\n>> at around we have \"ifdef NO_CURL\".\n>>\n>> Ævar?\n>\n> Makes sense to me, do you want to squash this + your proposed edit &\n> I'll pick it up if there's another version, or I can re-submit.\n\nOK. I'll squash in the 'move it further down' to the original\ncommit that removed DC_SHA1_SUBMODULE and added NO_DC_SHA1_SUBMODULE\nwhen rebuilding 'pu' branch.\n\nThanks.\n"},{"id":"334722","messageId":"xmqqy3m7y2mz.fsf@gitster.mtv.corp.google.com","threadId":"47432","inReplyTo":"xmqq609bziz5.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2017-12-12T22:58:44Z","receivedAt":"2017-12-12T22:58:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Ævar Arnfjörð Bjarmason writes:\n>\n>>> I actually think that the block can go even further down, perhaps\n>>> close to the run of choices \"what variant are we building?\" we make\n>>> at around we have \"ifdef NO_CURL\".\n>>>\n>>> Ævar?\n>>\n>> Makes sense to me, do you want to squash this + your proposed edit &\n>> I'll pick it up if there's another version, or I can re-submit.\n>\n> OK. I'll squash in the 'move it further down' to the original\n> commit that removed DC_SHA1_SUBMODULE and added NO_DC_SHA1_SUBMODULE\n> when rebuilding 'pu' branch.\n>\n> Thanks.\n\nAnother minor thing I noticed (which I do not have any squashable\nfix for) is that \"make distclean\" does not even work without\nsubmodule or this environment, which feels a bit too excessive. I\nhaven't tried to figure out how involved a fix for that would be yet\nand I do not mind leaving it broken if it would be too much work.\n\n"},{"id":"334725","messageId":"877etrcz28.fsf@evledraar.gmail.com","threadId":"47432","inReplyTo":"xmqqy3m7y2mz.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2017-12-12T23:21:35Z","receivedAt":"2017-12-12T23:21:43Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Dec 12 2017, Junio C. Hamano jotted:\n\n> Junio C Hamano writes:\n>\n>> Ævar Arnfjörð Bjarmason writes:\n>>\n>>>> I actually think that the block can go even further down, perhaps\n>>>> close to the run of choices \"what variant are we building?\" we make\n>>>> at around we have \"ifdef NO_CURL\".\n>>>>\n>>>> Ævar?\n>>>\n>>> Makes sense to me, do you want to squash this + your proposed edit &\n>>> I'll pick it up if there's another version, or I can re-submit.\n>>\n>> OK. I'll squash in the 'move it further down' to the original\n>> commit that removed DC_SHA1_SUBMODULE and added NO_DC_SHA1_SUBMODULE\n>> when rebuilding 'pu' branch.\n>>\n>> Thanks.\n>\n> Another minor thing I noticed (which I do not have any squashable\n> fix for) is that \"make distclean\" does not even work without\n> submodule or this environment, which feels a bit too excessive. I\n> haven't tried to figure out how involved a fix for that would be yet\n> and I do not mind leaving it broken if it would be too much work.\n\nYes, that sucks. I can't think of a better fix for it than this massive\nhack of outsourcing printing the error to the header itself, which'll\nonly be executed if we actually compile:\n\n diff --git a/Makefile b/Makefile\n index ba3e061edd..881cf55159 100644\n --- a/Makefile\n +++ b/Makefile\n @@ -1022,9 +1022,7 @@ GIT_USER_AGENT = git/$(GIT_VERSION)\n\n ifndef DC_SHA1_EXTERNAL\n ifneq ($(wildcard sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h)\n -$(error The sha1collisiondetection submodule is not checked out. \\\n -Please make it available, either by cloning with --recurse-submodules, \\\n -or by running \"git submodule update --init\".)\n + BASIC_CFLAGS += -DERROR_SHA1COLLISIONDETECTION_SUBMODULE\n endif\n else\n ifdef NO_DC_SHA1_SUBMODULE\n diff --git a/sha1dc_git.h b/sha1dc_git.h\n index be1d48abbe..93a9be976a 100644\n --- a/sha1dc_git.h\n +++ b/sha1dc_git.h\n @@ -1,5 +1,11 @@\n /* Plumbing with collition-detecting SHA1 code */\n\n +#ifdef ERROR_SHA1COLLISIONDETECTION_SUBMODULE\n +#error \"The sha1collisiondetection submodule is not checked out.\" \\\n + \"Please make it available, either by cloning with --recurse-submodules,\" \\\n + \"or by running \\\"git submodule update --init\\\".\"\n +#endif\n +\n #ifdef DC_SHA1_EXTERNAL\n #include \n #else\n\nBut maybe I missed a way to make the $(error) path work only if certain\ntargets were about to be compiled.\n"}]}