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

From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: "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 13:39:55 +0200	[thread overview]
Message-ID: <20210829133955.74d921e9@windsurf> (raw)
In-Reply-To: <20210828224740.GS2661@scaer>

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.

Best regards,

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot