[1/4] post-{build, images} hooks: export BUILD_DIR too

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Also export BUILD_DIR for post-{build,images} hooks, so they do have
a place to store generated files.

Note: this will be more einteresting for the instrumentation of steps,
to come in a later patch.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
 Makefile                         | 4 ++--
 docs/manual/customize-rootfs.txt | 5 +++--
 package/Makefile.in              | 5 +++++
 3 files changed, 10 insertions(+), 4 deletions(-)

"Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
>From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
>Also export BUILD_DIR for post-{build,images} hooks, so they do have
>a place to store generated files.
>
>Note: this will be more einteresting for the instrumentation of steps,
>to come in a later patch.
>
>Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>---
> Makefile                         | 4 ++--
> docs/manual/customize-rootfs.txt | 5 +++--
> package/Makefile.in              | 5 +++++
> 3 files changed, 10 insertions(+), 4 deletions(-)
>
>diff --git a/Makefile b/Makefile
>index f266e2d..0113f75 100644
>--- a/Makefile
>+++ b/Makefile
>@@ -511,7 +511,7 @@ endif
> 
> 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
> 		$(call MESSAGE,"Executing post-build script $(s)"); \
>-		$(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
>+		$(USER_HOOKS_EXTRA_ENV) $(s) $(TARGET_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
> 
> ifeq ($(BR2_ENABLE_LOCALE_PURGE),y)
> LOCALE_WHITELIST=$(BUILD_DIR)/locales.nopurge
>@@ -557,7 +557,7 @@ endif
> target-post-image:
> 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_IMAGE_SCRIPT)), \
> 		$(call MESSAGE,"Executing post-image script $(s)"); \
>-		$(s) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
>+		$(USER_HOOKS_EXTRA_ENV) $(s) $(BINARIES_DIR) $(call qstrip,$(BR2_ROOTFS_POST_SCRIPT_ARGS))$(sep))
> 
> toolchain-eclipse-register:
> 	./support/scripts/eclipse-register-toolchain `readlink -f $(O)` $(notdir $(TARGET_CROSS)) $(BR2_ARCH)
>diff --git a/docs/manual/customize-rootfs.txt b/docs/manual/customize-rootfs.txt
>index 49a6b4b..450b3d5 100644
>--- a/docs/manual/customize-rootfs.txt
>+++ b/docs/manual/customize-rootfs.txt
>@@ -41,6 +41,7 @@ there are a few ways to customize the resulting target filesystem.
>     - +BUILDROOT_CONFIG+: the path to the Buildroot .config file
>     - +HOST_DIR+, +STAGING_DIR+, +TARGET_DIR+: see
>       xref:generic-package-reference[]
>+    - +BUILD_DIR+: the directory where packages are extracted and built
>     - +BINARIES_DIR+: the place where all binary files (aka images) are
>       stored
>     - +BASE_DIR+: the base output directory
>@@ -78,8 +79,8 @@ in one of these _post-image scripts_ will require special handling
> 
> Just like for the _post-build scripts_ mentioned above, you also have
> access to the following environment variables from your _post-image
>-scripts_: +BUILDROOT_CONFIG+, +HOST_DIR+, +STAGING_DIR+, +TARGET_DIR+,
>-+BINARIES_DIR+ and +BASE_DIR+.
>+scripts_: +BUILDROOT_CONFIG+, +BUILD_DIR+, +HOST_DIR+, +STAGING_DIR+,
>++TARGET_DIR+, +BINARIES_DIR+ and +BASE_DIR+.
> 
> Additionally, each of the +BR2_ROOTFS_POST_BUILD_SCRIPT+ and
> +BR2_ROOTFS_POST_IMAGE_SCRIPT+ scripts will be passed the arguments
>diff --git a/package/Makefile.in b/package/Makefile.in
>index 612f3c7..2224f27 100644
>--- a/package/Makefile.in
>+++ b/package/Makefile.in
>@@ -275,6 +275,11 @@ HOST_MAKE_ENV=PATH=$(HOST_PATH) \
> 		PKG_CONFIG_LIBDIR="$(HOST_DIR)/usr/lib/pkgconfig" \
> 		PERLLIB="$(HOST_DIR)/usr/lib/perl"
> 
>+# This extra environment we can not export ourselves, so we have
>+# to expicitly pass it to user-supplied external hooks (eg.
>+# post-build, post-images)
>+USER_HOOKS_EXTRA_ENV=\
>+	BUILD_DIR=$(BUILD_DIR)

I'd add a bit more info here: we cannot export BUILD_DIR globally  because some packages, like uboot, also use this same variable for another purpose.

Best regards,
Thomas

Dear Thomas De Schampheleire,

On Sun, 27 Oct 2013 22:12:34 +0100, Thomas De Schampheleire wrote:

> >+# This extra environment we can not export ourselves, so we have
> >+# to expicitly pass it to user-supplied external hooks (eg.
> >+# post-build, post-images)
> >+USER_HOOKS_EXTRA_ENV=\
> >+	BUILD_DIR=$(BUILD_DIR)
> 
> I'd add a bit more info here: we cannot export BUILD_DIR globally  because some packages, like uboot, also use this same variable for another purpose.

(Unfortunately, we've asked you to not wrapped patches you reply to,
but now, it has the consequence that you no longer wrap what you write.
Well, I guess with K9 it's hard to get both).

Regarding BUILD_DIR and other variables being exported, is it really
safe to export other variables with a relatively generic name such as
TARGET_DIR, BASE_DIR, HOST_DIR, BINARIES_DIR, etc. ? Should we instead
expose them to post-build/post-image scripts as BR2_<something> ? But
well, of course, it means breaking the existing API, which isn't nice.

Thomas

Thomas DS, All,

On 2013-10-27 22:12 +0100, Thomas De Schampheleire spake thusly:
> "Yann E. MORIN" <yann.morin.1998@free.fr> wrote:
> >From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> >
> >Also export BUILD_DIR for post-{build,images} hooks, so they do have
> >a place to store generated files.
[--SNIP--]
> >+# This extra environment we can not export ourselves, so we have
> >+# to expicitly pass it to user-supplied external hooks (eg.
> >+# post-build, post-images)
> >+USER_HOOKS_EXTRA_ENV=\
> >+	BUILD_DIR=$(BUILD_DIR)
> 
> I'd add a bit more info here: we cannot export BUILD_DIR globally
> because some packages, like uboot, also use this same variable for
> another purpose.

Will do, thanks.

Regards,
Yann E. MORIN.

Thomas P, All,

On 2013-10-28 06:17 +0100, Thomas Petazzoni spake thusly:
> Dear Thomas De Schampheleire,
> 
> On Sun, 27 Oct 2013 22:12:34 +0100, Thomas De Schampheleire wrote:
> 
> > >+# This extra environment we can not export ourselves, so we have
> > >+# to expicitly pass it to user-supplied external hooks (eg.
> > >+# post-build, post-images)
> > >+USER_HOOKS_EXTRA_ENV=\
> > >+	BUILD_DIR=$(BUILD_DIR)
[--SNIP--]
> Regarding BUILD_DIR and other variables being exported, is it really
> safe to export other variables with a relatively generic name such as
> TARGET_DIR, BASE_DIR, HOST_DIR, BINARIES_DIR, etc. ? Should we instead
> expose them to post-build/post-image scripts as BR2_<something> ? But
> well, of course, it means breaking the existing API, which isn't nice.

If we export all of those variables with this mechanism rathe than with
the make's 'export', then we again dwell on the sagfe side, and we need
not rename them.

We should have named them BR2_something from the beginning, even for our
internal use, to avoid any name-clashing with the calling environment.
For example:

    $ export BUILD_DIR=/path/to/my/project/build.dir
    $ mkdir -p "${BUILD_DIR}/buildroot.output"
    $ make -C "${BUILD_DIR}//buildroot.output"

Suppose the user has post-{build,image} or other rsource files (eg.
kernel .config) in its ${BUILD_DIR}, then his/her configuration is
broken.

But that's the way story goes, we fscked up at the beginning, we have to
live with them, now... :-(

Anyway, I'll keep this change as-is, and make a new cset that export all
these variables to using this new mechanism, so they only get exported
for post-{build,image} scripts (and the new steps-user-hook).

Regards,
Yann E. MORIN.

Hi Thomas,

On Mon, Oct 28, 2013 at 6:17 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
[..]
>
> (Unfortunately, we've asked you to not wrapped patches you reply to,
> but now, it has the consequence that you no longer wrap what you write.
> Well, I guess with K9 it's hard to get both).
>

Hmm :-o
I'm not sure if I can easily fix this with K9. My earlier bug report
and even partial patch on the k9 mailing list didn't get any response.
I'll check the source but I'll try not to write long sentences in the
mean time ;-)

Best regards,
Thomas