[TO-BE-TESTED] support/download/hg: implement repository cache

Message ID 20190205202433.25292-1-patrickdepinguin@gmail.com
State New
Headers show
Series
  • [TO-BE-TESTED] support/download/hg: implement repository cache
Related show

Commit Message

Thomas De Schampheleire Feb. 5, 2019, 8:24 p.m.
From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>

Similar to the git download helper, implement a repository cache for
Mercurial repositories.

The code is mostly guided by the implementation for git, with certain parts
copied almost verbatim.

Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
---
 support/download/hg | 81 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 78 insertions(+), 3 deletions(-)

Note: this patch only got limited testing so far and needs to be tested further.
But I already wanted to send it out for feedback and for those of you that want
to help test it.

Comments

Arnout Vandecappelle (Essensium/Mind) Feb. 7, 2019, 9:33 p.m. | #1
On 05/02/2019 21:24, Thomas De Schampheleire wrote:
> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> 
> Similar to the git download helper, implement a repository cache for
> Mercurial repositories.
> 
> The code is mostly guided by the implementation for git, with certain parts
> copied almost verbatim.
> 
> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> ---
>  support/download/hg | 81 +++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 78 insertions(+), 3 deletions(-)
> 
> Note: this patch only got limited testing so far and needs to be tested further.
> But I already wanted to send it out for feedback and for those of you that want
> to help test it.

 It would be a good idea to do the same kind of copy and pasting on
support/testing/tests/download/test_git and its helpers and include that as the
first patches in the series.

 BTW, do you already have an idea of performance improvement?


> diff --git a/support/download/hg b/support/download/hg
> index efb515fca5..bb5cc87969 100755
> --- a/support/download/hg
> +++ b/support/download/hg
> @@ -1,7 +1,7 @@
>  #!/usr/bin/env bash
>  
>  # We want to catch any unexpected failure, and exit immediately
> -set -e
> +set -E

 If you do this, the comment above is no longer valid...
      -e  Exit immediately if a command exits with a non-zero status.
      -E  If set, the ERR trap is inherited by shell functions.
are not the same thing!

 However, since AFAICS the only function in this file is the trap handler
itself, this seems to be a mistake, no?

 Ow, this really is inherited from git, introduced by commit b7efb43e86da96.
Yann, care to explain?

>  
>  # Download helper for hg, to be called from the download wrapper script
>  #
> @@ -10,11 +10,39 @@ set -e
>  #   -o FILE     Generate archive in FILE.
>  #   -u URI      Clone from repository at URI.
>  #   -c CSET     Use changeset (or revision) CSET.
> +#   -d DLDIR    Download directory path.

 Hm, looks like this is missing in the git helper :-) Yann?

>  #   -n NAME     Use basename NAME.
>  #
[snip]
> +	Do *not* work in that directory; your changes will eventually get
> +	lost. Do *not* even use it as a remote, or as the source for new

 This bit is actually rubbish: it's perfectly fine to clone it, as long as you
don't push to it. Oh well.

> +	worktrees; your commits will eventually get lost.
> +_EOF_
> +
> +if ! [ -d "${hg_cache}/.hg" ]; then

 Is the presence of a .hg directory a 100% reliable of identifying it as a
mercurial repo?

> +    # While it would be possible to create an empty repo and use pull
> +    # subsequently, we intentionally use clone the first time as it could be
> +    # faster than pull thanks to clonebundles, if enabled on the server.
> +    printf "Cloning repository from '${uri}' into '${hg_cache}'\n"
> +    _hg clone ${verbose} "${@}" --noupdate "'${uri}'" "'${hg_cache}'"
> +else
> +    printf "Pulling repository from '${uri}' into '${hg_cache}'\n"
> +    _hg pull ${verbose} "${@}" --repository "'${hg_cache}'" "'${uri}'"
> +fi

 Does hg always do a full mirror on clone/pull? Or doesn't it have the concept
of special refs that git has?

> +
> +# Check that the changeset does exist. If it does not, re-cloning from
> +# scratch won't help, so we don't want to trash the repository for a
> +# missing commit. We just exit without going through the ERR trap.
> +if ! _hg identify --repository "'${hg_cache}'" --rev "'${cset}'" >/dev/null 2>&1; then
> +    printf "Commit '%s' does not exist in this repository\n." "${cset}"
> +    exit 1
> +fi
> +
> +# Make sure that there is no working directory. This will clear any user
> +# temptation to work in this directory.

 Good plan :-)

 Regards,
 Arnout

> +_hg update --repository "'${hg_cache}'" null >/dev/null 2>&1
>  
> -_hg archive ${verbose} --repository "'${basename}'" --type tgz \
> +_hg archive ${verbose} --repository "'${hg_cache}'" --type tgz \
>              --prefix "'${basename}'" --rev "'${cset}'" \
>              - >"${output}"
>
Yann E. MORIN Feb. 8, 2019, 4:54 p.m. | #2
Arnout, All,

On 2019-02-07 22:33 +0100, Arnout Vandecappelle spake thusly:
> On 05/02/2019 21:24, Thomas De Schampheleire wrote:
> > From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > 
> > Similar to the git download helper, implement a repository cache for
> > Mercurial repositories.
> > 
> > The code is mostly guided by the implementation for git, with certain parts
> > copied almost verbatim.
> > 
> > Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> > ---
> > index efb515fca5..bb5cc87969 100755
> > --- a/support/download/hg
> > +++ b/support/download/hg
> > @@ -1,7 +1,7 @@
> >  #!/usr/bin/env bash
> >  
> >  # We want to catch any unexpected failure, and exit immediately
> > -set -e
> > +set -E
> 
>  If you do this, the comment above is no longer valid...
>       -e  Exit immediately if a command exits with a non-zero status.
>       -E  If set, the ERR trap is inherited by shell functions.
> are not the same thing!
> 
>  However, since AFAICS the only function in this file is the trap handler
> itself, this seems to be a mistake, no?
> 
>  Ow, this really is inherited from git, introduced by commit b7efb43e86da96.
> Yann, care to explain?

Yes, I care to explain. Even though I do write some crap most of the
time, there are in fact two functions in the git backend.

Like all backends, the actual tool is wrapped into a _git() function,
which was introduced more than three years ago, by commit 3f2bdd070
(support/download: protect from custom commands with spaces in args), as
a way to actually fix a bug actually reported by Thomas DS.

Yes, this makes it that the set -E is needed, after all.

> >  # Download helper for hg, to be called from the download wrapper script
> >  #
> > @@ -10,11 +10,39 @@ set -e
> >  #   -o FILE     Generate archive in FILE.
> >  #   -u URI      Clone from repository at URI.
> >  #   -c CSET     Use changeset (or revision) CSET.
> > +#   -d DLDIR    Download directory path.
>  Hm, looks like this is missing in the git helper :-) Yann?

As I said above: I do write a lot of crap. However, in this case, I'm
not the culprit; see commit 6d938bcb (download: git: introduce cache
feature).

But yeah, the comment-help is not accurate.

> >  #   -n NAME     Use basename NAME.
> >  #
> [snip]
> > +	Do *not* work in that directory; your changes will eventually get
> > +	lost. Do *not* even use it as a remote, or as the source for new
> 
>  This bit is actually rubbish: it's perfectly fine to clone it, as long as you
> don't push to it. Oh well.

Indeed, it is technically possible to clone from it. However, that means
that the 'origin' would point to that, and people would be very easily
tempted to push to it. As a consequence, they could loose their work.

And this is really a cache for use by Buildroot, and we really want
users not to rely on it.

Hence, we really suggest they do not use it at all, not even as their
'origin'.

[--SNIP--]
> > +# Make sure that there is no working directory. This will clear any user
> > +# temptation to work in this directory.
>  Good plan :-)

Hmm.. I think someone once said there was not problem leaving cruft in
BR2_DL_DIR, because it does not matter what's inside as it is a location
private to Buildroot... (not directly in those words, but the message
was subtly similar.)

Regards,
Yann E. MORIN.

>  Regards,
>  Arnout
> 
> > +_hg update --repository "'${hg_cache}'" null >/dev/null 2>&1
> >  
> > -_hg archive ${verbose} --repository "'${basename}'" --type tgz \
> > +_hg archive ${verbose} --repository "'${hg_cache}'" --type tgz \
> >              --prefix "'${basename}'" --rev "'${cset}'" \
> >              - >"${output}"
> >
Arnout Vandecappelle (Essensium/Mind) Feb. 8, 2019, 7:27 p.m. | #3
On 08/02/2019 17:54, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2019-02-07 22:33 +0100, Arnout Vandecappelle spake thusly:
>> On 05/02/2019 21:24, Thomas De Schampheleire wrote:
>>> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>>>
>>> Similar to the git download helper, implement a repository cache for
>>> Mercurial repositories.
>>>
>>> The code is mostly guided by the implementation for git, with certain parts
>>> copied almost verbatim.
>>>
>>> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>>> ---
>>> index efb515fca5..bb5cc87969 100755
>>> --- a/support/download/hg
>>> +++ b/support/download/hg
>>> @@ -1,7 +1,7 @@
>>>  #!/usr/bin/env bash
>>>  
>>>  # We want to catch any unexpected failure, and exit immediately
>>> -set -e
>>> +set -E
>>
>>  If you do this, the comment above is no longer valid...
>>       -e  Exit immediately if a command exits with a non-zero status.
>>       -E  If set, the ERR trap is inherited by shell functions.
>> are not the same thing!
>>
>>  However, since AFAICS the only function in this file is the trap handler
>> itself, this seems to be a mistake, no?
>>
>>  Ow, this really is inherited from git, introduced by commit b7efb43e86da96.
>> Yann, care to explain?
> 
> Yes, I care to explain. Even though I do write some crap most of the
> time,

 I never said that!

> there are in fact two functions in the git backend.

 Oh yes, I missed the _git/_hg function.

> 
> Like all backends, the actual tool is wrapped into a _git() function,
> which was introduced more than three years ago, by commit 3f2bdd070
> (support/download: protect from custom commands with spaces in args), as
> a way to actually fix a bug actually reported by Thomas DS.
> 
> Yes, this makes it that the set -E is needed, after all.

 But then the comment is wrong, no? Because -E does not imply -e and without -e
there is no immediate exit. Or am I missing something else?


 Regards,
 Arnout

[snip]
Yann E. MORIN Feb. 8, 2019, 7:54 p.m. | #4
Arnout, All,

On 2019-02-08 20:27 +0100, Arnout Vandecappelle spake thusly:
> On 08/02/2019 17:54, Yann E. MORIN wrote:
> > On 2019-02-07 22:33 +0100, Arnout Vandecappelle spake thusly:
> >> On 05/02/2019 21:24, Thomas De Schampheleire wrote:
> >>> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >>>
> >>> Similar to the git download helper, implement a repository cache for
> >>> Mercurial repositories.
> >>>
> >>> The code is mostly guided by the implementation for git, with certain parts
> >>> copied almost verbatim.
> >>>
> >>> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
> >>> ---
> >>> index efb515fca5..bb5cc87969 100755
> >>> --- a/support/download/hg
> >>> +++ b/support/download/hg
> >>> @@ -1,7 +1,7 @@
> >>>  #!/usr/bin/env bash
> >>>  
> >>>  # We want to catch any unexpected failure, and exit immediately
> >>> -set -e
> >>> +set -E
> >>  Ow, this really is inherited from git, introduced by commit b7efb43e86da96.
> >> Yann, care to explain?
> > 
> > Yes, I care to explain. Even though I do write some crap most of the
> > time,
>  I never said that!

I said it! :-)

[--SNIP--]
> > Like all backends, the actual tool is wrapped into a _git() function,
> > which was introduced more than three years ago, by commit 3f2bdd070
> > (support/download: protect from custom commands with spaces in args), as
> > a way to actually fix a bug actually reported by Thomas DS.
> > 
> > Yes, this makes it that the set -E is needed, after all.
> 
>  But then the comment is wrong, no? Because -E does not imply -e and without -e
> there is no immediate exit. Or am I missing something else?

Yes, it is partly incorrect. But consider that, with the ERR trap, we
_do_ catch unexpected failures, but we no longer _immediately_ exit
anymore, indeed. We just postpone the exit to after the second
tentative, in which case we do exit immediately from the ERR trap.

However, I was thinking that maybe we could revisit this try-again code,
and just exit on the first error and rely on the user to actually clean
the git tree if it ever gets corrupted.

We initially added that, in case the repository was uterly broken,
because our backend itself  was creating situations where it _could_
leave the repo in an inconsistent state, or the autobuilder code would
do that (e.g. timeout-kill-9 the build right in the middle of a git
action).

However, the git backend was fixed to supposedly always leave the tree
in a consistent state now, and the autobuilder code now timeouts from
the last step, not from the build start, so we can assume that not git
action would now be interrupted abruptely anymore...

Yet, I am a bit contrived, as the git backend is partly my fault, and so
I'm not entirely happy with it...

Regards,
Yann E. MORIN.
Arnout Vandecappelle (Essensium/Mind) Feb. 8, 2019, 8:54 p.m. | #5
On 08/02/2019 20:54, Yann E. MORIN wrote:
> Arnout, All,
> 
> On 2019-02-08 20:27 +0100, Arnout Vandecappelle spake thusly:
>> On 08/02/2019 17:54, Yann E. MORIN wrote:
>>> On 2019-02-07 22:33 +0100, Arnout Vandecappelle spake thusly:
>>>> On 05/02/2019 21:24, Thomas De Schampheleire wrote:
>>>>> From: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>>>>>
>>>>> Similar to the git download helper, implement a repository cache for
>>>>> Mercurial repositories.
>>>>>
>>>>> The code is mostly guided by the implementation for git, with certain parts
>>>>> copied almost verbatim.
>>>>>
>>>>> Signed-off-by: Thomas De Schampheleire <thomas.de_schampheleire@nokia.com>
>>>>> ---
>>>>> index efb515fca5..bb5cc87969 100755
>>>>> --- a/support/download/hg
>>>>> +++ b/support/download/hg
>>>>> @@ -1,7 +1,7 @@
>>>>>  #!/usr/bin/env bash
>>>>>  
>>>>>  # We want to catch any unexpected failure, and exit immediately
>>>>> -set -e
>>>>> +set -E
>>>>  Ow, this really is inherited from git, introduced by commit b7efb43e86da96.
>>>> Yann, care to explain?
>>>
>>> Yes, I care to explain. Even though I do write some crap most of the
>>> time,
>>  I never said that!
> 
> I said it! :-)
> 
> [--SNIP--]
>>> Like all backends, the actual tool is wrapped into a _git() function,
>>> which was introduced more than three years ago, by commit 3f2bdd070
>>> (support/download: protect from custom commands with spaces in args), as
>>> a way to actually fix a bug actually reported by Thomas DS.
>>>
>>> Yes, this makes it that the set -E is needed, after all.
>>
>>  But then the comment is wrong, no? Because -E does not imply -e and without -e
>> there is no immediate exit. Or am I missing something else?
> 
> Yes, it is partly incorrect. But consider that, with the ERR trap, we
> _do_ catch unexpected failures, 

 It seems I'm unable to interpret documentation correctly, because I read the
trap help text as stating that trap ERR only works if -e is set...

                                                                A SIGNAL_SPEC
    of ERR means to execute ARG each time a command's failure would cause the
    shell to exit when the -e option is enabled.


> but we no longer _immediately_ exit
> anymore, indeed. We just postpone the exit to after the second
> tentative, in which case we do exit immediately from the ERR trap.
> 
> However, I was thinking that maybe we could revisit this try-again code,
> and just exit on the first error and rely on the user to actually clean
> the git tree if it ever gets corrupted.
> 
> We initially added that, in case the repository was uterly broken,
> because our backend itself  was creating situations where it _could_
> leave the repo in an inconsistent state, or the autobuilder code would
> do that (e.g. timeout-kill-9 the build right in the middle of a git
> action).

 Kill -9 of git should never leave the repo in an inconsistent state anyway. The
only reasonable way to get an inconsistent repo state is randomly deleting files
from it (and disk failure, of course). But doesn't the autobuilder do that,
randomly delete files?


 Regards,
 Arnout

> 
> However, the git backend was fixed to supposedly always leave the tree
> in a consistent state now, and the autobuilder code now timeouts from
> the last step, not from the build start, so we can assume that not git
> action would now be interrupted abruptely anymore...
> 
> Yet, I am a bit contrived, as the git backend is partly my fault, and so
> I'm not entirely happy with it...
> 
> Regards,
> Yann E. MORIN.
>
Yann E. MORIN Feb. 8, 2019, 9:18 p.m. | #6
Arnout, All,

On 2019-02-08 21:54 +0100, Arnout Vandecappelle spake thusly:
> On 08/02/2019 20:54, Yann E. MORIN wrote:
[--SNIP--]
> > Yes, it is partly incorrect. But consider that, with the ERR trap, we
> > _do_ catch unexpected failures, 
> 
>  It seems I'm unable to interpret documentation correctly, because I read the
> trap help text as stating that trap ERR only works if -e is set...
> 
>                                                                 A SIGNAL_SPEC
>     of ERR means to execute ARG each time a command's failure would cause the
>     shell to exit when the -e option is enabled.

Yes, the sentence is ambiguous. But reading it carefully, they wrote
"would cause", i.e.:  "do A if B would cause C" does not require "B".

And indeed:

    $ cat ess.sh
    #!/bin/bash
    set -E
    foo() { echo Trying...; false; echo Bummer; }
    foo
    trap '{ echo ERR; exit 42; }' ERR
    foo

    $ ./ess.sh
    Trying...
    Bummer
    Trying...
    ERR

    $ echo $?
    42

> > but we no longer _immediately_ exit
> > anymore, indeed. We just postpone the exit to after the second
> > tentative, in which case we do exit immediately from the ERR trap.
> > 
> > However, I was thinking that maybe we could revisit this try-again code,
> > and just exit on the first error and rely on the user to actually clean
> > the git tree if it ever gets corrupted.
> > 
> > We initially added that, in case the repository was uterly broken,
> > because our backend itself  was creating situations where it _could_
> > leave the repo in an inconsistent state, or the autobuilder code would
> > do that (e.g. timeout-kill-9 the build right in the middle of a git
> > action).
> 
>  Kill -9 of git should never leave the repo in an inconsistent state anyway. The

Well, not sure what caused it, in the end, but we did have random git
failures, and not ncessarily caused by the autobuilders removing files
in .git/ (but that was quite some tiem ago now, so I don't recall the
details...)

> only reasonable way to get an inconsistent repo state is randomly deleting files
> from it (and disk failure, of course). But doesn't the autobuilder do that,
> randomly delete files?

It used to, but no longer does, descend into sub-directopries, so could
remove files in BR2_DL_DIR/pkg/git/.git/

    https://git.buildroot.org/buildroot-test/tree/scripts/autobuild-run#n291

    https://git.buildroot.org/buildroot-test/commit/scripts/autobuild-run?id=cb1c4829b1d1ab660736811101fb6d988a8d14e7

Regards,
Yann E. MORIN.

Patch

diff --git a/support/download/hg b/support/download/hg
index efb515fca5..bb5cc87969 100755
--- a/support/download/hg
+++ b/support/download/hg
@@ -1,7 +1,7 @@ 
 #!/usr/bin/env bash
 
 # We want to catch any unexpected failure, and exit immediately
-set -e
+set -E
 
 # Download helper for hg, to be called from the download wrapper script
 #
@@ -10,11 +10,39 @@  set -e
 #   -o FILE     Generate archive in FILE.
 #   -u URI      Clone from repository at URI.
 #   -c CSET     Use changeset (or revision) CSET.
+#   -d DLDIR    Download directory path.
 #   -n NAME     Use basename NAME.
 #
 # Environment:
 #   HG       : the hg command to call
 
+# Save our path and options in case we need to call ourselves again
+myname="${0}"
+declare -a OPTS=("${@}")
+
+# This function is called when an error occurs. Its job is to attempt a clone
+# from scratch (only once!) in case the hg tree is borked, or in case an
+# unexpected and unsupported situation arises with uncommitted stuff (e.g. if
+# the user manually mucked around in the hg cache).
+# Note that this function would also be triggered by connection errors during
+# the clone/pull.
+_on_error() {
+    local ret=${?}
+
+    printf "Detected a possibly corrupted hg cache.\n" >&2
+    if ${BR_HG_BACKEND_FIRST_FAULT:-false}; then
+        printf "This is the second time in a row; bailing out\n" >&2
+        exit ${ret}
+    fi
+    export BR_HG_BACKEND_FIRST_FAULT=true
+
+    printf "Removing it and starting afresh.\n" >&2
+
+    rm -rf "${hg_cache}"
+
+    exec "${myname}" "${OPTS[@]}" || exit ${ret}
+}
+
 verbose=
 while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     case "${OPT}" in
@@ -22,6 +50,7 @@  while getopts "${BR_BACKEND_DL_GETOPTS}" OPT; do
     o)  output="${OPTARG}";;
     u)  uri="${OPTARG}";;
     c)  cset="${OPTARG}";;
+    d)  dl_dir="${OPTARG}";;
     n)  basename="${OPTARG}";;
     :)  printf "option '%s' expects a mandatory argument\n" "${OPTARG}"; exit 1;;
     \?) printf "unknown option '%s'\n" "${OPTARG}" >&2; exit 1;;
@@ -36,8 +65,54 @@  _hg() {
     eval ${HG} "${@}"
 }
 
-_hg clone ${verbose} "${@}" --noupdate "'${uri}'" "'${basename}'"
+# Create and cd into the directory that will contain the local hg cache
+hg_cache="${dl_dir}/hg"
+mkdir -p "${hg_cache}"
+
+# Any error now should try to recover
+trap _on_error ERR
+
+# Create a warning file, that the user should not use the hg cache.
+# It's ours. Our precious.
+cat <<-_EOF_ >"${dl_dir}/hg.readme"
+	IMPORTANT NOTE!
+
+	The hg tree located in this directory is for the exclusive use
+	by Buildroot, which uses it as a local cache to reduce bandwidth
+	usage.
+
+	Buildroot *will* trash any changes in that tree whenever it needs
+	to use it. Buildroot may even remove it in case it detects the
+	repository may have been damaged or corrupted.
+
+	Do *not* work in that directory; your changes will eventually get
+	lost. Do *not* even use it as a remote, or as the source for new
+	worktrees; your commits will eventually get lost.
+_EOF_
+
+if ! [ -d "${hg_cache}/.hg" ]; then
+    # While it would be possible to create an empty repo and use pull
+    # subsequently, we intentionally use clone the first time as it could be
+    # faster than pull thanks to clonebundles, if enabled on the server.
+    printf "Cloning repository from '${uri}' into '${hg_cache}'\n"
+    _hg clone ${verbose} "${@}" --noupdate "'${uri}'" "'${hg_cache}'"
+else
+    printf "Pulling repository from '${uri}' into '${hg_cache}'\n"
+    _hg pull ${verbose} "${@}" --repository "'${hg_cache}'" "'${uri}'"
+fi
+
+# Check that the changeset does exist. If it does not, re-cloning from
+# scratch won't help, so we don't want to trash the repository for a
+# missing commit. We just exit without going through the ERR trap.
+if ! _hg identify --repository "'${hg_cache}'" --rev "'${cset}'" >/dev/null 2>&1; then
+    printf "Commit '%s' does not exist in this repository\n." "${cset}"
+    exit 1
+fi
+
+# Make sure that there is no working directory. This will clear any user
+# temptation to work in this directory.
+_hg update --repository "'${hg_cache}'" null >/dev/null 2>&1
 
-_hg archive ${verbose} --repository "'${basename}'" --type tgz \
+_hg archive ${verbose} --repository "'${hg_cache}'" --type tgz \
             --prefix "'${basename}'" --rev "'${cset}'" \
             - >"${output}"