Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH] pkg-download.mk: Fix shallow clone
@ 2012-12-02 10:56 Stephan Hoffmann
  2012-12-02 16:05 ` Peter Korsgaard
  0 siblings, 1 reply; 7+ messages in thread
From: Stephan Hoffmann @ 2012-12-02 10:56 UTC (permalink / raw)
  To: buildroot

Some git versions (e.g. 1.7.0) do not treat trying to shallow clone
a non existing branch or tag as a fatal error but report a warning
and clone HEAD instead. Thus the fallback mechanism does not work
in this case.

This patch introduces a check for the presence of the requested
version as a branch or tag before trying the shallow clone. It
also removes the need to do two clones when a sha1 is given as
a packege version.

Signed-off-by: Stephan Hoffmann <sho@relinux.de>
---
 package/pkg-download.mk |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 6c57244..627e22d 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -74,12 +74,19 @@ domainseparator=$(if $(1),$(1),/)
 ################################################################################
 
 # Try a shallow clone - but that only works if the version is a ref (tag or
-# branch). Fall back on a full clone if it's a generic sha1.
+# 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.
+#
+# Messages for the type of clone used are provided to ease debugging in case of
+# problems
 define DOWNLOAD_GIT
 	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	(pushd $(DL_DIR) > /dev/null && \
-	 ($(GIT) clone --depth 1 -b $($(PKG)_DL_VERSION) --bare $($(PKG)_SITE) $($(PKG)_BASE_NAME) || \
-	  $(GIT) clone --bare $($(PKG)_SITE) $($(PKG)_BASE_NAME)) && \
+	 ((test `git ls-remote  $($(PKG)_SITE) | cut -b 42- | grep $($(PKG)_DL_VERSION)` && \
+	   echo "Doing shallow clone" && \
+	   $(GIT) clone --depth 1 -b $($(PKG)_DL_VERSION) --bare $($(PKG)_SITE) $($(PKG)_BASE_NAME)) || \
+	  (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) | \
 		gzip -c > $(DL_DIR)/$($(PKG)_SOURCE) && \
-- 
1.7.0.4

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH] pkg-download.mk: Fix shallow clone
  2012-12-02 10:56 [Buildroot] [PATCH] pkg-download.mk: Fix shallow clone Stephan Hoffmann
@ 2012-12-02 16:05 ` Peter Korsgaard
  2012-12-06  2:14   ` Danomi Manchego
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Korsgaard @ 2012-12-02 16:05 UTC (permalink / raw)
  To: buildroot

>>>>> "Stephan" == Stephan Hoffmann <sho@relinux.de> writes:

 Stephan> Some git versions (e.g. 1.7.0) do not treat trying to shallow clone
 Stephan> a non existing branch or tag as a fatal error but report a warning
 Stephan> and clone HEAD instead. Thus the fallback mechanism does not work
 Stephan> in this case.

 Stephan> This patch introduces a check for the presence of the requested
 Stephan> version as a branch or tag before trying the shallow clone. It
 Stephan> also removes the need to do two clones when a sha1 is given as
 Stephan> a packege version.

Thanks, committed.

I really do think we need to move the git handling into an external
script, because this isn't really readable anymore - But that's for
post-2012.11.

I've changed the cut -b 42- into cut -f2- as that's more robust (and
readable imho).


-- 
Bye, Peter Korsgaard

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH] pkg-download.mk: Fix shallow clone
  2012-12-02 16:05 ` Peter Korsgaard
@ 2012-12-06  2:14   ` Danomi Manchego
  2012-12-06  7:03     ` Arnout Vandecappelle
  2012-12-06 10:34     ` Stephan Hoffmann
  0 siblings, 2 replies; 7+ messages in thread
From: Danomi Manchego @ 2012-12-06  2:14 UTC (permalink / raw)
  To: buildroot

Hi all,

I ran into a minor problem with the shallow clone changes.  The test for
doing a git shallow clone uses a grep.  From pkg-download.mk:

   ((test `git ls-remote  $($(PKG)_SITE) | cut -f 2- | grep
$($(PKG)_DL_VERSION)` && \

If the grep returns multiple matches, then the "test" will fail, and then
go on to do a full clone. The failure prints out:

   bash: test: too many arguments

It's because the grep filter matches multiple items in the ls-remote
output.  For example, if you try to get your kernel from "
http://arago-project.org/git/projects/linux-davinci.git", specifying a
version of "03.22.00.02", then you will get two matches:

   $ git ls-remote
http://arago-project.org/git/projects/linux-davinci.git| cut -f 2- |
grep 03.22.00.02
    refs/heads/03.22.00.02
    refs/tags/DEV_DAVINCIPSP_03.22.00.02

There are ways to massage the grep to do better (maybe use "grep -q", or
grep for "/string$" instead of just "string"), but the ls-remote command
seems to take a ref directly.  I.e.

   $ git ls-remote
http://arago-project.org/git/projects/linux-davinci.git03.22.00.02 |
cut -f 2-
   refs/heads/03.22.00.02
   $
   $ git ls-remote
http://arago-project.org/git/projects/linux-davinci.gitDEV_DAVINCIPSP_03.22.00.02
| cut -f 2-
   refs/tags/DEV_DAVINCIPSP_03.22.00.02
   $
   $ git ls-remote
http://arago-project.org/git/projects/linux-davinci.gitJUNK | cut -f
2-
   $

So I'm wondering why the recent changes in this area used grep at all,
rather than asking ls-remote directly.  I.e.:

 ((test `git ls-remote $($(PKG)_SITE) $($(PKG)_DL_VERSION) | cut -f 2-` && \

If there is feedback that the above is desirable, then I can send in the
patch.

Danomi -
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20121205/6418600c/attachment.html>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH] pkg-download.mk: Fix shallow clone
  2012-12-06  2:14   ` Danomi Manchego
@ 2012-12-06  7:03     ` Arnout Vandecappelle
  2012-12-06 10:59       ` Stephan Hoffmann
  2012-12-06 10:34     ` Stephan Hoffmann
  1 sibling, 1 reply; 7+ messages in thread
From: Arnout Vandecappelle @ 2012-12-06  7:03 UTC (permalink / raw)
  To: buildroot

On 06/12/12 03:14, Danomi Manchego wrote:
> So I'm wondering why the recent changes in this area used grep at all, rather than asking ls-remote directly.  I.e.:
>
> ((test `git ls-remote $($(PKG)_SITE) $($(PKG)_DL_VERSION) | cut -f 2-` && \
>
> If there is feedback that the above is desirable, then I can send in the patch.

  The argument of test should be quoted to avoid getting the same problem again
when the ref matches more than once for some reason.

  Or even better: use the exit code:

git ls-remote --exit-code $($(PKG)_SITE) $($(PKG)_DL_VERSION) > /dev/null 2>&1 && \

(hopefully the --exit-code option exists in older gits as well...)

  Regards,
  Arnout

-- 
Arnout Vandecappelle                               arnout at mind be
Senior Embedded Software Architect                 +32-16-286540
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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH] pkg-download.mk: Fix shallow clone
  2012-12-06  2:14   ` Danomi Manchego
  2012-12-06  7:03     ` Arnout Vandecappelle
@ 2012-12-06 10:34     ` Stephan Hoffmann
  1 sibling, 0 replies; 7+ messages in thread
From: Stephan Hoffmann @ 2012-12-06 10:34 UTC (permalink / raw)
  To: buildroot

Am 06.12.2012 03:14, schrieb Danomi Manchego:
> Hi all,
>
> I ran into a minor problem with the shallow clone changes.  The test
> for doing a git shallow clone uses a grep.  From pkg-download.mk
> <http://pkg-download.mk>:
>
>   ((test `git ls-remote  $($(PKG)_SITE) | cut -f 2- | grep
> $($(PKG)_DL_VERSION)` && \
>
> If the grep returns multiple matches, then the "test" will fail, and
> then go on to do a full clone. The failure prints out:
>
>    bash: test: too many arguments
>
> It's because the grep filter matches multiple items in the ls-remote
> output.  For example, if you try to get your kernel from
> "http://arago-project.org/git/projects/linux-davinci.git", specifying
> a version of "03.22.00.02", then you will get two matches:
>
>    $ git ls-remote
> http://arago-project.org/git/projects/linux-davinci.git | cut -f 2- |
> grep 03.22.00.02
>     refs/heads/03.22.00.02 <http://03.22.00.02>
>     refs/tags/DEV_DAVINCIPSP_03.22.00.02
>
> There are ways to massage the grep to do better (maybe use "grep -q",
> or grep for "/string$" instead of just "string"), but the ls-remote
> command seems to take a ref directly.  I.e.
>
>    $ git ls-remote
> http://arago-project.org/git/projects/linux-davinci.git 03.22.00.02 |
> cut -f 2-
>    refs/heads/03.22.00.02 <http://03.22.00.02>
>    $
>    $ git ls-remote
> http://arago-project.org/git/projects/linux-davinci.git
> DEV_DAVINCIPSP_03.22.00.02 | cut -f 2-
>    refs/tags/DEV_DAVINCIPSP_03.22.00.02
>    $
>    $ git ls-remote
> http://arago-project.org/git/projects/linux-davinci.git JUNK | cut -f 2-
>    $
>
> So I'm wondering why the recent changes in this area used grep at all,
> rather than asking ls-remote directly.  I.e.:
>
> ((test `git ls-remote $($(PKG)_SITE) $($(PKG)_DL_VERSION) | cut -f 2-`
> && \
Hello Danomi,

this is quite simple to answer: because I didn't find out this possibility.
>
> If there is feedback that the above is desirable, then I can send in
> the patch.
If you send a patch I am quite sure to be able to test it.

Kind regards

Stephan
>
> Danomi -
>


-- 
reLinux     -    Stephan Hoffmann
Am Schmidtgrund 124    50765 K?ln
Tel. +49.221.95595-19    Fax: -64
www.reLinux.de     sho at reLinux.de


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20121206/f6062f47/attachment.html>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH] pkg-download.mk: Fix shallow clone
  2012-12-06  7:03     ` Arnout Vandecappelle
@ 2012-12-06 10:59       ` Stephan Hoffmann
  2012-12-06 11:28         ` Danomi Manchego
  0 siblings, 1 reply; 7+ messages in thread
From: Stephan Hoffmann @ 2012-12-06 10:59 UTC (permalink / raw)
  To: buildroot

Am 06.12.2012 08:03, schrieb Arnout Vandecappelle:
> On 06/12/12 03:14, Danomi Manchego wrote:
>> So I'm wondering why the recent changes in this area used grep at
>> all, rather than asking ls-remote directly.  I.e.:
>>
>> ((test `git ls-remote $($(PKG)_SITE) $($(PKG)_DL_VERSION) | cut -f
>> 2-` && \
>>
>> If there is feedback that the above is desirable, then I can send in
>> the patch.
>
>  The argument of test should be quoted to avoid getting the same
> problem again
> when the ref matches more than once for some reason.
>
>  Or even better: use the exit code:
>
> git ls-remote --exit-code $($(PKG)_SITE) $($(PKG)_DL_VERSION) >
> /dev/null 2>&1 && \
>
> (hopefully the --exit-code option exists in older gits as well...)
Hi,

sorry to say, but my git 1.7.0 does not know anything about a
--exit-code option and ls-remote always gives a exit code of 0 as long
as the repository can be connected.

Maybe its worth to think about adding a host-git to buildroot to
overcome such compatibility issues?

Kind regards

Stephan
>
>  Regards,
>  Arnout
>


-- 
reLinux     -    Stephan Hoffmann
Am Schmidtgrund 124    50765 K?ln
Tel. +49.221.95595-19    Fax: -64
www.reLinux.de     sho at reLinux.de

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [Buildroot] [PATCH] pkg-download.mk: Fix shallow clone
  2012-12-06 10:59       ` Stephan Hoffmann
@ 2012-12-06 11:28         ` Danomi Manchego
  0 siblings, 0 replies; 7+ messages in thread
From: Danomi Manchego @ 2012-12-06 11:28 UTC (permalink / raw)
  To: buildroot

Arnout - thanks for the feedback.
Stephan - thanks for looking into the the --exit-status on older gits.

Since the --exit-status isn't universally available, maybe the simplest
thing to do is to add quotes, and drop the "cut" and "grep":

 ((test "`git ls-remote $($(PKG)_SITE) $($(PKG)_DL_VERSION)`" && \

Assume no objections or better ideas come in, I'll send in a patch tonight.

Thanks,
Danomi -
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20121206/651c3a47/attachment.html>

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2012-12-06 11:28 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-12-02 10:56 [Buildroot] [PATCH] pkg-download.mk: Fix shallow clone Stephan Hoffmann
2012-12-02 16:05 ` Peter Korsgaard
2012-12-06  2:14   ` Danomi Manchego
2012-12-06  7:03     ` Arnout Vandecappelle
2012-12-06 10:59       ` Stephan Hoffmann
2012-12-06 11:28         ` Danomi Manchego
2012-12-06 10:34     ` Stephan Hoffmann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox