* [Buildroot] [PATCH 1/9 v3] support/download: add download wrapper
2014-07-20 22:42 [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
@ 2014-07-20 22:42 ` Yann E. MORIN
2014-08-03 7:18 ` Thomas De Schampheleire
2014-07-20 22:42 ` [Buildroot] [PATCH 2/9 v3] support/download: convert bzr to use the wrapper Yann E. MORIN
` (8 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Yann E. MORIN @ 2014-07-20 22:42 UTC (permalink / raw)
To: buildroot
The download wrapper is responsible for ensuring the atomicity
of saving into $(BR2_DL_DIR).
It calls the appropriate download helper, telling it to save the
downloaded content to a temporary file in $(BUILD_DIR) (so it does
not clutter $(BR2_DL_DIR) with partial, failed downloads.
Then, only it the download helper was successful, does the wrapper
saves the downloaded content to the final location, yet still in a
teporary file, and finally atomically renames it to the final output
file.
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
support/download/wrapper | 99 ++++++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 99 insertions(+)
create mode 100755 support/download/wrapper
diff --git a/support/download/wrapper b/support/download/wrapper
new file mode 100755
index 0000000..bc0d486
--- /dev/null
+++ b/support/download/wrapper
@@ -0,0 +1,99 @@
+#!/bin/bash
+
+# This script is a wrapper to the other download helpers.
+# Its role is to ensure atomicity when saving downloaded files
+# back to BR2_DL_DIR, and not clutter BR2_DL_DIR with partial,
+# failed downloads.
+#
+# Call it with:
+# $1: name of the helper (eg. cvs, git, cp...)
+# $2: full path to the file in which to save the download
+# $*: additional arguments to the helper in $1
+# Environment:
+# BUILD_DIR: the path to Buildroot's build dir
+
+# To avoid cluttering BR2_DL_DIR, we download to a trashable
+# location, namely in $(BUILD_DIR).
+# Then, we move the downloaed file to a temporary file in the
+# same directory as the final output file.
+# This allows us to finally atomically rename it to its final
+# name.
+# If anything goes wrong, we just remove all the temporaries
+# created so far.
+
+# We want to catch any unexpected failure, and exit immediately.
+set -e
+
+helper="${1}"
+output="${2}"
+shift 2
+
+# tmpd is a temporary directory in which helpers may store intermediate
+# by-products of the download.
+# tmpf is the file in which the helpers should put the downloaded content.
+# tmpd located in $(BUILD_DIR), so as not to cluter the (precious)
+# $(BR2_DL_DIR)
+# We let the helpers create tmpf, so they are able to set whatever
+# permission bits they want (although we're only really interested in
+# the executable bit.)
+tmpd="$( mktemp -d "${BUILD_DIR}/.${output##*/}.XXXXXX" )"
+tmpf="${tmpd}/output"
+
+# Helpers expect to run in a directory that is *really* trashable, so
+# they are free to create whatever files and/or sub-dirs they might need.
+# Doing the 'cd' here rather than in all helpers is easier.
+cd "${tmpd}"
+
+# If the helper fails, we can just remove the temporary directory to
+# remove all the cruft it may have left behind. Then we just exit in
+# error too.
+if ! "${OLDPWD}/support/download/${helper}" "${tmpf}" "${@}"; then
+ rm -rf "${tmpd}"
+ exit 1
+fi
+
+# cd back to free the temp-dir, so we can remove it later
+cd "${OLDPWD}"
+
+# tmp_output is in the same directory as the final output, so we can
+# later move it atomically.
+tmp_output="$( mktemp "${output}.XXXXXX" )"
+
+# 'mktemp' creates files with 'go=-rwx', so the files are not accessible
+# to users other than the one doing the download (and root, of course).
+# This can be problematic when a shared BR2_DL_DIR is used by different
+# users (e.g. on a build server), where all users may write to the shared
+# location, since other users would not be allowed to read the files
+# another user downloaded.
+# So, we restore the 'go' access rights to a more sensible value, while
+# still abiding by the current user's umask. We must do that before the
+# final 'mv', so just do it now.
+# Some helpers (cp and scp) may create executable files, so we need to
+# carry the executable bit if needed.
+[ -x "${tmpf}" ] && new_mode=755 || new_mode=644
+new_mode=$( printf "%04o" $((0${new_mode} & ~0$(umask))) )
+chmod ${new_mode} "${tmp_output}"
+
+# We must *not* unlink tmp_output, otherwise there is a small window
+# during which another download process may create the same tmp_output
+# name (very, very unlikely; but not impossible.)
+# Using 'cp' is not reliable, since 'cp' may unlink the destination file
+# if it is unable to open it with O_WRONLY|O_TRUNC; see:
+# http://pubs.opengroup.org/onlinepubs/9699919799/utilities/cp.html
+# Since the destination filesystem can be anything, it might not support
+# O_TRUNC, so 'cp' would unlink it first.
+# Use 'cat' and append-redirection '>>' to save to the final location,
+# since that is the only way we can be 100% sure of the behaviour.
+if ! cat "${tmpf}" >>"${tmp_output}"; then
+ rm -rf "${tmpd}" "${tmp_output}"
+ exit 1
+fi
+rm -rf "${tmpd}"
+# tmp_output and output are on the same filesystem, so POSIX guarantees
+# that 'mv' is atomic, because it then uses rename() that POSIX mandates
+# to be atomic, see:
+# http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html
+if ! mv "${tmp_output}" "${output}"; then
+ rm -f "${tmp_output}"
+ exit 1
+fi
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Buildroot] [PATCH 1/9 v3] support/download: add download wrapper
2014-07-20 22:42 ` [Buildroot] [PATCH 1/9 v3] support/download: add download wrapper Yann E. MORIN
@ 2014-08-03 7:18 ` Thomas De Schampheleire
2014-08-03 7:27 ` Yann E. MORIN
0 siblings, 1 reply; 25+ messages in thread
From: Thomas De Schampheleire @ 2014-08-03 7:18 UTC (permalink / raw)
To: buildroot
Hi Yann,
"Yann E. MORIN" <yann.morin.1998@free.fr> schreef:
>The download wrapper is responsible for ensuring the atomicity
>of saving into $(BR2_DL_DIR).
>
>It calls the appropriate download helper, telling it to save the
>downloaded content to a temporary file in $(BUILD_DIR) (so it does
>not clutter $(BR2_DL_DIR) with partial, failed downloads.
>
>Then, only it the download helper was successful, does the wrapper
only if
>saves the downloaded content to the final location, yet still in a
does ... save
>teporary file, and finally atomically renames it to the final output
temporary
>file.
>
>Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>---
> support/download/wrapper | 99 ++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 99 insertions(+)
> create mode 100755 support/download/wrapper
>
>diff --git a/support/download/wrapper b/support/download/wrapper
>new file mode 100755
>index 0000000..bc0d486
>--- /dev/null
>+++ b/support/download/wrapper
>@@ -0,0 +1,99 @@
>+#!/bin/bash
>+
>+# This script is a wrapper to the other download helpers.
>+# Its role is to ensure atomicity when saving downloaded files
>+# back to BR2_DL_DIR, and not clutter BR2_DL_DIR with partial,
>+# failed downloads.
>+#
>+# Call it with:
>+# $1: name of the helper (eg. cvs, git, cp...)
>+# $2: full path to the file in which to save the download
>+# $*: additional arguments to the helper in $1
>+# Environment:
>+# BUILD_DIR: the path to Buildroot's build dir
>+
>+# To avoid cluttering BR2_DL_DIR, we download to a trashable
>+# location, namely in $(BUILD_DIR).
>+# Then, we move the downloaed file to a temporary file in the
downloaded
>+# same directory as the final output file.
>+# This allows us to finally atomically rename it to its final
>+# name.
>+# If anything goes wrong, we just remove all the temporaries
>+# created so far.
>+
>+# We want to catch any unexpected failure, and exit immediately.
>+set -e
>+
>+helper="${1}"
>+output="${2}"
>+shift 2
>+
>+# tmpd is a temporary directory in which helpers may store intermediate
>+# by-products of the download.
>+# tmpf is the file in which the helpers should put the downloaded content.
>+# tmpd located in $(BUILD_DIR), so as not to cluter the (precious)
tmpd is located
clutter
>+# $(BR2_DL_DIR)
>+# We let the helpers create tmpf, so they are able to set whatever
>+# permission bits they want (although we're only really interested in
>+# the executable bit.)
>+tmpd="$( mktemp -d "${BUILD_DIR}/.${output##*/}.XXXXXX" )"
>+tmpf="${tmpd}/output"
>+
>+# Helpers expect to run in a directory that is *really* trashable, so
>+# they are free to create whatever files and/or sub-dirs they might need.
>+# Doing the 'cd' here rather than in all helpers is easier.
>+cd "${tmpd}"
>+
>+# If the helper fails, we can just remove the temporary directory to
>+# remove all the cruft it may have left behind. Then we just exit in
>+# error too.
>+if ! "${OLDPWD}/support/download/${helper}" "${tmpf}" "${@}"; then
>+ rm -rf "${tmpd}"
>+ exit 1
>+fi
>+
>+# cd back to free the temp-dir, so we can remove it later
>+cd "${OLDPWD}"
>+
>+# tmp_output is in the same directory as the final output, so we can
>+# later move it atomically.
>+tmp_output="$( mktemp "${output}.XXXXXX" )"
>+
>+# 'mktemp' creates files with 'go=-rwx', so the files are not accessible
>+# to users other than the one doing the download (and root, of course).
>+# This can be problematic when a shared BR2_DL_DIR is used by different
>+# users (e.g. on a build server), where all users may write to the shared
>+# location, since other users would not be allowed to read the files
>+# another user downloaded.
>+# So, we restore the 'go' access rights to a more sensible value, while
>+# still abiding by the current user's umask. We must do that before the
>+# final 'mv', so just do it now.
>+# Some helpers (cp and scp) may create executable files, so we need to
>+# carry the executable bit if needed.
>+[ -x "${tmpf}" ] && new_mode=755 || new_mode=644
>+new_mode=$( printf "%04o" $((0${new_mode} & ~0$(umask))) )
>+chmod ${new_mode} "${tmp_output}"
>+
>+# We must *not* unlink tmp_output, otherwise there is a small window
>+# during which another download process may create the same tmp_output
>+# name (very, very unlikely; but not impossible.)
>+# Using 'cp' is not reliable, since 'cp' may unlink the destination file
>+# if it is unable to open it with O_WRONLY|O_TRUNC; see:
>+# http://pubs.opengroup.org/onlinepubs/9699919799/utilities/cp.html
>+# Since the destination filesystem can be anything, it might not support
>+# O_TRUNC, so 'cp' would unlink it first.
>+# Use 'cat' and append-redirection '>>' to save to the final location,
>+# since that is the only way we can be 100% sure of the behaviour.
>+if ! cat "${tmpf}" >>"${tmp_output}"; then
>+ rm -rf "${tmpd}" "${tmp_output}"
>+ exit 1
>+fi
>+rm -rf "${tmpd}"
>+# tmp_output and output are on the same filesystem, so POSIX guarantees
>+# that 'mv' is atomic, because it then uses rename() that POSIX mandates
>+# to be atomic, see:
>+# http://pubs.opengroup.org/onlinepubs/9699919799/functions/rename.html
>+if ! mv "${tmp_output}" "${output}"; then
>+ rm -f "${tmp_output}"
>+ exit 1
>+fi
Note that I hope we can apply this series in the current release still...
Best regards,
Thomas
^ permalink raw reply [flat|nested] 25+ messages in thread* [Buildroot] [PATCH 1/9 v3] support/download: add download wrapper
2014-08-03 7:18 ` Thomas De Schampheleire
@ 2014-08-03 7:27 ` Yann E. MORIN
0 siblings, 0 replies; 25+ messages in thread
From: Yann E. MORIN @ 2014-08-03 7:27 UTC (permalink / raw)
To: buildroot
Thomas, All,
On 2014-08-03 09:18 +0200, Thomas De Schampheleire spake thusly:
> "Yann E. MORIN" <yann.morin.1998@free.fr> schreef:
[--SNIP typoes--]
Typoes fixed, thanks!
> Note that I hope we can apply this series in the current release still...
So do I, so do I! ;-)
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 2/9 v3] support/download: convert bzr to use the wrapper
2014-07-20 22:42 [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
2014-07-20 22:42 ` [Buildroot] [PATCH 1/9 v3] support/download: add download wrapper Yann E. MORIN
@ 2014-07-20 22:42 ` Yann E. MORIN
2014-08-03 7:52 ` Thomas De Schampheleire
2014-07-20 22:42 ` [Buildroot] [PATCH 3/9 v3] support/download: convert localfiles " Yann E. MORIN
` (7 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Yann E. MORIN @ 2014-07-20 22:42 UTC (permalink / raw)
To: buildroot
This drastically simplifies the bzr helper, as it no longer has to
deal with atomically saving the downloaded archive.
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
package/pkg-download.mk | 2 +-
support/download/bzr | 36 ++++++++----------------------------
2 files changed, 9 insertions(+), 29 deletions(-)
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 118591c..6320338 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -112,7 +112,7 @@ endef
define DOWNLOAD_BZR
test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
- $(EXTRA_ENV) support/download/bzr $($(PKG)_SITE) $($(PKG)_DL_VERSION) $(DL_DIR)/$($(PKG)_SOURCE)
+ $(EXTRA_ENV) support/download/wrapper bzr $(DL_DIR)/$($(PKG)_SOURCE) $($(PKG)_SITE) $($(PKG)_DL_VERSION)
endef
define SOURCE_CHECK_BZR
diff --git a/support/download/bzr b/support/download/bzr
index 19d837d..24bb1d0 100755
--- a/support/download/bzr
+++ b/support/download/bzr
@@ -1,38 +1,18 @@
#!/bin/bash
-# We want to catch any command failure, and exit immediately
+# We want to catch any unexpected failure, and exit immediately
set -e
# Download helper for bzr
# Call it with:
-# $1: bzr repo
-# $2: bzr revision
-# $3: output file
+# $1: output file
+# $2: bzr repo
+# $3: bzr revision
# And this environment:
# BZR : the bzr command to call
-# BUILD_DIR: path to Buildroot's build dir
-repo="${1}"
-rev="${2}"
-output="${3}"
+output="${1}"
+repo="${2}"
+rev="${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)
-# - the we move to a temp file in the final location, so it is
-# on the same filesystem as the final file
-# - finally, we atomically rename to the final file
-
-ret=1
-if ${BZR} export --format=tgz "${tmp_dl}" "${repo}" -r "${rev}"; then
- if mv "${tmp_dl}" "${tmp_output}"; then
- mv "${tmp_output}" "${output}"
- ret=0
- fi
-fi
-
-# Cleanup
-rm -f "${tmp_dl}" "${tmp_output}"
-exit ${ret}
+${BZR} export --format=tgz "${output}" "${repo}" -r "${rev}"
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Buildroot] [PATCH 2/9 v3] support/download: convert bzr to use the wrapper
2014-07-20 22:42 ` [Buildroot] [PATCH 2/9 v3] support/download: convert bzr to use the wrapper Yann E. MORIN
@ 2014-08-03 7:52 ` Thomas De Schampheleire
2014-08-03 17:05 ` Yann E. MORIN
0 siblings, 1 reply; 25+ messages in thread
From: Thomas De Schampheleire @ 2014-08-03 7:52 UTC (permalink / raw)
To: buildroot
On Mon, Jul 21, 2014 at 12:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> This drastically simplifies the bzr helper, as it no longer has to
> deal with atomically saving the downloaded archive.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> package/pkg-download.mk | 2 +-
> support/download/bzr | 36 ++++++++----------------------------
> 2 files changed, 9 insertions(+), 29 deletions(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 118591c..6320338 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -112,7 +112,7 @@ endef
>
> define DOWNLOAD_BZR
> test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
> - $(EXTRA_ENV) support/download/bzr $($(PKG)_SITE) $($(PKG)_DL_VERSION) $(DL_DIR)/$($(PKG)_SOURCE)
> + $(EXTRA_ENV) support/download/wrapper bzr $(DL_DIR)/$($(PKG)_SOURCE) $($(PKG)_SITE) $($(PKG)_DL_VERSION)
> endef
>
> define SOURCE_CHECK_BZR
> diff --git a/support/download/bzr b/support/download/bzr
> index 19d837d..24bb1d0 100755
> --- a/support/download/bzr
> +++ b/support/download/bzr
> @@ -1,38 +1,18 @@
> #!/bin/bash
>
> -# We want to catch any command failure, and exit immediately
> +# We want to catch any unexpected failure, and exit immediately
> set -e
>
> # Download helper for bzr
> # Call it with:
Maybe we should mention somewhere here that this script is supposed to
be called through the wrapper, not directly. Currently, the 'Call it
with' seems to indicate that you should call it directly.
If you agree to this, then of course it applies to all helpers.
> -# $1: bzr repo
> -# $2: bzr revision
> -# $3: output file
> +# $1: output file
> +# $2: bzr repo
> +# $3: bzr revision
> # And this environment:
> # BZR : the bzr command to call
> -# BUILD_DIR: path to Buildroot's build dir
>
> -repo="${1}"
> -rev="${2}"
> -output="${3}"
> +output="${1}"
> +repo="${2}"
> +rev="${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)
> -# - the we move to a temp file in the final location, so it is
> -# on the same filesystem as the final file
> -# - finally, we atomically rename to the final file
> -
> -ret=1
> -if ${BZR} export --format=tgz "${tmp_dl}" "${repo}" -r "${rev}"; then
> - if mv "${tmp_dl}" "${tmp_output}"; then
> - mv "${tmp_output}" "${output}"
> - ret=0
> - fi
> -fi
> -
> -# Cleanup
> -rm -f "${tmp_dl}" "${tmp_output}"
> -exit ${ret}
> +${BZR} export --format=tgz "${output}" "${repo}" -r "${rev}"
I like the fact that the helpers are now so much easier, and the
duplication is removed!
Best regards,
Thomas
^ permalink raw reply [flat|nested] 25+ messages in thread* [Buildroot] [PATCH 2/9 v3] support/download: convert bzr to use the wrapper
2014-08-03 7:52 ` Thomas De Schampheleire
@ 2014-08-03 17:05 ` Yann E. MORIN
0 siblings, 0 replies; 25+ messages in thread
From: Yann E. MORIN @ 2014-08-03 17:05 UTC (permalink / raw)
To: buildroot
Thomas, All,
On 2014-08-03 09:52 +0200, Thomas De Schampheleire spake thusly:
> On Mon, Jul 21, 2014 at 12:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > This drastically simplifies the bzr helper, as it no longer has to
> > deal with atomically saving the downloaded archive.
> >
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > ---
> > package/pkg-download.mk | 2 +-
> > support/download/bzr | 36 ++++++++----------------------------
> > 2 files changed, 9 insertions(+), 29 deletions(-)
> >
> > diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> > index 118591c..6320338 100644
> > --- a/package/pkg-download.mk
> > +++ b/package/pkg-download.mk
> > @@ -112,7 +112,7 @@ endef
> >
> > define DOWNLOAD_BZR
> > test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
> > - $(EXTRA_ENV) support/download/bzr $($(PKG)_SITE) $($(PKG)_DL_VERSION) $(DL_DIR)/$($(PKG)_SOURCE)
> > + $(EXTRA_ENV) support/download/wrapper bzr $(DL_DIR)/$($(PKG)_SOURCE) $($(PKG)_SITE) $($(PKG)_DL_VERSION)
> > endef
> >
> > define SOURCE_CHECK_BZR
> > diff --git a/support/download/bzr b/support/download/bzr
> > index 19d837d..24bb1d0 100755
> > --- a/support/download/bzr
> > +++ b/support/download/bzr
> > @@ -1,38 +1,18 @@
> > #!/bin/bash
> >
> > -# We want to catch any command failure, and exit immediately
> > +# We want to catch any unexpected failure, and exit immediately
> > set -e
> >
> > # Download helper for bzr
> > # Call it with:
>
> Maybe we should mention somewhere here that this script is supposed to
> be called through the wrapper, not directly. Currently, the 'Call it
> with' seems to indicate that you should call it directly.
> If you agree to this, then of course it applies to all helpers.
Yep, I'm changing all of them with:
# Download helper for XXX, to be called from the download wrapper script
# Expected arguments:
[--SNIP--]
> > +${BZR} export --format=tgz "${output}" "${repo}" -r "${rev}"
>
> I like the fact that the helpers are now so much easier, and the
> duplication is removed!
Yep.
I also fixed the root dir of the archive, so it now has the basename of
the package as root dir (as discussed on IRC.)
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3/9 v3] support/download: convert localfiles to use the wrapper
2014-07-20 22:42 [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
2014-07-20 22:42 ` [Buildroot] [PATCH 1/9 v3] support/download: add download wrapper Yann E. MORIN
2014-07-20 22:42 ` [Buildroot] [PATCH 2/9 v3] support/download: convert bzr to use the wrapper Yann E. MORIN
@ 2014-07-20 22:42 ` Yann E. MORIN
2014-08-03 7:56 ` Thomas De Schampheleire
2014-08-03 17:39 ` Thomas De Schampheleire
2014-07-20 22:42 ` [Buildroot] [PATCH 4/9 v3] support/download: convert cvs " Yann E. MORIN
` (6 subsequent siblings)
9 siblings, 2 replies; 25+ messages in thread
From: Yann E. MORIN @ 2014-07-20 22:42 UTC (permalink / raw)
To: buildroot
This drastically simplifies the localfiles helper, as it no longer has
to deal with atomically saving the downloaded archive.
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
package/pkg-download.mk | 4 ++--
support/download/cp | 20 +++++---------------
2 files changed, 7 insertions(+), 17 deletions(-)
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 6320338..94febab 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -205,8 +205,8 @@ endef
define DOWNLOAD_LOCALFILES
test -e $(DL_DIR)/$(2) || \
- $(EXTRA_ENV) support/download/cp $(call stripurischeme,$(call qstrip,$(1))) \
- $(DL_DIR)/$(2) && \
+ $(EXTRA_ENV) support/download/wrapper cp $(DL_DIR)/$(2) \
+ $(call stripurischeme,$(call qstrip,$(1))) && \
$(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
endef
diff --git a/support/download/cp b/support/download/cp
index 8f6bc06..4945c56 100755
--- a/support/download/cp
+++ b/support/download/cp
@@ -5,22 +5,12 @@ set -e
# Download helper for cp
# Call it with:
-# $1: source file
-# $2: output file
+# $1: output file
+# $2: source file
# And this environment:
# LOCALFILES: the cp command to call
-source="${1}"
-output="${2}"
+output="${1}"
+source="${2}"
-tmp_output="$( mktemp "${output}.XXXXXX" )"
-
-ret=1
-if ${LOCALFILES} "${source}" "${tmp_output}"; then
- mv "${tmp_output}" "${output}"
- ret=0
-fi
-
-# Cleanup
-rm -f "${tmp_output}"
-exit ${ret}
+${LOCALFILES} "${source}" "${output}"
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Buildroot] [PATCH 3/9 v3] support/download: convert localfiles to use the wrapper
2014-07-20 22:42 ` [Buildroot] [PATCH 3/9 v3] support/download: convert localfiles " Yann E. MORIN
@ 2014-08-03 7:56 ` Thomas De Schampheleire
2014-08-03 17:06 ` Yann E. MORIN
2014-08-03 17:39 ` Thomas De Schampheleire
1 sibling, 1 reply; 25+ messages in thread
From: Thomas De Schampheleire @ 2014-08-03 7:56 UTC (permalink / raw)
To: buildroot
On Mon, Jul 21, 2014 at 12:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> This drastically simplifies the localfiles helper, as it no longer has
> to deal with atomically saving the downloaded archive.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> package/pkg-download.mk | 4 ++--
> support/download/cp | 20 +++++---------------
> 2 files changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 6320338..94febab 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -205,8 +205,8 @@ endef
>
> define DOWNLOAD_LOCALFILES
> test -e $(DL_DIR)/$(2) || \
> - $(EXTRA_ENV) support/download/cp $(call stripurischeme,$(call qstrip,$(1))) \
> - $(DL_DIR)/$(2) && \
> + $(EXTRA_ENV) support/download/wrapper cp $(DL_DIR)/$(2) \
> + $(call stripurischeme,$(call qstrip,$(1))) && \
> $(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
> endef
>
> diff --git a/support/download/cp b/support/download/cp
> index 8f6bc06..4945c56 100755
> --- a/support/download/cp
> +++ b/support/download/cp
> @@ -5,22 +5,12 @@ set -e
Here you didn't make the comment change command->unexpected before set -e.
Best regards,
Thomas
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3/9 v3] support/download: convert localfiles to use the wrapper
2014-08-03 7:56 ` Thomas De Schampheleire
@ 2014-08-03 17:06 ` Yann E. MORIN
0 siblings, 0 replies; 25+ messages in thread
From: Yann E. MORIN @ 2014-08-03 17:06 UTC (permalink / raw)
To: buildroot
Thomas, All,
On 2014-08-03 09:56 +0200, Thomas De Schampheleire spake thusly:
> On Mon, Jul 21, 2014 at 12:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> > This drastically simplifies the localfiles helper, as it no longer has
> > to deal with atomically saving the downloaded archive.
> >
> > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> > ---
> > package/pkg-download.mk | 4 ++--
> > support/download/cp | 20 +++++---------------
> > 2 files changed, 7 insertions(+), 17 deletions(-)
> >
> > diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> > index 6320338..94febab 100644
> > --- a/package/pkg-download.mk
> > +++ b/package/pkg-download.mk
> > @@ -205,8 +205,8 @@ endef
> >
> > define DOWNLOAD_LOCALFILES
> > test -e $(DL_DIR)/$(2) || \
> > - $(EXTRA_ENV) support/download/cp $(call stripurischeme,$(call qstrip,$(1))) \
> > - $(DL_DIR)/$(2) && \
> > + $(EXTRA_ENV) support/download/wrapper cp $(DL_DIR)/$(2) \
> > + $(call stripurischeme,$(call qstrip,$(1))) && \
> > $(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
> > endef
> >
> > diff --git a/support/download/cp b/support/download/cp
> > index 8f6bc06..4945c56 100755
> > --- a/support/download/cp
> > +++ b/support/download/cp
> > @@ -5,22 +5,12 @@ set -e
>
> Here you didn't make the comment change command->unexpected before set -e.
Yep, will do.
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 3/9 v3] support/download: convert localfiles to use the wrapper
2014-07-20 22:42 ` [Buildroot] [PATCH 3/9 v3] support/download: convert localfiles " Yann E. MORIN
2014-08-03 7:56 ` Thomas De Schampheleire
@ 2014-08-03 17:39 ` Thomas De Schampheleire
1 sibling, 0 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2014-08-03 17:39 UTC (permalink / raw)
To: buildroot
On Mon, Jul 21, 2014 at 12:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> This drastically simplifies the localfiles helper, as it no longer has
> to deal with atomically saving the downloaded archive.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> package/pkg-download.mk | 4 ++--
> support/download/cp | 20 +++++---------------
> 2 files changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 6320338..94febab 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -205,8 +205,8 @@ endef
>
> define DOWNLOAD_LOCALFILES
> test -e $(DL_DIR)/$(2) || \
> - $(EXTRA_ENV) support/download/cp $(call stripurischeme,$(call qstrip,$(1))) \
> - $(DL_DIR)/$(2) && \
> + $(EXTRA_ENV) support/download/wrapper cp $(DL_DIR)/$(2) \
> + $(call stripurischeme,$(call qstrip,$(1))) && \
> $(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
> endef
>
> diff --git a/support/download/cp b/support/download/cp
> index 8f6bc06..4945c56 100755
> --- a/support/download/cp
> +++ b/support/download/cp
> @@ -5,22 +5,12 @@ set -e
>
> # Download helper for cp
> # Call it with:
> -# $1: source file
> -# $2: output file
> +# $1: output file
> +# $2: source file
> # And this environment:
> # LOCALFILES: the cp command to call
>
> -source="${1}"
> -output="${2}"
> +output="${1}"
> +source="${2}"
>
> -tmp_output="$( mktemp "${output}.XXXXXX" )"
> -
> -ret=1
> -if ${LOCALFILES} "${source}" "${tmp_output}"; then
> - mv "${tmp_output}" "${output}"
> - ret=0
> -fi
> -
> -# Cleanup
> -rm -f "${tmp_output}"
> -exit ${ret}
> +${LOCALFILES} "${source}" "${output}"
> --
Tested-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
(Tested by setting BUSYBOX_SITE = file:///tmp and running 'make busybox-source')
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 4/9 v3] support/download: convert cvs to use the wrapper
2014-07-20 22:42 [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
` (2 preceding siblings ...)
2014-07-20 22:42 ` [Buildroot] [PATCH 3/9 v3] support/download: convert localfiles " Yann E. MORIN
@ 2014-07-20 22:42 ` Yann E. MORIN
2014-08-03 17:40 ` Thomas De Schampheleire
2014-07-20 22:42 ` [Buildroot] [PATCH 5/9 v3] support/download: convert git " Yann E. MORIN
` (5 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Yann E. MORIN @ 2014-07-20 22:42 UTC (permalink / raw)
To: buildroot
This drastically simplifies the cvs helper, as it no longer has to deal
with atomically saving the downloaded archive.
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
package/pkg-download.mk | 9 ++++++---
support/download/cvs | 50 ++++++++++++++-----------------------------------
2 files changed, 20 insertions(+), 39 deletions(-)
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 94febab..235adcb 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -125,9 +125,12 @@ endef
define DOWNLOAD_CVS
test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
- $(EXTRA_ENV) support/download/cvs $(call stripurischeme,$(call qstrip,$($(PKG)_SITE))) \
- $($(PKG)_DL_VERSION) $($(PKG)_RAWNAME) \
- $($(PKG)_BASE_NAME) $(DL_DIR)/$($(PKG)_SOURCE)
+ $(EXTRA_ENV) support/download/wrapper cvs \
+ $(DL_DIR)/$($(PKG)_SOURCE) \
+ $(call stripurischeme,$(call qstrip,$($(PKG)_SITE))) \
+ $($(PKG)_DL_VERSION) \
+ $($(PKG)_RAWNAME) \
+ $($(PKG)_BASE_NAME)
endef
# Not all CVS servers support ls/rls, use login to see if we can connect
diff --git a/support/download/cvs b/support/download/cvs
index 9aeed79..b4b0acc 100755
--- a/support/download/cvs
+++ b/support/download/cvs
@@ -1,47 +1,25 @@
#!/bin/bash
-# We want to catch any command failure, and exit immediately
+# We want to catch any unexpected failure, and exit immediately
set -e
# Download helper for cvs
# Call it with:
-# $1: cvs repo
-# $2: cvs revision
-# $3: package's name (eg. foobar)
-# $4: package's basename (eg. foobar-1.2.3)
-# $5: output file
+# $1: output file
+# $2: cvs repo
+# $3: cvs revision
+# $4: package's name (eg. foobar)
+# $5: package's basename (eg. foobar-1.2.3)
# And this environment:
# CVS : the cvs command to call
-# BUILD_DIR: path to Buildroot's build dir
-repo="${1}"
-rev="${2}"
-rawname="${3}"
-basename="${4}"
-output="${5}"
+output="${1}"
+repo="${2}"
+rev="${3}"
+rawname="${4}"
+basename="${5}"
-repodir="${basename}.tmp-cvs-checkout"
-tmp_output="$( mktemp "${output}.XXXXXX" )"
+${CVS} -z3 -d":pserver:anonymous@${repo}" \
+ co -d "${basename}" -r ":${rev}" -P "${rawname}"
-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
-
-ret=1
-if ${CVS} -z3 -d":pserver:anonymous@${repo}" \
- co -d "${repodir}" -r ":${rev}" -P "${rawname}"; then
- if tar czf "${tmp_output}" "${repodir}"; then
- mv "${tmp_output}" "${output}"
- ret=0
- fi
-fi
-
-# Cleanup
-rm -rf "${repodir}" "${tmp_output}"
-exit ${ret}
+tar czf "${output}" "${basename}"
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Buildroot] [PATCH 4/9 v3] support/download: convert cvs to use the wrapper
2014-07-20 22:42 ` [Buildroot] [PATCH 4/9 v3] support/download: convert cvs " Yann E. MORIN
@ 2014-08-03 17:40 ` Thomas De Schampheleire
0 siblings, 0 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2014-08-03 17:40 UTC (permalink / raw)
To: buildroot
On Mon, Jul 21, 2014 at 12:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> This drastically simplifies the cvs helper, as it no longer has to deal
> with atomically saving the downloaded archive.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> package/pkg-download.mk | 9 ++++++---
> support/download/cvs | 50 ++++++++++++++-----------------------------------
> 2 files changed, 20 insertions(+), 39 deletions(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 94febab..235adcb 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -125,9 +125,12 @@ endef
>
> define DOWNLOAD_CVS
> test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
> - $(EXTRA_ENV) support/download/cvs $(call stripurischeme,$(call qstrip,$($(PKG)_SITE))) \
> - $($(PKG)_DL_VERSION) $($(PKG)_RAWNAME) \
> - $($(PKG)_BASE_NAME) $(DL_DIR)/$($(PKG)_SOURCE)
> + $(EXTRA_ENV) support/download/wrapper cvs \
> + $(DL_DIR)/$($(PKG)_SOURCE) \
> + $(call stripurischeme,$(call qstrip,$($(PKG)_SITE))) \
> + $($(PKG)_DL_VERSION) \
> + $($(PKG)_RAWNAME) \
> + $($(PKG)_BASE_NAME)
> endef
>
> # Not all CVS servers support ls/rls, use login to see if we can connect
> diff --git a/support/download/cvs b/support/download/cvs
> index 9aeed79..b4b0acc 100755
> --- a/support/download/cvs
> +++ b/support/download/cvs
> @@ -1,47 +1,25 @@
> #!/bin/bash
>
> -# We want to catch any command failure, and exit immediately
> +# We want to catch any unexpected failure, and exit immediately
> set -e
>
> # Download helper for cvs
> # Call it with:
> -# $1: cvs repo
> -# $2: cvs revision
> -# $3: package's name (eg. foobar)
> -# $4: package's basename (eg. foobar-1.2.3)
> -# $5: output file
> +# $1: output file
> +# $2: cvs repo
> +# $3: cvs revision
> +# $4: package's name (eg. foobar)
> +# $5: package's basename (eg. foobar-1.2.3)
> # And this environment:
> # CVS : the cvs command to call
> -# BUILD_DIR: path to Buildroot's build dir
>
> -repo="${1}"
> -rev="${2}"
> -rawname="${3}"
> -basename="${4}"
> -output="${5}"
> +output="${1}"
> +repo="${2}"
> +rev="${3}"
> +rawname="${4}"
> +basename="${5}"
>
> -repodir="${basename}.tmp-cvs-checkout"
> -tmp_output="$( mktemp "${output}.XXXXXX" )"
> +${CVS} -z3 -d":pserver:anonymous@${repo}" \
> + co -d "${basename}" -r ":${rev}" -P "${rawname}"
>
> -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
> -
> -ret=1
> -if ${CVS} -z3 -d":pserver:anonymous@${repo}" \
> - co -d "${repodir}" -r ":${rev}" -P "${rawname}"; then
> - if tar czf "${tmp_output}" "${repodir}"; then
> - mv "${tmp_output}" "${output}"
> - ret=0
> - fi
> -fi
> -
> -# Cleanup
> -rm -rf "${repodir}" "${tmp_output}"
> -exit ${ret}
> +tar czf "${output}" "${basename}"
> --
Reviewed-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 5/9 v3] support/download: convert git to use the wrapper
2014-07-20 22:42 [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
` (3 preceding siblings ...)
2014-07-20 22:42 ` [Buildroot] [PATCH 4/9 v3] support/download: convert cvs " Yann E. MORIN
@ 2014-07-20 22:42 ` Yann E. MORIN
2014-08-03 17:41 ` Thomas De Schampheleire
2014-07-20 22:42 ` [Buildroot] [PATCH 6/9 v3] support/download: convert Hg " Yann E. MORIN
` (4 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Yann E. MORIN @ 2014-07-20 22:42 UTC (permalink / raw)
To: buildroot
This drastically simplifies the git helper, as it no longer has to deal
with atomically saving the downloaded archive.
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
package/pkg-download.mk | 7 ++++--
support/download/git | 58 +++++++++++++++----------------------------------
2 files changed, 23 insertions(+), 42 deletions(-)
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 235adcb..696ff3e 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -95,8 +95,11 @@ endef
# problems
define DOWNLOAD_GIT
test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
- $(EXTRA_ENV) support/download/git $($(PKG)_SITE) $($(PKG)_DL_VERSION) \
- $($(PKG)_BASE_NAME) $(DL_DIR)/$($(PKG)_SOURCE)
+ $(EXTRA_ENV) support/download/wrapper git \
+ $(DL_DIR)/$($(PKG)_SOURCE) \
+ $($(PKG)_SITE) \
+ $($(PKG)_DL_VERSION) \
+ $($(PKG)_BASE_NAME)
endef
# TODO: improve to check that the given PKG_DL_VERSION exists on the remote
diff --git a/support/download/git b/support/download/git
index 116e5a9..8d9505d 100755
--- a/support/download/git
+++ b/support/download/git
@@ -1,60 +1,38 @@
#!/bin/bash
-# We want to catch any command failure, and exit immediately
+# We want to catch any unexpected failure, and exit immediately
set -e
# Download helper for git
# Call it with:
-# $1: git repo
-# $2: git cset
-# $3: package's basename (eg. foobar-1.2.3)
-# $4: output file
+# $1: output file
+# $2: git repo
+# $3: git cset
+# $4: package's basename (eg. foobar-1.2.3)
# And this environment:
# GIT : the git command to call
-# BUILD_DIR: path to Buildroot's build dir
-repo="${1}"
-cset="${2}"
-basename="${3}"
-output="${4}"
+output="${1}"
+repo="${2}"
+cset="${3}"
+basename="${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)
-# - then we create the uncomporessed tarball in tht same trashable location
-# - then we create a temporary compressed tarball in the final location, so
-# 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}"
-
-git_done=0
+# Try to see if we can do a shallow clone, since it is faster
+# than a full clone.
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
+ if ${GIT} clone --depth 1 -b "${cset}" --bare "${repo}" "${basename}"; then
git_done=1
+ else
+ printf "Shallow clone failed, falling back to doing a full clone\n"
fi
fi
if [ ${git_done} -eq 0 ]; then
printf "Doing full clone\n"
- ${GIT} clone --bare "${repo}" "${repodir}"
+ ${GIT} clone --bare "${repo}" "${basename}"
fi
-ret=1
-pushd "${repodir}" >/dev/null
-if ${GIT} archive --prefix="${basename}/" -o "${tmp_tar}" \
- --format=tar "${cset}"; then
- if gzip -c "${tmp_tar}" >"${tmp_output}"; then
- mv "${tmp_output}" "${output}"
- ret=0
- fi
-fi
-popd >/dev/null
+GIT_DIR="${basename}" \
+${GIT} archive --prefix="${basename}/" -o "${output}" --format=tar "${cset}"
-rm -rf "${repodir}" "${tmp_tar}" "${tmp_output}"
-exit ${ret}
+gzip "${output}"
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Buildroot] [PATCH 5/9 v3] support/download: convert git to use the wrapper
2014-07-20 22:42 ` [Buildroot] [PATCH 5/9 v3] support/download: convert git " Yann E. MORIN
@ 2014-08-03 17:41 ` Thomas De Schampheleire
0 siblings, 0 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2014-08-03 17:41 UTC (permalink / raw)
To: buildroot
On Mon, Jul 21, 2014 at 12:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> This drastically simplifies the git helper, as it no longer has to deal
> with atomically saving the downloaded archive.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> package/pkg-download.mk | 7 ++++--
> support/download/git | 58 +++++++++++++++----------------------------------
> 2 files changed, 23 insertions(+), 42 deletions(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 235adcb..696ff3e 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -95,8 +95,11 @@ endef
> # problems
> define DOWNLOAD_GIT
> test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
> - $(EXTRA_ENV) support/download/git $($(PKG)_SITE) $($(PKG)_DL_VERSION) \
> - $($(PKG)_BASE_NAME) $(DL_DIR)/$($(PKG)_SOURCE)
> + $(EXTRA_ENV) support/download/wrapper git \
> + $(DL_DIR)/$($(PKG)_SOURCE) \
> + $($(PKG)_SITE) \
> + $($(PKG)_DL_VERSION) \
> + $($(PKG)_BASE_NAME)
> endef
>
> # TODO: improve to check that the given PKG_DL_VERSION exists on the remote
> diff --git a/support/download/git b/support/download/git
> index 116e5a9..8d9505d 100755
> --- a/support/download/git
> +++ b/support/download/git
> @@ -1,60 +1,38 @@
> #!/bin/bash
>
> -# We want to catch any command failure, and exit immediately
> +# We want to catch any unexpected failure, and exit immediately
> set -e
>
> # Download helper for git
> # Call it with:
> -# $1: git repo
> -# $2: git cset
> -# $3: package's basename (eg. foobar-1.2.3)
> -# $4: output file
> +# $1: output file
> +# $2: git repo
> +# $3: git cset
> +# $4: package's basename (eg. foobar-1.2.3)
> # And this environment:
> # GIT : the git command to call
> -# BUILD_DIR: path to Buildroot's build dir
>
> -repo="${1}"
> -cset="${2}"
> -basename="${3}"
> -output="${4}"
> +output="${1}"
> +repo="${2}"
> +cset="${3}"
> +basename="${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)
> -# - then we create the uncomporessed tarball in tht same trashable location
> -# - then we create a temporary compressed tarball in the final location, so
> -# 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}"
> -
> -git_done=0
> +# Try to see if we can do a shallow clone, since it is faster
> +# than a full clone.
> 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
> + if ${GIT} clone --depth 1 -b "${cset}" --bare "${repo}" "${basename}"; then
> git_done=1
> + else
> + printf "Shallow clone failed, falling back to doing a full clone\n"
> fi
> fi
> if [ ${git_done} -eq 0 ]; then
> printf "Doing full clone\n"
> - ${GIT} clone --bare "${repo}" "${repodir}"
> + ${GIT} clone --bare "${repo}" "${basename}"
> fi
>
> -ret=1
> -pushd "${repodir}" >/dev/null
> -if ${GIT} archive --prefix="${basename}/" -o "${tmp_tar}" \
> - --format=tar "${cset}"; then
> - if gzip -c "${tmp_tar}" >"${tmp_output}"; then
> - mv "${tmp_output}" "${output}"
> - ret=0
> - fi
> -fi
> -popd >/dev/null
> +GIT_DIR="${basename}" \
> +${GIT} archive --prefix="${basename}/" -o "${output}" --format=tar "${cset}"
>
> -rm -rf "${repodir}" "${tmp_tar}" "${tmp_output}"
> -exit ${ret}
> +gzip "${output}"
> --
> 1.9.1
>
Tested-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
(Tested by running 'make fmc-fsl-sdk-source')
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 6/9 v3] support/download: convert Hg to use the wrapper
2014-07-20 22:42 [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
` (4 preceding siblings ...)
2014-07-20 22:42 ` [Buildroot] [PATCH 5/9 v3] support/download: convert git " Yann E. MORIN
@ 2014-07-20 22:42 ` Yann E. MORIN
2014-08-03 17:41 ` Thomas De Schampheleire
2014-07-20 22:42 ` [Buildroot] [PATCH 7/9 v3] support/download: convert scp " Yann E. MORIN
` (3 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Yann E. MORIN @ 2014-07-20 22:42 UTC (permalink / raw)
To: buildroot
This drastically simplifies the hg helper, as it no longer has to deal
with atomically saving the downloaded archive.
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
package/pkg-download.mk | 7 +++++--
support/download/hg | 48 +++++++++++++-----------------------------------
2 files changed, 18 insertions(+), 37 deletions(-)
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 696ff3e..454179c 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -180,8 +180,11 @@ endef
define DOWNLOAD_HG
test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
- $(EXTRA_ENV) support/download/hg $($(PKG)_SITE) $($(PKG)_DL_VERSION) \
- $($(PKG)_BASE_NAME) $(DL_DIR)/$($(PKG)_SOURCE)
+ $(EXTRA_ENV) support/download/wrapper hg \
+ $(DL_DIR)/$($(PKG)_SOURCE) \
+ $($(PKG)_SITE) \
+ $($(PKG)_DL_VERSION) \
+ $($(PKG)_BASE_NAME)
endef
# TODO: improve to check that the given PKG_DL_VERSION exists on the remote
diff --git a/support/download/hg b/support/download/hg
index 6e9e26b..510ef8b 100755
--- a/support/download/hg
+++ b/support/download/hg
@@ -1,46 +1,24 @@
#!/bin/bash
-# We want to catch any command failure, and exit immediately
+# We want to catch any unexpected failure, and exit immediately
set -e
# Download helper for hg
# Call it with:
-# $1: hg repo
-# $2: hg cset
-# $3: package's basename (eg. foobar-1.2.3)
-# $4: output file
+# $1: output file
+# $2: hg repo
+# $3: hg cset
+# $4: package's basename (eg. foobar-1.2.3)
# And this environment:
# HG : the hg command to call
-# BUILD_DIR: path to Buildroot's build dir
-repo="${1}"
-cset="${2}"
-basename="${3}"
-output="${4}"
+output="${1}"
+repo="${2}"
+cset="${3}"
+basename="${4}"
-repodir="${basename}.tmp-hg-checkout"
-tmp_output="$( mktemp "${output}.XXXXXX" )"
+${HG} clone --noupdate --rev "${cset}" "${repo}" "${basename}"
-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
-
-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
- mv "${tmp_output}" "${output}"
- ret=0
- fi
-fi
-
-# Cleanup
-rm -rf "${repodir}" "${tmp_output}"
-exit ${ret}
+${HG} archive --repository "${basename}" --type tgz \
+ --prefix "${basename}" --rev "${cset}" \
+ "${output}"
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Buildroot] [PATCH 6/9 v3] support/download: convert Hg to use the wrapper
2014-07-20 22:42 ` [Buildroot] [PATCH 6/9 v3] support/download: convert Hg " Yann E. MORIN
@ 2014-08-03 17:41 ` Thomas De Schampheleire
0 siblings, 0 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2014-08-03 17:41 UTC (permalink / raw)
To: buildroot
On Mon, Jul 21, 2014 at 12:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> This drastically simplifies the hg helper, as it no longer has to deal
> with atomically saving the downloaded archive.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> package/pkg-download.mk | 7 +++++--
> support/download/hg | 48 +++++++++++++-----------------------------------
> 2 files changed, 18 insertions(+), 37 deletions(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 696ff3e..454179c 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -180,8 +180,11 @@ endef
>
> define DOWNLOAD_HG
> test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
> - $(EXTRA_ENV) support/download/hg $($(PKG)_SITE) $($(PKG)_DL_VERSION) \
> - $($(PKG)_BASE_NAME) $(DL_DIR)/$($(PKG)_SOURCE)
> + $(EXTRA_ENV) support/download/wrapper hg \
> + $(DL_DIR)/$($(PKG)_SOURCE) \
> + $($(PKG)_SITE) \
> + $($(PKG)_DL_VERSION) \
> + $($(PKG)_BASE_NAME)
> endef
>
> # TODO: improve to check that the given PKG_DL_VERSION exists on the remote
> diff --git a/support/download/hg b/support/download/hg
> index 6e9e26b..510ef8b 100755
> --- a/support/download/hg
> +++ b/support/download/hg
> @@ -1,46 +1,24 @@
> #!/bin/bash
>
> -# We want to catch any command failure, and exit immediately
> +# We want to catch any unexpected failure, and exit immediately
> set -e
>
> # Download helper for hg
> # Call it with:
> -# $1: hg repo
> -# $2: hg cset
> -# $3: package's basename (eg. foobar-1.2.3)
> -# $4: output file
> +# $1: output file
> +# $2: hg repo
> +# $3: hg cset
> +# $4: package's basename (eg. foobar-1.2.3)
> # And this environment:
> # HG : the hg command to call
> -# BUILD_DIR: path to Buildroot's build dir
>
> -repo="${1}"
> -cset="${2}"
> -basename="${3}"
> -output="${4}"
> +output="${1}"
> +repo="${2}"
> +cset="${3}"
> +basename="${4}"
>
> -repodir="${basename}.tmp-hg-checkout"
> -tmp_output="$( mktemp "${output}.XXXXXX" )"
> +${HG} clone --noupdate --rev "${cset}" "${repo}" "${basename}"
>
> -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
> -
> -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
> - mv "${tmp_output}" "${output}"
> - ret=0
> - fi
> -fi
> -
> -# Cleanup
> -rm -rf "${repodir}" "${tmp_output}"
> -exit ${ret}
> +${HG} archive --repository "${basename}" --type tgz \
> + --prefix "${basename}" --rev "${cset}" \
> + "${output}"
> --
> 1.9.1
Tested-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
(Tested by running 'make vim-source')
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 7/9 v3] support/download: convert scp to use the wrapper
2014-07-20 22:42 [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
` (5 preceding siblings ...)
2014-07-20 22:42 ` [Buildroot] [PATCH 6/9 v3] support/download: convert Hg " Yann E. MORIN
@ 2014-07-20 22:42 ` Yann E. MORIN
2014-08-03 17:42 ` Thomas De Schampheleire
2014-07-20 22:42 ` [Buildroot] [PATCH 8/9 v3] support/download: convert svn " Yann E. MORIN
` (2 subsequent siblings)
9 siblings, 1 reply; 25+ messages in thread
From: Yann E. MORIN @ 2014-07-20 22:42 UTC (permalink / raw)
To: buildroot
This drastically simplifies the scp helper, as it no longer has to deal
with atomically saving the downloaded archive.
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
package/pkg-download.mk | 5 +++--
support/download/scp | 24 ++++++------------------
2 files changed, 9 insertions(+), 20 deletions(-)
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 454179c..434eeaf 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -164,8 +164,9 @@ endef
# to prepend the path with a slash: scp://[user@]host:/absolutepath
define DOWNLOAD_SCP
test -e $(DL_DIR)/$(2) || \
- $(EXTRA_ENV) support/download/scp '$(call stripurischeme,$(call qstrip,$(1)))' \
- $(DL_DIR)/$(2) && \
+ $(EXTRA_ENV) support/download/wrapper scp \
+ $(DL_DIR)/$(2) \
+ '$(call stripurischeme,$(call qstrip,$(1)))' && \
$(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
endef
diff --git a/support/download/scp b/support/download/scp
index d3aad43..6ae545c 100755
--- a/support/download/scp
+++ b/support/download/scp
@@ -1,28 +1,16 @@
#!/bin/bash
-# We want to catch any command failure, and exit immediately
+# We want to catch any unexpected failure, and exit immediately
set -e
# Download helper for scp
# Call it with:
-# $1: URL
-# $2: output file
+# $1: output file
+# $2: URL
# And this environment:
# SCP : the scp command to call
-url="${1}"
-output="${2}"
-tmp_dl="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
-tmp_output="$( mktemp "${output}.XXXXXX" )"
+output="${1}"
+url="${2}"
-ret=1
-if ${SCP} "${url}" "${tmp_dl}"; then
- if mv "${tmp_dl}" "${tmp_output}"; then
- mv "${tmp_output}" "${output}"
- ret=0
- fi
-fi
-
-# Cleanup
-rm -f "${tmp_dl}" "${tmp_output}"
-exit ${ret}
+${SCP} "${url}" "${output}"
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Buildroot] [PATCH 7/9 v3] support/download: convert scp to use the wrapper
2014-07-20 22:42 ` [Buildroot] [PATCH 7/9 v3] support/download: convert scp " Yann E. MORIN
@ 2014-08-03 17:42 ` Thomas De Schampheleire
0 siblings, 0 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2014-08-03 17:42 UTC (permalink / raw)
To: buildroot
On Mon, Jul 21, 2014 at 12:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> This drastically simplifies the scp helper, as it no longer has to deal
> with atomically saving the downloaded archive.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> package/pkg-download.mk | 5 +++--
> support/download/scp | 24 ++++++------------------
> 2 files changed, 9 insertions(+), 20 deletions(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 454179c..434eeaf 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -164,8 +164,9 @@ endef
> # to prepend the path with a slash: scp://[user@]host:/absolutepath
> define DOWNLOAD_SCP
> test -e $(DL_DIR)/$(2) || \
> - $(EXTRA_ENV) support/download/scp '$(call stripurischeme,$(call qstrip,$(1)))' \
> - $(DL_DIR)/$(2) && \
> + $(EXTRA_ENV) support/download/wrapper scp \
> + $(DL_DIR)/$(2) \
> + '$(call stripurischeme,$(call qstrip,$(1)))' && \
> $(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
> endef
>
> diff --git a/support/download/scp b/support/download/scp
> index d3aad43..6ae545c 100755
> --- a/support/download/scp
> +++ b/support/download/scp
> @@ -1,28 +1,16 @@
> #!/bin/bash
>
> -# We want to catch any command failure, and exit immediately
> +# We want to catch any unexpected failure, and exit immediately
> set -e
>
> # Download helper for scp
> # Call it with:
> -# $1: URL
> -# $2: output file
> +# $1: output file
> +# $2: URL
> # And this environment:
> # SCP : the scp command to call
>
> -url="${1}"
> -output="${2}"
> -tmp_dl="$( mktemp "${BUILD_DIR}/.XXXXXX" )"
> -tmp_output="$( mktemp "${output}.XXXXXX" )"
> +output="${1}"
> +url="${2}"
>
> -ret=1
> -if ${SCP} "${url}" "${tmp_dl}"; then
> - if mv "${tmp_dl}" "${tmp_output}"; then
> - mv "${tmp_output}" "${output}"
> - ret=0
> - fi
> -fi
> -
> -# Cleanup
> -rm -f "${tmp_dl}" "${tmp_output}"
> -exit ${ret}
> +${SCP} "${url}" "${output}"
Tested-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
(Tested by setting a primary site to 'scp://localhost:/tmp' and
running 'make vim-source')
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 8/9 v3] support/download: convert svn to use the wrapper
2014-07-20 22:42 [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
` (6 preceding siblings ...)
2014-07-20 22:42 ` [Buildroot] [PATCH 7/9 v3] support/download: convert scp " Yann E. MORIN
@ 2014-07-20 22:42 ` Yann E. MORIN
2014-08-03 17:42 ` Thomas De Schampheleire
2014-07-20 22:42 ` [Buildroot] [PATCH 9/9 v3] support/download: convert wget " Yann E. MORIN
2014-07-27 14:13 ` [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads) Thomas Petazzoni
9 siblings, 1 reply; 25+ messages in thread
From: Yann E. MORIN @ 2014-07-20 22:42 UTC (permalink / raw)
To: buildroot
This drastically simplifies the svn helper, as it no longer has to deal
with atomically saving the downloaded archive.
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
package/pkg-download.mk | 6 ++++--
support/download/svn | 44 +++++++++++---------------------------------
2 files changed, 15 insertions(+), 35 deletions(-)
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index 434eeaf..b18864e 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -147,8 +147,10 @@ endef
define DOWNLOAD_SVN
test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
- $(EXTRA_ENV) support/download/svn $($(PKG)_SITE) $($(PKG)_DL_VERSION) \
- $($(PKG)_BASE_NAME) $(DL_DIR)/$($(PKG)_SOURCE)
+ $(EXTRA_ENV) support/download/wrapper svn \
+ $(DL_DIR)/$($(PKG)_SOURCE) \
+ $($(PKG)_SITE) $($(PKG)_DL_VERSION) \
+ $($(PKG)_BASE_NAME)
endef
define SOURCE_CHECK_SVN
diff --git a/support/download/svn b/support/download/svn
index 39cbbcb..36ba520 100755
--- a/support/download/svn
+++ b/support/download/svn
@@ -1,44 +1,22 @@
#!/bin/bash
-# We want to catch any command failure, and exit immediately
+# We want to catch any unexpected failure, and exit immediately
set -e
# Download helper for svn
# Call it with:
-# $1: svn repo
-# $2: svn revision
-# $3: package's basename (eg. foobar-1.2.3)
-# $4: output file
+# $1: output file
+# $2: svn repo
+# $3: svn revision
+# $4: package's basename (eg. foobar-1.2.3)
# And this environment:
# SVN : the svn command to call
-# BUILD_DIR: path to Buildroot's build dir
-repo="${1}"
-rev="${2}"
-basename="${3}"
-output="${4}"
+output="${1}"
+repo="${2}"
+rev="${3}"
+basename="${4}"
-repodir="${basename}.tmp-svn-checkout"
-tmp_output="$( mktemp "${output}.XXXXXX" )"
+${SVN} export "${repo}@${rev}" "${basename}"
-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
-
-ret=1
-if ${SVN} export "${repo}@${rev}" "${repodir}"; then
- if tar czf "${tmp_output}" "${repodir}"; then
- mv "${tmp_output}" "${output}"
- ret=0
- fi
-fi
-
-# Cleanup
-rm -rf "${repodir}" "${tmp_output}"
-exit ${ret}
+tar czf "${output}" "${basename}"
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Buildroot] [PATCH 8/9 v3] support/download: convert svn to use the wrapper
2014-07-20 22:42 ` [Buildroot] [PATCH 8/9 v3] support/download: convert svn " Yann E. MORIN
@ 2014-08-03 17:42 ` Thomas De Schampheleire
0 siblings, 0 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2014-08-03 17:42 UTC (permalink / raw)
To: buildroot
On Mon, Jul 21, 2014 at 12:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> This drastically simplifies the svn helper, as it no longer has to deal
> with atomically saving the downloaded archive.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> package/pkg-download.mk | 6 ++++--
> support/download/svn | 44 +++++++++++---------------------------------
> 2 files changed, 15 insertions(+), 35 deletions(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index 434eeaf..b18864e 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -147,8 +147,10 @@ endef
>
> define DOWNLOAD_SVN
> test -e $(DL_DIR)/$($(PKG)_SOURCE) || \
> - $(EXTRA_ENV) support/download/svn $($(PKG)_SITE) $($(PKG)_DL_VERSION) \
> - $($(PKG)_BASE_NAME) $(DL_DIR)/$($(PKG)_SOURCE)
> + $(EXTRA_ENV) support/download/wrapper svn \
> + $(DL_DIR)/$($(PKG)_SOURCE) \
> + $($(PKG)_SITE) $($(PKG)_DL_VERSION) \
> + $($(PKG)_BASE_NAME)
> endef
>
> define SOURCE_CHECK_SVN
> diff --git a/support/download/svn b/support/download/svn
> index 39cbbcb..36ba520 100755
> --- a/support/download/svn
> +++ b/support/download/svn
> @@ -1,44 +1,22 @@
> #!/bin/bash
>
> -# We want to catch any command failure, and exit immediately
> +# We want to catch any unexpected failure, and exit immediately
> set -e
>
> # Download helper for svn
> # Call it with:
> -# $1: svn repo
> -# $2: svn revision
> -# $3: package's basename (eg. foobar-1.2.3)
> -# $4: output file
> +# $1: output file
> +# $2: svn repo
> +# $3: svn revision
> +# $4: package's basename (eg. foobar-1.2.3)
> # And this environment:
> # SVN : the svn command to call
> -# BUILD_DIR: path to Buildroot's build dir
>
> -repo="${1}"
> -rev="${2}"
> -basename="${3}"
> -output="${4}"
> +output="${1}"
> +repo="${2}"
> +rev="${3}"
> +basename="${4}"
>
> -repodir="${basename}.tmp-svn-checkout"
> -tmp_output="$( mktemp "${output}.XXXXXX" )"
> +${SVN} export "${repo}@${rev}" "${basename}"
>
> -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
> -
> -ret=1
> -if ${SVN} export "${repo}@${rev}" "${repodir}"; then
> - if tar czf "${tmp_output}" "${repodir}"; then
> - mv "${tmp_output}" "${output}"
> - ret=0
> - fi
> -fi
> -
> -# Cleanup
> -rm -rf "${repodir}" "${tmp_output}"
> -exit ${ret}
> +tar czf "${output}" "${basename}"
> --
> 1.9.1
Tested-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
(Tested by running 'make open2300-source')
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 9/9 v3] support/download: convert wget to use the wrapper
2014-07-20 22:42 [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
` (7 preceding siblings ...)
2014-07-20 22:42 ` [Buildroot] [PATCH 8/9 v3] support/download: convert svn " Yann E. MORIN
@ 2014-07-20 22:42 ` Yann E. MORIN
2014-08-03 17:43 ` Thomas De Schampheleire
2014-07-27 14:13 ` [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads) Thomas Petazzoni
9 siblings, 1 reply; 25+ messages in thread
From: Yann E. MORIN @ 2014-07-20 22:42 UTC (permalink / raw)
To: buildroot
This drastically simplifies the wget helper, as it no longer has to deal
with atomically saving the downloaded archive.
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
---
package/pkg-download.mk | 4 +++-
support/download/wget | 31 ++++++-------------------------
2 files changed, 9 insertions(+), 26 deletions(-)
diff --git a/package/pkg-download.mk b/package/pkg-download.mk
index b18864e..6f5b080 100644
--- a/package/pkg-download.mk
+++ b/package/pkg-download.mk
@@ -203,7 +203,9 @@ endef
define DOWNLOAD_WGET
test -e $(DL_DIR)/$(2) || \
- $(EXTRA_ENV) support/download/wget '$(call qstrip,$(1))' $(DL_DIR)/$(2) && \
+ $(EXTRA_ENV) support/download/wrapper wget \
+ $(DL_DIR)/$(2) \
+ '$(call qstrip,$(1))' && \
$(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
endef
diff --git a/support/download/wget b/support/download/wget
index cbeca3b..84f0c53 100755
--- a/support/download/wget
+++ b/support/download/wget
@@ -1,35 +1,16 @@
#!/bin/bash
-# We want to catch any command failure, and exit immediately
+# We want to catch any unexpected failure, and exit immediately
set -e
# Download helper for wget
# Call it with:
-# $1: URL
-# $2: output file
+# $1: output file
+# $2: URL
# And this environment:
# WGET : the wget command to call
-# BUILD_DIR: path to Buildroot's build dir
-url="${1}"
-output="${2}"
+output="${1}"
+url="${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)
-# - then we copy to 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
-
-ret=1
-if ${WGET} -O "${tmp_dl}" "${url}"; then
- if mv "${tmp_dl}" "${tmp_output}"; then
- mv "${tmp_output}" "${output}"
- ret=0
- fi
-fi
-
-rm -f "${tmp_dl}" "${tmp_output}"
-exit ${ret}
+${WGET} -O "${output}" "${url}"
--
1.9.1
^ permalink raw reply related [flat|nested] 25+ messages in thread* [Buildroot] [PATCH 9/9 v3] support/download: convert wget to use the wrapper
2014-07-20 22:42 ` [Buildroot] [PATCH 9/9 v3] support/download: convert wget " Yann E. MORIN
@ 2014-08-03 17:43 ` Thomas De Schampheleire
0 siblings, 0 replies; 25+ messages in thread
From: Thomas De Schampheleire @ 2014-08-03 17:43 UTC (permalink / raw)
To: buildroot
On Mon, Jul 21, 2014 at 12:42 AM, Yann E. MORIN <yann.morin.1998@free.fr> wrote:
> This drastically simplifies the wget helper, as it no longer has to deal
> with atomically saving the downloaded archive.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> ---
> package/pkg-download.mk | 4 +++-
> support/download/wget | 31 ++++++-------------------------
> 2 files changed, 9 insertions(+), 26 deletions(-)
>
> diff --git a/package/pkg-download.mk b/package/pkg-download.mk
> index b18864e..6f5b080 100644
> --- a/package/pkg-download.mk
> +++ b/package/pkg-download.mk
> @@ -203,7 +203,9 @@ endef
>
> define DOWNLOAD_WGET
> test -e $(DL_DIR)/$(2) || \
> - $(EXTRA_ENV) support/download/wget '$(call qstrip,$(1))' $(DL_DIR)/$(2) && \
> + $(EXTRA_ENV) support/download/wrapper wget \
> + $(DL_DIR)/$(2) \
> + '$(call qstrip,$(1))' && \
> $(call VERIFY_HASH,$(PKGDIR)/$($(PKG)_NAME).hash,$(DL_DIR)/$(2))
> endef
>
> diff --git a/support/download/wget b/support/download/wget
> index cbeca3b..84f0c53 100755
> --- a/support/download/wget
> +++ b/support/download/wget
> @@ -1,35 +1,16 @@
> #!/bin/bash
>
> -# We want to catch any command failure, and exit immediately
> +# We want to catch any unexpected failure, and exit immediately
> set -e
>
> # Download helper for wget
> # Call it with:
> -# $1: URL
> -# $2: output file
> +# $1: output file
> +# $2: URL
> # And this environment:
> # WGET : the wget command to call
> -# BUILD_DIR: path to Buildroot's build dir
>
> -url="${1}"
> -output="${2}"
> +output="${1}"
> +url="${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)
> -# - then we copy to 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
> -
> -ret=1
> -if ${WGET} -O "${tmp_dl}" "${url}"; then
> - if mv "${tmp_dl}" "${tmp_output}"; then
> - mv "${tmp_output}" "${output}"
> - ret=0
> - fi
> -fi
> -
> -rm -f "${tmp_dl}" "${tmp_output}"
> -exit ${ret}
> +${WGET} -O "${output}" "${url}"
> --
> 1.9.1
Tested-by: Thomas De Schampheleire <thomas.de.schampheleire@gmail.com>
(Tested by running 'make busybox-source')
^ permalink raw reply [flat|nested] 25+ messages in thread
* [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads)
2014-07-20 22:42 [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads) Yann E. MORIN
` (8 preceding siblings ...)
2014-07-20 22:42 ` [Buildroot] [PATCH 9/9 v3] support/download: convert wget " Yann E. MORIN
@ 2014-07-27 14:13 ` Thomas Petazzoni
2014-07-27 19:58 ` Yann E. MORIN
9 siblings, 1 reply; 25+ messages in thread
From: Thomas Petazzoni @ 2014-07-27 14:13 UTC (permalink / raw)
To: buildroot
Hello,
On Mon, 21 Jul 2014 00:42:22 +0200, Yann E. MORIN wrote:
> This series is a follow-up to the previous download series. Following
> the advice from Arnout, I introduced a wrapper script that is responsible
> for the saving atomically in BR2_DL_DIR. If also manages all the temporary
> files, and properly cleans up.
>
> Consequently, the helpers are now nuch more simple, and no longer need to
> duplicate the complex handling of atomicity in BR2_DL_DIR, nor do they
> need to manage temporary files.
In principle, it certainly looks good. I'm just wondering if we really
need the separate scripts for each of the download methods, since they
do just one command basically. What about a single
support/scripts/download-helper script, which does the common stuff +
contains a switch to call the appropriate sub-functions for each
download method?
What do others think? Could some others test the patch series and give
their feedback?
Thanks,
Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
^ permalink raw reply [flat|nested] 25+ messages in thread* [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads)
2014-07-27 14:13 ` [Buildroot] [PATCH 0/9 v3] Cleanups in download helpers (branch yem/check-downloads) Thomas Petazzoni
@ 2014-07-27 19:58 ` Yann E. MORIN
0 siblings, 0 replies; 25+ messages in thread
From: Yann E. MORIN @ 2014-07-27 19:58 UTC (permalink / raw)
To: buildroot
Thomas, All,
On 2014-07-27 16:13 +0200, Thomas Petazzoni spake thusly:
> On Mon, 21 Jul 2014 00:42:22 +0200, Yann E. MORIN wrote:
> > This series is a follow-up to the previous download series. Following
> > the advice from Arnout, I introduced a wrapper script that is responsible
> > for the saving atomically in BR2_DL_DIR. If also manages all the temporary
> > files, and properly cleans up.
> >
> > Consequently, the helpers are now nuch more simple, and no longer need to
> > duplicate the complex handling of atomicity in BR2_DL_DIR, nor do they
> > need to manage temporary files.
>
> In principle, it certainly looks good. I'm just wondering if we really
> need the separate scripts for each of the download methods, since they
> do just one command basically. What about a single
> support/scripts/download-helper script, which does the common stuff +
> contains a switch to call the appropriate sub-functions for each
> download method?
I thought about this, too. But I can see at least two reasons against:
- bloat: the download script would get pretty large. That can be OK in
itself, although I do not like it too much. Having the download
helpers properly split into separate files makes it easier to manage
and maintain, I think.
- simplicity: since the wrapper calls to the helpers, it has a single
command for which to catch failure. The error path is very simple.
If the download script was to call functions, then those function
should not be able to cause the script to exit() or we would not be
able to clean up behind ourselves (and 'set -e' has no effect in an
'if' statement). Granted, we could use traps, but Arnout and I do
not like them very much: it's easy to mis-use them, and miss
updating them later on; having a linear code-flow is easier to
handle.
So I still prefer to use separate scripts for the helpers, rather than
move them to functions in the download script itself.
Of course, should that really be a blocker, I can rethink that. But be
real quick, since -rc1 is approaching, and I'd like to fix it before -rc1
(since it impacts the mirror set up by Peter on mirror.buildroot.org.)
> What do others think? Could some others test the patch series and give
> their feedback?
Yes, please! ;-)
Regards,
Yann E. MORIN.
--
.-----------------.--------------------.------------------.--------------------.
| Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ |
| +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. |
'------------------------------^-------^------------------^--------------------'
^ permalink raw reply [flat|nested] 25+ messages in thread