All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Ramsay Jones <ramsay@ramsayjones.plus.com>,
	GIT Mailing-list <git@vger.kernel.org>
Subject: Re: [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak
Date: Wed, 13 Dec 2017 00:21:35 +0100	[thread overview]
Message-ID: <877etrcz28.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <xmqqy3m7y2mz.fsf@gitster.mtv.corp.google.com>


On Tue, Dec 12 2017, Junio C. Hamano jotted:

> Junio C Hamano <gitster@pobox.com> writes:
>
>> Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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 <sha1dc/sha1.h>
     #else

But maybe I missed a way to make the $(error) path work only if certain
targets were about to be compiled.

      reply	other threads:[~2017-12-12 23:21 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-12 12:47 [PATCH] Makefile: allow NO_DC_SHA1_SUBMODULE to be set in config.mak Ramsay Jones
2017-12-12 19:53 ` Junio C Hamano
2017-12-12 19:57   ` Junio C Hamano
2017-12-12 21:11     ` Jonathan Tan
2017-12-12 21:06   ` Ævar Arnfjörð Bjarmason
2017-12-12 22:20     ` Junio C Hamano
2017-12-12 22:58       ` Junio C Hamano
2017-12-12 23:21         ` Ævar Arnfjörð Bjarmason [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=877etrcz28.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=ramsay@ramsayjones.plus.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.