diff mbox series

package/pkg-generic.mk: fix generation of package list in reinstall

Message ID 20201105140401.1356161-1-unixmania@gmail.com
State New
Headers show
Series package/pkg-generic.mk: fix generation of package list in reinstall | expand

Commit Message

Carlos Santos Nov. 5, 2020, 2:04 p.m. UTC
From: Carlos Santos <unixmania@gmail.com>

Running "make <pkg>-reinstall" always shows a message like these:

comm: /.../build/pkg-x.y.z/.files-list.before: No such file or directory
comm: /.../build/pkg-x.y.z/.files-list-staging.before: No such file or directory
comm: /.../build/pkg-x.y.z/.files-list-host.before: No such file or directory

This happens because pkg_size_after is called in the .stamp_installed
target and removes .files-list$(2).before, so in a second installation
the file is not there anymore. This results in an empty .files-list.txt
file.

Move the pkg_size_{before,after} calls from the .stamp_configured and
.stamp_installed targets to the .stamp_{host,staging,target}_installed
targets, respectively. This ensures that every pkg_size_after call has
an immediately corresponding pkg_size_before call.

Signed-off-by: Carlos Santos <unixmania@gmail.com>
---
 package/pkg-generic.mk | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Thomas Petazzoni Nov. 5, 2020, 4:33 p.m. UTC | #1
Hello Carlos,

On Thu,  5 Nov 2020 11:04:01 -0300
unixmania@gmail.com wrote:

> From: Carlos Santos <unixmania@gmail.com>
> 
> Running "make <pkg>-reinstall" always shows a message like these:
> 
> comm: /.../build/pkg-x.y.z/.files-list.before: No such file or directory
> comm: /.../build/pkg-x.y.z/.files-list-staging.before: No such file or directory
> comm: /.../build/pkg-x.y.z/.files-list-host.before: No such file or directory
> 
> This happens because pkg_size_after is called in the .stamp_installed
> target and removes .files-list$(2).before, so in a second installation
> the file is not there anymore. This results in an empty .files-list.txt
> file.
> 
> Move the pkg_size_{before,after} calls from the .stamp_configured and
> .stamp_installed targets to the .stamp_{host,staging,target}_installed
> targets, respectively. This ensures that every pkg_size_after call has
> an immediately corresponding pkg_size_before call.

Unfortunately, this breaks the very reason why the .stamp_installed
step was added, as explained in the commit log for the change that
introduced this:

commit f2a12a8e9fa292ab61d99f963683a97f1283b7a3
Author: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Date:   Thu Apr 30 11:52:40 2020 +0200

    package/pkg-generic.mk: introduce final 'install' step
    
    We currently have four different install steps: target installation,
    staging installation, images installation and host installation. These
    steps are directly triggered from the $(1)-install make target, so
    there is no place where we can run some logic once all installation
    steps have completed.
    
    However, as part of improving the reliability of the logic done in
    step_pkg_size_before and step_pkg_size_after to detect the files
    installed by packages, we would in fact need to run some logic after
    all installation steps have completed. This will allow us to make sure
    that all files are detected, even if a host package installs something
    in the target directory, or if a target package installs something in
    the host directory.
    
    To achieve this, this commit implements a new stamp file,
    .stamp_installed, which is a step that depends on all four install
    steps. Currently, this step does nothing except creating the stamp
    file.
    
    Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
    [yann.morin.1998@free.fr: remove stampfile on foo-reinstall]
    Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>

I'll try to think about the issue you've reported and see if I can come
up with some useful suggestions.

Thanks,

Thomas
Carlos Santos Nov. 5, 2020, 5:21 p.m. UTC | #2
On Thu, Nov 5, 2020 at 1:33 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> Hello Carlos,
>
> On Thu,  5 Nov 2020 11:04:01 -0300
> unixmania@gmail.com wrote:
>
> > From: Carlos Santos <unixmania@gmail.com>
> >
> > Running "make <pkg>-reinstall" always shows a message like these:
> >
> > comm: /.../build/pkg-x.y.z/.files-list.before: No such file or directory
> > comm: /.../build/pkg-x.y.z/.files-list-staging.before: No such file or directory
> > comm: /.../build/pkg-x.y.z/.files-list-host.before: No such file or directory
> >
> > This happens because pkg_size_after is called in the .stamp_installed
> > target and removes .files-list$(2).before, so in a second installation
> > the file is not there anymore. This results in an empty .files-list.txt
> > file.
> >
> > Move the pkg_size_{before,after} calls from the .stamp_configured and
> > .stamp_installed targets to the .stamp_{host,staging,target}_installed
> > targets, respectively. This ensures that every pkg_size_after call has
> > an immediately corresponding pkg_size_before call.
>
> Unfortunately, this breaks the very reason why the .stamp_installed
> step was added, as explained in the commit log for the change that
> introduced this:
>
> commit f2a12a8e9fa292ab61d99f963683a97f1283b7a3
> Author: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
> Date:   Thu Apr 30 11:52:40 2020 +0200
>
>     package/pkg-generic.mk: introduce final 'install' step
>
>     We currently have four different install steps: target installation,
>     staging installation, images installation and host installation. These
>     steps are directly triggered from the $(1)-install make target, so
>     there is no place where we can run some logic once all installation
>     steps have completed.
>
>     However, as part of improving the reliability of the logic done in
>     step_pkg_size_before and step_pkg_size_after to detect the files
>     installed by packages, we would in fact need to run some logic after
>     all installation steps have completed. This will allow us to make sure
>     that all files are detected, even if a host package installs something
>     in the target directory, or if a target package installs something in
>     the host directory.
>
>     To achieve this, this commit implements a new stamp file,
>     .stamp_installed, which is a step that depends on all four install
>     steps. Currently, this step does nothing except creating the stamp
>     file.
>
>     Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
>     [yann.morin.1998@free.fr: remove stampfile on foo-reinstall]
>     Signed-off-by: Yann E. MORIN <yann.morin.1998@free.fr>
>
> I'll try to think about the issue you've reported and see if I can come
> up with some useful suggestions.
>
> Thanks,
>
> Thomas

I understand but wouldn't be a bug if a host package installed things
on the target directory and vice-versa? Maybe the right thing to do is
to ensure that this does not happen.


--
Carlos Santos <unixmania@gmail.com>
Thomas Petazzoni Nov. 5, 2020, 9:16 p.m. UTC | #3
On Thu, 5 Nov 2020 14:21:21 -0300
Carlos Santos <unixmania@gmail.com> wrote:

> > I'll try to think about the issue you've reported and see if I can come
> > up with some useful suggestions.
> >
> I understand but wouldn't be a bug if a host package installed things
> on the target directory and vice-versa? Maybe the right thing to do is
> to ensure that this does not happen.

This can certainly be debated, and it would clearly be nicer if only
host packages installed stuff in HOST_DIR, only the staging
installation steps installed things in STAGING_DIR, and only target
installation steps installed things in TARGET_DIR. Unfortunately, we're
not there for a number of packages I'm afraid.

For example, qt5 is a target package, but it installs all its host
tools in HOST_DIR, and splitting up into a host-qt5 package is clearly
not trivial.

Another example: the external toolchain packages are all target
packages, as they install the libraries/headers in STAGING_DIR, the
libraries in TARGET_DIR... but they also install the cross-compiler and
other cross-tools in HOST_DIR.

Thomas
diff mbox series

Patch

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 54de03da03..b13ecc5d8e 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -232,9 +232,6 @@  $(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_size_before,$(TARGET_DIR))
-	@$(call pkg_size_before,$(STAGING_DIR),-staging)
-	@$(call pkg_size_before,$(HOST_DIR),-host)
 	$(call fixup-libtool-files,$(NAME),$(STAGING_DIR))
 	$(foreach hook,$($(PKG)_PRE_CONFIGURE_HOOKS),$(call $(hook))$(sep))
 	$($(PKG)_CONFIGURE_CMDS)
@@ -256,9 +253,11 @@  $(BUILD_DIR)/%/.stamp_built::
 $(BUILD_DIR)/%/.stamp_host_installed:
 	@$(call step_start,install-host)
 	@$(call MESSAGE,"Installing to host directory")
+	@$(call pkg_size_before,$(HOST_DIR),-host)
 	$(foreach hook,$($(PKG)_PRE_INSTALL_HOOKS),$(call $(hook))$(sep))
 	+$($(PKG)_INSTALL_CMDS)
 	$(foreach hook,$($(PKG)_POST_INSTALL_HOOKS),$(call $(hook))$(sep))
+	@$(call pkg_size_after,$(HOST_DIR),-host)
 	@$(call step_end,install-host)
 	$(Q)touch $@
 
@@ -285,6 +284,7 @@  $(BUILD_DIR)/%/.stamp_host_installed:
 $(BUILD_DIR)/%/.stamp_staging_installed:
 	@$(call step_start,install-staging)
 	@$(call MESSAGE,"Installing to staging directory")
+	@$(call pkg_size_before,$(STAGING_DIR),-staging)
 	$(foreach hook,$($(PKG)_PRE_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
 	+$($(PKG)_INSTALL_STAGING_CMDS)
 	$(foreach hook,$($(PKG)_POST_INSTALL_STAGING_HOOKS),$(call $(hook))$(sep))
@@ -320,6 +320,7 @@  $(BUILD_DIR)/%/.stamp_staging_installed:
 			mv "$${la}.fixed" "$${la}"; \
 		fi || exit 1; \
 	done
+	@$(call pkg_size_after,$(STAGING_DIR),-staging)
 	@$(call step_end,install-staging)
 	$(Q)touch $@
 
@@ -337,6 +338,7 @@  $(BUILD_DIR)/%/.stamp_images_installed:
 $(BUILD_DIR)/%/.stamp_target_installed:
 	@$(call step_start,install-target)
 	@$(call MESSAGE,"Installing to target")
+	@$(call pkg_size_before,$(TARGET_DIR))
 	$(foreach hook,$($(PKG)_PRE_INSTALL_TARGET_HOOKS),$(call $(hook))$(sep))
 	+$($(PKG)_INSTALL_TARGET_CMDS)
 	$(if $(BR2_INIT_SYSTEMD),\
@@ -350,15 +352,13 @@  $(BUILD_DIR)/%/.stamp_target_installed:
 	$(Q)if test -n "$($(PKG)_CONFIG_SCRIPTS)" ; then \
 		$(RM) -f $(addprefix $(TARGET_DIR)/usr/bin/,$($(PKG)_CONFIG_SCRIPTS)) ; \
 	fi
+	@$(call pkg_size_after,$(TARGET_DIR))
 	@$(call step_end,install-target)
 	$(Q)touch $@
 
 # Final installation step, completed when all installation steps
 # (host, images, staging, target) have completed
 $(BUILD_DIR)/%/.stamp_installed:
-	@$(call pkg_size_after,$(TARGET_DIR))
-	@$(call pkg_size_after,$(STAGING_DIR),-staging)
-	@$(call pkg_size_after,$(HOST_DIR),-host)
 	@$(call check_bin_arch)
 	$(Q)touch $@