[Buildroot] [PATCH 1/1] package: make package download from Mercurial repositories more robust

[Buildroot] [PATCH 1/1] package: make package download from Mercurial repositories more robust

[Mathias De Maré]

Previously, when cloning a Mercurial repository,
aborting the clone could (in some cases) leave behind an empty directory.
As a result, a later clone would fail.

Signed-off-by: Mathias De Mar? <mathias.demare@gmail.com>
---
 package/pkg-download.mk | 1 +
 1 file changed, 1 insertion(+)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 758b211..c00689b 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -178,6 +178,7 @@ endef
 define DOWNLOAD_HG
 	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	(pushd $(DL_DIR) > /dev/null && \
+	rm -rf $($(PKG)_BASE_NAME) && \
 	$(HG) clone --noupdate --rev $($(PKG)_DL_VERSION) $($(PKG)_SITE) $($(PKG)_BASE_NAME) && \
 	$(HG) archive --repository $($(PKG)_BASE_NAME) --type tgz --prefix $($(PKG)_BASE_NAME)/ \
 	              --rev $($(PKG)_DL_VERSION) $(DL_DIR)/$($(PKG)_SOURCE) && \
-- 
1.8.1.2

[Buildroot] [PATCH 1/1] package: make package download from Mercurial repositories more robust

[Thomas De Schampheleire]

On Tue, Dec 10, 2013 at 7:31 PM, Mathias De Mar?
<mathias.demare@gmail.com> wrote:
> Previously, when cloning a Mercurial repository,
> aborting the clone could (in some cases) leave behind an empty directory.
> As a result, a later clone would fail.
>
> Signed-off-by: Mathias De Mar? <mathias.demare@gmail.com>
> ---
>  package/pkg-download.mk | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 758b211..c00689b 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -178,6 +178,7 @@ endef
>  define DOWNLOAD_HG
>         test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
>         (pushd $(DL_DIR) > /dev/null && \
> +       rm -rf $($(PKG)_BASE_NAME) && \
>         $(HG) clone --noupdate --rev $($(PKG)_DL_VERSION) $($(PKG)_SITE) $($(PKG)_BASE_NAME) && \
>         $(HG) archive --repository $($(PKG)_BASE_NAME) --type tgz --prefix $($(PKG)_BASE_NAME)/ \
>                       --rev $($(PKG)_DL_VERSION) $(DL_DIR)/$($(PKG)_SOURCE) && \

Acked-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

[Buildroot] [PATCH 1/1] package: make package download from Mercurial repositories more robust

[Thomas De Schampheleire]

On Tue, Dec 10, 2013 at 9:10 PM, Thomas De Schampheleire
<patrickdepinguin@gmail.com> wrote:
> On Tue, Dec 10, 2013 at 7:31 PM, Mathias De Mar?
> <mathias.demare@gmail.com> wrote:
>> Previously, when cloning a Mercurial repository,
>> aborting the clone could (in some cases) leave behind an empty directory.
>> As a result, a later clone would fail.
>>
>> Signed-off-by: Mathias De Mar? <mathias.demare@gmail.com>
>> ---
>>  package/pkg-download.mk | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
>> index 758b211..c00689b 100644
>> --- a/package/pkg-download.mk
>> +++ b/package/pkg-download.mk
>> @@ -178,6 +178,7 @@ endef
>>  define DOWNLOAD_HG
>>         test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
>>         (pushd $(DL_DIR) > /dev/null && \
>> +       rm -rf $($(PKG)_BASE_NAME) && \
>>         $(HG) clone --noupdate --rev $($(PKG)_DL_VERSION) $($(PKG)_SITE) $($(PKG)_BASE_NAME) && \
>>         $(HG) archive --repository $($(PKG)_BASE_NAME) --type tgz --prefix $($(PKG)_BASE_NAME)/ \
>>                       --rev $($(PKG)_DL_VERSION) $(DL_DIR)/$($(PKG)_SOURCE) && \
>
> Acked-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>

It seems that patchwork did not pick up this ack, nor the reply
itself... Is this a known issue?

[Buildroot] [PATCH 1/1] package: make package download from Mercurial repositories more robust

[Mathias De Maré]

Hello,

should I do anything extra to get this patch into buildroot?

Greetings,
Mathias


On Wed, Dec 11, 2013 at 7:12 PM, Thomas De Schampheleire <
patrickdepinguin@gmail.com> wrote:

> On Tue, Dec 10, 2013 at 9:10 PM, Thomas De Schampheleire
> <patrickdepinguin@gmail.com> wrote:
> > On Tue, Dec 10, 2013 at 7:31 PM, Mathias De Mar?
> > <mathias.demare@gmail.com> wrote:
> >> Previously, when cloning a Mercurial repository,
> >> aborting the clone could (in some cases) leave behind an empty
> directory.
> >> As a result, a later clone would fail.
> >>
> >> Signed-off-by: Mathias De Mar? <mathias.demare@gmail.com>
> >> ---
> >>  package/pkg-download.mk | 1 +
> >>  1 file changed, 1 insertion(+)
> >>
> >> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> >> index 758b211..c00689b 100644
> >> --- a/package/pkg-download.mk
> >> +++ b/package/pkg-download.mk
> >> @@ -178,6 +178,7 @@ endef
> >>  define DOWNLOAD_HG
> >>         test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
> >>         (pushd $(DL_DIR) > /dev/null && \
> >> +       rm -rf $($(PKG)_BASE_NAME) && \
> >>         $(HG) clone --noupdate --rev $($(PKG)_DL_VERSION)
> $($(PKG)_SITE) $($(PKG)_BASE_NAME) && \
> >>         $(HG) archive --repository $($(PKG)_BASE_NAME) --type tgz
> --prefix $($(PKG)_BASE_NAME)/ \
> >>                       --rev $($(PKG)_DL_VERSION)
> $(DL_DIR)/$($(PKG)_SOURCE) && \
> >
> > Acked-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
>
> It seems that patchwork did not pick up this ack, nor the reply
> itself... Is this a known issue?
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20131216/879c8805/attachment.html>

[Buildroot] [PATCH 1/1] package: make package download from Mercurial repositories more robust

[Thomas Petazzoni]

Dear Mathias De Mar?,

On Tue, 10 Dec 2013 19:31:30 +0100, Mathias De Mar? wrote:
> Previously, when cloning a Mercurial repository,
> aborting the clone could (in some cases) leave behind an empty directory.
> As a result, a later clone would fail.
> 
> Signed-off-by: Mathias De Mar? <mathias.demare@gmail.com>
> ---
>  package/pkg-download.mk | 1 +
>  1 file changed, 1 insertion(+)

Applied, thanks. I've manually added Thomas DS Acked-by, since it was
not automatically picked up by patchwork for some reason.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] package: make package download from Mercurial repositories more robust

[Mathias De Maré]

Previously, after cloning a Mercurial repository,
an interrupted 'hg archive' operation would leave behind a partial
archive. This caused future archive operations to fail.

Signed-off-by: Mathias De Mar? <mathias.demare@gmail.com>
---
 package/pkg-download.mk | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 85842de..6f53f53 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -176,13 +176,19 @@ define SHOW_EXTERNAL_DEPS_SCP
 endef
 
 
+# Download a file using Mercurial.
+# In case a previous clone failed, remove the directory.
+# To handle interrupted archive operations, use a temporary directory,
+# and remove this directory before archiving again.
 define DOWNLOAD_HG
 	test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
 	(pushd $(DL_DIR) > /dev/null && \
 	rm -rf $($(PKG)_BASE_NAME) && \
 	$(HG) clone --noupdate --rev $($(PKG)_DL_VERSION) $($(PKG)_SITE) $($(PKG)_BASE_NAME) && \
+	rm -rf $(DL_DIR)/$($(PKG)_SOURCE).tmp && \
 	$(HG) archive --repository $($(PKG)_BASE_NAME) --type tgz --prefix $($(PKG)_BASE_NAME)/ \
-	              --rev $($(PKG)_DL_VERSION) $(DL_DIR)/$($(PKG)_SOURCE) && \
+			--rev $($(PKG)_DL_VERSION) $(DL_DIR)/$($(PKG)_SOURCE).tmp && \
+	mv $(DL_DIR)/$($(PKG)_SOURCE).tmp $(DL_DIR)/$($(PKG)_SOURCE) && \
 	rm -rf $($(PKG)_DL_DIR) && \
 	popd > /dev/null)
 endef
-- 
1.8.1.2

[Buildroot] [PATCH 1/1] package: make package download from Mercurial repositories more robust

[Thomas De Schampheleire]

Hi Mathias, Yann,

On Wed, Mar 5, 2014 at 7:54 AM, Mathias De Mar?
<mathias.demare@gmail.com> wrote:
> Previously, after cloning a Mercurial repository,
> an interrupted 'hg archive' operation would leave behind a partial
> archive. This caused future archive operations to fail.
>
> Signed-off-by: Mathias De Mar? <mathias.demare@gmail.com>
> ---
>  package/pkg-download.mk | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 85842de..6f53f53 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -176,13 +176,19 @@ define SHOW_EXTERNAL_DEPS_SCP
>  endef
>
>
> +# Download a file using Mercurial.
> +# In case a previous clone failed, remove the directory.
> +# To handle interrupted archive operations, use a temporary directory,
> +# and remove this directory before archiving again.
>  define DOWNLOAD_HG
>         test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
>         (pushd $(DL_DIR) > /dev/null && \
>         rm -rf $($(PKG)_BASE_NAME) && \
>         $(HG) clone --noupdate --rev $($(PKG)_DL_VERSION) $($(PKG)_SITE) $($(PKG)_BASE_NAME) && \
> +       rm -rf $(DL_DIR)/$($(PKG)_SOURCE).tmp && \
>         $(HG) archive --repository $($(PKG)_BASE_NAME) --type tgz --prefix $($(PKG)_BASE_NAME)/ \
> -                     --rev $($(PKG)_DL_VERSION) $(DL_DIR)/$($(PKG)_SOURCE) && \
> +                       --rev $($(PKG)_DL_VERSION) $(DL_DIR)/$($(PKG)_SOURCE).tmp && \
> +       mv $(DL_DIR)/$($(PKG)_SOURCE).tmp $(DL_DIR)/$($(PKG)_SOURCE) && \
>         rm -rf $($(PKG)_DL_DIR) && \
>         popd > /dev/null)
>  endef

When the operation is indeed interrupted, the tmp file will not be removed.
For the wget helper, there is a line:
... ||  (rm -f $(DL_DIR)/$(2).tmp ; exit 1)

that I believe should do exactly that. Could you check this?

Also, Yann Morin has very recently sent some patches to move the
download helpers to shell scripts (see
http://patchwork.ozlabs.org/project/buildroot/list/?page=1 and search
for pkg-infra)
I suggested him to add similar robustness to all download helpers.
In one way or another, we should line up: either by accepting your
patch first and let Yann rebase, or alternatively by having Yann take
in your patch and integrate it into his new shell script mechanism.

Yann: what is your preference?

Thanks,
Thomas

[Buildroot] [PATCH 1/1] package: make package download from Mercurial repositories more robust

[Mathias De Maré]

Hello Thomas,

On Wed, Mar 5, 2014 at 9:12 AM, Thomas De Schampheleire <
patrickdepinguin@gmail.com> wrote:

> Hi Mathias, Yann,
>
> On Wed, Mar 5, 2014 at 7:54 AM, Mathias De Mar?
> <mathias.demare@gmail.com> wrote:
> > Previously, after cloning a Mercurial repository,
> > an interrupted 'hg archive' operation would leave behind a partial
> > archive. This caused future archive operations to fail.
> >
> > Signed-off-by: Mathias De Mar? <mathias.demare@gmail.com>
> > ---
> >  package/pkg-download.mk | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> > index 85842de..6f53f53 100644
> > --- a/package/pkg-download.mk
> > +++ b/package/pkg-download.mk
> > @@ -176,13 +176,19 @@ define SHOW_EXTERNAL_DEPS_SCP
> >  endef
> >
> >
> > +# Download a file using Mercurial.
> > +# In case a previous clone failed, remove the directory.
> > +# To handle interrupted archive operations, use a temporary directory,
> > +# and remove this directory before archiving again.
> >  define DOWNLOAD_HG
> >         test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
> >         (pushd $(DL_DIR) > /dev/null && \
> >         rm -rf $($(PKG)_BASE_NAME) && \
> >         $(HG) clone --noupdate --rev $($(PKG)_DL_VERSION) $($(PKG)_SITE)
> $($(PKG)_BASE_NAME) && \
> > +       rm -rf $(DL_DIR)/$($(PKG)_SOURCE).tmp && \
> >         $(HG) archive --repository $($(PKG)_BASE_NAME) --type tgz
> --prefix $($(PKG)_BASE_NAME)/ \
> > -                     --rev $($(PKG)_DL_VERSION)
> $(DL_DIR)/$($(PKG)_SOURCE) && \
> > +                       --rev $($(PKG)_DL_VERSION)
> $(DL_DIR)/$($(PKG)_SOURCE).tmp && \
> > +       mv $(DL_DIR)/$($(PKG)_SOURCE).tmp $(DL_DIR)/$($(PKG)_SOURCE) && \
> >         rm -rf $($(PKG)_DL_DIR) && \
> >         popd > /dev/null)
> >  endef
>
> When the operation is indeed interrupted, the tmp file will not be removed.
> For the wget helper, there is a line:
> ... ||  (rm -f $(DL_DIR)/$(2).tmp ; exit 1)
>
> that I believe should do exactly that. Could you check this?
>
This is correct, the wget helper apparently does the cleanup while running.
The change I've made to the Mercurial helper does the cleanup when starting
the next build. I don't really have a preference either way.

Greetings,
Mathias
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20140305/fa67f12b/attachment.html>

[Buildroot] [PATCH 1/1] package: make package download from Mercurial repositories more robust

[Yann E. MORIN]

Mathias, Thomas,

On 2014-03-05 09:12 +0100, Thomas De Schampheleire spake thusly:
> On Wed, Mar 5, 2014 at 7:54 AM, Mathias De Mar?
> <mathias.demare@gmail.com> wrote:
> > Previously, after cloning a Mercurial repository,
> > an interrupted 'hg archive' operation would leave behind a partial
> > archive. This caused future archive operations to fail.
> >
> > Signed-off-by: Mathias De Mar? <mathias.demare@gmail.com>
> > ---
> >  package/pkg-download.mk | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> > index 85842de..6f53f53 100644
> > --- a/package/pkg-download.mk
> > +++ b/package/pkg-download.mk
> > @@ -176,13 +176,19 @@ define SHOW_EXTERNAL_DEPS_SCP
> >  endef
> >
> >
> > +# Download a file using Mercurial.
> > +# In case a previous clone failed, remove the directory.
> > +# To handle interrupted archive operations, use a temporary directory,
> > +# and remove this directory before archiving again.
> >  define DOWNLOAD_HG
> >         test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
> >         (pushd $(DL_DIR) > /dev/null && \
> >         rm -rf $($(PKG)_BASE_NAME) && \
> >         $(HG) clone --noupdate --rev $($(PKG)_DL_VERSION) $($(PKG)_SITE) $($(PKG)_BASE_NAME) && \
> > +       rm -rf $(DL_DIR)/$($(PKG)_SOURCE).tmp && \
> >         $(HG) archive --repository $($(PKG)_BASE_NAME) --type tgz --prefix $($(PKG)_BASE_NAME)/ \
> > -                     --rev $($(PKG)_DL_VERSION) $(DL_DIR)/$($(PKG)_SOURCE) && \
> > +                       --rev $($(PKG)_DL_VERSION) $(DL_DIR)/$($(PKG)_SOURCE).tmp && \
> > +       mv $(DL_DIR)/$($(PKG)_SOURCE).tmp $(DL_DIR)/$($(PKG)_SOURCE) && \
> >         rm -rf $($(PKG)_DL_DIR) && \
> >         popd > /dev/null)
> >  endef
> 
> When the operation is indeed interrupted, the tmp file will not be removed.
> For the wget helper, there is a line:
> ... ||  (rm -f $(DL_DIR)/$(2).tmp ; exit 1)
> 
> that I believe should do exactly that. Could you check this?
> 
> Also, Yann Morin has very recently sent some patches to move the
> download helpers to shell scripts (see
> http://patchwork.ozlabs.org/project/buildroot/list/?page=1 and search
> for pkg-infra)
> I suggested him to add similar robustness to all download helpers.
> In one way or another, we should line up: either by accepting your
> patch first and let Yann rebase, or alternatively by having Yann take
> in your patch and integrate it into his new shell script mechanism.
> 
> Yann: what is your preference?

I already have implemented that in my series. I was waiting for a bit
more reviews before re-posting (and letting some time pass after the
debacle of my last patchset...).

I guess I'll just re-post that series tonight, then.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'