# [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak 8 messages from 2017-12-12 to 2017-12-12. Participants: Ramsay Jones, Junio C Hamano, Ævar Arnfjörð Bjarmason, Jonathan Tan. Thread: https://gitlist.dev/t/47432 ## Ramsay Jones, 2017-12-12 12:47 Subject: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak Message-ID: <7272c221-c1b6-ca41-3be4-329275e94b8a@ramsayjones.plus.com> URL: https://gitlist.dev/e/7272c221-c1b6-ca41-3be4-329275e94b8a%40ramsayjones.plus.com ``` Signed-off-by: Ramsay Jones --- Hi Junio, Could you please add (or squash) this on top of the 'ab/sha1dc-build' branch, so that I can build with NO_DC_SHA1_SUBMODULE=NoThanks in my config.mak. [If I were to get a vote, I would vote no to the submodule. ;-) ] Thanks! ATB, Ramsay Jones Makefile | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 929b49b04..91bbb0ed8 100644 --- a/Makefile +++ b/Makefile @@ -1042,6 +1042,10 @@ EXTLIBS = GIT_USER_AGENT = git/$(GIT_VERSION) +include config.mak.uname +-include config.mak.autogen +-include config.mak + ifndef NO_DC_SHA1_SUBMODULE ifndef DC_SHA1_EXTERNAL ifneq ($(wildcard sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h) @@ -1053,10 +1057,6 @@ whatever reason define NO_DC_SHA1_SUBMODULE=NoThanks) endif endif -include config.mak.uname --include config.mak.autogen --include config.mak - ifdef DEVELOPER CFLAGS += $(DEVELOPER_CFLAGS) endif -- 2.15.0 ``` ## Junio C Hamano, 2017-12-12 19:53 Subject: Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak Message-ID: URL: https://gitlist.dev/e/xmqqk1xr204t.fsf%40gitster.mtv.corp.google.com In-Reply-To: <7272c221-c1b6-ca41-3be4-329275e94b8a@ramsayjones.plus.com> ``` Ramsay Jones writes: > Signed-off-by: Ramsay Jones > --- > > Hi Junio, > > Could you please add (or squash) this on top of the 'ab/sha1dc-build' > branch, so that I can build with NO_DC_SHA1_SUBMODULE=NoThanks in my > config.mak. Makes sense. The patch looks scary by appearing to move the includes far to the front of the Makefile, but it in fact is moving the NO_DC_SHA1_SUBMODULE block slightly down and it is a sensible and safe move. I actually think that the block can go even further down, perhaps close to the run of choices "what variant are we building?" we make at around we have "ifdef NO_CURL". Ævar? > diff --git a/Makefile b/Makefile > index 929b49b04..91bbb0ed8 100644 > --- a/Makefile > +++ b/Makefile > @@ -1042,6 +1042,10 @@ EXTLIBS = > > GIT_USER_AGENT = git/$(GIT_VERSION) > > +include config.mak.uname > +-include config.mak.autogen > +-include config.mak > + > ifndef NO_DC_SHA1_SUBMODULE > ifndef DC_SHA1_EXTERNAL > ifneq ($(wildcard sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h) > @@ -1053,10 +1057,6 @@ whatever reason define NO_DC_SHA1_SUBMODULE=NoThanks) > endif > endif > > -include config.mak.uname > --include config.mak.autogen > --include config.mak > - > ifdef DEVELOPER > CFLAGS += $(DEVELOPER_CFLAGS) > endif ``` ## Junio C Hamano, 2017-12-12 19:57 Subject: Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak Message-ID: URL: https://gitlist.dev/e/xmqqfu8f1zyv.fsf%40gitster.mtv.corp.google.com In-Reply-To: ``` Junio C Hamano writes: > Makes sense. The patch looks scary by appearing to move the > includes far to the front of the Makefile, but it in fact is moving > the NO_DC_SHA1_SUBMODULE block slightly down and it is a sensible > and safe move. A completely unrelated tangent. This was a good change to try your anchored diff on. Viewing this change with $ git show --anchored='include config.mak' looks a lot less scary than the way it is shown by default (which matches what was posted on the list). Good job. ``` ## Ævar Arnfjörð Bjarmason, 2017-12-12 21:06 Subject: Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak Message-ID: URL: https://gitlist.dev/e/CACBZZX4DFXYNZyU-QPrun2g4zTpt8u6yE6DsEGsQvOEhUNRd8Q%40mail.gmail.com In-Reply-To: ``` On Tue, Dec 12, 2017 at 8:53 PM, Junio C Hamano wrote: > Ramsay Jones writes: > >> Signed-off-by: Ramsay Jones >> --- >> >> Hi Junio, >> >> Could you please add (or squash) this on top of the 'ab/sha1dc-build' >> branch, so that I can build with NO_DC_SHA1_SUBMODULE=NoThanks in my >> config.mak. > > Makes sense. The patch looks scary by appearing to move the > includes far to the front of the Makefile, but it in fact is moving > the NO_DC_SHA1_SUBMODULE block slightly down and it is a sensible > and safe move. > > I actually think that the block can go even further down, perhaps > close to the run of choices "what variant are we building?" we make > at around we have "ifdef NO_CURL". > > Ævar? Makes sense to me, do you want to squash this + your proposed edit & I'll pick it up if there's another version, or I can re-submit. >> diff --git a/Makefile b/Makefile >> index 929b49b04..91bbb0ed8 100644 >> --- a/Makefile >> +++ b/Makefile >> @@ -1042,6 +1042,10 @@ EXTLIBS = >> >> GIT_USER_AGENT = git/$(GIT_VERSION) >> >> +include config.mak.uname >> +-include config.mak.autogen >> +-include config.mak >> + >> ifndef NO_DC_SHA1_SUBMODULE >> ifndef DC_SHA1_EXTERNAL >> ifneq ($(wildcard sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h) >> @@ -1053,10 +1057,6 @@ whatever reason define NO_DC_SHA1_SUBMODULE=NoThanks) >> endif >> endif >> >> -include config.mak.uname >> --include config.mak.autogen >> --include config.mak >> - >> ifdef DEVELOPER >> CFLAGS += $(DEVELOPER_CFLAGS) >> endif ``` ## Jonathan Tan, 2017-12-12 21:11 Subject: Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak Message-ID: <20171212131158.ec87dcde2a74dd879068cd95@google.com> URL: https://gitlist.dev/e/20171212131158.ec87dcde2a74dd879068cd95%40google.com In-Reply-To: ``` On Tue, 12 Dec 2017 11:57:28 -0800 Junio C Hamano wrote: > Junio C Hamano writes: > > > Makes sense. The patch looks scary by appearing to move the > > includes far to the front of the Makefile, but it in fact is moving > > the NO_DC_SHA1_SUBMODULE block slightly down and it is a sensible > > and safe move. > > A completely unrelated tangent. This was a good change to try your > anchored diff on. Viewing this change with > > $ git show --anchored='include config.mak' > > looks a lot less scary than the way it is shown by default (which > matches what was posted on the list). > > Good job. Thanks, glad that it helped. ``` ## Junio C Hamano, 2017-12-12 22:20 Subject: Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak Message-ID: URL: https://gitlist.dev/e/xmqq609bziz5.fsf%40gitster.mtv.corp.google.com In-Reply-To: ``` Ævar Arnfjörð Bjarmason writes: >> I actually think that the block can go even further down, perhaps >> close to the run of choices "what variant are we building?" we make >> at around we have "ifdef NO_CURL". >> >> Ævar? > > Makes sense to me, do you want to squash this + your proposed edit & > I'll pick it up if there's another version, or I can re-submit. OK. I'll squash in the 'move it further down' to the original commit that removed DC_SHA1_SUBMODULE and added NO_DC_SHA1_SUBMODULE when rebuilding 'pu' branch. Thanks. ``` ## Junio C Hamano, 2017-12-12 22:58 Subject: Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak Message-ID: URL: https://gitlist.dev/e/xmqqy3m7y2mz.fsf%40gitster.mtv.corp.google.com In-Reply-To: ``` Junio C Hamano writes: > Ævar Arnfjörð Bjarmason writes: > >>> I actually think that the block can go even further down, perhaps >>> close to the run of choices "what variant are we building?" we make >>> at around we have "ifdef NO_CURL". >>> >>> Ævar? >> >> Makes sense to me, do you want to squash this + your proposed edit & >> I'll pick it up if there's another version, or I can re-submit. > > OK. I'll squash in the 'move it further down' to the original > commit that removed DC_SHA1_SUBMODULE and added NO_DC_SHA1_SUBMODULE > when rebuilding 'pu' branch. > > Thanks. Another minor thing I noticed (which I do not have any squashable fix for) is that "make distclean" does not even work without submodule or this environment, which feels a bit too excessive. I haven't tried to figure out how involved a fix for that would be yet and I do not mind leaving it broken if it would be too much work. ``` ## Ævar Arnfjörð Bjarmason, 2017-12-12 23:21 Subject: Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak Message-ID: <877etrcz28.fsf@evledraar.gmail.com> URL: https://gitlist.dev/e/877etrcz28.fsf%40evledraar.gmail.com In-Reply-To: ``` On Tue, Dec 12 2017, Junio C. Hamano jotted: > Junio C Hamano writes: > >> Ævar Arnfjörð Bjarmason writes: >> >>>> I actually think that the block can go even further down, perhaps >>>> close to the run of choices "what variant are we building?" we make >>>> at around we have "ifdef NO_CURL". >>>> >>>> Ævar? >>> >>> Makes sense to me, do you want to squash this + your proposed edit & >>> I'll pick it up if there's another version, or I can re-submit. >> >> OK. I'll squash in the 'move it further down' to the original >> commit that removed DC_SHA1_SUBMODULE and added NO_DC_SHA1_SUBMODULE >> when rebuilding 'pu' branch. >> >> Thanks. > > Another minor thing I noticed (which I do not have any squashable > fix for) is that "make distclean" does not even work without > submodule or this environment, which feels a bit too excessive. I > haven't tried to figure out how involved a fix for that would be yet > and I do not mind leaving it broken if it would be too much work. Yes, that sucks. I can't think of a better fix for it than this massive hack of outsourcing printing the error to the header itself, which'll only be executed if we actually compile: diff --git a/Makefile b/Makefile index ba3e061edd..881cf55159 100644 --- a/Makefile +++ b/Makefile @@ -1022,9 +1022,7 @@ GIT_USER_AGENT = git/$(GIT_VERSION) ifndef DC_SHA1_EXTERNAL ifneq ($(wildcard sha1collisiondetection/lib/sha1.h),sha1collisiondetection/lib/sha1.h) -$(error The sha1collisiondetection submodule is not checked out. \ -Please make it available, either by cloning with --recurse-submodules, \ -or by running "git submodule update --init".) + BASIC_CFLAGS += -DERROR_SHA1COLLISIONDETECTION_SUBMODULE endif else ifdef NO_DC_SHA1_SUBMODULE diff --git a/sha1dc_git.h b/sha1dc_git.h index be1d48abbe..93a9be976a 100644 --- a/sha1dc_git.h +++ b/sha1dc_git.h @@ -1,5 +1,11 @@ /* Plumbing with collition-detecting SHA1 code */ +#ifdef ERROR_SHA1COLLISIONDETECTION_SUBMODULE +#error "The sha1collisiondetection submodule is not checked out." \ + "Please make it available, either by cloning with --recurse-submodules," \ + "or by running \"git submodule update --init\"." +#endif + #ifdef DC_SHA1_EXTERNAL #include #else But maybe I missed a way to make the $(error) path work only if certain targets were about to be compiled. ```