Submitter | via Mercurial-devel |
---|---|
Date | Oct. 9, 2018, 9:29 p.m. |
Message ID | <186a840796aae2aa48ca.1539120565@cuben.lan> |
Download | mbox | patch |
Permalink | /patch/35591/ |
State | New |
Headers | show |
Comments
On Tue, 09 Oct 2018 23:29:25 +0200, Antonio Muci via Mercurial-devel wrote: > # HG changeset patch > # User muxator <a.mux@inwind.it> > # Date 1539116185 -7200 > # Tue Oct 09 22:16:25 2018 +0200 > # Node ID 186a840796aae2aa48ca5228fc7225779e3e0ee9 > # Parent f774dfa1595d4935879d2abbb8b4c802d8af0c65 > packaging: blindly factor out trap's cleanup function in builddeb > > This commit blindly extracts builddeb's trap routine in a dedicated function. > While doing so, I think two bugs are exposed, which will be addressed in the > next commits: > > - single quoting around '$CLEANUP' will always evaluate to the literal > '$CLEANUP' regardless of the variable's value. The "if" will always be true. > - the removal operation will not expand $PWD (and a variable expansion would > need double quotes, anyways. > > diff --git a/contrib/packaging/builddeb b/contrib/packaging/builddeb > --- a/contrib/packaging/builddeb > +++ b/contrib/packaging/builddeb > @@ -13,6 +13,13 @@ CLEANUP=1 > DISTID=`(lsb_release -is 2> /dev/null | tr '[:upper:]' '[:lower:]') || echo debian` > CODENAME=`lsb_release -cs 2> /dev/null || echo unknown` > DEBFLAGS=-b > + > +cleanup() { > + if [ '$CLEANUP' ]; then > + rm -r '$PWD/debian'; > + fi > +} > + > while [ "$1" ]; do > case "$1" in > --distid ) > @@ -44,7 +51,7 @@ while [ "$1" ]; do > esac > done > > -trap "if [ '$CLEANUP' ] ; then rm -r '$PWD/debian' ; fi" EXIT Just for the record, $CLEANUP and $PWD would be expanded here, while setting up the trap. So, yeah, PWD="'" would make it crash.
Patch
diff --git a/contrib/packaging/builddeb b/contrib/packaging/builddeb --- a/contrib/packaging/builddeb +++ b/contrib/packaging/builddeb @@ -13,6 +13,13 @@ CLEANUP=1 DISTID=`(lsb_release -is 2> /dev/null | tr '[:upper:]' '[:lower:]') || echo debian` CODENAME=`lsb_release -cs 2> /dev/null || echo unknown` DEBFLAGS=-b + +cleanup() { + if [ '$CLEANUP' ]; then + rm -r '$PWD/debian'; + fi +} + while [ "$1" ]; do case "$1" in --distid ) @@ -44,7 +51,7 @@ while [ "$1" ]; do esac done -trap "if [ '$CLEANUP' ] ; then rm -r '$PWD/debian' ; fi" EXIT +trap 'cleanup' EXIT set -u