[Buildroot] [PATCH 1/1] utils/docker-run: fix support for git-worktrees

[Buildroot] [PATCH 1/1] utils/docker-run: fix support for git-worktrees

[Brandon Maier via buildroot]

The docker-run script attempts to support git-new-workdirs and
git-worktrees by resolving the symlink at '$GIT_DIR/config' to get the
true $GIT_DIR. However this does not work for git-worktrees as they do
not use symlinks, instead they change the $GIT_DIR into a regular file
that contains the path to the real $GIT_DIR. To complicate things
further, we actually want the $GIT_COMMON_DIR which is the superset of a
worktree's $GIT_DIR.

git-rev-parse supports the '--git-common-dir' which will resolve the
$GIT_COMMON_DIR for us. However it does not work for git-new-workdirs,
so we still need to detect and handle them.

Signed-off-by: Brandon Maier <brandon.maier@collins.com>
---
 utils/docker-run | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/utils/docker-run b/utils/docker-run
index 17c587a484..db279dca68 100755
--- a/utils/docker-run
+++ b/utils/docker-run
@@ -2,8 +2,13 @@
 set -o errexit -o pipefail
 DIR=$(dirname "${0}")
 MAIN_DIR=$(readlink -f "${DIR}/..")
-# GIT_DIR to support workdirs/worktrees
-GIT_DIR="$(dirname "$(realpath "${MAIN_DIR}/.git/config")")"
+if [ -L "${MAIN_DIR}/.git/config" ]; then
+    # Support git-new-workdir
+    GIT_DIR="$(dirname "$(realpath "${MAIN_DIR}/.git/config")")"
+else
+    # Support git-worktree
+    GIT_DIR="$(cd "$MAIN_DIR" && readlink -f "$(git rev-parse --git-common-dir)")"
+fi
 # shellcheck disable=SC2016
 IMAGE=$(grep ^image: "${MAIN_DIR}/.gitlab-ci.yml" | \
         sed -e 's,^image: ,,g' | sed -e 's,\$CI_REGISTRY,registry.gitlab.com,g')
-- 
2.41.0

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] utils/docker-run: fix support for git-worktrees

[Yann E. MORIN]

Brandon, All,

On 2023-07-28 21:32 +0000, Brandon Maier via buildroot spake thusly:
> The docker-run script attempts to support git-new-workdirs and
> git-worktrees by resolving the symlink at '$GIT_DIR/config' to get the
> true $GIT_DIR. However this does not work for git-worktrees as they do
> not use symlinks, instead they change the $GIT_DIR into a regular file
> that contains the path to the real $GIT_DIR. To complicate things
> further, we actually want the $GIT_COMMON_DIR which is the superset of a
> worktree's $GIT_DIR.
> 
> git-rev-parse supports the '--git-common-dir' which will resolve the
> $GIT_COMMON_DIR for us. However it does not work for git-new-workdirs,
> so we still need to detect and handle them.
> 
> Signed-off-by: Brandon Maier <brandon.maier@collins.com>

Thanks, I've pushed it to master with a few changes:

  - support git versions before --git-common-dir was introduced
  - don't mount GIT_DIR if unknown (i.e. not needed)
  - fix expanding MAIN_DIR

See below..

> ---
>  utils/docker-run | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/utils/docker-run b/utils/docker-run
> index 17c587a484..db279dca68 100755
> --- a/utils/docker-run
> +++ b/utils/docker-run
> @@ -2,8 +2,13 @@
>  set -o errexit -o pipefail
>  DIR=$(dirname "${0}")
>  MAIN_DIR=$(readlink -f "${DIR}/..")
> -# GIT_DIR to support workdirs/worktrees
> -GIT_DIR="$(dirname "$(realpath "${MAIN_DIR}/.git/config")")"
> +if [ -L "${MAIN_DIR}/.git/config" ]; then
> +    # Support git-new-workdir
> +    GIT_DIR="$(dirname "$(realpath "${MAIN_DIR}/.git/config")")"
> +else
> +    # Support git-worktree
> +    GIT_DIR="$(cd "$MAIN_DIR" && readlink -f "$(git rev-parse --git-common-dir)")"
                      ^^^^^^^^^
Expansion should use curly braces, like everywhere else.

--git-common-dir was only introduced in git 2.10.0, which is most
probably not available in enterprise-grade distros like RHEL (RHEL 7 is
still maintained, and it is based on Fedora 19; there's only a docker
image for Fedora 20, and that has git 1.9).

So I've added --no-flags, that has been present since at least 1.0.0,
which means that, on git versions that did not support worktrees, we'll
get an empty GIT_DIR, so that's OK: it means we're in the main working
copy, and thus we don't need the mount.

Applied to master with those changes, thanks.

Regards,
Yann E. MORIN.

> +fi
>  # shellcheck disable=SC2016
>  IMAGE=$(grep ^image: "${MAIN_DIR}/.gitlab-ci.yml" | \
>          sed -e 's,^image: ,,g' | sed -e 's,\$CI_REGISTRY,registry.gitlab.com,g')
> -- 
> 2.41.0
> 
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  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 mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [External] Re: [PATCH 1/1] utils/docker-run: fix support for git-worktrees

[Maier, Brandon L Collins via buildroot]

Yann,

> -----Original Message-----
> From: Yann E. MORIN <yann.morin.1998@free.fr>

> > +    # Support git-worktree
> > +    GIT_DIR="$(cd "$MAIN_DIR" && readlink -f "$(git rev-parse --git-
> common-dir)")"
>                       ^^^^^^^^^
> Expansion should use curly braces, like everywhere else.
>
> --git-common-dir was only introduced in git 2.10.0, which is most
> probably not available in enterprise-grade distros like RHEL (RHEL 7 is
> still maintained, and it is based on Fedora 19; there's only a docker
> image for Fedora 20, and that has git 1.9).
>
> So I've added --no-flags, that has been present since at least 1.0.0,
> which means that, on git versions that did not support worktrees, we'll
> get an empty GIT_DIR, so that's OK: it means we're in the main working
> copy, and thus we don't need the mount.
>
> Applied to master with those changes, thanks.

All the changes look good to me, and verified still works on my end.

Thanks,
Brandon
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [External] Re: [PATCH 1/1] utils/docker-run: fix support for git-worktrees

[Maier, Brandon L Collins via buildroot]

Yann,

> -----Original Message-----
> From: Maier, Brandon L Collins
> Sent: Monday, July 31, 2023 12:37 PM
> To: Yann E. MORIN <yann.morin.1998@free.fr>
> Cc: buildroot@buildroot.org; Ricardo Martincoski
> <ricardo.martincoski@datacom.com.br>
> Subject: RE: [External] Re: [Buildroot] [PATCH 1/1] utils/docker-run: fix
> support for git-worktrees
>
> Yann,
>
> > -----Original Message-----
> > From: Yann E. MORIN <yann.morin.1998@free.fr>
>
> > > +    # Support git-worktree
> > > +    GIT_DIR="$(cd "$MAIN_DIR" && readlink -f "$(git rev-parse --git-
> > common-dir)")"
> >                       ^^^^^^^^^
> > Expansion should use curly braces, like everywhere else.
> >
> > --git-common-dir was only introduced in git 2.10.0, which is most
> > probably not available in enterprise-grade distros like RHEL (RHEL 7 is
> > still maintained, and it is based on Fedora 19; there's only a docker
> > image for Fedora 20, and that has git 1.9).
> >
> > So I've added --no-flags, that has been present since at least 1.0.0,
> > which means that, on git versions that did not support worktrees, we'll
> > get an empty GIT_DIR, so that's OK: it means we're in the main working
> > copy, and thus we don't need the mount.
> >
> > Applied to master with those changes, thanks.
>
> All the changes look good to me, and verified still works on my end.

I was looking at this again, and there's a problem with one of the changes. If the script's working directory is not at the root of the repo, then the `readlink -e "$GIT_DIR"` will fail because GIT_DIR is relative to the root of the repo. My original patch had the readlink run inside the `cd $MAIN_DIR` so that it would resolve the absolute path correctly.

Here's the output with `set -x` if I try running it from another directory.

$ ./buildroot2/utils/docker-run echo hi
++ dirname ./buildroot2/utils/docker-run
+ DIR=./buildroot2/utils
++ readlink -f ./buildroot2/utils/..
+ MAIN_DIR=/local_build_dir/blmaier/buildroot-update/buildroot2
+ '[' -L /local_build_dir/blmaier/buildroot-update/buildroot2/.git/config ']'
++ cd /local_build_dir/blmaier/buildroot-update/buildroot2
++ git rev-parse --no-flags --git-common-dir
+ GIT_DIR=.git
++ grep '^image:' /local_build_dir/blmaier/buildroot-update/buildroot2/.gitlab-ci.yml
++ sed -e 's,^image: ,,g'
++ sed -e 's,\$CI_REGISTRY,registry.gitlab.com,g'
+ IMAGE=registry.gitlab.com/buildroot.org/buildroot/base:20230207.1123
+ docker_opts=(-i --rm --user "$(id -u):$(id -g)" --mount "type=bind,src=${MAIN_DIR},dst=${MAIN_DIR}" --workdir "${MAIN_DIR}")
++ id -u
++ id -g
+ declare -a docker_opts
+ '[' .git ']'
++ readlink -e .git
+ GIT_DIR=

Which it dies at that last command as the readlink returns an error and `set -e` is enabled.

Thanks,
Brandon
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [External] Re: [PATCH 1/1] utils/docker-run: fix support for git-worktrees

[Yann E. MORIN]

Brandon, All,

On 2023-07-31 17:53 +0000, Maier, Brandon L                            Collins spake thusly:
> > > From: Yann E. MORIN <yann.morin.1998@free.fr>
> > > --git-common-dir was only introduced in git 2.10.0, which is most
> > > probably not available in enterprise-grade distros like RHEL (RHEL 7 is
> > > still maintained, and it is based on Fedora 19; there's only a docker
> > > image for Fedora 20, and that has git 1.9).
> > >
> > > So I've added --no-flags, that has been present since at least 1.0.0,
> > > which means that, on git versions that did not support worktrees, we'll
> > > get an empty GIT_DIR, so that's OK: it means we're in the main working
> > > copy, and thus we don't need the mount.
> > >
> > > Applied to master with those changes, thanks.
> >
> > All the changes look good to me, and verified still works on my end.
> 
> I was looking at this again, and there's a problem with one of the
> changes. If the script's working directory is not at the root of the
> repo, then the `readlink -e "$GIT_DIR"` will fail because GIT_DIR is
> relative to the root of the repo.

Ah, damned...

> My original patch had the readlink run inside the `cd $MAIN_DIR` so
> that it would resolve the absolute path correctly.

Yes, I saw that, but I forgot to carry that logic on when I added
support for older git versions that did not know about --git-common-dir

Thanks for checking this! I'll push a quick fix.

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 mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH 1/1] utils/docker-run: fix support for git-worktrees

[Peter Korsgaard]

>>>>> "Brandon" == Brandon Maier via buildroot <buildroot@buildroot.org> writes:

 > The docker-run script attempts to support git-new-workdirs and
 > git-worktrees by resolving the symlink at '$GIT_DIR/config' to get the
 > true $GIT_DIR. However this does not work for git-worktrees as they do
 > not use symlinks, instead they change the $GIT_DIR into a regular file
 > that contains the path to the real $GIT_DIR. To complicate things
 > further, we actually want the $GIT_COMMON_DIR which is the superset of a
 > worktree's $GIT_DIR.

 > git-rev-parse supports the '--git-common-dir' which will resolve the
 > $GIT_COMMON_DIR for us. However it does not work for git-new-workdirs,
 > so we still need to detect and handle them.

 > Signed-off-by: Brandon Maier <brandon.maier@collins.com>

Committed to 2023.02.x and 2023.05.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot