support/download: rename internal 'verbose' variable where applicable

From: Thomas De Schampheleire <[email protected]>

Even though that most download commands actually print some output, like progress indication or other messages, the actual command used is not. This makes it hard to analyze a build log when you are not fully familiar with the typical output of said log.

Update the download helpers to do just that, respecting any quiet/verbose flag so that a silent make (make -s) does not get more verbose.

Note: getting rid of the duplication of the command in the script is not straightforward without breaking support for arguments with spaces.

Signed-off-by Thomas De Schampheleire <[email protected]>

---
 support/download/bzr  | 3 +++
 support/download/cvs  | 3 +++
 support/download/file | 3 +++
 support/download/git  | 3 +++
 support/download/hg   | 3 +++
 support/download/scp  | 3 +++
 support/download/svn  | 3 +++
 support/download/wget | 3 +++
 8 files changed, 24 insertions(+)

support/download/bzr +0 -0

index 7cc6890a30..379a8db753 100755

support/download/bzr +1 -0

+    fi
     eval ${BZR} "${@}"
 }
 

support/download/cvs +3 -0

index 463d70c220..04f030ff54 100755
@@ -39,6 +39,9 @@ shift $((OPTIND-1)) # Get rid of our options
 # ). Since nobody sane will put large code bases in CVS, a timeout of
 # 10 minutes should do the trick.
 _cvs() {
+    if [ -z "${quiet}" ]; then
+        echo timeout 10m ${CVS} "${@}"
+    fi
     eval timeout 10m ${CVS} "${@}"
 }
 

support/download/file +3 -0

index e52fcf2c8c..7870a2f27c 100755
@@ -36,6 +36,9 @@ shift $((OPTIND-1)) # Get rid of our options
 # Caller needs to single-quote its arguments to prevent them from
 # being expanded a second time (in case there are spaces in them)
 _localfiles() {
+    if [ -n "${verbose}" ]; then
+        echo ${LOCALFILES} "${@}"
+    fi
     eval ${LOCALFILES} "${@}"
 }
 

support/download/git +3 -0

index 01e0f214cf..369c256f75 100755
@@ -79,6 +79,9 @@ trap _on_error ERR
 # Caller needs to single-quote its arguments to prevent them from
 # being expanded a second time (in case there are spaces in them)
 _git() {
+    if [ -z "${quiet}" ]; then
+        echo GIT_DIR="${git_cache}/.git" ${GIT} "${@}"
+    fi
     eval GIT_DIR="${git_cache}/.git" ${GIT} "${@}"
 }
 

support/download/hg +3 -0

index c8149c9c91..0dd27d78a2 100755
@@ -33,6 +33,9 @@ shift $((OPTIND-1)) # Get rid of our options
 # Caller needs to single-quote its arguments to prevent them from
 # being expanded a second time (in case there are spaces in them)
 _hg() {
+    if [ -z "${quiet}" ]; then
+        echo ${HG} "${@}"
+    fi
     eval ${HG} "${@}"
 }
 

support/download/scp +3 -0

index 636d66c66a..e2c9710992 100755
@@ -31,6 +31,9 @@ shift $((OPTIND-1)) # Get rid of our options
 # Caller needs to single-quote its arguments to prevent them from
 # being expanded a second time (in case there are spaces in them)
 _scp() {
+    if [ -z "${quiet}" ]; then
+        echo ${SCP} "${@}"
+    fi
     eval ${SCP} "${@}"
 }
 

support/download/svn +3 -0

index ab9bd85f45..e71ca804aa 100755
@@ -40,6 +40,9 @@ shift $((OPTIND-1)) # Get rid of our options
 # Caller needs to single-quote its arguments to prevent them from
 # being expanded a second time (in case there are spaces in them)
 _svn() {
+    if [ -z "${quiet}" ]; then
+        echo ${SVN} "${@}"
+    fi
     eval ${SVN} "${@}"
 }
 

support/download/wget +3 -0

index 1bcb1e4b00..5867f37f6f 100755
@@ -33,6 +33,9 @@ shift $((OPTIND-1)) # Get rid of our options
 # Caller needs to single-quote its arguments to prevent them from
 # being expanded a second time (in case there are spaces in them)
 _wget() {
+    if [ -z "${quiet}" ]; then
+        echo ${WGET} "${@}"
+    fi
     eval ${WGET} "${@}"
 }
 

Signature · 6 lines

2.26.2

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

On 2021-01-15 16:00 +0100, Thomas De Schampheleire spake thusly:

Quoted 9 lines

From: Thomas De Schampheleire <[email protected]>

Even though that most download commands actually print some output, like progress indication or other messages, the actual command used is not. This makes it hard to analyze a build log when you are not fully familiar with the typical output of said log.

Update the download helpers to do just that, respecting any quiet/verbose flag so that a silent make (make -s) does not get more verbose.

Quoted 20 lines

Note: getting rid of the duplication of the command in the script is not straightforward without breaking support for arguments with spaces.

Signed-off-by Thomas De Schampheleire <[email protected]>

---
 support/download/bzr  | 3 +++
 support/download/cvs  | 3 +++
 support/download/file | 3 +++
 support/download/git  | 3 +++
 support/download/hg   | 3 +++
 support/download/scp  | 3 +++
 support/download/svn  | 3 +++
 support/download/wget | 3 +++
 8 files changed, 24 insertions(+)

support/download/bzr +0 -0

index 7cc6890a30..379a8db753 100755

support/download/bzr shift $((OPTIND-1)) # Get rid of our options · line 38

Quoted 5 lines

support/download/bzr +2 -0

@@ -34,6 +34,9 @@ shift $((OPTIND-1)) # Get rid of our options
 # Caller needs to single-quote its arguments to prevent them from
 # being expanded a second time (in case there are spaces in them)
 _bzr() {
+    if [ -z "${quiet}" ]; then
+        echo ${BZR} "${@}"

I've switched away from echo, to use printf instead.

Applied to master with that fixed, thanks.

Regards,
Yann E. MORIN.

Quoted 109 lines

support/download/bzr +1 -0

+    fi
     eval ${BZR} "${@}"
 }
 

support/download/cvs +3 -0

index 463d70c220..04f030ff54 100755
@@ -39,6 +39,9 @@ shift $((OPTIND-1)) # Get rid of our options
 # ). Since nobody sane will put large code bases in CVS, a timeout of
 # 10 minutes should do the trick.
 _cvs() {
+    if [ -z "${quiet}" ]; then
+        echo timeout 10m ${CVS} "${@}"
+    fi
     eval timeout 10m ${CVS} "${@}"
 }
 

support/download/file +3 -0

index e52fcf2c8c..7870a2f27c 100755
@@ -36,6 +36,9 @@ shift $((OPTIND-1)) # Get rid of our options
 # Caller needs to single-quote its arguments to prevent them from
 # being expanded a second time (in case there are spaces in them)
 _localfiles() {
+    if [ -n "${verbose}" ]; then
+        echo ${LOCALFILES} "${@}"
+    fi
     eval ${LOCALFILES} "${@}"
 }
 

support/download/git +3 -0

index 01e0f214cf..369c256f75 100755
@@ -79,6 +79,9 @@ trap _on_error ERR
 # Caller needs to single-quote its arguments to prevent them from
 # being expanded a second time (in case there are spaces in them)
 _git() {
+    if [ -z "${quiet}" ]; then
+        echo GIT_DIR="${git_cache}/.git" ${GIT} "${@}"
+    fi
     eval GIT_DIR="${git_cache}/.git" ${GIT} "${@}"
 }
 

support/download/hg +3 -0

index c8149c9c91..0dd27d78a2 100755
@@ -33,6 +33,9 @@ shift $((OPTIND-1)) # Get rid of our options
 # Caller needs to single-quote its arguments to prevent them from
 # being expanded a second time (in case there are spaces in them)
 _hg() {
+    if [ -z "${quiet}" ]; then
+        echo ${HG} "${@}"
+    fi
     eval ${HG} "${@}"
 }
 

support/download/scp +3 -0

index 636d66c66a..e2c9710992 100755
@@ -31,6 +31,9 @@ shift $((OPTIND-1)) # Get rid of our options
 # Caller needs to single-quote its arguments to prevent them from
 # being expanded a second time (in case there are spaces in them)
 _scp() {
+    if [ -z "${quiet}" ]; then
+        echo ${SCP} "${@}"
+    fi
     eval ${SCP} "${@}"
 }
 

support/download/svn +3 -0

index ab9bd85f45..e71ca804aa 100755
@@ -40,6 +40,9 @@ shift $((OPTIND-1)) # Get rid of our options
 # Caller needs to single-quote its arguments to prevent them from
 # being expanded a second time (in case there are spaces in them)
 _svn() {
+    if [ -z "${quiet}" ]; then
+        echo ${SVN} "${@}"
+    fi
     eval ${SVN} "${@}"
 }
 

support/download/wget +3 -0

index 1bcb1e4b00..5867f37f6f 100755
@@ -33,6 +33,9 @@ shift $((OPTIND-1)) # Get rid of our options
 # Caller needs to single-quote its arguments to prevent them from
 # being expanded a second time (in case there are spaces in them)
 _wget() {
+    if [ -z "${quiet}" ]; then
+        echo ${WGET} "${@}"
+    fi
     eval ${WGET} "${@}"
 }
 

Signature · 6 lines

2.26.2

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Signature · 10 lines

.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Thomas, All,

On 2021-03-16 23:29 +0100, Yann E. MORIN spake thusly:

Quoted 10 lines

On 2021-01-15 16:00 +0100, Thomas De Schampheleire spake thusly:

Quoted 9 lines

From: Thomas De Schampheleire <[email protected]>

Even though that most download commands actually print some output, like progress indication or other messages, the actual command used is not. This makes it hard to analyze a build log when you are not fully familiar with the typical output of said log.

Update the download helpers to do just that, respecting any quiet/verbose flag so that a silent make (make -s) does not get more verbose.

Note that this breaks the git downloads, as reported by Matthias:

    https://bugs.busybox.net/show_bug.cgi?id=13631

It most probably also breaks the svn downloads too. So I know you are primarily using Hg at your place, but clearly the git and svn backends were not tested, and I overlooked the impact of that change when applying. :-/

However, I don't see an easy way out.

My first idea was to test whether stdout was a terminal or not, e.g.:

    if [ -z "${quiet}" -a -t 1 ]; then
        printf ...
    fi

So that the command would not be printed when called in a subshell like is used to get the date:

    date="$( _git ... )"

But that means the commands would not be printed for people that log the output, like so:

    $ make 2 >&1|tee build.log
or with:
    $ ./utils/brmake

either being probably a well-established use-case (I use that almost exclusively...)

Another idea I had was to force quiet in such case:

    date="$( quiet=-q _git ... )"

But I find it pretty ugly, because it just papers over the problem...

El jue, 18 mar 2021 a las 22:01, Yann E. MORIN
(<[email protected]>) escribi?:

Quoted 12 lines

Thomas, All,

On 2021-03-16 23:29 +0100, Yann E. MORIN spake thusly:

Quoted 8 lines

On 2021-01-15 16:00 +0100, Thomas De Schampheleire spake thusly:

Quoted 7 lines

From: Thomas De Schampheleire <[email protected]>

Even though that most download commands actually print some output, like progress indication or other messages, the actual command used is not. This makes it hard to analyze a build log when you are not fully familiar with the typical output of said log.

Quoted 5 lines

Quoted 2 lines

Quoted 2 lines

Update the download helpers to do just that, respecting any quiet/verbose flag so that a silent make (make -s) does not get more verbose.

Note that this breaks the git downloads, as reported by Matthias:

    https://bugs.busybox.net/show_bug.cgi?id=13631

oops :-o
I indeed tested hg, wget, and scp downloads, but not git/svn. The change seemed so trivial (*famous last words).

As the problem is caused by adding additional output in something where the output is parsed, a solution would be to send the 'printf' output to stderr iso stdout.

Another solution is indeed to use a custom _git/_svn for the above three cases. Actually you want a '__quiet_git', but the need to handle spaces in args correctly makes it more complex, if one does not want to duplicate code. One would need something like:

_git() {
    printf
    _quiet_git() "$@"
}

_quiet_git() {
<actual command>
}
but aside from the fact I'm unsure if this works correctly for the argument passing (including support for spaces), the fact that the printf and the actual command are duplicating the command now in two _different_ functions, is pretty fragile.

A simpler version of this is what you already suggested but disliked, hacking the 'quiet' variable for this purpose.
Or alternatively, if you like that better, add another variable, a 'boolean' perhaps, to steer this quietness.

And finally, the other approach is to revert this change for git and svn, but leave it enabled for the other helpers which just have 'normal' usage.

Of the above, what are your thoughts?

Thanks,
Thomas
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

Quoted 2 lines

Quoted 2 lines

Note that this breaks the git downloads, as reported by Matthias:

    https://bugs.busybox.net/show_bug.cgi?id=13631

Now the quesiton: do we want to apply this to all helpers, so that they are homogeneous, or do we do that only for git and svn?

My gut feeling is that we should do it for all, because it is relatively trivial to do, and then we have all backends using the same constructs, so they are easier to reason about.

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

Note that this breaks the git downloads, as reported by Matthias:

    https://bugs.busybox.net/show_bug.cgi?id=13631

FWIW, we've been bitten by this, too. Not our main branch, mind you. But on the "let's build the latest upstream branch and see if there are any surprises coming down the pipe" branch. And it sure found a little surprise for us.

Quoted 6 lines

Now the quesiton: do we want to apply this to all helpers, so that they are homogeneous, or do we do that only for git and svn?

My gut feeling is that we should do it for all, because it is relatively trivial to do, and then we have all backends using the same constructs, so they are easier to reason about.

My vote would be to keep them homogeneous, too. It'll also provide a facility that can be used in the future if anybody needs to add a feature to the other downloaders that would otherwise cause similar problems there.

Markus, All,

On 2021-03-19 11:24 -0700, Markus Mayer spake thusly:

Quoted 4 lines

FWIW, we've been bitten by this, too. Not our main branch, mind you. But on the "let's build the latest upstream branch and see if there are any surprises coming down the pipe" branch. And it sure found a little surprise for us.

Yeah, that was a serious issue, but we're still pretty early in the development phase, so now is a good time to break things! ;-)

Hopefully that's been settled now, with commit b70ce56651, that's been just merged. Feedback greatly appreciated if there's still feedback (or not).

From: Thomas De Schampheleire <[email protected]>

Most 'verbose' variable inside the download helpers actually mean 'quiet'. I.e. they are assigned in case quiet operation is requested, and empty in case of non-quiet operation. Using the name 'verbose' for such a variable is confusing, especially when you want to test the variable on emptiness or non-emptiness (in a subsequent commit).

Signed-off-by Thomas De Schampheleire <[email protected]>

---
 support/download/bzr  | 6 +++---
 support/download/cvs  | 6 +++---
 support/download/git  | 4 ++--
 support/download/hg   | 8 ++++----
 support/download/scp  | 6 +++---
 support/download/svn  | 6 +++---
 support/download/wget | 6 +++---
 7 files changed, 21 insertions(+), 21 deletions(-)

support/download/bzr +3 -3

index 5289a421cd..7cc6890a30 100755
@@ -16,10 +16,10 @@ set -e
 #   BZR      : the bzr command to call
 
 
-verbose=
+quiet=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
-    q)  verbose=-q;;
+    q)  quiet=-q;;
     o)  output="${OPTARG}";;
     u)  uri="${OPTARG}";;
     c)  cset="${OPTARG}";;

@@ -53,6 +53,6 @@ if [ ${bzr_version} -ge ${bzr_min_version} ]; then
     timestamp_opt="--per-file-timestamps"
 fi
 
-_bzr export ${verbose} --root="'${basename}/'" --format=tgz \
+_bzr export ${quiet} --root="'${basename}/'" --format=tgz \
     ${timestamp_opt} - "${@}" "'${uri}'" -r "'${cset}'" \
     >"${output}"

support/download/cvs +3 -3

index 9d0dc3cb3a..463d70c220 100755
@@ -16,10 +16,10 @@ set -e
 # Environment:
 #   CVS      : the cvs command to call
 
-verbose=
+quiet=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
-    q)  verbose=-Q;;
+    q)  quiet=-Q;;
     o)  output="${OPTARG}";;
     u)  uri="${OPTARG#*://}";;
     c)  rev="${OPTARG}";;

@@ -57,7 +57,7 @@ if [[ ! "${uri}" =~ ^: ]]; then
 fi
 
 export TZ=UTC
-_cvs ${verbose} -z3 -d"'${uri}'" \
+_cvs ${quiet} -z3 -d"'${uri}'" \
      co "${@}" -d "'${basename}'" ${select} "'${rev}'" -P "'${rawname}'"
 
 tar czf "${output}" "${basename}"

support/download/git +2 -2

index fa98198fe0..01e0f214cf 100755
@@ -50,11 +50,11 @@ _on_error() {
     exec "${myname}" "${OPTS[@]}" || exit ${ret}
 }
 
-verbose=
+quiet=
 recurse=0
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
-    q)  verbose=-q; exec >/dev/null;;
+    q)  quiet=-q; exec >/dev/null;;
     r)  recurse=1;;
     o)  output="${OPTARG}";;
     u)  uri="${OPTARG}";;

support/download/hg +4 -4

index efb515fca5..c8149c9c91 100755
@@ -15,10 +15,10 @@ set -e
 # Environment:
 #   HG       : the hg command to call
 
-verbose=
+quiet=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
-    q)  verbose=-q;;
+    q)  quiet=-q;;
     o)  output="${OPTARG}";;
     u)  uri="${OPTARG}";;
     c)  cset="${OPTARG}";;

@@ -36,8 +36,8 @@ _hg() {
     eval ${HG} "${@}"
 }
 
-_hg clone ${verbose} "${@}" --noupdate "'${uri}'" "'${basename}'"
+_hg clone ${quiet} "${@}" --noupdate "'${uri}'" "'${basename}'"
 
-_hg archive ${verbose} --repository "'${basename}'" --type tgz \
+_hg archive ${quiet} --repository "'${basename}'" --type tgz \
             --prefix "'${basename}'" --rev "'${cset}'" \
             - >"${output}"

support/download/scp +3 -3

index 80cf495c4e..636d66c66a 100755
@@ -14,10 +14,10 @@ set -e
 # Environment:
 #   SCP       : the scp command to call
 
-verbose=
+quiet=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
-    q)  verbose=-q;;
+    q)  quiet=-q;;
     o)  output="${OPTARG}";;
     f)  filename="${OPTARG}";;
     u)  uri="${OPTARG}";;

@@ -37,4 +37,4 @@ _scp() {
 # Remove any scheme prefix
 uri="${uri##scp://}"
 
-_scp ${verbose} "${@}" "'${uri}/${filename}'" "'${output}'"
+_scp ${quiet} "${@}" "'${uri}/${filename}'" "'${output}'"

support/download/svn +3 -3

index 839dccaf62..ab9bd85f45 100755
@@ -22,10 +22,10 @@ set -e
 
 . "${0%/*}/helpers"
 
-verbose=
+quiet=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
-    q)  verbose=-q;;
+    q)  quiet=-q;;
     o)  output="${OPTARG}";;
     u)  uri="${OPTARG}";;
     c)  rev="${OPTARG}";;

@@ -43,7 +43,7 @@ _svn() {
     eval ${SVN} "${@}"
 }
 
-_svn export ${verbose} "${@}" "'${uri}@${rev}'" "'${basename}'"
+_svn export ${quiet} "${@}" "'${uri}@${rev}'" "'${basename}'"
 
 # Get the date of the revision, to generate reproducible archives.
 # The output format is YYYY-MM-DDTHH:MM:SS.mmmuuuZ (i.e. always in the

support/download/wget +3 -3

index c69e6071aa..1bcb1e4b00 100755
@@ -15,10 +15,10 @@ set -e
 # Environment:
 #   WGET     : the wget command to call
 
-verbose=
+quiet=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
-    q)  verbose=-q;;
+    q)  quiet=-q;;
     o)  output="${OPTARG}";;
     f)  filename="${OPTARG}";;
     u)  url="${OPTARG}";;

@@ -40,4 +40,4 @@ _wget() {
 # mirror
 [ -n "${encode}" ] && filename=${filename//\?/%3F}
 
-_wget ${verbose} "${@}" -O "'${output}'" "'${url}/${filename}'"
+_wget ${quiet} "${@}" -O "'${output}'" "'${url}/${filename}'"

Signature · 6 lines

2.26.2

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot