From mboxrd@z Thu Jan  1 00:00:00 1970
From: Yann E. MORIN <yann.morin.1998@free.fr>
Date: Tue, 22 Jun 2021 23:15:10 +0200
Subject: [Buildroot] [PATCH 12/15] package/pkg-generic.mk: Generate
 final rsync exclude file list
In-Reply-To: <20210621141130.48654-13-herve.codina@bootlin.com>
References: <20210621141130.48654-1-herve.codina@bootlin.com>
 <20210621141130.48654-13-herve.codina@bootlin.com>
Message-ID: <20210622211510.GP44262@scaer>
List-Id: <buildroot.busybox.net>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: buildroot@busybox.net

Herv?, All,

On 2021-06-21 16:11 +0200, Herve Codina spake thusly:
> The final rsync performed at {host,target}-finalize steps
> need to be done by rsyncing files generated by each packages
> without looking at files generated by other packages the current
> package depends on. This is needed to avoid overwrites in final
> {HOST,TARGET}_DIR.
> 
> In order to prepare the final rsync, an exclusion list is generated.
> This list lists files that are not generated by the current package and
> so files that need to be excluded from the final rsync.
> 
> Note also that the files list was not based on .files-list.{before,after}.
> During .file-list.{before,after} built for host directory, staging
> sub-directory (ie <toolchain>/sysroot) is filtered out.
> The final rsync exclusion list needs to take into account the full
> {host,target} directory to avoid final overwrites.

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...)

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.

> Signed-off-by: Herve Codina <herve.codina@bootlin.com>
> ---
>  package/pkg-generic.mk | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 918e2381af..546370af7a 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -162,6 +162,30 @@ define pkg_size_after
>  	rm -f $($(PKG)_DIR)/.files-list$(2).after
>  endef
>  
> +# Functions to collect final rsync exclusion files
> +
> +# $(1): base directory to search in
> +# $(2): suffix of file (optional)
> +define pkg_final_rsync_before
> +	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).before
> +endef
> +
> +# $(1): base directory to search in
> +# $(2): suffix of file (optional)
> +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

> +endef
> +
>  define check_bin_arch
>  	support/scripts/check-bin-arch -p $($(PKG)_NAME) \
>  		-l $($(PKG)_DIR)/.files-list.txt \
> @@ -267,6 +291,8 @@ $(BUILD_DIR)/%/.stamp_configured:
>  	@$(call MESSAGE,"Configuring")
>  	$(Q)mkdir -p $(HOST_DIR) $(TARGET_DIR) $(STAGING_DIR) $(BINARIES_DIR)
>  	$(call prepare-per-package-directory,$($(PKG)_FINAL_DEPENDENCIES))
> +	@$(call pkg_final_rsync_before,$(TARGET_DIR))
> +	@$(call pkg_final_rsync_before,$(HOST_DIR),-host)

The more I look at this, the more I believe we should resurrect the idea
of a 'prepare' step that goes in-between 'patch' and 'configure'...

Regards,
Yann E. MORIN.

>  	@$(call pkg_size_before,$(TARGET_DIR))
>  	@$(call pkg_size_before,$(STAGING_DIR),-staging)
>  	@$(call pkg_size_before,$(HOST_DIR),-host)
> @@ -404,6 +430,8 @@ $(BUILD_DIR)/%/.stamp_installed:
>  	@$(call pkg_size_after,$(STAGING_DIR),-staging)
>  	@$(call pkg_size_after,$(HOST_DIR),-host)
>  	@$(call check_bin_arch)
> +	@$(call pkg_final_rsync_after,$(TARGET_DIR))
> +	@$(call pkg_final_rsync_after,$(HOST_DIR),-host)
>  	@$(call pkg_detect_overwrite_after,$(TARGET_DIR))
>  	@$(call pkg_detect_overwrite_after,$(HOST_DIR),-host)
>  	$(Q)touch $@
> -- 
> 2.31.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'