Yasuhito FUTATSUKI wrote on Mon, Mar 01, 2021 at 10:55:51 +0900:
> On 2021/02/26 22:52, Daniel Shahaf wrote:
> > Yasuhito FUTATSUKI wrote on Thu, Feb 25, 2021 at 21:43:15 +0900:
> >> On 2021/02/22 13:39, Karl Fogel wrote:
> >>> I felt I should close the loop in this thread:
> >>>
> >>> I decided to leave this be for now. Even though it is mildly annoying
> >>> that the Subversion test suite exhibits a few spurious failures on
> >>> systems that have only 'python3' and not 'python', it turns out that most
> >>> of the time most of my systems have 'python'. So the real-world effects
> >>> here are pretty minor for me, and apparently for others, and there are
> >>> more important things I could work on in Subversion. If anyone wants to
> >>> fix this minor problem, there are enough pointers in this thread I think.
> >>
> >> Then, I made patches. It also allows pypy to run the test without
> >> 'python'.
> >>
> >> (A) Replace the shebang line of svneditor.py from svneditor.py
> >> at running the configure script. (fix-svneditor-in-test-patch-a.txt)
> >> As @PYTHON@ can be basename only, relative path, or absolute path,
> >> "/usr/bin/env" is still needed.
> >>
> >> (B) Make a script to launch svneditor.py using @PYTHON@, svneditor.sh
> >> at running the configure scriptt. (fix-svneditor-in-test-patch-b.txt)
> >>
> >> If @PYTHON@ is always absolute path, we can use shebang line without
> >> '/usr/bin/env' in patch (A).
> >>
> >> Cheers,
> >
> > I take it the two patches are alternatives? If so, I prefer (B), because:
> >
> > - .in files get in the way of the edit-compile-fix cycle, and the file
> > that (B) makes .in is less likely to need to be edited.
> >
> > - Using @PYTHON@ in a #!/bin/sh script is closer to its existing uses
> > (in Makefile.in, for example) than using it on the #! line.
>
> Thank you for the comment. It is reasonable.
>
> So I looked at patch (B) more closely and updated, so that the substituted
> string is not interpreted specially by the shell. As this uses function,
> it does not work with v7 sh but POSIX shells allows to use it.
Thanks for the revision. I'm happy to leave it to you to choose between
the two versions of (B). I'll just share two (more) observations:
> +++ subversion/tests/cmdline/svneditor.sh.in (working copy)
> @@ -0,0 +1,40 @@
> +print_python_path()
> +{
> +cat <<"_SVN_EOF"
> +@PYTHON@x
> +_SVN_EOF
> +}
I don't see any value of @PYTHON@ that would benefit from being
carefully unescaped like this, since Makefile.in already assumes that
@PYTHON@ can be used either bare or in double quotes:
% grep -n @PYTHON@ Makefile.in
398:PYTHON = @PYTHON@
% fgrep -n '$(PYTHON)' Makefile.in
551: @if test "$(PYTHON)" != "none"; then \
629: $(PYTHON) $(top_srcdir)/build/run_tests.py \
657: @env PYTHON=$(PYTHON) THREADED=$(THREADED) MAKE=$(MAKE) \
⋮
%
> +python="$(print_python_path)"
> +python="${python%x}"
> +
> +script_dir="`cd $(dirname "$0"); pwd`"
> +
If «$0» needs quoting, then quotes are needed around the «$(…)»
construct as well.
Personally, I'd use two sets of «--» guards as well (one for «cd» and
one for «dirname»), even if there's no (current) case where that
matters. Readability and cribbing compatibility are things.
Cheers,
Daniel
> +exec "$python" "$script_dir"/svneditor.py "$@"
