mbox series

[00/15] Overwritten file detection and fixes, one more step to TLP build

Message ID 20210621141130.48654-1-herve.codina@bootlin.com
Headers show
Series Overwritten file detection and fixes, one more step to TLP build | expand

Message

Herve Codina June 21, 2021, 2:11 p.m. UTC
Hi,

This series fixes some issues in the TLP build feature.
The starting point was the work done by Thomas Petazzoni on overwrites
detection (PATCH 1).

Based on this work, this series fixes some overwrites, introduces and uses
<PKG>_PER_PACKAGE_TWEAK_HOOKS. This variable is used to collect specific
per-package path tweaks needed by per-package build.
With this tweaks collected in this variable, they can be re-applied
when they are needed (<pkg>_{reconfigure,rebuild,reinstall}).

It also reworks the final {HOST,TARGET}_DIR rsync.
For each packages only files generated by the package are rsynced (PATCH 13).
This is done using an exclusion list computed in (PATCH 12).
In order to avoid modifications performed in target-finalize step to be seen
as overwrites on next build, the final {HOST,TARGET}_DIR do not contain any
hardlink anymore and are a full copy of expected files and directories (PATCH 14).

The last patch (PATCH 15) fixes <pkg>_{reconfigure,rebuild,reinstall} recreating
per-package {HOST,TARGET}_DIR from scratch.
Indeed, for a given package, re-generating the same file is detected as an overwrite.
The simplest way to avoid this is to fully discard previous per-package {HOST,TARGET}_DIR
directory and recreate them.

To have a TLP build fully fonctionnal, several things are still missing:
  - Be sure that all buildroot packages do not perform any overwrites.
  - Undo tweak stuff that have be done for per-package build (fixup-libtool-files,
    fixup-python-files, <PKG>_PER_PACKAGE_TWEAK_HOOKS) after collecting files
    in final {HOST,STAGING}_DIR.
    This mainly means reverting .../per-package/xxxxx/... path to point to
    the correct ones in final {HOST,STAGING}_DIR.

I hope this series will help in TLP build feature.

Best regards,
Hervé Codina

Herve Codina (14):
  package/e2fsprogs: fix fsck overwrite in HOST_DIR
  package/pkg-generic.mk: Remove Info documents dir entry
  package/pkg-generic.mk: Fix .la files overwrite detection
  package/pkg-generic.mk: Perform .la files fixup in per-package
    HOST_DIR
  package/pkg-generic.mk: Introduce <PKG>_PER_PACKAGE_TWEAK_HOOKS
  package/apr-util: Use <PKG>_PER_PACKAGE_TWEAK_HOOKS
  package/apache: Move APACHE_FIXUP_APR_LIBTOOL to
    <PKG>_PER_PACKAGE_TWEAK_HOOKS
  package/pkg-python: Remove _sysconfigdata*.pyc files when
    _sysconfigdata*.py are changed
  package/pkg-generic.mk: Move python fixup to generic package
    infrastructure
  package/owfs: Remove Python sysconfigdata fixup
  package/pkg-generic.mk: Generate final rsync exclude file list
  Makefile: Rsync global {TARGET,HOST}_DIR using exclusion file list
  Makefile: Breaks hardlinks in global {TARGET,HOST}_DIR on per-package
    build
  package/pkg-generic.mk: Fix per-package
    <pkg>-{reconfigure,rebuild,reinstall}

Thomas Petazzoni (1):
  package/pkg-generic.mk: detect files overwritten in TARGET_DIR and
    HOST_DIR

 Makefile                       |  24 +++++++-
 package/apache/apache.mk       |   2 +-
 package/apr-util/apr-util.mk   |  12 ++--
 package/e2fsprogs/e2fsprogs.mk |   1 +
 package/owfs/owfs.mk           |   9 ---
 package/pkg-generic.mk         | 103 ++++++++++++++++++++++++++++++++-
 package/pkg-python.mk          |  10 ----
 7 files changed, 134 insertions(+), 27 deletions(-)

Comments

Arnout Vandecappelle June 21, 2021, 8:42 p.m. UTC | #1
Hi Hervé,

 Great that someone is picking thuis up!

On 21/06/2021 16:11, Herve Codina wrote:
> Hi,
> 
> This series fixes some issues in the TLP build feature.
> The starting point was the work done by Thomas Petazzoni on overwrites
> detection (PATCH 1).

 I've marked [1] as Changes Requested.

 However, it seems the tests [2][3][4] are missing from this series. Could you
include them next time you post it?

> Based on this work, this series fixes some overwrites, introduces and uses
> <PKG>_PER_PACKAGE_TWEAK_HOOKS. This variable is used to collect specific
> per-package path tweaks needed by per-package build.
> With this tweaks collected in this variable, they can be re-applied
> when they are needed (<pkg>_{reconfigure,rebuild,reinstall}).

 That's excellent!


> It also reworks the final {HOST,TARGET}_DIR rsync.
> For each packages only files generated by the package are rsynced (PATCH 13).
> This is done using an exclusion list computed in (PATCH 12).
> In order to avoid modifications performed in target-finalize step to be seen
> as overwrites on next build, the final {HOST,TARGET}_DIR do not contain any
> hardlink anymore and are a full copy of expected files and directories (PATCH 14).

 Hm, that will have quite the impact on disk size, no? In non-PPS builds,
HOST_DIR is typically about 1/3 of the total size (the other 2/3 is BUILD_DIR,
the rest is negligible). By doing a non-hardlinked copy, the size of HOST_DIR is
basically going to double, right?

 I still think it's acceptable, but it's something to take into account...


> The last patch (PATCH 15) fixes <pkg>_{reconfigure,rebuild,reinstall} recreating
> per-package {HOST,TARGET}_DIR from scratch.
> Indeed, for a given package, re-generating the same file is detected as an overwrite.
> The simplest way to avoid this is to fully discard previous per-package {HOST,TARGET}_DIR
> directory and recreate them.

 +1 to that!


> To have a TLP build fully fonctionnal, several things are still missing:
>   - Be sure that all buildroot packages do not perform any overwrites.

 I guess that's easy: increase the chance of PER_PACKAGE enabled in
genrandconfig, and see what happens to the autobuilders. Or is there anything
else missing?

>   - Undo tweak stuff that have be done for per-package build (fixup-libtool-files,
>     fixup-python-files, <PKG>_PER_PACKAGE_TWEAK_HOOKS) after collecting files
>     in final {HOST,STAGING}_DIR.
>     This mainly means reverting .../per-package/xxxxx/... path to point to
>     the correct ones in final {HOST,STAGING}_DIR.
> 
> I hope this series will help in TLP build feature.


 Regards,
 Arnout

[1]
https://patchwork.ozlabs.org/project/buildroot/patch/20200430095249.782597-9-thomas.petazzoni@bootlin.com/
[2]
https://patchwork.ozlabs.org/project/buildroot/patch/20200430095249.782597-10-thomas.petazzoni@bootlin.com/
[3]
https://patchwork.ozlabs.org/project/buildroot/patch/20200430095249.782597-11-thomas.petazzoni@bootlin.com/
[4]
https://patchwork.ozlabs.org/project/buildroot/patch/20200430095249.782597-12-thomas.petazzoni@bootlin.com/



> 
> Best regards,
> Hervé Codina
> 
> Herve Codina (14):
>   package/e2fsprogs: fix fsck overwrite in HOST_DIR
>   package/pkg-generic.mk: Remove Info documents dir entry
>   package/pkg-generic.mk: Fix .la files overwrite detection
>   package/pkg-generic.mk: Perform .la files fixup in per-package
>     HOST_DIR
>   package/pkg-generic.mk: Introduce <PKG>_PER_PACKAGE_TWEAK_HOOKS
>   package/apr-util: Use <PKG>_PER_PACKAGE_TWEAK_HOOKS
>   package/apache: Move APACHE_FIXUP_APR_LIBTOOL to
>     <PKG>_PER_PACKAGE_TWEAK_HOOKS
>   package/pkg-python: Remove _sysconfigdata*.pyc files when
>     _sysconfigdata*.py are changed
>   package/pkg-generic.mk: Move python fixup to generic package
>     infrastructure
>   package/owfs: Remove Python sysconfigdata fixup
>   package/pkg-generic.mk: Generate final rsync exclude file list
>   Makefile: Rsync global {TARGET,HOST}_DIR using exclusion file list
>   Makefile: Breaks hardlinks in global {TARGET,HOST}_DIR on per-package
>     build
>   package/pkg-generic.mk: Fix per-package
>     <pkg>-{reconfigure,rebuild,reinstall}
> 
> Thomas Petazzoni (1):
>   package/pkg-generic.mk: detect files overwritten in TARGET_DIR and
>     HOST_DIR
> 
>  Makefile                       |  24 +++++++-
>  package/apache/apache.mk       |   2 +-
>  package/apr-util/apr-util.mk   |  12 ++--
>  package/e2fsprogs/e2fsprogs.mk |   1 +
>  package/owfs/owfs.mk           |   9 ---
>  package/pkg-generic.mk         | 103 ++++++++++++++++++++++++++++++++-
>  package/pkg-python.mk          |  10 ----
>  7 files changed, 134 insertions(+), 27 deletions(-)
>
Yann E. MORIN June 24, 2021, 8:53 p.m. UTC | #2
Hervé, All,

On 2021-06-21 16:11 +0200, Herve Codina spake thusly:
> This series fixes some issues in the TLP build feature.
> The starting point was the work done by Thomas Petazzoni on overwrites
> detection (PATCH 1).
[--SNIP--]

Thanks for working on this complex topic. :-)

I'm now done with my review; I may have missed details of lesser
importance, so we'll deal with those later; Thomas also provided some
feedback. I see that you have already taken initial reviews into
account, and that you seem nicely on track for a v2.

As such, I've marked this entire series as "changes requested" in
patchwork.

Can you add me in Cc of the full series when you later respin, please?

Regards,
Yann E. MORIN.
Andreas Naumann June 25, 2021, 9:08 a.m. UTC | #3
Hi Herve, all,

On 21.06.21 16:11, Herve Codina wrote:
> Hi,
> 
> This series fixes some issues in the TLP build feature.
> The starting point was the work done by Thomas Petazzoni on overwrites
> detection (PATCH 1).

after a long time I have recently picked up playing with TLP build 
again. As part of that I found a problem in qt5 where I sent a patch two 
days ago "qt5: Fix sporadic build failure during top-level parallel 
build" which is caused by manipulation of the hard-linked qt.conf in 
HOST_DIR from different qt5 packages.

Now I see your promising set, but the matter is quite complex and so I 
wonder if the "overwritten file detection" would a) uncover the qt.conf 
problem and b)if your patch series somehow fixes it in a generic way?


regards,
Andreas



> 
> Based on this work, this series fixes some overwrites, introduces and uses
> <PKG>_PER_PACKAGE_TWEAK_HOOKS. This variable is used to collect specific
> per-package path tweaks needed by per-package build.
> With this tweaks collected in this variable, they can be re-applied
> when they are needed (<pkg>_{reconfigure,rebuild,reinstall}).
> 
> It also reworks the final {HOST,TARGET}_DIR rsync.
> For each packages only files generated by the package are rsynced (PATCH 13).
> This is done using an exclusion list computed in (PATCH 12).
> In order to avoid modifications performed in target-finalize step to be seen
> as overwrites on next build, the final {HOST,TARGET}_DIR do not contain any
> hardlink anymore and are a full copy of expected files and directories (PATCH 14).
> 
> The last patch (PATCH 15) fixes <pkg>_{reconfigure,rebuild,reinstall} recreating
> per-package {HOST,TARGET}_DIR from scratch.
> Indeed, for a given package, re-generating the same file is detected as an overwrite.
> The simplest way to avoid this is to fully discard previous per-package {HOST,TARGET}_DIR
> directory and recreate them.
> 
> To have a TLP build fully fonctionnal, several things are still missing:
>    - Be sure that all buildroot packages do not perform any overwrites.
>    - Undo tweak stuff that have be done for per-package build (fixup-libtool-files,
>      fixup-python-files, <PKG>_PER_PACKAGE_TWEAK_HOOKS) after collecting files
>      in final {HOST,STAGING}_DIR.
>      This mainly means reverting .../per-package/xxxxx/... path to point to
>      the correct ones in final {HOST,STAGING}_DIR.
> 
> I hope this series will help in TLP build feature.
> 
> Best regards,
> Hervé Codina
> 
> Herve Codina (14):
>    package/e2fsprogs: fix fsck overwrite in HOST_DIR
>    package/pkg-generic.mk: Remove Info documents dir entry
>    package/pkg-generic.mk: Fix .la files overwrite detection
>    package/pkg-generic.mk: Perform .la files fixup in per-package
>      HOST_DIR
>    package/pkg-generic.mk: Introduce <PKG>_PER_PACKAGE_TWEAK_HOOKS
>    package/apr-util: Use <PKG>_PER_PACKAGE_TWEAK_HOOKS
>    package/apache: Move APACHE_FIXUP_APR_LIBTOOL to
>      <PKG>_PER_PACKAGE_TWEAK_HOOKS
>    package/pkg-python: Remove _sysconfigdata*.pyc files when
>      _sysconfigdata*.py are changed
>    package/pkg-generic.mk: Move python fixup to generic package
>      infrastructure
>    package/owfs: Remove Python sysconfigdata fixup
>    package/pkg-generic.mk: Generate final rsync exclude file list
>    Makefile: Rsync global {TARGET,HOST}_DIR using exclusion file list
>    Makefile: Breaks hardlinks in global {TARGET,HOST}_DIR on per-package
>      build
>    package/pkg-generic.mk: Fix per-package
>      <pkg>-{reconfigure,rebuild,reinstall}
> 
> Thomas Petazzoni (1):
>    package/pkg-generic.mk: detect files overwritten in TARGET_DIR and
>      HOST_DIR
> 
>   Makefile                       |  24 +++++++-
>   package/apache/apache.mk       |   2 +-
>   package/apr-util/apr-util.mk   |  12 ++--
>   package/e2fsprogs/e2fsprogs.mk |   1 +
>   package/owfs/owfs.mk           |   9 ---
>   package/pkg-generic.mk         | 103 ++++++++++++++++++++++++++++++++-
>   package/pkg-python.mk          |  10 ----
>   7 files changed, 134 insertions(+), 27 deletions(-)
>
Herve Codina June 25, 2021, 1:13 p.m. UTC | #4
Hi,

On Fri, 25 Jun 2021 11:08:43 +0200
Andreas Naumann <dev@andin.de> wrote:

> after a long time I have recently picked up playing with TLP build 
> again. As part of that I found a problem in qt5 where I sent a patch two 
> days ago "qt5: Fix sporadic build failure during top-level parallel 
> build" which is caused by manipulation of the hard-linked qt.conf in 
> HOST_DIR from different qt5 packages.
> 
> Now I see your promising set, but the matter is quite complex and so I 
> wonder if the "overwritten file detection" would a) uncover the qt.conf 
> problem and b)if your patch series somehow fixes it in a generic way?
> 
> 

I quickly looked at your patch and it looks like an overwrite issue.

Your patch:
---- 8< ----
 # compiled into the Qt library. We need it to make "qmake" relocatable and
 # tweak the per-package install pathes
 define QT5_INSTALL_QT_CONF
+	rm -f $(HOST_DIR)/bin/qt.conf
 	sed -e "s|@@HOST_DIR@@|$(HOST_DIR)|" -e "s|@@STAGING_DIR@@|$(STAGING_DIR)|" \
 		$(QT5BASE_PKGDIR)/qt.conf.in > $(HOST_DIR)/bin/qt.conf
 endef
---- 8< ----

This overwrite is done by sed whose output is redirected $(HOST_DIR)/bin/qt.conf

With TLP this overwite leads to package A seeing modification done by package B

Without your patch and with overwrite detection present in my series
the overwrite should be detected and the build stopped.

My series do not contains any fixes for this specific Qt5 overwrite.
This is a concrete example of what I said still missing ...

Your 'rm' breaks the hardlink and so the following sed does not perform
overwrite anymore.
That's the correct fix.

To go further with interactions with my current series still ongoing and not ready
to be merged, the entire macro (QT5_INSTALL_QT_CONF) should be moved to
<PKG>_PER_PACKAGE_TWEAK_HOOKS and QT5_QT_CONF_FIXUP is no more needed.

Hervé
Andreas Naumann June 25, 2021, 2:55 p.m. UTC | #5
Hi Herve,

On 25.06.21 15:13, Herve Codina wrote:
> Hi,
> 
> On Fri, 25 Jun 2021 11:08:43 +0200
> Andreas Naumann <dev@andin.de> wrote:
> 
>> after a long time I have recently picked up playing with TLP build
>> again. As part of that I found a problem in qt5 where I sent a patch two
>> days ago "qt5: Fix sporadic build failure during top-level parallel
>> build" which is caused by manipulation of the hard-linked qt.conf in
>> HOST_DIR from different qt5 packages.
>>
>> Now I see your promising set, but the matter is quite complex and so I
>> wonder if the "overwritten file detection" would a) uncover the qt.conf
>> problem and b)if your patch series somehow fixes it in a generic way?
>>
>>
> 
> I quickly looked at your patch and it looks like an overwrite issue.
> 
> Your patch:
> ---- 8< ----
>   # compiled into the Qt library. We need it to make "qmake" relocatable and
>   # tweak the per-package install pathes
>   define QT5_INSTALL_QT_CONF
> +	rm -f $(HOST_DIR)/bin/qt.conf
>   	sed -e "s|@@HOST_DIR@@|$(HOST_DIR)|" -e "s|@@STAGING_DIR@@|$(STAGING_DIR)|" \
>   		$(QT5BASE_PKGDIR)/qt.conf.in > $(HOST_DIR)/bin/qt.conf
>   endef
> ---- 8< ----
> 
> This overwrite is done by sed whose output is redirected $(HOST_DIR)/bin/qt.conf
> 
> With TLP this overwite leads to package A seeing modification done by package B
> 
> Without your patch and with overwrite detection present in my series
> the overwrite should be detected and the build stopped.
> 
> My series do not contains any fixes for this specific Qt5 overwrite.
> This is a concrete example of what I said still missing ...

Thank you for the confirmation.

> 
> Your 'rm' breaks the hardlink and so the following sed does not perform
> overwrite anymore.
> That's the correct fix.
> 
> To go further with interactions with my current series still ongoing and not ready
> to be merged, the entire macro (QT5_INSTALL_QT_CONF) should be moved to
> <PKG>_PER_PACKAGE_TWEAK_HOOKS and QT5_QT_CONF_FIXUP is no more needed.

Ok, I'll try to catch the point when your set is accepted and the repost 
an updated version.


have a nice weekend,
Andreas


> 
> Hervé
>
Herve Codina July 6, 2021, 2:15 p.m. UTC | #6
Hi,

On Mon, 21 Jun 2021 22:42:18 +0200
Arnout Vandecappelle <arnout@mind.be> wrote:

>  However, it seems the tests [2][3][4] are missing from this series. Could you
> include them next time you post it?

Added in v2 series.