Re: [PATCH 1/1] Makefile: improve SPARSE_FLAGS customisation

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 4, 2019, 20:15 UTC
Message-ID: <xmqqy36vtimu.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <365a9539-4ce1-121a-ec8c-2e52a5828091@ramsayjones.plus.com>

Ramsay Jones <ramsay@ramsayjones.plus.com> writes:

Show 20 quoted lines

> On 04/02/2019 18:12, Junio C Hamano wrote:
>> Ramsay Jones <ramsay@ramsayjones.plus.com> writes:
>> 
>>>> Thanks for a detailed and clear explanation here and in the cover
>>>> letter.  I agree with the motivation and most of the things I see in
>>>> this patch, but one thing that stands out at me is if we still want
>>>> to += append to SP_EXTRA_FLAGS in target specific way.  Before this
>>>> patch, because SPARSE_FLAGS was a dual use variable, it needed +=
>>>> appending to it in these two places, but that rationale is gone with
>>>> this patch.
>>>
>>> As Luc surmised, in his reply, my intention was that SP_EXTRA_FLAGS
>>> should be used for any 'internal' settings (not just the target
>>> specific settings), whereas SPARSE_FLAGS would now be used _only_ for
>>> user customisation.
>> 
>> OK, if that is the case, then not using "+= append" on SP_EXTRA_FLAGS
>
> Err, no, that clearly wouldn't be an improvement! As I said above,
> this is not just for target specific settings.

Ah, do you mean that there may be globally applicable internal setting? I would have expected that such an option would be done directly on the command line, e.g.

$(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
	$(QUIET_SP)cgcc -no-compile $(ALL_CFLAGS) $(EXTRA_CPPFLAGS) \
		$(SPARSE_FLAGS) $(SP_EXTRA_FLAGS) \
		-Wsparse-settings-for-everybody $<

But it is fine either way, as long as the purpose of the macro is documented clearly enough ;-)

Thanks.