[Buildroot] [PATCH v3 0/3] [RFC] Reinstall Targets

[Buildroot] [PATCH v3 0/3] [RFC] Reinstall Targets

[Doug Kehn]

This RFC proposes adding the following targets to pkg-generic.mk:
    <package>-reinstall
    <package>-reinstall-target
    <package>-reinstall-staging
    <package>-reinstall-images
    <package>-reinstall-host

These targets remove the package's .stamp_*_installed file to allow the
reinstall to execute.  The only method I presently know of to reinstall a
package is to make <package>-rebuild.  The proposed targets allow a package
reinstall without, unnecessarily incurring a rebuild of the package.

In addition to allowing a reinstall to staging and/or target output
directories, the proposed targets allow selected packages to be
installed to a different 'target' directory.  For example, I attempt to
keep software updates as small as possible; therefore, I cherry-pick
files to include in the update package.  If I know packages foo and bar
have been updated, the following will install those packages to a
directory that can be further processed to generate the desired update
package.

make TARGET_DIR=${PWD}/update foo-reinstall-target

make TARGET_DIR=${PWD}/update bar-reinstall-target

Doug Kehn (3):
  pkg-generic.mk: reinstall targets
  manual: reinstall targets
  manual: style update

 docs/manual/package-make-target.txt | 26 ++++++++++++++++++++------
 package/pkg-generic.mk              | 22 ++++++++++++++++++++++
 2 files changed, 42 insertions(+), 6 deletions(-)

---
Changes v2 -> v3:
  - Move style updates to separate patch (requested by Thomos)

Changes v1 -> v2:
  - Reworked to include reinstall target (requested by Thomas)
  - Updated documentation

-- 
1.9.1

[Buildroot] [PATCH v3 1/3] pkg-generic.mk: reinstall targets

[Doug Kehn]

Add reinstall targets for host, target, staging, and images variants.
clean-for-reinstall targets added to remove package
.stamp_target_install file to allow package install.  Additionally, when
OVERRIDE_SRCDIR is provided, .stamp_rsynced is removed to ensure pakcage
is up to date before reinstalling.

Signed-off-by: Doug Kehn <rdkehn@yahoo.com>
---
 package/pkg-generic.mk | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index 9643a30..0fe7059 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -480,34 +480,51 @@ $(1):			$(1)-install
 
 ifeq ($$($(2)_TYPE),host)
 $(1)-install:	        $(1)-install-host
+$(1)-reinstall:		$(1)-reinstall-host
 else
 $(1)-install:		$(1)-install-staging $(1)-install-target $(1)-install-images
+$(1)-reinstall:		$(1)-reinstall-staging $(1)-reinstall-target $(1)-reinstall-images
 endif
 
 ifeq ($$($(2)_INSTALL_TARGET),YES)
 $(1)-install-target:		$$($(2)_TARGET_INSTALL_TARGET)
+$(1)-reinstall-target:		$(1)-clean-for-reinstall-target $$($(2)_TARGET_INSTALL_TARGET)
+$(1)-clean-for-reinstall-target:$(1)-clean-for-reinstall
+			rm -f $$($(2)_TARGET_INSTALL_TARGET)
 $$($(2)_TARGET_INSTALL_TARGET):	$$($(2)_TARGET_BUILD)
 else
 $(1)-install-target:
+$(1)-reinstall-target:
 endif
 
 ifeq ($$($(2)_INSTALL_STAGING),YES)
 $(1)-install-staging:			$$($(2)_TARGET_INSTALL_STAGING)
+$(1)-reinstall-staging:			$(1)-clean-for-reinstall-staging $$($(2)_TARGET_INSTALL_STAGING)
+$(1)-clean-for-reinstall-staging:	$(1)-clean-for-reinstall
+			rm -f $$($(2)_TARGET_INSTALL_STAGING)
 $$($(2)_TARGET_INSTALL_STAGING):	$$($(2)_TARGET_BUILD)
 # Some packages use install-staging stuff for install-target
 $$($(2)_TARGET_INSTALL_TARGET):		$$($(2)_TARGET_INSTALL_STAGING)
 else
 $(1)-install-staging:
+$(1)-reinstall-staging:
 endif
 
 ifeq ($$($(2)_INSTALL_IMAGES),YES)
 $(1)-install-images:		$$($(2)_TARGET_INSTALL_IMAGES)
+$(1)-reinstall-images:		$(1)-clean-for-reinstall-images $$($(2)_TARGET_INSTALL_IMAGES)
+$(1)-clean-for-reinstall-images:$(1)-clean-for-reinstall
+			rm -f $$($(2)_TARGET_INSTALL_IMAGES)
 $$($(2)_TARGET_INSTALL_IMAGES):	$$($(2)_TARGET_BUILD)
 else
 $(1)-install-images:
+$(1)-reinstall-images:
 endif
 
 $(1)-install-host:      	$$($(2)_TARGET_INSTALL_HOST)
+$(1)-reinstall-host:      	$(1)-clean-for-reinstall-host $$($(2)_TARGET_INSTALL_HOST)
+$(1)-clean-for-reinstall-host:	$(1)-clean-for-reinstall
+			rm -f $$($(2)_TARGET_INSTALL_HOST)
 $$($(2)_TARGET_INSTALL_HOST):	$$($(2)_TARGET_BUILD)
 
 $(1)-build:		$$($(2)_TARGET_BUILD)
@@ -545,6 +562,8 @@ $$($(2)_TARGET_EXTRACT):	$$($(2)_TARGET_SOURCE)
 $(1)-depends:		$$($(2)_FINAL_DEPENDENCIES)
 
 $(1)-source:		$$($(2)_TARGET_SOURCE)
+
+$(1)-clean-for-reinstall:
 else
 # In the package override case, the sequence of steps
 #  source, by rsyncing
@@ -563,6 +582,9 @@ $(1)-extract:		$(1)-rsync
 $(1)-rsync:		$$($(2)_TARGET_RSYNC)
 
 $(1)-source:		$$($(2)_TARGET_RSYNC_SOURCE)
+
+$(1)-clean-for-reinstall:
+			rm -f $$($(2)_TARGET_RSYNC)
 endif
 
 $(1)-show-depends:
-- 
1.9.1

[Buildroot] [PATCH v3 1/3] pkg-generic.mk: reinstall targets

[Yann E. MORIN]

Doug, All,

Sorry to come back to this so late...

On 2014-11-28 09:25 -0600, Doug Kehn spake thusly:
> Add reinstall targets for host, target, staging, and images variants.
> clean-for-reinstall targets added to remove package
> .stamp_target_install file to allow package install.  Additionally, when
> OVERRIDE_SRCDIR is provided, .stamp_rsynced is removed to ensure pakcage
> is up to date before reinstalling.
> 
> Signed-off-by: Doug Kehn <rdkehn@yahoo.com>

I have some comments below...

> ---
>  package/pkg-generic.mk | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index 9643a30..0fe7059 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -480,34 +480,51 @@ $(1):			$(1)-install
>  
>  ifeq ($$($(2)_TYPE),host)
>  $(1)-install:	        $(1)-install-host
> +$(1)-reinstall:		$(1)-reinstall-host

Indentation here (and in other places in that file) is off, but that's
not your fault: the existing code is already borked.

>  else
>  $(1)-install:		$(1)-install-staging $(1)-install-target $(1)-install-images
> +$(1)-reinstall:		$(1)-reinstall-staging $(1)-reinstall-target $(1)-reinstall-images
>  endif
>  
>  ifeq ($$($(2)_INSTALL_TARGET),YES)
>  $(1)-install-target:		$$($(2)_TARGET_INSTALL_TARGET)
> +$(1)-reinstall-target:		$(1)-clean-for-reinstall-target $$($(2)_TARGET_INSTALL_TARGET)
> +$(1)-clean-for-reinstall-target:$(1)-clean-for-reinstall
> +			rm -f $$($(2)_TARGET_INSTALL_TARGET)

I am not very happy with the fact that we add new actions here; the rest
of the code block is so far only concerned about ordering, not the
actual commands to execute.

The only other similar make target that removes stuff is the
$(1)-dirclean target, that uses a fake stamp-file (that it actually does
not touch) to actually does the cleanup.

I agree this is a bit convoluted, but I'd prefer we keep the current
layout: the code around here gets concerned about ordering, and we
actually implement the rules above (around line 248-250), using a fake
stamp file (obviously, one for each new -reinstall target).

Are you still interested in this? Would you mind looking at that?

Again, sorry for the delay in reviewing this series... :-/

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH v3 1/3] pkg-generic.mk: reinstall targets

[Thomas Petazzoni]

Dear Yann E. MORIN,

On Sun, 1 Feb 2015 11:55:01 +0100, Yann E. MORIN wrote:

> I am not very happy with the fact that we add new actions here; the rest
> of the code block is so far only concerned about ordering, not the
> actual commands to execute.
> 
> The only other similar make target that removes stuff is the
> $(1)-dirclean target, that uses a fake stamp-file (that it actually does
> not touch) to actually does the cleanup.

I am not sure what you mean here. Look at the existing
$(1)-clean-for-rebuild and $(1)-clean-for-reconfigure targets. They
also do some commands to remove the stamp files. So what Doug has done
here is to simply replicate the same logic, but removing less stamp
files to only retrigger installation and not configuration and/or
compilation.

So I believe the implementation proposed by Doug is well-aligned with
what we already do. Unless I misunderstood your comment, obviously.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v3 1/3] pkg-generic.mk: reinstall targets

[Yann E. MORIN]

Thomas, Doug, All,

On 2015-02-01 11:59 +0100, Thomas Petazzoni spake thusly:
> On Sun, 1 Feb 2015 11:55:01 +0100, Yann E. MORIN wrote:
> > I am not very happy with the fact that we add new actions here; the rest
> > of the code block is so far only concerned about ordering, not the
> > actual commands to execute.
> > 
> > The only other similar make target that removes stuff is the
> > $(1)-dirclean target, that uses a fake stamp-file (that it actually does
> > not touch) to actually does the cleanup.
> 
> I am not sure what you mean here. Look at the existing
> $(1)-clean-for-rebuild and $(1)-clean-for-reconfigure targets. They
> also do some commands to remove the stamp files. So what Doug has done
> here is to simply replicate the same logic, but removing less stamp
> files to only retrigger installation and not configuration and/or
> compilation.

Ah, indeed, I missed it as it was below the code touched for the
reinstall targets.

> So I believe the implementation proposed by Doug is well-aligned with
> what we already do. Unless I misunderstood your comment, obviously.

No, that's Ok, I did not look further than the part actually touched by
the patch.

So, obviously, I withdraw my comment.

Regards,
Yann E. MORIN.

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

[Buildroot] [PATCH v3 1/3] pkg-generic.mk: reinstall targets

[Thomas Petazzoni]

Dear Doug Kehn,

On Fri, 28 Nov 2014 09:25:04 -0600, Doug Kehn wrote:
> Add reinstall targets for host, target, staging, and images variants.
> clean-for-reinstall targets added to remove package
> .stamp_target_install file to allow package install.  Additionally, when
> OVERRIDE_SRCDIR is provided, .stamp_rsynced is removed to ensure pakcage
> is up to date before reinstalling.
> 
> Signed-off-by: Doug Kehn <rdkehn@yahoo.com>
> ---
>  package/pkg-generic.mk | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)

Thanks, we applied your patch, but with a much simplified version.
Since having <pkg>-reinstall-{images,host,staging,target} was probably
not necessary, we just implemented one <pkg>-reinstall target, that
reinstalls whatever the package wants to install. This allows to make
the implementation a lot more similar to the <pkg>-rebuild and
<pkg>-reconfigure implementations.

See
http://git.buildroot.net/buildroot/commit/?id=b7bc44d22d0fddde7c964a2fd259775ea8d0bf9f
for the final commit.

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v3 2/3] manual: reinstall targets

[Doug Kehn]

Add reinstall target information to package-make-target.txt.

Signed-off-by: Doug Kehn <rdkehn@yahoo.com>
---
 docs/manual/package-make-target.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/docs/manual/package-make-target.txt b/docs/manual/package-make-target.txt
index 25d3f44..0f0aced 100644
--- a/docs/manual/package-make-target.txt
+++ b/docs/manual/package-make-target.txt
@@ -77,4 +77,18 @@ directly in the build directory
 makes sense when using the +OVERRIDE_SRCDIR+ feature or when you modified a
 file directly in the build directory
 
+| +reinstall-staging+ |
+*target package:* Re-run the installation of the package in the staging
+directory - this only makes sense when overriding +STAGING_DIR+
+
+| +reinstall-target+  |
+*target package:* Re-run the installation of the package in the target
+directory - this only makes sense when overriding +TARGET_DIR+
+
+| +reinstall+         |
+*target package:* Run the 2 previous re-installation commands
+
+*host package:* Re-run the installation of the package in the host
+directory
+
 |===================================================
-- 
1.9.1

[Buildroot] [PATCH v3 2/3] manual: reinstall targets

[Thomas Petazzoni]

Dear Doug Kehn,

On Fri, 28 Nov 2014 09:25:05 -0600, Doug Kehn wrote:
> Add reinstall target information to package-make-target.txt.
> 
> Signed-off-by: Doug Kehn <rdkehn@yahoo.com>
> ---
>  docs/manual/package-make-target.txt | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Thanks, applied, after adjusting to the fairly significant changes we
did on PATCH 1/3.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v3 2/3] manual: reinstall targets

[rdkehn at yahoo.com]

On Tue, Feb 03, 2015 at 06:11:47PM +0100, Thomas Petazzoni wrote:
Hi Thomas,

> Dear Doug Kehn,
> 
> On Fri, 28 Nov 2014 09:25:05 -0600, Doug Kehn wrote:
> > Add reinstall target information to package-make-target.txt.
> > 
> > Signed-off-by: Doug Kehn <rdkehn@yahoo.com>
> > ---
> >  docs/manual/package-make-target.txt | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> 
> Thanks, applied, after adjusting to the fairly significant changes we
> did on PATCH 1/3.

Agree, simpler is always better.  Thank you for your consideration.

Regards,
...doug

[Buildroot] [PATCH v3 3/3] manual: style update

[Doug Kehn]

Align column separators.

Signed-off-by: Doug Kehn <rdkehn@yahoo.com>
---
 docs/manual/package-make-target.txt | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/docs/manual/package-make-target.txt b/docs/manual/package-make-target.txt
index 0f0aced..35d787e 100644
--- a/docs/manual/package-make-target.txt
+++ b/docs/manual/package-make-target.txt
@@ -57,23 +57,23 @@ Additionally, there are some other useful make targets:
 
 [width="90%",cols="^1,4",options="header"]
 |===================================================
-| command/target    | Description
+| command/target      | Description
 
-| +show-depends+    | Displays the dependencies required to build the
+| +show-depends+      | Displays the dependencies required to build the
 package
 
-| +graph-depends+   | Generate a dependency graph of the package, in the
+| +graph-depends+     | Generate a dependency graph of the package, in the
 context of the current Buildroot configuration. See
 xref:graph-depends[this section] for more details about dependency
 graphs.
 
-| +dirclean+        | Remove the whole package build directory
+| +dirclean+          | Remove the whole package build directory
 
-| +rebuild+         | Re-run the compilation commands - this only makes
+| +rebuild+           | Re-run the compilation commands - this only makes
 sense when using the +OVERRIDE_SRCDIR+ feature or when you modified a file
 directly in the build directory
 
-| +reconfigure+     | Re-run the configure commands, then rebuild - this only
+| +reconfigure+       | Re-run the configure commands, then rebuild - this only
 makes sense when using the +OVERRIDE_SRCDIR+ feature or when you modified a
 file directly in the build directory
 
-- 
1.9.1

[Buildroot] [PATCH v3 3/3] manual: style update

[Thomas Petazzoni]

Dear Doug Kehn,

On Fri, 28 Nov 2014 09:25:06 -0600, Doug Kehn wrote:
> Align column separators.
> 
> Signed-off-by: Doug Kehn <rdkehn@yahoo.com>
> ---
>  docs/manual/package-make-target.txt | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)

This patch was no longer needed after our changes to PATCH 2/3, so we
marked it as "Non Applicable" in our patch tracking system.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH v3 0/3] [RFC] Reinstall Targets

[Yann E. MORIN]

Doug, All,

On 2014-11-28 09:25 -0600, Doug Kehn spake thusly:
> This RFC proposes adding the following targets to pkg-generic.mk:
>     <package>-reinstall
>     <package>-reinstall-target
>     <package>-reinstall-staging
>     <package>-reinstall-images
>     <package>-reinstall-host
> 
> These targets remove the package's .stamp_*_installed file to allow the
> reinstall to execute.  The only method I presently know of to reinstall a
> package is to make <package>-rebuild.  The proposed targets allow a package
> reinstall without, unnecessarily incurring a rebuild of the package.

We've discussed this series during the Patchwork cleanupsession
yesterday.

Peter was concerned about the overhead of those new rules, so it was
decided I would do some testing before/after this pach was applied.

Here are the results of my testing.

All testing was done on the same machine, Core-i5 @2.5GHz, 12GiB RAM,
and Samsung 840pro SSD, machine otherwise compeltely idle. Before
running the tests, I warmed the cache with: tar cf - . >/dev/null

All tests were run on top of ed592d6: poppler: Install files into staging
(master at the time I did the tests). Tests were out-of-tree, and the
output was consigned to oblivion with >/dev/null an executed in the
displayed sequence.

                                            Before      After

  * make defconfig                           1.28        1.27
    make defconfig                           0.34        0.34
    [disable luajit]
    make help                                2.72        2.79

  * make defconfig allyespackageconfig       1.34        1.38
    make defconfig allyespackageconfig       0.44        0.44
    [disable luajit]
    make help                                2.77        2.87
    [switch to external toolchain, make allyespackageconfig, disable luajit]
    make toolchain                          21.05       21.73

[*] marks starting from a clean output directory.

I had to disable luajit, because both it and lua are providers of
luainterpreter.

So, it seems the overhead is negligible.

So, the conclusion is: OK! ;-)

I'll do some further review on the code a bit later, but otherwise we
decided to accept that feature.

Thank you!

Regards,
Yann E. MORIN.

> In addition to allowing a reinstall to staging and/or target output
> directories, the proposed targets allow selected packages to be
> installed to a different 'target' directory.  For example, I attempt to
> keep software updates as small as possible; therefore, I cherry-pick
> files to include in the update package.  If I know packages foo and bar
> have been updated, the following will install those packages to a
> directory that can be further processed to generate the desired update
> package.
> 
> make TARGET_DIR=${PWD}/update foo-reinstall-target
> 
> make TARGET_DIR=${PWD}/update bar-reinstall-target
> 
> Doug Kehn (3):
>   pkg-generic.mk: reinstall targets
>   manual: reinstall targets
>   manual: style update
> 
>  docs/manual/package-make-target.txt | 26 ++++++++++++++++++++------
>  package/pkg-generic.mk              | 22 ++++++++++++++++++++++
>  2 files changed, 42 insertions(+), 6 deletions(-)
> 
> ---
> Changes v2 -> v3:
>   - Move style updates to separate patch (requested by Thomos)
> 
> Changes v1 -> v2:
>   - Reworked to include reinstall target (requested by Thomas)
>   - Updated documentation
> 
> -- 
> 1.9.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 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'