Skip to contentSkip to conversation

Cleanups in download helpers (branch yem/check-downloads)

PATCH Superseded Yann E. MORIN 3 participants 11 messages Jul 7, 2014 – Jul 8, 2014

v2 of 4 Superseded Latest is v4 →

lore
Share & tools
lore

Yann E. MORIN

author
#0 lore raw

v2 cover Cleanups in download helpers (branch yem/check-downloads)

Hello All!

This series is a follow-up to the previous download series, and brings in:
- fixes the git and bzr helpers
- fixes a potential race in parallel downloads
- simplifies the localfiles helper
- postpones creating temp files/dirs only at the point they are needed
- cleanups and harmonises the naming of temp files/dirs

Changes v1 -> v2:
- fix the git helper
- add the git comment in a single patch
- fix left-overs in the cvs helper
- fix double-dot typo in scp helper
- harmoise the svn and cvs helpers wrt. tar options

Regards,
Yann E. MORIN.

The following changes since commit 1473c8c4c819aa5b544eb2d9368c510ada3ec6c0:

linux: bump default to version 3.15.4 (2014-07-07 19:44:52 +0200)

are available in the git repository at:

git://gitorious.org/buildroot/buildroot.git yem/check-downloads

for you to fetch changes up to b16d43fcd8082ce3444c04e8db2ef2e896b40834:

support/download: rationalise naming and use of the temporary files (2014-07-07 22:08:06 +0200)

----------------------------------------------------------------
Yann E. MORIN (6):
support/download: fix the bzr helper
support/download: fix the git helper
support/download: properly use temp files support/download: simplify the local-files helper support/download: only create final temp file when needed support/download: rationalise naming and use of the temporary files

 Config.in               |  4 ----
 Config.in.legacy        | 11 +++++++++++
 package/pkg-download.mk |  1 -
 support/download/bzr    |  9 ++++-----
 support/download/cp     |  6 ++++--
 support/download/cvs    | 19 +++++++++++--------
 support/download/git    | 26 ++++++++++++++------------
 support/download/hg     | 11 +++--------
 support/download/scp    |  8 +++++---
 support/download/svn    | 21 ++++++++++++---------
 support/download/wget   |  7 +++----
 11 files changed, 67 insertions(+), 56 deletions(-)
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.  |
'------------------------------^-------^------------------^--------------------'
author
#1 lore raw

PATCH v2 1/6 support/download: fix the bzr helper

bzr uses the name of the extension of the output file to known what output format to use: tar, tgz, tar.bz2... If no extension is recognised, bzr will output to a directory.

Since we use 'mktemp .XXXXXX' to generate temporary files, it obviously never ends with a recognised extension. Thus, bzr expects the output to be a directory, and fails since it is a file.

Fix that by forcing the output format.

Signed-off-by "Yann E. MORIN" <[email protected]>
Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>
---
 support/download/bzr | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
support/download/bzr +1 -1
index 3f52ee9..19d837d 100755
@@ -26,7 +26,7 @@ tmp_output="$( mktemp "${output}.XXXXXX" )"
 # - finally, we atomically rename to the final file
 
 ret=1
-if ${BZR} export "${tmp_dl}" "${repo}" -r "${rev}"; then
+if ${BZR} export --format=tgz "${tmp_dl}" "${repo}" -r "${rev}"; then
     if mv "${tmp_dl}" "${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
Signature · 1 line
1.9.1
#2 lore raw

PATCH v2 1/6 support/download: fix the bzr helper

Quoted 6 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

bzr uses the name of the extension of the output file to known what output format to use: tar, tgz, tar.bz2... If no extension is recognised, bzr will output to a directory.

Quoted 8 lines

Since we use 'mktemp .XXXXXX' to generate temporary files, it obviously never ends with a recognised extension. Thus, bzr expects the output to be a directory, and fails since it is a file.

Fix that by forcing the output format.

Signed-off-by "Yann E. MORIN" <[email protected]>
Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

Committed, thanks.

Signature · 1 line
Bye, Peter Korsgaard
author
#3 lore raw

PATCH v2 2/6 support/download: fix the git helper

In reply to Yann E. MORIN

When switching the git helper over to a shell script, a special case was not carried over: in case the remote has the required reference, we attempt a shallow clone, using --depth 1. However, this is not supported when the remote is accessed with the http protocol.

Therefore, the download fails.

What happened before the conversion to a shell script was that the helper in the Makefile would fallback to doing a full-clone.

This is the case and behaviour that were lost in the conversion.

To avoid making the script too complex, we only attempt a full clone if needed. And we decide that a full clone is needed by default; we decide it is unnecessary if the remote has the needed reference *and* the shallow clone was successful.

Signed-off-by "Yann E. MORIN" <[email protected]>
---
 support/download/git | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
support/download/git +6 -2
index badb491..116e5a9 100755
@@ -33,10 +33,14 @@ cd "${BUILD_DIR}"
 # Remove leftovers from a previous failed run
 rm -rf "${repodir}"
 
+git_done=0
 if [ -n "$(${GIT} ls-remote "${repo}" "${cset}" 2>&1)" ]; then
     printf "Doing shallow clone\n"
-    ${GIT} clone --depth 1 -b "${cset}" --bare "${repo}" "${repodir}"
-else
+    if ${GIT} clone --depth 1 -b "${cset}" --bare "${repo}" "${repodir}"; then
+        git_done=1
+    fi
+fi
+if [ ${git_done} -eq 0 ]; then
     printf "Doing full clone\n"
     ${GIT} clone --bare "${repo}" "${repodir}"
 fi
Signature · 1 line
1.9.1
#4 lore raw

PATCH v2 2/6 support/download: fix the git helper

Quoted 10 lines · Yann E. MORIN

When switching the git helper over to a shell script, a special case was not carried over: in case the remote has the required reference, we attempt a shallow clone, using --depth 1. However, this is not supported when the remote is accessed with the http protocol.

Therefore, the download fails.

What happened before the conversion to a shell script was that the helper in the Makefile would fallback to doing a full-clone.

Quoted 8 lines

This is the case and behaviour that were lost in the conversion.

To avoid making the script too complex, we only attempt a full clone if needed. And we decide that a full clone is needed by default; we decide it is unnecessary if the remote has the needed reference *and* the shallow clone was successful.

Signed-off-by "Yann E. MORIN" <[email protected]>
Reviewed-by Arnout Vandecappelle (Essensium/Mind) <[email protected]>

Actually, the ls-remote step is a bit useless: a failing shallow clone will not
be slower, and if it succeeds it wasn't necessary to begin with.

Regards,
Arnout

Quoted 26 lines
---
 support/download/git | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
support/download/git +6 -2
index badb491..116e5a9 100755
@@ -33,10 +33,14 @@ cd "${BUILD_DIR}"
 # Remove leftovers from a previous failed run
 rm -rf "${repodir}"
 
+git_done=0
 if [ -n "$(${GIT} ls-remote "${repo}" "${cset}" 2>&1)" ]; then
     printf "Doing shallow clone\n"
-    ${GIT} clone --depth 1 -b "${cset}" --bare "${repo}" "${repodir}"
-else
+    if ${GIT} clone --depth 1 -b "${cset}" --bare "${repo}" "${repodir}"; then
+        git_done=1
+    fi
+fi
+if [ ${git_done} -eq 0 ]; then
     printf "Doing full clone\n"
     ${GIT} clone --bare "${repo}" "${repodir}"
 fi
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
#5 lore raw

PATCH v2 2/6 support/download: fix the git helper

Quoted 2 lines · Arnout Vandecappelle

Actually, the ls-remote step is a bit useless: a failing shallow clone will not
be slower, and if it succeeds it wasn't necessary to begin with.

d'Oh, silly me, that was introduced in d1f5fc29e in order to support pre-1.7.0
git versions that don't error out on a failing shallow clone. So it's not useless at all.

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
#6 lore raw

PATCH v2 2/6 support/download: fix the git helper

In reply to Yann E. MORIN
Quoted 12 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Yann" == Yann E MORIN <[email protected]> writes:

When switching the git helper over to a shell script, a special case was not carried over: in case the remote has the required reference, we attempt a shallow clone, using --depth 1. However, this is not supported when the remote is accessed with the http protocol.

Therefore, the download fails.

What happened before the conversion to a shell script was that the helper in the Makefile would fallback to doing a full-clone.

Quoted 8 lines

This is the case and behaviour that were lost in the conversion.

To avoid making the script too complex, we only attempt a full clone if needed. And we decide that a full clone is needed by default; we decide it is unnecessary if the remote has the needed reference *and* the shallow clone was successful.

Signed-off-by "Yann E. MORIN" <[email protected]>

Committed, thanks.

Signature · 1 line
Bye, Peter Korsgaard
author
#7 lore raw

PATCH v2 3/6 support/download: properly use temp files

In reply to Yann E. MORIN

When using 'mv' to move files to temp files, the existing temp file is forcibly removed by 'mv', and a new file is created.

Although this should have little impact to us, there is still a race conditions in case two parallel downloads would step on each other's toes, and it goes like that:

Process 1 Process 2
mktemp tmp_output
mv tmp_dl tmp_output
removes tmp_output
mktemp tmp_ouput
writes to tmp_output
mv tmp_dl tmp_output
removes tmp_output writes to tmp_output
mv tmp_output output
mv tmp_output output

In this case, mktemp has the opportunity to create the same tmp_output temporary file, and we trash the content from one with the content from the other, and the last to do the final 'mv' breaks horibly.

Instead, use 'cat', which guarantees that tmp_output is not removed before writing to it.

This complexifies a bit the local-files (cp) helper, but better safe than sorry.

(Note: this is a purely theoretical issue, I did not observe it yet, but it is slated to happen one day or the other, and will be very hard to debug then.)

Signed-off-by "Yann E. MORIN" <[email protected]>
---
 support/download/bzr  | 2 +-
 support/download/cp   | 9 ++++++---
 support/download/cvs  | 2 +-
 support/download/git  | 4 ++--
 support/download/hg   | 2 +-
 support/download/scp  | 2 +-
 support/download/svn  | 2 +-
 support/download/wget | 2 +-
 8 files changed, 14 insertions(+), 11 deletions(-)
support/download/bzr +1 -1
index 19d837d..f86fa82 100755
@@ -27,7 +27,7 @@ tmp_output="$( mktemp "${output}.XXXXXX" )"
 
 ret=1
 if ${BZR} export --format=tgz "${tmp_dl}" "${repo}" -r "${rev}"; then
-    if mv "${tmp_dl}" "${tmp_output}"; then
+    if cat "${tmp_dl}" >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
     fi
support/download/cp +6 -3
index 8f6bc06..e73159b 100755
@@ -13,12 +13,15 @@ set -e
 source="${1}"
 output="${2}"
 
+tmp_dl="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
 tmp_output="$( mktemp "${output}.XXXXXX" )"
 
 ret=1
-if ${LOCALFILES} "${source}" "${tmp_output}"; then
-    mv "${tmp_output}" "${output}"
-    ret=0
+if ${LOCALFILES} "${source}" "${tmp_dl}"; then
+    if cat "${tmp_dl}" >"${tmp_output}"; then
+        mv "${tmp_output}" "${output}"
+        ret=0
+    fi
 fi
 
 # Cleanup
support/download/cvs +1 -1
index 9aeed79..a8ab080 100755
@@ -36,7 +36,7 @@ rm -rf "${repodir}"
 ret=1
 if ${CVS} -z3 -d":pserver:anonymous@${repo}" \
            co -d "${repodir}" -r ":${rev}" -P "${rawname}"; then
-    if tar czf "${tmp_output}" "${repodir}"; then
+    if tar czf - "${repodir}" >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
     fi
support/download/git +2 -2
index 116e5a9..bc5aabc 100755
@@ -47,8 +47,8 @@ fi
 
 ret=1
 pushd "${repodir}" >/dev/null
-if ${GIT} archive --prefix="${basename}/" -o "${tmp_tar}" \
-                  --format=tar "${cset}"; then
+if ${GIT} archive --prefix="${basename}/" --format=tar "${cset}" \
+                  >"${tmp_tar}"; then
     if gzip -c "${tmp_tar}" >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
support/download/hg +1 -1
index 6e9e26b..8b36db9 100755
@@ -35,7 +35,7 @@ ret=1
 if ${HG} clone --noupdate --rev "${cset}" "${repo}" "${repodir}"; then
     if ${HG} archive --repository "${repodir}" --type tgz \
                      --prefix "${basename}" --rev "${cset}" \
-                     "${tmp_output}"; then
+                     - >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
     fi
support/download/scp +1 -1
index d3aad43..e16ece5 100755
@@ -17,7 +17,7 @@ tmp_output="$( mktemp "${output}.XXXXXX" )"
 
 ret=1
 if ${SCP} "${url}" "${tmp_dl}"; then
-    if mv "${tmp_dl}" "${tmp_output}"; then
+    if cat "${tmp_dl}" >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
     fi
support/download/svn +1 -1
index 39cbbcb..259d3ed 100755
@@ -33,7 +33,7 @@ rm -rf "${repodir}"
 
 ret=1
 if ${SVN} export "${repo}@${rev}" "${repodir}"; then
-    if tar czf "${tmp_output}" "${repodir}"; then
+    if tar czf - "${repodir}" >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
     fi
support/download/wget +1 -1
index cbeca3b..0f71108 100755
@@ -25,7 +25,7 @@ tmp_output="$( mktemp "${output}.XXXXXX" )"
 
 ret=1
 if ${WGET} -O "${tmp_dl}" "${url}"; then
-    if mv "${tmp_dl}" "${tmp_output}"; then
+    if cat "${tmp_dl}" >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
     fi
Signature · 1 line
1.9.1
author
#8 lore raw

PATCH v2 4/6 support/download: simplify the local-files helper

In reply to Yann E. MORIN

Currently, the local-files download helper behaves like all the other download helpers, by first copying into the BUILD_DIR, then to BR2_DL_DIR, and then rename the final file.

This does two copies, for the sake of using the LOCALFILES command.

Just get rid of the intermediate copy to BUILD_DIR. Instead, directly copy to the final temp file and rename that.

This does a single copy, but we lose the file access mode, so we just reinstate them (in case it's a self-extracting executable used in a br2-external package, for example.)

Signed-off-by "Yann E. MORIN" <[email protected]>

---
One may argue that we could just do a symlink dest -> source. But if source is on a network mount, it may vanish any time (eg. when undocking your laptop for example ;-) ). So we really need to do a copy.

---
 Config.in               |  4 ----
 Config.in.legacy        | 11 +++++++++++
 package/pkg-download.mk |  1 -
 support/download/cp     | 13 ++++++-------
 4 files changed, 17 insertions(+), 12 deletions(-)
Config.in +0 -4
index 50968fb..9e81d77 100644
@@ -71,10 +71,6 @@ config BR2_CVS
 	string "CVS command"
 	default "cvs"
 
-config BR2_LOCALFILES
-	string "Local files retrieval command"
-	default "cp"
-
 config BR2_SCP
 	string "Secure copy (scp) command"
 	default "scp"
Config.in.legacy +11 -0
index a2c7846..aa49806 100644
@@ -101,6 +101,17 @@ endif
 ###############################################################################
 comment "Legacy options removed in 2014.08"
 
+config BR2_LOCALFILES
+	string "Local files retrieval command has been removed"
+	help
+	  The option to specify how to copy local files has been removed.
+	  It is now handled by a helper script in support/download/cp.
+
+config BR2_LOCALFILES_WRAP
+	bool
+	default y if BR2_LOCALFILES != ""
+	select BR2_LEGACY
+
 config BR2_KERNEL_HEADERS_3_8
 	bool "kernel headers version 3.8.x are no longer supported"
 	select BR2_KERNEL_HEADERS_3_9
package/pkg-download.mk +0 -1
index 7f208d5..fb52ae0 100644
@@ -16,7 +16,6 @@ export GIT := $(call qstrip,$(BR2_GIT))
 export HG := $(call qstrip,$(BR2_HG)) $(QUIET)
 export SCP := $(call qstrip,$(BR2_SCP)) $(QUIET)
 SSH := $(call qstrip,$(BR2_SSH)) $(QUIET)
-export LOCALFILES := $(call qstrip,$(BR2_LOCALFILES))
 
 # Default spider mode is 'DOWNLOAD'. Other possible values are 'SOURCE_CHECK'
 # used by the _source-check target and 'SHOW_EXTERNAL_DEPS', used by the
support/download/cp +6 -7
index e73159b..e1e7337 100755
@@ -8,20 +8,19 @@ set -e
 #   $1: source file
 #   $2: output file
 # And this environment:
-#   LOCALFILES: the cp command to call
+#   (nothing special)
 
 source="${1}"
 output="${2}"
 
-tmp_dl="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
 tmp_output="$( mktemp "${output}.XXXXXX" )"
 
 ret=1
-if ${LOCALFILES} "${source}" "${tmp_dl}"; then
-    if cat "${tmp_dl}" >"${tmp_output}"; then
-        mv "${tmp_output}" "${output}"
-        ret=0
-    fi
+if cat "${source}" >"${tmp_output}"; then
+    mode="$( stat -c '%a' "${source}" )"
+    chmod "${mode}" "${tmp_output}"
+    mv "${tmp_output}" "${output}"
+    ret=0
 fi
 
 # Cleanup
Signature · 1 line
1.9.1
author
#9 lore raw

PATCH v2 5/6 support/download: only create final temp file when needed

In reply to Yann E. MORIN

Create the temp file in the final location only when it is needed.

This avoids the nasty experience of seeing lots of temp files in BR2_DL_DIR, that would linger around in case the downloads fails.

Add a comment on why we don't clean-up after git.

Reported-by Peter Korsgaard <[email protected]>
Signed-off-by "Yann E. MORIN" <[email protected]>

---
Changes v1 -> v2:
- fold the git comment in this commit

---
 support/download/bzr  | 2 +-
 support/download/cvs  | 2 +-
 support/download/git  | 7 +++++--
 support/download/hg   | 2 +-
 support/download/scp  | 2 +-
 support/download/svn  | 2 +-
 support/download/wget | 2 +-
 7 files changed, 11 insertions(+), 8 deletions(-)
support/download/bzr +1 -1
index f86fa82..a2cb440 100755
@@ -17,7 +17,6 @@ rev="${2}"
 output="${3}"
 
 tmp_dl="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
-tmp_output="$( mktemp "${output}.XXXXXX" )"
 
 # Play tic-tac-toe with temp files
 # - first, we download to a trashable location (the build-dir)

@@ -27,6 +26,7 @@ tmp_output="$( mktemp "${output}.XXXXXX" )"
 
 ret=1
 if ${BZR} export --format=tgz "${tmp_dl}" "${repo}" -r "${rev}"; then
+    tmp_output="$( mktemp "${output}.XXXXXX" )"
     if cat "${tmp_dl}" >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
support/download/cvs +1 -1
index a8ab080..22863d8 100755
@@ -21,7 +21,6 @@ basename="${4}"
 output="${5}"
 
 repodir="${basename}.tmp-cvs-checkout"
-tmp_output="$( mktemp "${output}.XXXXXX" )"
 
 cd "${BUILD_DIR}"
 # Remove leftovers from a previous failed run

@@ -36,6 +35,7 @@ rm -rf "${repodir}"
 ret=1
 if ${CVS} -z3 -d":pserver:anonymous@${repo}" \
            co -d "${repodir}" -r ":${rev}" -P "${rawname}"; then
+    tmp_output="$( mktemp "${output}.XXXXXX" )"
     if tar czf - "${repodir}" >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
support/download/git +5 -2
index bc5aabc..e8e9055 100755
@@ -19,8 +19,6 @@ basename="${3}"
 output="${4}"
 
 repodir="${basename}.tmp-git-checkout"
-tmp_tar="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
-tmp_output="$( mktemp "${output}.XXXXXX" )"
 
 # Play tic-tac-toe with temp files
 # - first, we download to a trashable location (the build-dir)

@@ -33,6 +31,9 @@ cd "${BUILD_DIR}"
 # Remove leftovers from a previous failed run
 rm -rf "${repodir}"
 
+# Upon failure, git cleans behind itself, so no need to catch failures
+# here. The only case when git would not clean up, is if it gets killed
+# with SIGKILL, which is user-initiated, so we let the user handle that.
 git_done=0
 if [ -n "$(${GIT} ls-remote "${repo}" "${cset}" 2>&1)" ]; then
     printf "Doing shallow clone\n"

@@ -47,8 +48,10 @@ fi
 
 ret=1
 pushd "${repodir}" >/dev/null
+tmp_tar="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
 if ${GIT} archive --prefix="${basename}/" --format=tar "${cset}" \
                   >"${tmp_tar}"; then
+    tmp_output="$( mktemp "${output}.XXXXXX" )"
     if gzip -c "${tmp_tar}" >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
support/download/hg +1 -1
index 8b36db9..7e5c9eb 100755
@@ -19,7 +19,6 @@ basename="${3}"
 output="${4}"
 
 repodir="${basename}.tmp-hg-checkout"
-tmp_output="$( mktemp "${output}.XXXXXX" )"
 
 cd "${BUILD_DIR}"
 # Remove leftovers from a previous failed run

@@ -33,6 +32,7 @@ rm -rf "${repodir}"
 
 ret=1
 if ${HG} clone --noupdate --rev "${cset}" "${repo}" "${repodir}"; then
+    tmp_output="$( mktemp "${output}.XXXXXX" )"
     if ${HG} archive --repository "${repodir}" --type tgz \
                      --prefix "${basename}" --rev "${cset}" \
                      - >"${tmp_output}"; then
support/download/scp +1 -1
index e16ece5..1cc18de 100755
@@ -13,10 +13,10 @@ set -e
 url="${1}"
 output="${2}"
 tmp_dl="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
-tmp_output="$( mktemp "${output}.XXXXXX" )"
 
 ret=1
 if ${SCP} "${url}" "${tmp_dl}"; then
+    tmp_output="$( mktemp "${output}.XXXXXX" )"
     if cat "${tmp_dl}" >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
support/download/svn +1 -1
index 259d3ed..232d887 100755
@@ -19,7 +19,6 @@ basename="${3}"
 output="${4}"
 
 repodir="${basename}.tmp-svn-checkout"
-tmp_output="$( mktemp "${output}.XXXXXX" )"
 
 cd "${BUILD_DIR}"
 # Remove leftovers from a previous failed run

@@ -33,6 +32,7 @@ rm -rf "${repodir}"
 
 ret=1
 if ${SVN} export "${repo}@${rev}" "${repodir}"; then
+    tmp_output="$( mktemp "${output}.XXXXXX" )"
     if tar czf - "${repodir}" >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
support/download/wget +1 -1
index 0f71108..e961d71 100755
@@ -15,7 +15,6 @@ url="${1}"
 output="${2}"
 
 tmp_dl="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
-tmp_output="$( mktemp "${output}.XXXXXX" )"
 
 # Play tic-tac-toe with temp files
 # - first, we download to a trashable location (the build-dir)

@@ -25,6 +24,7 @@ tmp_output="$( mktemp "${output}.XXXXXX" )"
 
 ret=1
 if ${WGET} -O "${tmp_dl}" "${url}"; then
+    tmp_output="$( mktemp "${output}.XXXXXX" )"
     if cat "${tmp_dl}" >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
Signature · 1 line
1.9.1
author
#10 lore raw

PATCH v2 6/6 support/download: rationalise naming and use of the temporary files

In reply to Yann E. MORIN

Currently, we have temporary files to store 'downloaded' files or repositories, and they all are created using 'mktemp .XXXXXX'. Also, temporary repositories are named in a inconsistent manner.

This poses a problem in case something goes wrong: it is not possible to easily see what temp file corresponds to what attempted download.

Make all this a bit more homogeneous:
- name the temporary files and directories after the final file,

    with this mktemp pattern:   ${BUILD_DIR}/.${output##*/}.XXXXXX

This ensures it is easy to correlate a temp file or dir to the associated download attempt.

Signed-off-by "Yann E. MORIN" <[email protected]>

---
Notes:
- I was not able to test those helpers, because we have no package

    using them: cp, cvs, scp;
  - I tested bzr with our sole package using bazaar, so corner cases
    might spring to our attention at unexpected times;

- the cvs helper uses a little trick, due to the fact that cvs is so ancient it has never learned the ability to generate archives on its own.

Tested with this defconfig, which exercises all possible helpers:
BR2_x86_i686=y
BR2_TOOLCHAIN_EXTERNAL=y # wget
BR2_PACKAGE_DVB_APPS=y # hg
BR2_PACKAGE_DTV_SCAN_TABLES=y # git
BR2_PACKAGE_FIS=y # svn
BR2_PACKAGE_PYTHON=y # wget, needed for pynfc:
BR2_PACKAGE_PYTHON_NFC=y # bzr

---
Changes v1 -> v2:
- remove left-over hunk in cvs helper
- fix double-dot in scp helper
- use the tar transform trick in svn helper, too

---
 support/download/bzr  |  3 +--
 support/download/cvs  | 17 ++++++++++-------
 support/download/git  |  7 +------
 support/download/hg   |  7 +------
 support/download/scp  |  4 +++-
 support/download/svn  | 19 +++++++++++--------
 support/download/wget |  3 +--
 7 files changed, 28 insertions(+), 32 deletions(-)
support/download/bzr +1 -2
index a2cb440..ff01bff 100755
@@ -16,8 +16,6 @@ repo="${1}"
 rev="${2}"
 output="${3}"
 
-tmp_dl="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
-
 # Play tic-tac-toe with temp files
 # - first, we download to a trashable location (the build-dir)
 # - the we move to a temp file in the final location, so it is

@@ -25,6 +23,7 @@ tmp_dl="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
 # - finally, we atomically rename to the final file
 
 ret=1
+tmp_dl="$( mktemp "${BUILD_DIR}/.${output##*/}.XXXXXX" )"
 if ${BZR} export --format=tgz "${tmp_dl}" "${repo}" -r "${rev}"; then
     tmp_output="$( mktemp "${output}.XXXXXX" )"
     if cat "${tmp_dl}" >"${tmp_output}"; then
support/download/cvs +10 -7
index 22863d8..b580eda 100755
@@ -20,23 +20,26 @@ rawname="${3}"
 basename="${4}"
 output="${5}"
 
-repodir="${basename}.tmp-cvs-checkout"
-
-cd "${BUILD_DIR}"
-# Remove leftovers from a previous failed run
-rm -rf "${repodir}"
-
 # Play tic-tac-toe with temp files
 # - first, we download to a trashable location (the build-dir)
 # - then we create a temporary tarball in the final location, so it is
 #   on the same filesystem as the final file
 # - finally, we atomically rename to the final file
 
+# Since cvs by itself is not capable of generating archives, we use
+# a little trick of tar, to transform the filenames as they are being
+# added to the archive, by replacing the leadin '.' with the basename
+# of the package. Note: basename is just that, a basename, so it does
+# not contain any '/', so we need not escape them for the transform
+# expression.
+
 ret=1
+repodir="$( mktemp -d "${BUILD_DIR}/.${output##*/}.XXXXXX" )"
 if ${CVS} -z3 -d":pserver:anonymous@${repo}" \
            co -d "${repodir}" -r ":${rev}" -P "${rawname}"; then
     tmp_output="$( mktemp "${output}.XXXXXX" )"
-    if tar czf - "${repodir}" >"${tmp_output}"; then
+    if tar czf - -C "${repodir}" --transform "s/^\./${basename}/;" . \
+           >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
     fi
support/download/git +1 -6
index e8e9055..c53d87f 100755
@@ -18,8 +18,6 @@ cset="${2}"
 basename="${3}"
 output="${4}"
 
-repodir="${basename}.tmp-git-checkout"
-
 # Play tic-tac-toe with temp files
 # - first, we download to a trashable location (the build-dir)
 # - then we create the uncomporessed tarball in tht same trashable location

@@ -27,14 +25,11 @@ repodir="${basename}.tmp-git-checkout"
 #   it is on the same filesystem as the final file
 # - finally, we atomically rename to the final file
 
-cd "${BUILD_DIR}"
-# Remove leftovers from a previous failed run
-rm -rf "${repodir}"
-
 # Upon failure, git cleans behind itself, so no need to catch failures
 # here. The only case when git would not clean up, is if it gets killed
 # with SIGKILL, which is user-initiated, so we let the user handle that.
 git_done=0
+repodir="$( mktemp -d "${BUILD_DIR}/.${output##*/}.XXXXXX" )"
 if [ -n "$(${GIT} ls-remote "${repo}" "${cset}" 2>&1)" ]; then
     printf "Doing shallow clone\n"
     if ${GIT} clone --depth 1 -b "${cset}" --bare "${repo}" "${repodir}"; then
support/download/hg +1 -6
index 7e5c9eb..104d034 100755
@@ -18,12 +18,6 @@ cset="${2}"
 basename="${3}"
 output="${4}"
 
-repodir="${basename}.tmp-hg-checkout"
-
-cd "${BUILD_DIR}"
-# Remove leftovers from a previous failed run
-rm -rf "${repodir}"
-
 # Play tic-tac-toe with temp files
 # - first, we download to a trashable location (the build-dir)
 # - then we create a temporary tarball in the final location, so it is

@@ -31,6 +25,7 @@ rm -rf "${repodir}"
 # - finally, we atomically rename to the final file
 
 ret=1
+repodir="$( mktemp -d "${BUILD_DIR}/.${output##*/}.XXXXXX" )"
 if ${HG} clone --noupdate --rev "${cset}" "${repo}" "${repodir}"; then
     tmp_output="$( mktemp "${output}.XXXXXX" )"
     if ${HG} archive --repository "${repodir}" --type tgz \
support/download/scp +3 -1
index 1cc18de..6aa3a23 100755
@@ -12,9 +12,11 @@ set -e
 
 url="${1}"
 output="${2}"
-tmp_dl="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
+tmp_dl="$( mktemp "${BUILD_DIR}/.${output##*/}.XXXXXX" )"
 
 ret=1
+# Note: it looks like scp does not unlink the destination,
+# which is what we want (observed by stracing scp.)
 if ${SCP} "${url}" "${tmp_dl}"; then
     tmp_output="$( mktemp "${output}.XXXXXX" )"
     if cat "${tmp_dl}" >"${tmp_output}"; then
support/download/svn +11 -8
index 232d887..7a46d0f 100755
@@ -18,22 +18,25 @@ rev="${2}"
 basename="${3}"
 output="${4}"
 
-repodir="${basename}.tmp-svn-checkout"
-
-cd "${BUILD_DIR}"
-# Remove leftovers from a previous failed run
-rm -rf "${repodir}"
-
 # Play tic-tac-toe with temp files
 # - first, we download to a trashable location (the build-dir)
 # - then we create a temporary tarball in the final location, so it is
 #   on the same filesystem as the final file
 # - finally, we atomically rename to the final file
 
+# Since svn by itself is not capable of generating archives, we use
+# a little trick of tar, to transform the filenames as they are being
+# added to the archive, by replacing the leadin '.' with the basename
+# of the package. Note: basename is just that, a basename, so it does
+# not contain any '/', so we need not escape them for the transform
+# expression.
+
 ret=1
-if ${SVN} export "${repo}@${rev}" "${repodir}"; then
+repodir="$( mktemp -d "${BUILD_DIR}/.${output##*/}.XXXXXX" )"
+if ${SVN} export --force "${repo}@${rev}" "${repodir}"; then
     tmp_output="$( mktemp "${output}.XXXXXX" )"
-    if tar czf - "${repodir}" >"${tmp_output}"; then
+    if tar czf - -C "${repodir}" --transform "s/^\./${basename}/;" . \
+           >"${tmp_output}"; then
         mv "${tmp_output}" "${output}"
         ret=0
     fi
support/download/wget +1 -2
index e961d71..9bb6700 100755
@@ -14,8 +14,6 @@ set -e
 url="${1}"
 output="${2}"
 
-tmp_dl="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
-
 # Play tic-tac-toe with temp files
 # - first, we download to a trashable location (the build-dir)
 # - then we copy to a temporary tarball in the final location, so it is

@@ -23,6 +21,7 @@ tmp_dl="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
 # - finally, we atomically rename to the final file
 
 ret=1
+tmp_dl="$( mktemp "${BUILD_DIR}/.${output##*/}.XXXXXX" )"
 if ${WGET} -O "${tmp_dl}" "${url}"; then
     tmp_output="$( mktemp "${output}.XXXXXX" )"
     if cat "${tmp_dl}" >"${tmp_output}"; then
Signature · 1 line
1.9.1