package: make package download from Mercurial repositories more robust

PATCH New Mathias De Maré 3 participants 4 messages Mar 5, 2014

Share & tools

Mathias De Maré

author

2014-03-05 06:54 UTC #0 lore raw

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

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? <[email protected]>

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

package/pkg-download.mk +7 -1

index 85842de..6f53f53 100644
@@ -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

Signature · 1 line

1.8.1.2

Thomas De Schampheleire

2014-03-05 08:12 UTC #1 lore raw

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

Hi Mathias, Yann,

Quoted 24 lines · 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? <[email protected]>

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

package/pkg-download.mk +4 -0

index 85842de..6f53f53 100644
@@ -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 && \

package/pkg-download.mk define SHOW_EXTERNAL_DEPS_SCP · line 194

Quoted 10 lines

package/pkg-download.mk +3 -1

        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

Mathias De Maré

author

2014-03-05 10:33 UTC #2 lore raw

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

Hello Thomas,

Quoted 43 lines · Thomas De Schampheleire

Hi Mathias, Yann,
Quoted 16 lines · 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? <[email protected]>
---
 package/pkg-download.mk | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
package/pkg-download.mk +0 -0
index 85842de..6f53f53 100644
@@ -176,13 +176,19 @@ define SHOW_EXTERNAL_DEPS_SCP
 endef
 
package/pkg-download.mk define SHOW_EXTERNAL_DEPS_SCP · line 187
Quoted 10 lines
package/pkg-download.mk +4 -0
 
+# 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) && \
package/pkg-download.mk define SHOW_EXTERNAL_DEPS_SCP · line 189
Quoted 2 lines
package/pkg-download.mk +1 -0
+       rm -rf $(DL_DIR)/$($(PKG)_SOURCE).tmp && \
        $(HG) archive --repository $($(PKG)_BASE_NAME) --type tgz
--prefix $($(PKG)_BASE_NAME)/ \
package/pkg-download.mk define SHOW_EXTERNAL_DEPS_SCP · line 185
Quoted 1 line
package/pkg-download.mk +0 -1
-                     --rev $($(PKG)_DL_VERSION)
$(DL_DIR)/$($(PKG)_SOURCE) && \
package/pkg-download.mk define SHOW_EXTERNAL_DEPS_SCP · line 190
Quoted 1 line
package/pkg-download.mk +1 -0
+                       --rev $($(PKG)_DL_VERSION)
$(DL_DIR)/$($(PKG)_SOURCE).tmp && \
package/pkg-download.mk define SHOW_EXTERNAL_DEPS_SCP · line 194
Quoted 4 lines
package/pkg-download.mk +1 -0
+       mv $(DL_DIR)/$($(PKG)_SOURCE).tmp $(DL_DIR)/$($(PKG)_SOURCE) && \
        rm -rf $($(PKG)_DL_DIR) && \
        popd > /dev/null)
 endef

Quoted 6 lines

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>

Yann E. MORIN

2014-03-05 16:59 UTC #3 lore raw

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

In reply to Thomas De Schampheleire

Mathias, Thomas,

On 2014-03-05 09:12 +0100, Thomas De Schampheleire spake thusly:

Quoted 52 lines

Quoted 24 lines · 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? <[email protected]>
---
 package/pkg-download.mk | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
package/pkg-download.mk +4 -0
index 85842de..6f53f53 100644
@@ -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 && \
package/pkg-download.mk define SHOW_EXTERNAL_DEPS_SCP · line 194
Quoted 10 lines
package/pkg-download.mk +3 -1
        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.

Quoted 1 line

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.

Signature · 6 lines

.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'