All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/6 v2] support/download: fix the bzr helper
  2014-07-07 21:44 [Buildroot] [PATCH 0/6 v2] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
@ 2014-07-07 21:44 ` Yann E. MORIN
  2014-07-08  8:53   ` Peter Korsgaard
  2014-07-07 21:44 ` [Buildroot] [PATCH 2/6 v2] support/download: fix the git helper Yann E. MORIN
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Yann E. MORIN @ 2014-07-07 21:44 UTC (permalink / raw)
  To: buildroot

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" <yann.morin.1998@free.fr>
Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>
---
 support/download/bzr | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/support/download/bzr b/support/download/bzr
index 3f52ee9..19d837d 100755
--- a/support/download/bzr
+++ b/support/download/bzr
@@ -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
-- 
1.9.1

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

* [Buildroot] [PATCH 2/6 v2] support/download: fix the git helper
  2014-07-07 21:44 [Buildroot] [PATCH 0/6 v2] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
  2014-07-07 21:44 ` [Buildroot] [PATCH 1/6 v2] support/download: fix the bzr helper Yann E. MORIN
@ 2014-07-07 21:44 ` Yann E. MORIN
  2014-07-08 16:45   ` Arnout Vandecappelle
  2014-07-08 21:27   ` Peter Korsgaard
  2014-07-07 21:44 ` [Buildroot] [PATCH 3/6 v2] support/download: properly use temp files Yann E. MORIN
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 11+ messages in thread
From: Yann E. MORIN @ 2014-07-07 21:44 UTC (permalink / raw)
  To: buildroot

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" <yann.morin.1998@free.fr>
---
 support/download/git | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/support/download/git b/support/download/git
index badb491..116e5a9 100755
--- a/support/download/git
+++ b/support/download/git
@@ -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
-- 
1.9.1

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

* [Buildroot] [PATCH 0/6 v2] Cleanups in download helpers (branch yem/check-downloads)
@ 2014-07-07 21:44 Yann E. MORIN
  2014-07-07 21:44 ` [Buildroot] [PATCH 1/6 v2] support/download: fix the bzr helper Yann E. MORIN
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Yann E. MORIN @ 2014-07-07 21:44 UTC (permalink / raw)
  To: buildroot

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(-)

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

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

* [Buildroot] [PATCH 3/6 v2] support/download: properly use temp files
  2014-07-07 21:44 [Buildroot] [PATCH 0/6 v2] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
  2014-07-07 21:44 ` [Buildroot] [PATCH 1/6 v2] support/download: fix the bzr helper Yann E. MORIN
  2014-07-07 21:44 ` [Buildroot] [PATCH 2/6 v2] support/download: fix the git helper Yann E. MORIN
@ 2014-07-07 21:44 ` Yann E. MORIN
  2014-07-07 21:44 ` [Buildroot] [PATCH 4/6 v2] support/download: simplify the local-files helper Yann E. MORIN
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Yann E. MORIN @ 2014-07-07 21:44 UTC (permalink / raw)
  To: buildroot

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" <yann.morin.1998@free.fr>
---
 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(-)

diff --git a/support/download/bzr b/support/download/bzr
index 19d837d..f86fa82 100755
--- a/support/download/bzr
+++ b/support/download/bzr
@@ -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
diff --git a/support/download/cp b/support/download/cp
index 8f6bc06..e73159b 100755
--- a/support/download/cp
+++ b/support/download/cp
@@ -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
diff --git a/support/download/cvs b/support/download/cvs
index 9aeed79..a8ab080 100755
--- a/support/download/cvs
+++ b/support/download/cvs
@@ -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
diff --git a/support/download/git b/support/download/git
index 116e5a9..bc5aabc 100755
--- a/support/download/git
+++ b/support/download/git
@@ -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
diff --git a/support/download/hg b/support/download/hg
index 6e9e26b..8b36db9 100755
--- a/support/download/hg
+++ b/support/download/hg
@@ -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
diff --git a/support/download/scp b/support/download/scp
index d3aad43..e16ece5 100755
--- a/support/download/scp
+++ b/support/download/scp
@@ -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
diff --git a/support/download/svn b/support/download/svn
index 39cbbcb..259d3ed 100755
--- a/support/download/svn
+++ b/support/download/svn
@@ -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
diff --git a/support/download/wget b/support/download/wget
index cbeca3b..0f71108 100755
--- a/support/download/wget
+++ b/support/download/wget
@@ -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
-- 
1.9.1

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

* [Buildroot] [PATCH 4/6 v2] support/download: simplify the local-files helper
  2014-07-07 21:44 [Buildroot] [PATCH 0/6 v2] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
                   ` (2 preceding siblings ...)
  2014-07-07 21:44 ` [Buildroot] [PATCH 3/6 v2] support/download: properly use temp files Yann E. MORIN
@ 2014-07-07 21:44 ` Yann E. MORIN
  2014-07-07 21:44 ` [Buildroot] [PATCH 5/6 v2] support/download: only create final temp file when needed Yann E. MORIN
  2014-07-07 21:44 ` [Buildroot] [PATCH 6/6 v2] support/download: rationalise naming and use of the temporary files Yann E. MORIN
  5 siblings, 0 replies; 11+ messages in thread
From: Yann E. MORIN @ 2014-07-07 21:44 UTC (permalink / raw)
  To: buildroot

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" <yann.morin.1998@free.fr>

---
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(-)

diff --git a/Config.in b/Config.in
index 50968fb..9e81d77 100644
--- a/Config.in
+++ b/Config.in
@@ -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"
diff --git a/Config.in.legacy b/Config.in.legacy
index a2c7846..aa49806 100644
--- a/Config.in.legacy
+++ b/Config.in.legacy
@@ -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
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 7f208d5..fb52ae0 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -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
diff --git a/support/download/cp b/support/download/cp
index e73159b..e1e7337 100755
--- a/support/download/cp
+++ b/support/download/cp
@@ -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
-- 
1.9.1

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

* [Buildroot] [PATCH 5/6 v2] support/download: only create final temp file when needed
  2014-07-07 21:44 [Buildroot] [PATCH 0/6 v2] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
                   ` (3 preceding siblings ...)
  2014-07-07 21:44 ` [Buildroot] [PATCH 4/6 v2] support/download: simplify the local-files helper Yann E. MORIN
@ 2014-07-07 21:44 ` Yann E. MORIN
  2014-07-07 21:44 ` [Buildroot] [PATCH 6/6 v2] support/download: rationalise naming and use of the temporary files Yann E. MORIN
  5 siblings, 0 replies; 11+ messages in thread
From: Yann E. MORIN @ 2014-07-07 21:44 UTC (permalink / raw)
  To: buildroot

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 <jacmet@uclibc.org>
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

---
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(-)

diff --git a/support/download/bzr b/support/download/bzr
index f86fa82..a2cb440 100755
--- a/support/download/bzr
+++ b/support/download/bzr
@@ -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
diff --git a/support/download/cvs b/support/download/cvs
index a8ab080..22863d8 100755
--- a/support/download/cvs
+++ b/support/download/cvs
@@ -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
diff --git a/support/download/git b/support/download/git
index bc5aabc..e8e9055 100755
--- a/support/download/git
+++ b/support/download/git
@@ -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
diff --git a/support/download/hg b/support/download/hg
index 8b36db9..7e5c9eb 100755
--- a/support/download/hg
+++ b/support/download/hg
@@ -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
diff --git a/support/download/scp b/support/download/scp
index e16ece5..1cc18de 100755
--- a/support/download/scp
+++ b/support/download/scp
@@ -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
diff --git a/support/download/svn b/support/download/svn
index 259d3ed..232d887 100755
--- a/support/download/svn
+++ b/support/download/svn
@@ -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
diff --git a/support/download/wget b/support/download/wget
index 0f71108..e961d71 100755
--- a/support/download/wget
+++ b/support/download/wget
@@ -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
-- 
1.9.1

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

* [Buildroot] [PATCH 6/6 v2] support/download: rationalise naming and use of the temporary files
  2014-07-07 21:44 [Buildroot] [PATCH 0/6 v2] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
                   ` (4 preceding siblings ...)
  2014-07-07 21:44 ` [Buildroot] [PATCH 5/6 v2] support/download: only create final temp file when needed Yann E. MORIN
@ 2014-07-07 21:44 ` Yann E. MORIN
  5 siblings, 0 replies; 11+ messages in thread
From: Yann E. MORIN @ 2014-07-07 21:44 UTC (permalink / raw)
  To: buildroot

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" <yann.morin.1998@free.fr>

---
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(-)

diff --git a/support/download/bzr b/support/download/bzr
index a2cb440..ff01bff 100755
--- a/support/download/bzr
+++ b/support/download/bzr
@@ -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
diff --git a/support/download/cvs b/support/download/cvs
index 22863d8..b580eda 100755
--- a/support/download/cvs
+++ b/support/download/cvs
@@ -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
diff --git a/support/download/git b/support/download/git
index e8e9055..c53d87f 100755
--- a/support/download/git
+++ b/support/download/git
@@ -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
diff --git a/support/download/hg b/support/download/hg
index 7e5c9eb..104d034 100755
--- a/support/download/hg
+++ b/support/download/hg
@@ -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 \
diff --git a/support/download/scp b/support/download/scp
index 1cc18de..6aa3a23 100755
--- a/support/download/scp
+++ b/support/download/scp
@@ -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
diff --git a/support/download/svn b/support/download/svn
index 232d887..7a46d0f 100755
--- a/support/download/svn
+++ b/support/download/svn
@@ -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
diff --git a/support/download/wget b/support/download/wget
index e961d71..9bb6700 100755
--- a/support/download/wget
+++ b/support/download/wget
@@ -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
-- 
1.9.1

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

* [Buildroot] [PATCH 1/6 v2] support/download: fix the bzr helper
  2014-07-07 21:44 ` [Buildroot] [PATCH 1/6 v2] support/download: fix the bzr helper Yann E. MORIN
@ 2014-07-08  8:53   ` Peter Korsgaard
  0 siblings, 0 replies; 11+ messages in thread
From: Peter Korsgaard @ 2014-07-08  8:53 UTC (permalink / raw)
  To: buildroot

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> 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.

 > 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" <yann.morin.1998@free.fr>
 > Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

Committed, thanks.

-- 
Bye, Peter Korsgaard

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

* [Buildroot] [PATCH 2/6 v2] support/download: fix the git helper
  2014-07-07 21:44 ` [Buildroot] [PATCH 2/6 v2] support/download: fix the git helper Yann E. MORIN
@ 2014-07-08 16:45   ` Arnout Vandecappelle
  2014-07-08 21:39     ` Arnout Vandecappelle
  2014-07-08 21:27   ` Peter Korsgaard
  1 sibling, 1 reply; 11+ messages in thread
From: Arnout Vandecappelle @ 2014-07-08 16:45 UTC (permalink / raw)
  To: buildroot

On 07/07/14 23:44, Yann E. MORIN wrote:
> 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" <yann.morin.1998@free.fr>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 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

> ---
>  support/download/git | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/support/download/git b/support/download/git
> index badb491..116e5a9 100755
> --- a/support/download/git
> +++ b/support/download/git
> @@ -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
> 


-- 
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

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

* [Buildroot] [PATCH 2/6 v2] support/download: fix the git helper
  2014-07-07 21:44 ` [Buildroot] [PATCH 2/6 v2] support/download: fix the git helper Yann E. MORIN
  2014-07-08 16:45   ` Arnout Vandecappelle
@ 2014-07-08 21:27   ` Peter Korsgaard
  1 sibling, 0 replies; 11+ messages in thread
From: Peter Korsgaard @ 2014-07-08 21:27 UTC (permalink / raw)
  To: buildroot

>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> 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.

 > 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" <yann.morin.1998@free.fr>

Committed, thanks.

-- 
Bye, Peter Korsgaard

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

* [Buildroot] [PATCH 2/6 v2] support/download: fix the git helper
  2014-07-08 16:45   ` Arnout Vandecappelle
@ 2014-07-08 21:39     ` Arnout Vandecappelle
  0 siblings, 0 replies; 11+ messages in thread
From: Arnout Vandecappelle @ 2014-07-08 21:39 UTC (permalink / raw)
  To: buildroot

On 08/07/14 18:45, Arnout Vandecappelle wrote:
>  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

-- 
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

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

end of thread, other threads:[~2014-07-08 21:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-07-07 21:44 [Buildroot] [PATCH 0/6 v2] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
2014-07-07 21:44 ` [Buildroot] [PATCH 1/6 v2] support/download: fix the bzr helper Yann E. MORIN
2014-07-08  8:53   ` Peter Korsgaard
2014-07-07 21:44 ` [Buildroot] [PATCH 2/6 v2] support/download: fix the git helper Yann E. MORIN
2014-07-08 16:45   ` Arnout Vandecappelle
2014-07-08 21:39     ` Arnout Vandecappelle
2014-07-08 21:27   ` Peter Korsgaard
2014-07-07 21:44 ` [Buildroot] [PATCH 3/6 v2] support/download: properly use temp files Yann E. MORIN
2014-07-07 21:44 ` [Buildroot] [PATCH 4/6 v2] support/download: simplify the local-files helper Yann E. MORIN
2014-07-07 21:44 ` [Buildroot] [PATCH 5/6 v2] support/download: only create final temp file when needed Yann E. MORIN
2014-07-07 21:44 ` [Buildroot] [PATCH 6/6 v2] support/download: rationalise naming and use of the temporary files Yann E. MORIN

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.