[Buildroot] [PATCH 12/15] package/pkg-generic.mk: Generate final rsync exclude file list

[Herve Codina]

Hi,

On Tue, 22 Jun 2021 23:15:10 +0200
"Yann E. MORIN" <yann.morin.1998 at free.fr> wrote:

> Ideally, we would in fact redirect the package installation to an emoty
> directory, so that we can actually find what it installs.
> 
> However, as has been discussed in the past, this is fraught with
> unworkable issues. For example, some paths may be hard-coded at
> configure time and/or build time, and thus the package would still
> install in the original stagin we presented it with (for target/, this
> is not an isue, because target/ is never looked at during configure or
> build, only at install time). Or a package may try to modify an existing
> file (bad, but still). Or a file just assumes that tdirectory structure
> exists (bad, but eh...)

Modifying existing files is no more allowed -> Overwrites detection.

> 
> So, yes, the rsync exclusion list is a good workaround.
> 
> I see you have provided detailed comit logs, that is great. Still, for
> such core stuff, I think they still miss the bigger picture, like I
> explained above for example, and which should have been part of the
> commit log to explain why we resort to an exclusion list rather than the
> more obvious and simple empty-DESTDIR.
> 

I can add at the end of commit log :
--- 8< ---
Using an empty directory for per-package installation directory would be the
simplest way to find what a package installs.
However, as it has been discussed in the past, this is fraught with unworkable
issues. For example, some paths may be hard-coded at configure time and/or build
time, and thus the package would still install in the original stagin we presented
it with (for target/, this is not an isue, because target/ is never looked at
during configure or build, only at install time). Or a package installation
process just assumes that the directory structure exists (bad, but eh...).
--- 8< ---

Is that ok for you ?


> > +define pkg_final_rsync_after
> > +	cd $(1); \
> > +	LC_ALL=C find . \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
> > +		| LC_ALL=C sort > $($(PKG)_DIR)/.files-final-rsync$(2).after
> > +	LC_ALL=C comm -2 \
> > +		$($(PKG)_DIR)/.files-final-rsync$(2).before \
> > +		$($(PKG)_DIR)/.files-final-rsync$(2).after \
> > +		| sed -r -e 's/^[^,]+,./- /' \
> > +		> $($(PKG)_DIR)/.files-final-rsync$(2).exclude_rsync
> > +	rm -f $($(PKG)_DIR)/.files-final-rsync$(2).after  
> 
> You forgot to remove .files-final-rsync$(2).before

I cannot remove it in this patch.

.files-final-rsync$(2).before is generated by .stamp_configured rule calling
pkg_final_rsync_before call and if we run 'make && make foo-rebuild' the file
generated by first make invocation will be no more present for second make
invocation.

We can remove .files-final-rsync$(2).before only after patch
"package/pkg-generic.mk: Fix per-package <pkg>-{reconfigure,rebuild,reinstall}"
Indeed, in this patch, pkg_final_rsync_after is called after the per-package
host and target dir are recreated from scratch.

I will remove .files-final-rsync$(2).before in a dedicated patch after
"package/pkg-generic.mk: Fix per-package <pkg>-{reconfigure,rebuild,reinstall}"

Is that ok for you ?

Hervé

-- 
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com