Re: [PATCH 1/4] Makefile: don't error out under DC_SHA1_EXTERNAL if DC_SHA1_SUBMODULE=auto

[Jeff King <peff@peff.net>]

On Tue, Nov 28, 2017 at 09:32:11PM +0000, Ævar Arnfjörð Bjarmason wrote:

> Fix a logic error in the initial introduction of DC_SHA1_EXTERNAL. If
> git.git has a sha1collisiondetection submodule checked out the logic
> to set DC_SHA1_SUBMODULE=auto would interact badly with the check for
> whether DC_SHA1_SUBMODULE was set.
> 
> It would error out, meaning that there's no way to build git with
> DC_SHA1_EXTERNAL=YesPlease without deinit-ing the submodule.
> 
> Instead, adjust the logic to only fire if the variable is to something
> else than "auto" which would mean it's a mistake on the part of
> whoever's building git, not just the Makefile tripping over its own
> logic.

This all makes sense, and I agree your patch is an improvement.

One minor whitespace nit:

> diff --git a/Makefile b/Makefile
> index e53750ca01..8fe8278126 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1497,7 +1497,9 @@ else
>  	LIB_OBJS += sha1dc_git.o
>  ifdef DC_SHA1_EXTERNAL
>  	ifdef DC_SHA1_SUBMODULE
> +ifneq ($(DC_SHA1_SUBMODULE),auto)
>  $(error Only set DC_SHA1_EXTERNAL or DC_SHA1_SUBMODULE, not both)
> +endif
>  	endif

The indentation here is funky. Unfortunately I think $(error) can't be
tab-indented, so it has to either stay at the left-most side, or we have
to use spaces.

But the ifneq/endif pair can be indented. Ordinarily I'd say it's fine
to keep it at the outermost (because of the weird error indent), but
note that we're _inside_ an already-indented ifdef/endif pair. We should
either stay inside there, or we should put the outer one to the left for
consistency.

-Peff