Change in coreboot[master]: payloads/external/GRUB2: fix constantly rebuilding due to git checkout

[Michael Niewöhner (Code Review)]

Michael Niewöhner has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/36343 )

Change subject: payloads/external/GRUB2: fix constantly rebuilding due to git checkout
......................................................................

payloads/external/GRUB2: fix constantly rebuilding due to git checkout

Multiple git checkouts cause GRUB2 to constantly rebuild even if there
were no changes to code or config. This is due to changing timestamps.

Fix this by not creating or switching branches but instead rely on
`git checkout -f` which does not touch existing unchanged files.

Change-Id: I7cf66f63268de973a654146a0a47c3d5ca516d4d
Signed-off-by: Michael Niewöhner 
---
M payloads/external/GRUB2/Makefile
1 file changed, 4 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/36343/1

diff --git a/payloads/external/GRUB2/Makefile b/payloads/external/GRUB2/Makefile
index b433bc0..7654f5f 100644
--- a/payloads/external/GRUB2/Makefile
+++ b/payloads/external/GRUB2/Makefile
@@ -1,4 +1,4 @@
-TAG-$(CONFIG_GRUB2_MASTER)=
+TAG-$(CONFIG_GRUB2_MASTER)=origin/HEAD
 TAG-$(CONFIG_GRUB2_REVISION)=$(CONFIG_GRUB2_REVISION_ID)
 TAG-$(CONFIG_GRUB2_STABLE)=grub-2.04
 NAME-$(CONFIG_GRUB2_MASTER)=HEAD
@@ -15,14 +15,9 @@

checkout:
    echo "    GIT        GRUB2 $(NAME-y)"
-	test -d grub2 || \
-		git clone $(project_git_repo) $(project_dir)
-	cd grub2 && \
-		git checkout master && \
-		git pull; \
-		test -n "$(TAG-y)" && \
-			git branch -f $(NAME-y) $(TAG-y) && \
-			git checkout $(NAME-y) || true
+	test -d grub2 || git clone $(project_git_repo) $(project_dir)
+	git -C grub2 fetch
+	git -C grub2 checkout -f $(TAG-y)

grub2/build/config.h: $(CONFIG_DEP) | checkout
    echo "    CONFIG     GRUB2 $(NAME-y)"

[Arthur Heymans (Code Review)]

Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36343 )

Change subject: payloads/external/GRUB2: fix constantly rebuilding due to git checkout
......................................................................

Patch Set 1: Code-Review+1

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36343 )

Change subject: payloads/external/GRUB2: fix constantly rebuilding due to git checkout
......................................................................

Patch Set 1:

(2 comments)

https://review.coreboot.org/c/coreboot/+/36343/1/payloads/external/GRUB2/Mak... 
File payloads/external/GRUB2/Makefile:

https://review.coreboot.org/c/coreboot/+/36343/1/payloads/external/GRUB2/Mak... 
PS1, Line 18: grub2
$(project_dir)

https://review.coreboot.org/c/coreboot/+/36343/1/payloads/external/GRUB2/Mak... 
PS1, Line 19: grub2
...

[Michael Niewöhner (Code Review)]

Hello Arthur Heymans, Paul Menzel, build bot (Jenkins), Nico Huber, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36343

to look at the new patch set (#2).

Change subject: payloads/external/GRUB2: fix constantly rebuilding due to git checkout
......................................................................

payloads/external/GRUB2: fix constantly rebuilding due to git checkout

Multiple git checkouts cause GRUB2 to constantly rebuild even if there
were no changes to code or config. This is due to changing timestamps.

Fix this by not creating or switching branches but instead rely on
`git checkout -f` which does not touch existing unchanged files.

Change-Id: I7cf66f63268de973a654146a0a47c3d5ca516d4d
Signed-off-by: Michael Niewöhner 
---
M payloads/external/GRUB2/Makefile
1 file changed, 4 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/36343/2

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36343 )

Change subject: payloads/external/GRUB2: fix constantly rebuilding due to git checkout
......................................................................

Uploaded patch set 2.

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36343 )

Change subject: payloads/external/GRUB2: fix constantly rebuilding due to git checkout
......................................................................

Patch Set 2:

(2 comments)

https://review.coreboot.org/c/coreboot/+/36343/1/payloads/external/GRUB2/Mak... 
File payloads/external/GRUB2/Makefile:

https://review.coreboot.org/c/coreboot/+/36343/1/payloads/external/GRUB2/Mak... 
PS1, Line 18: grub2
> $(project_dir)

Done

https://review.coreboot.org/c/coreboot/+/36343/1/payloads/external/GRUB2/Mak... 
PS1, Line 19: grub2
> ...

Done

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36343 )

Change subject: payloads/external/GRUB2: fix constantly rebuilding due to git checkout
......................................................................

Patch Set 2:

Is it possible that the old version maintained local changes and the
new doesn't? If so, that could lead to a lot of frustration for users
that are used to it.

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36343 )

Change subject: payloads/external/GRUB2: fix constantly rebuilding due to git checkout
......................................................................

Patch Set 2:
> Patch Set 2:
>
> Is it possible that the old version maintained local changes and the
> new doesn't? If so, that could lead to a lot of frustration for users
> that are used to it.

hm, possible but frustrating, too, because one needs to handle merge conflicts while running `make`

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36343 )

Change subject: payloads/external/GRUB2: fix constantly rebuilding due to git checkout
......................................................................

Patch Set 2: Code-Review+2
> > Is it possible that the old version maintained local changes and the
> > new doesn't? If so, that could lead to a lot of frustration for users
> > that are used to it.
>
> hm, possible but frustrating, too, because one needs to handle merge conflicts while running `make`

Yeah, just saying we might break somebody's workflow. We could try to
handle it more nicely. For instance, issue a warning if the index/tree
aren't clean and skip the checkout in that case?

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36343 )

Change subject: payloads/external/GRUB2: fix constantly rebuilding due to git checkout
......................................................................

Uploaded patch set 3.

[Michael Niewöhner (Code Review)]

Hello Arthur Heymans, Paul Menzel, build bot (Jenkins), Nico Huber, Martin Roth,

I'd like you to reexamine a change. Please visit

https://review.coreboot.org/c/coreboot/+/36343

to look at the new patch set (#3).

Change subject: payloads/external/GRUB2: fix constantly rebuilding due to git checkout
......................................................................

payloads/external/GRUB2: fix constantly rebuilding due to git checkout

Multiple git checkouts cause GRUB2 to constantly rebuild even if there
were no changes to code or config. This is due to changing timestamps.

Fix this by not creating or switching branches but instead rely on
`git checkout -f` which does not touch existing unchanged files.

To be sure to not break anyones workflow checkout is skipped and a
warning gets printed if the tree/index is unclean.

Change-Id: I7cf66f63268de973a654146a0a47c3d5ca516d4d
Signed-off-by: Michael Niewöhner 
---
M payloads/external/GRUB2/Makefile
1 file changed, 9 insertions(+), 9 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/36343/3

[Michael Niewöhner (Code Review)]

Michael Niewöhner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36343 )

Change subject: payloads/external/GRUB2: fix constantly rebuilding due to git checkout
......................................................................

Patch Set 2:
> Patch Set 2: Code-Review+2
> > > Is it possible that the old version maintained local changes and the
> > > new doesn't? If so, that could lead to a lot of frustration for users
> > > that are used to it.
> >
> > hm, possible but frustrating, too, because one needs to handle merge conflicts while running `make`
>
> Yeah, just saying we might break somebody's workflow. We could try to
> handle it more nicely. For instance, issue a warning if the index/tree
> aren't clean and skip the checkout in that case?

Done!

[Nico Huber (Code Review)]

Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/36343 )

Change subject: payloads/external/GRUB2: fix constantly rebuilding due to git checkout
......................................................................

Patch Set 3: Code-Review+2

Usage of the `ifeq` instead of a shell script looks a bit unorthodox, but
whatever, it should work. Thanks for taking care of dirty trees!

[Patrick Georgi (Code Review)]

Patrick Georgi has submitted this change. ( https://review.coreboot.org/c/coreboot/+/36343 )

Change subject: payloads/external/GRUB2: fix constantly rebuilding due to git checkout
......................................................................

payloads/external/GRUB2: fix constantly rebuilding due to git checkout

Multiple git checkouts cause GRUB2 to constantly rebuild even if there
were no changes to code or config. This is due to changing timestamps.

Fix this by not creating or switching branches but instead rely on
`git checkout -f` which does not touch existing unchanged files.

To be sure to not break anyones workflow checkout is skipped and a
warning gets printed if the tree/index is unclean.

Change-Id: I7cf66f63268de973a654146a0a47c3d5ca516d4d
Signed-off-by: Michael Niewöhner 
Reviewed-on: https://review.coreboot.org/c/coreboot/+/36343
Reviewed-by: Nico Huber 
Tested-by: build bot (Jenkins) 
---
M payloads/external/GRUB2/Makefile
1 file changed, 9 insertions(+), 9 deletions(-)

Approvals:
  build bot (Jenkins): Verified
  Nico Huber: Looks good to me, approved

diff --git a/payloads/external/GRUB2/Makefile b/payloads/external/GRUB2/Makefile
index b433bc0..20afdc3 100644
--- a/payloads/external/GRUB2/Makefile
+++ b/payloads/external/GRUB2/Makefile
@@ -1,4 +1,4 @@
-TAG-$(CONFIG_GRUB2_MASTER)=
+TAG-$(CONFIG_GRUB2_MASTER)=origin/HEAD
 TAG-$(CONFIG_GRUB2_REVISION)=$(CONFIG_GRUB2_REVISION_ID)
 TAG-$(CONFIG_GRUB2_STABLE)=grub-2.04
 NAME-$(CONFIG_GRUB2_MASTER)=HEAD
@@ -15,14 +15,14 @@

checkout:
    echo "    GIT        GRUB2 $(NAME-y)"
-	test -d grub2 || \
-		git clone $(project_git_repo) $(project_dir)
-	cd grub2 && \
-		git checkout master && \
-		git pull; \
-		test -n "$(TAG-y)" && \
-			git branch -f $(NAME-y) $(TAG-y) && \
-			git checkout $(NAME-y) || true
+	test -d $(project_dir) || git clone $(project_git_repo) $(project_dir)
+	git -C $(project_dir) fetch
+ifeq ("$(shell git -C $(project_dir) status --ignored=no --untracked-files=no --porcelain)",)
+	git -C $(project_dir) checkout -f $(TAG-y)
+else
+	echo "WARNING: index/tree not clean, skipping update / force checkout."
+	echo "         Checkout manually with `git -C $(project_dir) checkout -f`."
+endif

grub2/build/config.h: $(CONFIG_DEP) | checkout
    echo "    CONFIG     GRUB2 $(NAME-y)"