Skip to contentSkip to conversation

pkg-download: take some more care of <PKG_>VERSION with git

PATCH New Patrick Boettcher 5 participants 7 messages Apr 2, 2013 – Aug 14, 2013

lore
Share & tools
lore

Patrick Boettcher

author
#0 lore raw

PATCH v2 1/1 pkg-download: take some more care of <PKG_>VERSION with git

This replaces the one-liner patch I made last week after a comment by Thomas De Schampheleire. (it is now a 5-liner)

I added comment to explain why using HEAD is better in the current situation. I did not change the hg-part as it is different to git - after a quick read it won't work at all when VERSION is not set in a packge.

Please apply if appropriate and helpful (it is for me).

Patrick Boettcher (1):
pkg-download: take some more care of <PKG_>VERSION when METHOD is git

 package/pkg-download.mk |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

--
1.7.10.4

author
#1 lore raw

PATCH 1/1 pkg-download: take some more care of <PKG_>VERSION when METHOD is git

If a package's SITE_METHOD is set to git, but the VERSION is not set it fails to create a tar-ball, this patch is fixes this by always using HEAD as git-archive reference.

HEAD will work in both cases:
1) if VERSION is set the clone's HEAD is VERSION,
2) if VERSION is not set the clone's HEAD is the default branch.

Signed-off-by Patrick Boettcher <[email protected]>
---
 package/pkg-download.mk |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
package/pkg-download.mk +5 -1
index 1705fcd..610ceb3 100644
@@ -77,6 +77,10 @@ domainseparator=$(if $(1),$(1),/)
 # branch). Before trying to do a shallow clone we check if $($(PKG)_DL_VERSION)
 # is in the list provided by git ls-remote. If not we fall back on a full clone.
 #
+# In any case we archive HEAD (and not $($(PKG)_DL_VERSION)): if VERSION is not
+# set the default branch is checked out and git-archive will fail if no refspec
+# is given. HEAD will continue to work if VERSION is set.
+#
 # Messages for the type of clone used are provided to ease debugging in case of
 # problems
 define DOWNLOAD_GIT

@@ -88,7 +92,7 @@ define DOWNLOAD_GIT
 	  (echo "Doing full clone" && \
 	   $(GIT) clone --bare $($(PKG)_SITE) $($(PKG)_BASE_NAME))) && \
 	pushd $($(PKG)_BASE_NAME) > /dev/null && \
-	$(GIT) archive --format=tar --prefix=$($(PKG)_BASE_NAME)/ $($(PKG)_DL_VERSION) | \
+	$(GIT) archive --format=tar --prefix=$($(PKG)_BASE_NAME)/ HEAD | \
 		gzip -c > $(DL_DIR)/$($(PKG)_SOURCE) && \
 	popd > /dev/null && \
 	rm -rf $($(PKG)_DL_DIR) && \
Signature · 1 line
1.7.10.4
#2 lore raw

PATCH v2 1/1 pkg-download: take some more care of <PKG_>VERSION with git

In reply to Patrick Boettcher

Hello,

Quoted 8 lines · Patrick Boettcher

This replaces the one-liner patch I made last week after a comment by Thomas De Schampheleire. (it is now a 5-liner)

I added comment to explain why using HEAD is better in the current situation. I did not change the hg-part as it is different to git - after a quick read it won't work at all when VERSION is not set in a packge.

Quoted 7 lines

Please apply if appropriate and helpful (it is for me).

Patrick Boettcher (1):
pkg-download: take some more care of <PKG_>VERSION when METHOD is git

 package/pkg-download.mk |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Thomas, Arnout, Gustavo, some thoughts about this patch?

I'm not a big fan of the fact that <pkg>_VERSION would be optional for the git site method, but mandatory for the hg site method, but it's not really a strong opinion.

Thanks,

Thomas

Signature · 4 lines
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com
#3 lore raw

PATCH v2 1/1 pkg-download: take some more care of <PKG_>VERSION with git

Quoted 16 lines · Thomas Petazzoni

Hello,

Quoted 13 lines · Patrick Boettcher

This replaces the one-liner patch I made last week after a comment by Thomas De Schampheleire. (it is now a 5-liner)

I added comment to explain why using HEAD is better in the current situation. I did not change the hg-part as it is different to git - after a quick read it won't work at all when VERSION is not set in a packge.

Please apply if appropriate and helpful (it is for me).

Patrick Boettcher (1):
pkg-download: take some more care of <PKG_>VERSION when METHOD is git

Quoted 8 lines
Quoted 2 lines
  package/pkg-download.mk |    6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)

Thomas, Arnout, Gustavo, some thoughts about this patch?

I'm not a big fan of the fact that <pkg>_VERSION would be optional for the git site method, but mandatory for the hg site method, but it's not really a strong opinion.

Considering the commit message of 3a7a0a which you just committed:

"Specifying a floating tag like HEAD for a repository version is bad practice, as it results in non-reproducible builds."

I would say this is a bad idea. Instead, there could be a check that PKG_VERSION is not empty.

Also, I think the patch is just wrong: if you specify a sha instead of
a branch or tag, then it will check out master's HEAD instead of the specified sha...

Regards,
Arnout

Signature · 6 lines
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
#4 lore raw

PATCH v2 1/1 pkg-download: take some more care of <PKG_>VERSION with git

Quoted 30 lines · Arnout Vandecappelle
Quoted 22 lines · Thomas Petazzoni

Hello,

Quoted 9 lines · Patrick Boettcher

This replaces the one-liner patch I made last week after a comment by Thomas De Schampheleire. (it is now a 5-liner)

I added comment to explain why using HEAD is better in the current situation. I did not change the hg-part as it is different to git - after a quick read it won't work at all when VERSION is not set in a packge.

Quoted 7 lines

Please apply if appropriate and helpful (it is for me).

Patrick Boettcher (1):
pkg-download: take some more care of <PKG_>VERSION when METHOD is git

  package/pkg-download.mk |    6 +++++-
  1 file changed, 5 insertions(+), 1 deletion(-)
Quoted 5 lines

Thomas, Arnout, Gustavo, some thoughts about this patch?

I'm not a big fan of the fact that <pkg>_VERSION would be optional for the git site method, but mandatory for the hg site method, but it's not really a strong opinion.

Quoted 7 lines

Considering the commit message of 3a7a0a which you just committed:

"Specifying a floating tag like HEAD for a repository version is bad practice, as it results in non-reproducible builds."

I would say this is a bad idea. Instead, there could be a check that PKG_VERSION is not empty.

I agree with this suggestion, this would be a better solution for the mentioned problem.

Best regards,
Thomas

#5 lore raw

PATCH v2 1/1 pkg-download: take some more care of <PKG_>VERSION with git

In reply to Thomas Petazzoni

Hi,

Quoted 16 lines · Thomas Petazzoni

Hello,

Quoted 13 lines · Patrick Boettcher

This replaces the one-liner patch I made last week after a comment by Thomas De Schampheleire. (it is now a 5-liner)

I added comment to explain why using HEAD is better in the current situation. I did not change the hg-part as it is different to git - after a quick read it won't work at all when VERSION is not set in a packge.

Please apply if appropriate and helpful (it is for me).

Patrick Boettcher (1):
pkg-download: take some more care of <PKG_>VERSION when METHOD is git

Quoted 8 lines
Quoted 2 lines
 package/pkg-download.mk |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

Thomas, Arnout, Gustavo, some thoughts about this patch?

I'm not a big fan of the fact that <pkg>_VERSION would be optional for the git site method, but mandatory for the hg site method, but it's not really a strong opinion.

As I commented [1] on the original patch [2], I think git and Mercurial should be treated equally where possible, so the patch as-is is not yet ok.
Additionally, Arnout correctly noted in [3] that the patch is not correct in all cases.

[1] http://lists.busybox.net/pipermail/buildroot/2013-March/070035.html
[2] http://lists.busybox.net/pipermail/buildroot/2013-March/070032.html
[3] http://lists.busybox.net/pipermail/buildroot/2013-April/070123.html

Best regards,
Thomas

#6 lore raw

PATCH v2 1/1 pkg-download: take some more care of <PKG_>VERSION with git

In reply to Thomas Petazzoni
Quoted 5 lines · Thomas Petazzoni

Thomas, Arnout, Gustavo, some thoughts about this patch?

I'm not a big fan of the fact that <pkg>_VERSION would be optional for the git site method, but mandatory for the hg site method, but it's not really a strong opinion.

I agree with Arnout.
It MAY make life simpler for some people, but it opens the "moving target" can of worms.
IF we accept moving targets then policy should be to never use it in a buildroot release for anything.
Regards.