Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx> writes: > On 04/02/2019 18:12, Junio C Hamano wrote: >> Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx> 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.
