[Buildroot] [git commit] core/pkg-infra: restore completeness of packages files lists

[Buildroot] [git commit] core/pkg-infra: restore completeness of packages files lists

[Peter Korsgaard]

commit: https://git.buildroot.net/buildroot/commit/?id=3c8f0d9efae92772c90521322a083e4f73985dd1
branch: https://git.buildroot.net/buildroot/commit/?id=refs/heads/master

In commit 7fb6e782542f (core/instrumentation: shave minutes off the
build time), the built stampfile is used as a reference to detect files
installed by a package.

However, packages may install files keeping their mtime intact, and we
end up not detecting this. For example, the internal skeleton package
will install (e.g.) /etc/passwd with an mtime of when the file was
created in $(TOP_DIR), which could be the time the git repository was
checked out; that mtime is always older than the build stamp file, so
files installed by the skeleton package are never accounted for to that
package, or to any other package for that matters.

We switch to an alternate solution, which consists of storing some extra
metadata per file, so that we can more reasily detect modifications to
the files. Then we compare the state before the package is installed (by
reusing the existing list) and after the package is installed, compare
that to list any new file or modified files (in reality, ignoring
untouched and removed files). Finally, we store the file->package
association in the global list and store the new stat list as the global
list.

The format used for the .stat file is:

mtime:inode:perms:filetype:size,filename

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Peter Korsgaard <peter@korsgaard.com>
Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: Arnout Vandecappelle <arnout@mind.be>
Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
Cc: Trent Piepho <tpiepho@impinj.com>

[Peter: rename files, reformat, only look for files and symlinks and pass
	LC_ALL=C to comm as pointed out by Thomas De Schampheleire]
Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
 package/pkg-generic.mk | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index f5cab2b9c2..6168b40e89 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -63,13 +63,21 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
 # $(2): base directory to search in
 # $(3): suffix of file  (optional)
 define step_pkg_size_inner
+	@touch $(BUILD_DIR)/.files-list$(3).stat
 	@touch $(BUILD_DIR)/packages-file-list$(3).txt
 	$(SED) '/^$(1),/d' $(BUILD_DIR)/packages-file-list$(3).txt
 	cd $(2); \
-	find . \( -type f -o -type l \) \
-		-newer $($(PKG)_DIR)/.stamp_built \
-		-exec printf '$(1),%s\n' {} + \
+	LC_ALL=C find . \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
+		| LC_ALL=C sort > $(BUILD_DIR)/.files-list$(3).new
+	LC_ALL=C comm -13 \
+		$(BUILD_DIR)/.files-list$(3).stat \
+		$(BUILD_DIR)/.files-list$(3).new \
+		> $($(PKG)_BUILDDIR)/.files-list$(3).txt
+	sed -r -e 's/^[^,]+/$(1)/' \
+		$($(PKG)_BUILDDIR)/.files-list$(3).txt \
 		>> $(BUILD_DIR)/packages-file-list$(3).txt
+	mv $(BUILD_DIR)/.files-list$(3).new \
+		$(BUILD_DIR)/.files-list$(3).stat
 endef
 
 define step_pkg_size

[Buildroot] [git commit] core/pkg-infra: restore completeness of packages files lists

[Peter Korsgaard]

>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes:

 > commit: https://git.buildroot.net/buildroot/commit/?id=3c8f0d9efae92772c90521322a083e4f73985dd1
 > branch: https://git.buildroot.net/buildroot/commit/?id=refs/heads/master

 > In commit 7fb6e782542f (core/instrumentation: shave minutes off the
 > build time), the built stampfile is used as a reference to detect files
 > installed by a package.

 > However, packages may install files keeping their mtime intact, and we
 > end up not detecting this. For example, the internal skeleton package
 > will install (e.g.) /etc/passwd with an mtime of when the file was
 > created in $(TOP_DIR), which could be the time the git repository was
 > checked out; that mtime is always older than the build stamp file, so
 > files installed by the skeleton package are never accounted for to that
 > package, or to any other package for that matters.

 > We switch to an alternate solution, which consists of storing some extra
 > metadata per file, so that we can more reasily detect modifications to
 > the files. Then we compare the state before the package is installed (by
 > reusing the existing list) and after the package is installed, compare
 > that to list any new file or modified files (in reality, ignoring
 > untouched and removed files). Finally, we store the file->package
 > association in the global list and store the new stat list as the global
 > list.

 > The format used for the .stat file is:

 > mtime:inode:perms:filetype:size,filename

 > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
 > Cc: Peter Korsgaard <peter@korsgaard.com>
 > Cc: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
 > Cc: Arnout Vandecappelle <arnout@mind.be>
 > Cc: Thomas De Schampheleire <patrickdepinguin@gmail.com>
 > Cc: Trent Piepho <tpiepho@impinj.com>

 > [Peter: rename files, reformat, only look for files and symlinks and pass
 > 	LC_ALL=C to comm as pointed out by Thomas De Schampheleire]

Committed to 2018.02.x and 2018.11.x, thanks.

-- 
Bye, Peter Korsgaard

[Buildroot] [git commit] core/pkg-infra: restore completeness of packages files lists

[Jan Kundrát]

>  package/pkg-generic.mk | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index f5cab2b9c2..6168b40e89 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -63,13 +63,21 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
>  # $(2): base directory to search in
>  # $(3): suffix of file  (optional)
>  define step_pkg_size_inner
> +	@touch $(BUILD_DIR)/.files-list$(3).stat
>  	@touch $(BUILD_DIR)/packages-file-list$(3).txt
>  	$(SED) '/^$(1),/d' $(BUILD_DIR)/packages-file-list$(3).txt
>  	cd $(2); \
> -	find . \( -type f -o -type l \) \
> -		-newer $($(PKG)_DIR)/.stamp_built \
> -		-exec printf '$(1),%s\n' {} + \
> +	LC_ALL=C find . \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
> +		| LC_ALL=C sort > $(BUILD_DIR)/.files-list$(3).new
> +	LC_ALL=C comm -13 \
> +		$(BUILD_DIR)/.files-list$(3).stat \
> +		$(BUILD_DIR)/.files-list$(3).new \
> +		> $($(PKG)_BUILDDIR)/.files-list$(3).txt
> +	sed -r -e 's/^[^,]+/$(1)/' \
> +		$($(PKG)_BUILDDIR)/.files-list$(3).txt \
>  		>> $(BUILD_DIR)/packages-file-list$(3).txt
> +	mv $(BUILD_DIR)/.files-list$(3).new \
> +		$(BUILD_DIR)/.files-list$(3).stat

This breaks top-level parallel build. Here's an example how it looks when 
it breaks:

2019-03-06T12:50:02 >>> expat 2.2.6 Fixing libtool files
2019-03-06T12:50:02 Making install in gnulib-lib
2019-03-06T12:50:02 /usr/bin/make  install-am
2019-03-06T12:50:02 >>> ethtool 4.19 Installing to target
2019-03-06T12:50:02 
PATH="/home/jkt/work/prog/_build/br-cfb/host/bin:/home/jkt/work/prog/_build/br-cfb/host/sbin:/home/jkt/.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/home/jkt/
.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/8.1.0:/usr/lib/llvm/6/bin:/usr/lib/llvm/4/bin:/usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/games/bin" 
 /usr/bin/make  DESTDIR=/home/jkt/work/prog/_build/br-cfb/target install -C 
/home/jkt/work/prog/_build/br-cfb/build/ethtool-4.19/
2019-03-06T12:50:02 /bin/mkdir -p 
'/home/jkt/work/prog/_build/br-cfb/target/usr/sbin'
2019-03-06T12:50:02 /bin/mkdir -p 
'/home/jkt/work/prog/_build/br-cfb/host/lib'
2019-03-06T12:50:02 /bin/sh ../libtool   --mode=install /usr/bin/install -c 
  libgettextlib.la '/home/jkt/work/prog/_build/br-cfb/host/lib'
2019-03-06T12:50:02 /usr/bin/install -c ethtool 
'/home/jkt/work/prog/_build/br-cfb/target/usr/sbin'
2019-03-06T12:50:02 comm: 
/home/jkt/work/prog/_build/br-cfb/build/.files-list-staging.new: No such 
file or directory
2019-03-06T12:50:02 make[1]: *** [package/pkg-generic.mk:287: 
/home/jkt/work/prog/_build/br-cfb/build/expat-2.2.6/.stamp_staging_installed] 
Error 1
2019-03-06T12:50:02 make[1]: *** Waiting for unfinished jobs....

I can't offer much insight into how to best fix this, unfortunately, 
whether to add some locking or whether to make this per-package.

This is just with a patched top-level Makefile to remove the .NOTPARALLEL 
stanza, with no per-package build dire patches atec.

With kind regards,
Jan

[Buildroot] [git commit] core/pkg-infra: restore completeness of packages files lists

[Yann E. MORIN]

Jan, All,

On 2019-03-06 12:57 +0100, Jan Kundr?t spake thusly:
> > package/pkg-generic.mk | 14 +++++++++++---
> > 1 file changed, 11 insertions(+), 3 deletions(-)
> >
> >diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> >index f5cab2b9c2..6168b40e89 100644
> >--- a/package/pkg-generic.mk
> >+++ b/package/pkg-generic.mk
> >@@ -63,13 +63,21 @@ GLOBAL_INSTRUMENTATION_HOOKS += step_time
> > # $(2): base directory to search in
> > # $(3): suffix of file  (optional)
> > define step_pkg_size_inner
> >+	@touch $(BUILD_DIR)/.files-list$(3).stat
> > 	@touch $(BUILD_DIR)/packages-file-list$(3).txt
> > 	$(SED) '/^$(1),/d' $(BUILD_DIR)/packages-file-list$(3).txt
> > 	cd $(2); \
> >-	find . \( -type f -o -type l \) \
> >-		-newer $($(PKG)_DIR)/.stamp_built \
> >-		-exec printf '$(1),%s\n' {} + \
> >+	LC_ALL=C find . \( -type f -o -type l \) -printf '%T@:%i:%#m:%y:%s,%p\n' \
> >+		| LC_ALL=C sort > $(BUILD_DIR)/.files-list$(3).new
> >+	LC_ALL=C comm -13 \
> >+		$(BUILD_DIR)/.files-list$(3).stat \
> >+		$(BUILD_DIR)/.files-list$(3).new \
> >+		> $($(PKG)_BUILDDIR)/.files-list$(3).txt
> >+	sed -r -e 's/^[^,]+/$(1)/' \
> >+		$($(PKG)_BUILDDIR)/.files-list$(3).txt \
> > 		>> $(BUILD_DIR)/packages-file-list$(3).txt
> >+	mv $(BUILD_DIR)/.files-list$(3).new \
> >+		$(BUILD_DIR)/.files-list$(3).stat
> 
> This breaks top-level parallel build.

Yes, that's not unexpected, and I'm not entirely surprised.

So, TLPB is not yet supported, so fixing the files lists was deemed more
important than allowing an as-yet unsupported (and known to anyway
already break in other surprising and silent ways) "feature".

We also discussed this at the devloers days in Brussels, where I pointed
out that this would also break for PPSH (aka per-package staging+host,
aka PPD, perp-package directory). Again, fixing it for the upcoming LTS
was more important.

> Here's an example how it looks when it
> breaks:
[--SNIP--]
> I can't offer much insight into how to best fix this, unfortunately, whether
> to add some locking or whether to make this per-package.
> 
> This is just with a patched top-level Makefile to remove the .NOTPARALLEL
> stanza, with no per-package build dire patches atec.

And it is known that just removing .NOTPRALLEL would otherwise open a
can of worms, with much more subtle breakage anyway, like concurrent
acces to direcotries or files in target/  (install(1) is racy in how
it tests if a file exists and then creates it).

We've just made this breakage more verbose, now. ;-]

But really, the real way to TLPB is with PPD anyway.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [git commit] core/pkg-infra: restore completeness of packages files lists

[Andreas Naumann]

Hi Jan,

> 
> This breaks top-level parallel build. Here's an example how it looks 
> when it breaks:
> 
> 2019-03-06T12:50:02 >>> expat 2.2.6 Fixing libtool files
> 2019-03-06T12:50:02 Making install in gnulib-lib
> 2019-03-06T12:50:02 /usr/bin/make? install-am
> 2019-03-06T12:50:02 >>> ethtool 4.19 Installing to target
> 2019-03-06T12:50:02 
> PATH="/home/jkt/work/prog/_build/br-cfb/host/bin:/home/jkt/work/prog/_build/br-cfb/host/sbin:/home/jkt/.local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/home/jkt/ 
> 
> .local/bin:/home/jkt/bin/:/opt/qtc/bin:/opt/darktable/bin:/usr/x86_64-pc-linux-gnu/gcc-bin/8.1.0:/usr/lib/llvm/6/bin:/usr/lib/llvm/4/bin:/usr/local/bin:/usr/bin:/bin:/opt/bin:/usr/games/bin" 
> /usr/bin/make? DESTDIR=/home/jkt/work/prog/_build/br-cfb/target install 
> -C /home/jkt/work/prog/_build/br-cfb/build/ethtool-4.19/
> 2019-03-06T12:50:02 /bin/mkdir -p 
> '/home/jkt/work/prog/_build/br-cfb/target/usr/sbin'
> 2019-03-06T12:50:02 /bin/mkdir -p 
> '/home/jkt/work/prog/_build/br-cfb/host/lib'
> 2019-03-06T12:50:02 /bin/sh ../libtool?? --mode=install /usr/bin/install 
> -c ?libgettextlib.la '/home/jkt/work/prog/_build/br-cfb/host/lib'
> 2019-03-06T12:50:02 /usr/bin/install -c ethtool 
> '/home/jkt/work/prog/_build/br-cfb/target/usr/sbin'
> 2019-03-06T12:50:02 comm: 
> /home/jkt/work/prog/_build/br-cfb/build/.files-list-staging.new: No such 
> file or directory
> 2019-03-06T12:50:02 make[1]: *** [package/pkg-generic.mk:287: 
> /home/jkt/work/prog/_build/br-cfb/build/expat-2.2.6/.stamp_staging_installed] 
> Error 1
> 2019-03-06T12:50:02 make[1]: *** Waiting for unfinished jobs....

I've sent a patch patch for this 2 days ago (core/pkg-infra: Fix package 
files statistics for parallel build). I dont know if it will be accepted 
but would be interested if it works for you.


best regards,
Andreas

> 
> I can't offer much insight into how to best fix this, unfortunately, 
> whether to add some locking or whether to make this per-package.
> 
> This is just with a patched top-level Makefile to remove the 
> .NOTPARALLEL stanza, with no per-package build dire patches atec.
> 
> With kind regards,
> Jan
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
>