[Buildroot] [PATCH v2] pkg-download: take some more care of <PKG_>VERSION with git

[Buildroot] [PATCH v2] pkg-download: take some more care of <PKG_>VERSION with git

[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

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

--
1.7.10.4

[Buildroot] [PATCH] pkg-download: take some more care of <PKG_>VERSION when METHOD is git

[Patrick Boettcher]

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 <Patrick.Boettcher@parrot.com>
---
 package/pkg-download.mk |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 1705fcd..610ceb3 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -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) && \
-- 
1.7.10.4

[Buildroot] [PATCH v2] pkg-download: take some more care of <PKG_>VERSION with git

[Thomas Petazzoni]

Hello,

On Tue,  2 Apr 2013 21:40:08 +0200, Patrick Boettcher wrote:
> 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(-)

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
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

[Buildroot] [PATCH v2] pkg-download: take some more care of <PKG_>VERSION with git

[Arnout Vandecappelle]

On 13/08/13 12:15, Thomas Petazzoni wrote:
> Hello,
>
> On Tue,  2 Apr 2013 21:40:08 +0200, Patrick Boettcher wrote:
>> 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(-)
>
> 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
-- 
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

[Buildroot] [PATCH v2] pkg-download: take some more care of <PKG_>VERSION with git

[Thomas De Schampheleire]

On Tue, Aug 13, 2013 at 6:40 PM, Arnout Vandecappelle <arnout@mind.be> wrote:
> On 13/08/13 12:15, Thomas Petazzoni wrote:
>>
>> Hello,
>>
>> On Tue,  2 Apr 2013 21:40:08 +0200, Patrick Boettcher wrote:
>>>
>>> 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(-)
>>
>>
>> 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.

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

Best regards,
Thomas

[Buildroot] [PATCH v2] pkg-download: take some more care of <PKG_>VERSION with git

[Thomas De Schampheleire]

Hi,

On Tue, Aug 13, 2013 at 12:15 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> Hello,
>
> On Tue,  2 Apr 2013 21:40:08 +0200, Patrick Boettcher wrote:
>> 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(-)
>
> 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

[Buildroot] [PATCH v2] pkg-download: take some more care of <PKG_>VERSION with git

[Gustavo Zacarias]

On 08/13/2013 07:15 AM, Thomas Petazzoni wrote:
> 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.