* [Buildroot] [PATCH 1/1] utils/docker-run: fix support for git-worktrees
@ 2023-07-28 21:32 Brandon Maier via buildroot
2023-07-29 15:33 ` Yann E. MORIN
2023-08-30 21:59 ` [Buildroot] " Peter Korsgaard
0 siblings, 2 replies; 6+ messages in thread
From: Brandon Maier via buildroot @ 2023-07-28 21:32 UTC (permalink / raw)
To: buildroot; +Cc: Brandon Maier, Ricardo Martincoski
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [Buildroot] [PATCH 1/1] utils/docker-run: fix support for git-worktrees
2023-07-28 21:32 [Buildroot] [PATCH 1/1] utils/docker-run: fix support for git-worktrees Brandon Maier via buildroot
@ 2023-07-29 15:33 ` Yann E. MORIN
2023-07-31 17:36 ` [Buildroot] [External] " Maier, Brandon L Collins via buildroot
2023-08-30 21:59 ` [Buildroot] " Peter Korsgaard
1 sibling, 1 reply; 6+ messages in thread
From: Yann E. MORIN @ 2023-07-29 15:33 UTC (permalink / raw)
To: Brandon Maier; +Cc: Ricardo Martincoski, buildroot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Buildroot] [External] Re: [PATCH 1/1] utils/docker-run: fix support for git-worktrees
2023-07-29 15:33 ` Yann E. MORIN
@ 2023-07-31 17:36 ` Maier, Brandon L Collins via buildroot
2023-07-31 17:53 ` Maier, Brandon L Collins via buildroot
0 siblings, 1 reply; 6+ messages in thread
From: Maier, Brandon L Collins via buildroot @ 2023-07-31 17:36 UTC (permalink / raw)
To: Yann E. MORIN; +Cc: Ricardo Martincoski, 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Buildroot] [External] Re: [PATCH 1/1] utils/docker-run: fix support for git-worktrees
2023-07-31 17:36 ` [Buildroot] [External] " Maier, Brandon L Collins via buildroot
@ 2023-07-31 17:53 ` Maier, Brandon L Collins via buildroot
2023-07-31 18:50 ` Yann E. MORIN
0 siblings, 1 reply; 6+ messages in thread
From: Maier, Brandon L Collins via buildroot @ 2023-07-31 17:53 UTC (permalink / raw)
To: Yann E. MORIN; +Cc: Ricardo Martincoski, 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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Buildroot] [External] Re: [PATCH 1/1] utils/docker-run: fix support for git-worktrees
2023-07-31 17:53 ` Maier, Brandon L Collins via buildroot
@ 2023-07-31 18:50 ` Yann E. MORIN
0 siblings, 0 replies; 6+ messages in thread
From: Yann E. MORIN @ 2023-07-31 18:50 UTC (permalink / raw)
To: Maier, Brandon L Collins
Cc: Ricardo Martincoski, buildroot
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Buildroot] [PATCH 1/1] utils/docker-run: fix support for git-worktrees
2023-07-28 21:32 [Buildroot] [PATCH 1/1] utils/docker-run: fix support for git-worktrees Brandon Maier via buildroot
2023-07-29 15:33 ` Yann E. MORIN
@ 2023-08-30 21:59 ` Peter Korsgaard
1 sibling, 0 replies; 6+ messages in thread
From: Peter Korsgaard @ 2023-08-30 21:59 UTC (permalink / raw)
To: Brandon Maier via buildroot; +Cc: Ricardo Martincoski, Brandon Maier
>>>>> "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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-08-30 21:59 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-28 21:32 [Buildroot] [PATCH 1/1] utils/docker-run: fix support for git-worktrees Brandon Maier via buildroot
2023-07-29 15:33 ` Yann E. MORIN
2023-07-31 17:36 ` [Buildroot] [External] " Maier, Brandon L Collins via buildroot
2023-07-31 17:53 ` Maier, Brandon L Collins via buildroot
2023-07-31 18:50 ` Yann E. MORIN
2023-08-30 21:59 ` [Buildroot] " Peter Korsgaard
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).