* [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.