Re: [Buildroot] [PATCH v3 07/16] package/pkg-generic.mk: detect files overwritten in TARGET_DIR and HOST_DIR

From: Arnout Vandecappelle <arnout@mind.be>
To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>,
	"Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Herve Codina <herve.codina@bootlin.com>,
	Naumann Andreas <ANaumann@ultratronik.de>,
	Peter Seiderer <ps.report@gmx.net>,
	Julien Corjon <corjon.j@ecagroup.com>,
	buildroot@buildroot.org,
	Ricardo Martincoski <ricardo.martincoski@gmail.com>
Subject: Re: [Buildroot] [PATCH v3 07/16] package/pkg-generic.mk: detect files overwritten in TARGET_DIR and HOST_DIR
Date: Sun, 29 Aug 2021 17:01:36 +0200	[thread overview]
Message-ID: <8b85fc92-b792-dccc-6258-bc6a07a03848@mind.be> (raw)
In-Reply-To: <20210829133955.74d921e9@windsurf>

On 29/08/2021 13:39, Thomas Petazzoni wrote:
> Hello Yann,
> 
> First of all, thanks a lot for reviewing and merging bits of this patch
> series, I'm glad to see we're making progress with the TLP stuff.
> 
> On Sun, 29 Aug 2021 00:47:40 +0200
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> 
>> However, what prompted me from applying for now, is that this new
>> detection is a hard error.
>>
>> Previously, check-uniq-files was just emitting warnings, but would not
>> prevent the build from failing. Now, with this patch, even an innocuous
>> overwrite (e.g. because a post-build script deletes the file, or the
>> content of the file really does not matter at runtime), the build will
>> fail.
>>
>> I.e. configurations that are currently working with PPD, despite the
>> overwrite, will suddenly no longer build.
>>
>> OTOH, if we do not make that a hard-error, we will never detect most
>> issues, because users will never spot those warnings and wil enver
>> report issues, and the autobuilders will not fail and we will not
>> notice either...
>>
>> One solution is to add a configuration knob to make that a hard-error,
>> like we have the paranoid libs/headers check:
>>
>>     config BR2_PPD_OVERWRITE_STRICT
>>         bool "Strict file overwrite detection"
>>         depends on BR2_PER_PACKAGE_DIRECTORIES
>>         help
>>           Say 'y' here to turn the file overwrite detection
>>           to a hard error. By default, only warnings will be
>>           printed.
> 
> We had some discussion with Hervé back when he worked on this, and I
> disagreed with adding an option. When BR2_PER_PACKAGE_DIRECTORIES=y, a
> file overwrite must be a hard error, as the result of the build is
> incorrect if there is an overwrite. It's not the "latest" package that
> wins in an overwrite situation, like it does in a non-PPD case.
> 
> So I really think this must be a hard error for PPD builds, and just a
> warning for non-PPD builds.
> 
> Yes, for PPD builds, it means users will get failures, but those
> failures are pointing to real problems.
> 
> So, my preference would be to merge as an unconditional check, and see
> how it goes. Perhaps the situation will be so bad that we will have to
> make it conditional, but I would prefer to have it unconditional first
> and see the impact.

 I was originally with Yann, but these arguments convinced that it is indeed
better to not have the option (for now).

 Regards,
 Arnout

_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot