Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes to correct missing pathname quoting in various tools. #7

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

sshambar
Copy link

  • build-aux/bootstrap.in: Quote paths in source includes.
  • build-aux/extract-trace: Quote paths in source includes, correct
    time-stamp-pattern.
  • build-aux/extract-trace (func_find_tool): Quote paths.
  • build-aux/funclib.sh (func_path_progs): Quote paths.
  • build-aux/inline-source: Quote paths in source includes, correct
    time-stamp-pattern.
  • build-aux/inline-source (func_include): quote paths used in awk
    script, and add support for "correctly-quoted" include paths
    (. "echo ..path") to awk script.
  • tests/test-option-parser.sh: Update helper usage to support correct
    path quoting.
  • bootstrap: Regenerate.

Signed-off-by: Scott Shambarger [email protected]

* build-aux/bootstrap.in: Quote paths in source includes.
* build-aux/extract-trace: Quote paths in source includes, correct
time-stamp-pattern.
* build-aux/extract-trace (func_find_tool): Quote paths.
* build-aux/funclib.sh (func_path_progs): Quote paths.
* build-aux/inline-source: Quote paths in source includes, correct
time-stamp-pattern.
* build-aux/inline-source (func_include): quote paths used in awk
script, and add support for "correctly-quoted" include paths
(. "`echo ..`path") to awk script.
* tests/test-option-parser.sh: Update helper usage to support correct
path quoting.
* bootstrap: Regenerate.

Signed-off-by: Scott Shambarger <[email protected]>
@@ -32,7 +32,7 @@ check_output_inner ()
func_quote pretty ${1+"$@"}
$ECHO "[[ output check ]] args: $func_quote_result"

output=`$helper ${1+"$@"} 2>/dev/null`
output=`$__GL_ALL_SHELLS_SHELL "$helper" ${1+"$@"} 2>/dev/null`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your work!

Maybe it is detail, but could we have rather $helper properly quoted and later eval()ed to not copy the ugly $__GL_ALL_SHELLS_SHELL on multiple places?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On 2016-08-17 14:38, Pavel Raiskup wrote:

Thanks for your work!

Maybe it is detail, but could we have rather $helper properly quoted and later eval()ed to not copy the ugly $__GL_ALL_SHELLS_SHELL on multiple places?

Sure, I actually did that initially, but then wondered if it might
affect the quote nesting. I'll go back and test it with eval and append
the patch.

Thanks,

Scott

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pavel,

As I suspected, the eval solution breaks several tests as it swallows
empty parameters passed via ${1+"@"}. Here's the closest I could come
up with:

func_quote_arg eval "$abs_srcdir"
_G_qabs_srcdir=$func_quote_arg_result
helper="$__GL_ALL_SHELLS_SHELL
$_G_qabs_srcdir/test-option-parser-helper"

output=eval $helper ${1+"$@"} 2>/dev/null

Tests similar to <-t ''> broke with this solution... if you see another
method to handle the word splitting in the shell argument, I'd willing
to it (I don't think we want to redefine IFS here...)

Thanks,

Scott

On 2016-08-17 14:38, Pavel Raiskup wrote:

In tests/test-option-parser.sh [1]:

@@ -32,7 +32,7 @@ check_output_inner ()
func_quote pretty ${1+"$@"}
$ECHO "[[ output check ]] args: $func_quote_result"

  • output=$helper ${1+"$@"} 2>/dev/null
  • output=$__GL_ALL_SHELLS_SHELL "$helper" ${1+"$@"} 2>/dev/null

Thanks for your work!

Maybe it is detail, but could we have rather $helper properly quoted and later eval()ed to not copy the ugly $__GL_ALL_SHELLS_SHELL on multiple places?

You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub [2], or mute the thread [3].

Links:

[1]
#7 (comment)
[2]
https://github.com/gnulib-modules/bootstrap/pull/7/files/20a6d66266904c00c29f943b71c4f8424db1b4d5#r75113724
[3]
https://github.com/notifications/unsubscribe-auth/AAnGuNF8tF3x--QsugNT8rGQcLxcp_JJks5qgwCygaJpZM4JlzUb

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, forgot to finish. Other solution is to define a quick shell var,
eg:

hshell=$__GL_ALL_SHELLS_SHELL

output=$hshell "$helper" ${1+"$@"} 2>/dev/null

... which is pretty clean looking :)

Scott

On 2016-08-17 14:38, Pavel Raiskup wrote:

In tests/test-option-parser.sh [1]:

@@ -32,7 +32,7 @@ check_output_inner ()
func_quote pretty ${1+"$@"}
$ECHO "[[ output check ]] args: $func_quote_result"

  • output=$helper ${1+"$@"} 2>/dev/null
  • output=$__GL_ALL_SHELLS_SHELL "$helper" ${1+"$@"} 2>/dev/null

Thanks for your work!

Maybe it is detail, but could we have rather $helper properly quoted and later eval()ed to not copy the ugly $__GL_ALL_SHELLS_SHELL on multiple places?

You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub [2], or mute the thread [3].

Links:

[1]
#7 (comment)
[2]
https://github.com/gnulib-modules/bootstrap/pull/7/files/20a6d66266904c00c29f943b71c4f8424db1b4d5#r75113724
[3]
https://github.com/notifications/unsubscribe-auth/AAnGuNF8tF3x--QsugNT8rGQcLxcp_JJks5qgwCygaJpZM4JlzUb

@@ -2355,11 +2355,11 @@ func_version ()
# <https://github.com/gnulib-modules/bootstrap/issues>

# Make sure we've evaluated scripts we depend on.
test -z "$progpath" && . `echo "$0" |${SED-sed} 's|[^/]*$||'`/funclib.sh
test extract-trace = "$progname" && . `echo "$0" |${SED-sed} 's|[^/]*$||'`/options-parser
test -z "$progpath" && . "`echo "$0" |${SED-sed} 's|[^/]*$||'`funclib.sh"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test -z "$progpath" && . `echo "$0" |${SED-sed} 's|[^/]*$||'`/funclib.sh
test extract-trace = "$progname" && . `echo "$0" |${SED-sed} 's|[^/]*$||'`/options-parser
test -z "$progpath" && . "`echo "$0" |${SED-sed} 's|[^/]*$||'`funclib.sh"
test extract-trace = "$progname" && . "`echo "$0" |${SED-sed} 's|[^/]*$||'`options-parser"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar " ... " issue.

. `echo "$0" |${SED-sed} 's|[^/]*$||'`"extract-trace"
. "`echo "$0" |${SED-sed} 's|[^/]*$||'`funclib.sh"
. "`echo "$0" |${SED-sed} 's|[^/]*$||'`options-parser"
. "`echo "$0" |${SED-sed} 's|[^/]*$||'`extract-trace"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly. Workaround would be separate variables?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants