diff mbox series

[1/2] package/Makefile.in: set GIT_DIR=. in {HOST, TARGET}_MAKE_ENV

Message ID 20240111105621.2321957-1-peter@korsgaard.com
State Accepted
Headers show
Series [1/2] package/Makefile.in: set GIT_DIR=. in {HOST, TARGET}_MAKE_ENV | expand

Commit Message

Peter Korsgaard Jan. 11, 2024, 10:56 a.m. UTC 
A number of packages try to detect if they are running in a git repo and run
git describe at build time instead of using the hard coded version number if
it succeed, leading to odd version numbers as they end up picking up the
Buildroot git version if building inside a Buildroot git checkout, E.G.:

rauc --version
rauc 2023.11-562-g9c954953b4+

This is because rauc builds with meson and uses vcs_tag:

https://github.com/rauc/rauc/blob/v1.11/meson.build#L168-L171

https://mesonbuild.com/Reference-manual_functions.html#vcs_tag

Another example is micropython, where we already work around it by passing
GIT_DIR=.

In the context of Buildroot the packages are never built in their own git
checkout, so pass GIT_DIR=.  to ensure git doesn't walk back up the
directory tree and finds the Buildroot git repo, which fixes the rauc (and
similar) issues.

>>> rauc 1.11 Building
..
ninja: Entering directory `/home/peko/source/buildroot/output-rauc/build/rauc-1.11//build'
[1/29] Generating version.h with a custom command
fatal: not a git repository: '.'

cat output-rauc/build/rauc-1.11/build/version.h
 #define PACKAGE_STRING "rauc 1.11"

 #define PACKAGE_VERSION "1.11"

Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
---
 package/Makefile.in | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Yann E. MORIN Jan. 11, 2024, 8:07 p.m. UTC | #1 
Peter, All,

On 2024-01-11 11:56 +0100, Peter Korsgaard spake thusly:
> A number of packages try to detect if they are running in a git repo and run
> git describe at build time instead of using the hard coded version number if
> it succeed, leading to odd version numbers as they end up picking up the
> Buildroot git version if building inside a Buildroot git checkout, E.G.:
> 
> rauc --version
> rauc 2023.11-562-g9c954953b4+
> 
> This is because rauc builds with meson and uses vcs_tag:
> 
> https://github.com/rauc/rauc/blob/v1.11/meson.build#L168-L171
> 
> https://mesonbuild.com/Reference-manual_functions.html#vcs_tag
> 
> Another example is micropython, where we already work around it by passing
> GIT_DIR=.
> 
> In the context of Buildroot the packages are never built in their own git
> checkout, so pass GIT_DIR=.  to ensure git doesn't walk back up the
> directory tree and finds the Buildroot git repo, which fixes the rauc (and
> similar) issues.
> 
> >>> rauc 1.11 Building
> ..
> ninja: Entering directory `/home/peko/source/buildroot/output-rauc/build/rauc-1.11//build'
> [1/29] Generating version.h with a custom command
> fatal: not a git repository: '.'
> 
> cat output-rauc/build/rauc-1.11/build/version.h
>  #define PACKAGE_STRING "rauc 1.11"
> 
>  #define PACKAGE_VERSION "1.11"
> 
> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>

Applied to master, thanks.

Regards,
Yann E. MORIN.

> ---
>  package/Makefile.in | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/package/Makefile.in b/package/Makefile.in
> index 9fbe960759..3e276d23d6 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -263,7 +263,9 @@ export PERL=$(shell which perl)
>  # finds this perl module by exporting the proper value for PERL5LIB.
>  export PERL5LIB=$(HOST_DIR)/lib/perl
>  
> -TARGET_MAKE_ENV = PATH=$(BR_PATH)
> +TARGET_MAKE_ENV = \
> +	GIT_DIR=. \
> +	PATH=$(BR_PATH)
>  
>  TARGET_CONFIGURE_OPTS = \
>  	$(TARGET_MAKE_ENV) \
> @@ -307,6 +309,7 @@ TARGET_CONFIGURE_OPTS = \
>  
>  
>  HOST_MAKE_ENV = \
> +	GIT_DIR=. \
>  	PATH=$(BR_PATH) \
>  	PKG_CONFIG="$(PKG_CONFIG_HOST_BINARY)" \
>  	PKG_CONFIG_SYSROOT_DIR="/" \
> -- 
> 2.39.2
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot
Arnout Vandecappelle Jan. 11, 2024, 9:10 p.m. UTC | #2 
On 11/01/2024 11:56, Peter Korsgaard wrote:
> A number of packages try to detect if they are running in a git repo and run
> git describe at build time instead of using the hard coded version number if
> it succeed, leading to odd version numbers as they end up picking up the
> Buildroot git version if building inside a Buildroot git checkout, E.G.:
> 
> rauc --version
> rauc 2023.11-562-g9c954953b4+
> 
> This is because rauc builds with meson and uses vcs_tag:
> 
> https://github.com/rauc/rauc/blob/v1.11/meson.build#L168-L171
> 
> https://mesonbuild.com/Reference-manual_functions.html#vcs_tag
> 
> Another example is micropython, where we already work around it by passing
> GIT_DIR=.
> 
> In the context of Buildroot the packages are never built in their own git
> checkout, so pass GIT_DIR=.  to ensure git doesn't walk back up the
> directory tree and finds the Buildroot git repo, which fixes the rauc (and
> similar) issues.
> 
>>>> rauc 1.11 Building
> ..
> ninja: Entering directory `/home/peko/source/buildroot/output-rauc/build/rauc-1.11//build'
> [1/29] Generating version.h with a custom command
> fatal: not a git repository: '.'
> 
> cat output-rauc/build/rauc-1.11/build/version.h
>   #define PACKAGE_STRING "rauc 1.11"
> 
>   #define PACKAGE_VERSION "1.11"
> 
> Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
> ---
>   package/Makefile.in | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/package/Makefile.in b/package/Makefile.in
> index 9fbe960759..3e276d23d6 100644
> --- a/package/Makefile.in
> +++ b/package/Makefile.in
> @@ -263,7 +263,9 @@ export PERL=$(shell which perl)
>   # finds this perl module by exporting the proper value for PERL5LIB.
>   export PERL5LIB=$(HOST_DIR)/lib/perl
>   
> -TARGET_MAKE_ENV = PATH=$(BR_PATH)
> +TARGET_MAKE_ENV = \
> +	GIT_DIR=. \
> +	PATH=$(BR_PATH)

  There are a number of infras that don't use TARGET_MAKE_ENV: golang, luarocks, 
perl, and python. Of these, I think only golang has a big risk of having the 
same issue. So shouldn't we add it there as well? And perhaps to python as well.


  Regards,
  Arnout

>   
>   TARGET_CONFIGURE_OPTS = \
>   	$(TARGET_MAKE_ENV) \
> @@ -307,6 +309,7 @@ TARGET_CONFIGURE_OPTS = \
>   
>   
>   HOST_MAKE_ENV = \
> +	GIT_DIR=. \
>   	PATH=$(BR_PATH) \
>   	PKG_CONFIG="$(PKG_CONFIG_HOST_BINARY)" \
>   	PKG_CONFIG_SYSROOT_DIR="/" \
Peter Korsgaard Jan. 12, 2024, 7:14 a.m. UTC | #3 
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

Hi,

 >> # finds this perl module by exporting the proper value for PERL5LIB.
 >> export PERL5LIB=$(HOST_DIR)/lib/perl
 >> -TARGET_MAKE_ENV = PATH=$(BR_PATH)
 >> +TARGET_MAKE_ENV = \
 >> +	GIT_DIR=. \
 >> +	PATH=$(BR_PATH)

 >  There are a number of infras that don't use TARGET_MAKE_ENV: golang,
 >  luarocks, perl, and python. Of these, I think only golang has a big
 >  risk of having the same issue. So shouldn't we add it there as well?
 >  And perhaps to python as well.

Possibly, yes. We have to be a bit careful to not cause issues with the
module vendoring, but it should be doable.

Will you send patches?
Peter Korsgaard Jan. 13, 2024, 8:30 p.m. UTC | #4 
>>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes:

 > A number of packages try to detect if they are running in a git repo and run
 > git describe at build time instead of using the hard coded version number if
 > it succeed, leading to odd version numbers as they end up picking up the
 > Buildroot git version if building inside a Buildroot git checkout, E.G.:

 > rauc --version
 > rauc 2023.11-562-g9c954953b4+

 > This is because rauc builds with meson and uses vcs_tag:

 > https://github.com/rauc/rauc/blob/v1.11/meson.build#L168-L171

 > https://mesonbuild.com/Reference-manual_functions.html#vcs_tag

 > Another example is micropython, where we already work around it by passing
 > GIT_DIR=.

 > In the context of Buildroot the packages are never built in their own git
 > checkout, so pass GIT_DIR=.  to ensure git doesn't walk back up the
 > directory tree and finds the Buildroot git repo, which fixes the rauc (and
 > similar) issues.

 >>>> rauc 1.11 Building
 > ..
 > ninja: Entering directory `/home/peko/source/buildroot/output-rauc/build/rauc-1.11//build'
 > [1/29] Generating version.h with a custom command
 > fatal: not a git repository: '.'

 > cat output-rauc/build/rauc-1.11/build/version.h
 >  #define PACKAGE_STRING "rauc 1.11"

 >  #define PACKAGE_VERSION "1.11"

 > Signed-off-by: Peter Korsgaard <peter@korsgaard.com>

Committed to 2023.02.x and 2023.11.x, thanks.
Jan Kundrát Jan. 16, 2024, 3:32 p.m. UTC | #5 
> In the context of Buildroot the packages are never built in their own git
> checkout, so pass GIT_DIR=.  to ensure git doesn't walk back up the
> directory tree and finds the Buildroot git repo, which fixes the rauc (and
> similar) issues.

This has caused a regression for us.

We're using Zuul CI [1] which is set up so that any patch that's uploaded 
for review in any of our internal projects also triggers a test build 
through Buildroot. Some of these builds would use sequences of yet-unmerged 
patches, so tracking version info of each component is crucial, which is 
why we're puting all that into the target's /etc/os-release, as well as 
relaying on the build system to call an equivalent of a `git describe` for 
each of our projects.

For this feature, we're relying on a few hooks:

- Zuul itself produces the correct content as a git checkout in a scratch 
directory,

- Buildroot is configured via `local.mk` to use that checkout as a source 
[2]

- a custom hook sets up the redirection via the `gitdir` statement in the 
buidlroot's copy of the source directory [3], this redirection however  
takes effect at runtime (see [4])

- the build script of each app then simply runs `git describe` with a few 
flags for output formatting; the apps currently do not have a way of 
accepting some externally provided version strings.

Some of our projects use git submodules, which means that manipulating 
.gitdir has to be done carefuly. The TL;DR version is that this is a bit 
more complex than just putting a version override into a file. In short, 
there's already a git checkout that's been prepared with accurate version 
info, and it makes sense to use that info as-is -- or it has worked well 
for us in the past three years :), up until this commit.

How are you guys solving this problem in your CI systems? Is there a chance 
of undoing this global GIT_DIR which, unfortunately, takes precedence over 
the projects' own .gitdir mechanism?

With kind regards,
Jan

[1] https://zuul-ci.org/
[2] 
https://gerrit.cesnet.cz/plugins/gitiles/CzechLight/br2-external/+/b1066e39fe136c4996f7d13b6003a958c1ea40a2/dev-setup-git.sh#10
[3] 
https://gerrit.cesnet.cz/plugins/gitiles/CzechLight/br2-external/+/b1066e39fe136c4996f7d13b6003a958c1ea40a2/dev-setup-git.sh#31
[4] 
https://gerrit.cesnet.cz/plugins/gitiles/CzechLight/br2-external/+/7a9d8b0d8f3cefedf708368bba463c89556dfa9e%5E%21/
Peter Korsgaard Jan. 17, 2024, 7:27 a.m. UTC | #6 
>>>>> "Jan" == Jan Kundrát via buildroot <buildroot@buildroot.org> writes:

 >> In the context of Buildroot the packages are never built in their own git
 >> checkout, so pass GIT_DIR=.  to ensure git doesn't walk back up the
 >> directory tree and finds the Buildroot git repo, which fixes the rauc (and
 >> similar) issues.

 > This has caused a regression for us.

Sorry to hear that.

 > We're using Zuul CI [1] which is set up so that any patch that's
 > uploaded for review in any of our internal projects also triggers a
 > test build through Buildroot. Some of these builds would use sequences
 > of yet-unmerged patches, so tracking version info of each component is
 > crucial, which is why we're puting all that into the target's
 > /etc/os-release, as well as relaying on the build system to call an
 > equivalent of a `git describe` for each of our projects.

 > For this feature, we're relying on a few hooks:

 > - Zuul itself produces the correct content as a git checkout in a
 >   scratch directory,

 > - Buildroot is configured via `local.mk` to use that checkout as a
 >   source [2]

 > - a custom hook sets up the redirection via the `gitdir` statement in
 >   the buidlroot's copy of the source directory [3], this redirection
 >   however  takes effect at runtime (see [4])

I don't understand why you do > \$(@D)/.git, I was would have
expected >> \$(@D)/.git/config but OK.


 > - the build script of each app then simply runs `git describe` with a
 >   few flags for output formatting; the apps currently do not have a
 >   way of accepting some externally provided version strings.

 > Some of our projects use git submodules, which means that manipulating
 > .gitdir has to be done carefuly. The TL;DR version is that this is a
 > bit more complex than just putting a version override into a file. In
 > short, there's already a git checkout that's been prepared with
 > accurate version info, and it makes sense to use that info as-is -- or
 > it has worked well for us in the past three years :), up until this
 > commit.

I guess those are local/custom packages that you have in your external
tree? Can you pass GIT_DIR=.git to them (or make them unset GIT_DIR) to
make them use your prepared .git directory?


 > How are you guys solving this problem in your CI systems?

This is not applicable to the CI done for Buildroot itself (where we
want to build the versions specified in the .mk files). For $WORK, I
have populated a file in a post-build script like your os-release file.


 > Is there a chance of undoing this global GIT_DIR which,
 > unfortunately, takes precedence over the projects' own .gitdir
 > mechanism?

The problem this change tried to fix is real, so the question is how we
can continue to fix this issue (without having to play whack-a-mole for
each package) which still supporting a use case like yours.

Thanks for the heads up!
Jan Kundrát Jan. 18, 2024, 1:56 a.m. UTC | #7 
> I guess those are local/custom packages that you have in your external
> tree? Can you pass GIT_DIR=.git to them (or make them unset GIT_DIR) to
> make them use your prepared .git directory?

Yes, these are local packages whose build recipes live in our br2-external 
tree.

I tried to unset the GIT_DIR from both $(PKG)_PRE_CONFIGURE_HOOKS and 
$(PKG)_PRE_BULD_HOOKS, but calling `unset GIT_DIR` has no effect in there. 
That makes sense, because it's used like this in the pkg-cmake.mk:

define $(2)_BUILD_CMDS
	$$(TARGET_MAKE_ENV) $$($$(PKG)_BUILD_ENV) $$(BR2_CMAKE) --build 
$$($$(PKG)_BUILDDIR) -j$(PARALLEL_JOBS) $$($$(PKG)_BUILD_OPTS)

...so it won't propagate because it's set explicitly just before the build 
command executes.

In the end, I did it like this:

  XXX_HACK_GIT_DIR = GIT_DIR=.git

...and then for each project:

  FOO_BUILD_ENV += $(XXX_HACK_GIT_DIR)
  BAR_BUILD_ENV += $(XXX_HACK_GIT_DIR)

Cheers,
Jan
Mircea Gliga Jan. 26, 2024, 9:16 a.m. UTC | #8 
Hi all,

> In the context of Buildroot the packages are never built in their own git
> checkout, so pass GIT_DIR=.  to ensure git doesn't walk back up the
> directory tree and finds the Buildroot git repo, which fixes the rauc (and
> similar) issues.

>
> -TARGET_MAKE_ENV = PATH=$(BR_PATH)
> +TARGET_MAKE_ENV = \
> +       GIT_DIR=. \
> +       PATH=$(BR_PATH)
>
>  TARGET_CONFIGURE_OPTS = \
>         $(TARGET_MAKE_ENV) \
> @@ -307,6 +309,7 @@ TARGET_CONFIGURE_OPTS = \

This will also break the usage of package/environment-setup. The
GIT_DIR=. leaks in
the environment script `environment-setup` useful when a developer
wants to use a Buildroot
generated SDK to build an external project.
So, on a developer machine after sourcing the environment script, all
git commands fail:

$ git status
fatal: not a git repository: '.'

Best regards
Mircea
Peter Korsgaard Jan. 26, 2024, 9:35 a.m. UTC | #9 
>>>>> "Mircea" == Mircea Gliga <gliga.mircea@gmail.com> writes:

 > Hi all,
 >> In the context of Buildroot the packages are never built in their own git
 >> checkout, so pass GIT_DIR=.  to ensure git doesn't walk back up the
 >> directory tree and finds the Buildroot git repo, which fixes the rauc (and
 >> similar) issues.

 >> 
 >> -TARGET_MAKE_ENV = PATH=$(BR_PATH)
 >> +TARGET_MAKE_ENV = \
 >> +       GIT_DIR=. \
 >> +       PATH=$(BR_PATH)
 >> 
 >> TARGET_CONFIGURE_OPTS = \
 >> $(TARGET_MAKE_ENV) \
 >> @@ -307,6 +309,7 @@ TARGET_CONFIGURE_OPTS = \

 > This will also break the usage of package/environment-setup. The
 > GIT_DIR=. leaks in
 > the environment script `environment-setup` useful when a developer
 > wants to use a Buildroot
 > generated SDK to build an external project.
 > So, on a developer machine after sourcing the environment script, all
 > git commands fail:

 > $ git status
 > fatal: not a git repository: '.'

Thanks for the report, I've sent a fix here:

https://patchwork.ozlabs.org/project/buildroot/patch/20240126093441.2130693-1-peter@korsgaard.com/
diff mbox series

Patch

diff --git a/package/Makefile.in b/package/Makefile.in
index 9fbe960759..3e276d23d6 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -263,7 +263,9 @@  export PERL=$(shell which perl)
 # finds this perl module by exporting the proper value for PERL5LIB.
 export PERL5LIB=$(HOST_DIR)/lib/perl
 
-TARGET_MAKE_ENV = PATH=$(BR_PATH)
+TARGET_MAKE_ENV = \
+	GIT_DIR=. \
+	PATH=$(BR_PATH)
 
 TARGET_CONFIGURE_OPTS = \
 	$(TARGET_MAKE_ENV) \
@@ -307,6 +309,7 @@  TARGET_CONFIGURE_OPTS = \
 
 
 HOST_MAKE_ENV = \
+	GIT_DIR=. \
 	PATH=$(BR_PATH) \
 	PKG_CONFIG="$(PKG_CONFIG_HOST_BINARY)" \
 	PKG_CONFIG_SYSROOT_DIR="/" \