[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH v3 2/9] Makefiles: add "shared.mak", move ".DELETE_ON_ERROR" to it
From:       Ævar Arnfjörð Bjarmason <avarab () gmail ! com>
Date:       2022-02-28 16:34:53
Message-ID: 220228.86zgmb2blb.gmgdl () evledraar ! gmail ! com
[Download RAW message or body]


On Mon, Feb 28 2022, Phillip Wood wrote:

> On 28/02/2022 11:16, Ævar Arnfjörð Bjarmason wrote:
>> On Mon, Feb 28 2022, Phillip Wood wrote:
>> 
>>> Hi Ævar
>>>
>>> On 25/02/2022 09:04, Ævar Arnfjörð Bjarmason wrote:
>>>> We have various behavior that's shared across our Makefiles, or that
>>>> really should be (e.g. via defined templates). Let's create a
>>>> top-level "shared.mak" to house those sorts of things, and start by
>>>> adding the ".DELETE_ON_ERROR" flag to it.
>>>> See my own 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR"
>>>> flag, 2021-06-29) and db10fc6c09f (doc: simplify Makefile using
>>>> .DELETE_ON_ERROR, 2021-05-21) for the addition and use of the
>>>> ".DELETE_ON_ERROR" flag.
>>>> This does have the potential downside that if
>>>> e.g. templates/Makefile
>>>> would like to include this "shared.mak" in the future the semantics of
>>>> such a Makefile will change, but as noted in the above commits (and
>>>> GNU make's own documentation) any such change would be for the better,
>>>> so it's safe to do this.
>>>
>>> I was confused about the mention of templates/Makefile in this
>>> paragraph, it seems to be saying that the behavior would change in the
>>> future if we included shared.mak in templates/Makefile but this patch
>>> does exactly that.
>> Yes, oops! It's a zombie comment that I forgot to adjust from an
>> earlier
>> version of this where that wasn't the case. Will adjust & re-roll.
>> 
>>> Also does this patch mean we're now using .DELETE_ON_ERROR in places
>>> where we were not previously using it?
>> Yes, we'll now use it in those other Makefiles as well. The commits
>> referenced in the second paragrap of the commit message argue for this
>> being safe, and I've reviewed the logic myself & don't expect any
>> problems with it.
>
> Thanks for elaborating, maybe it is worth spelling explicitly in the
> commit message that this is turning on .DELETE_ON_ERROR in places we 
> didn't previously use it. I had a look at the commit message you
> referenced and it seems to make a good case for using
> .DELETE_ON_ERROR. Having a shared makefile for common code makes sense
> and the speed ups from some of the other commits are nice.

Thanks, yes, willdo :) I have a re-roll of this queued up for
submission, after sitting on it for a bit longer to shake out any
potential last-minute issues...
[prev in list] [next in list] [prev in thread] [next in thread]