You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2010/12/07 09:23:37 UTC

Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

CC += dev@

Daniel Näslund wrote on Mon, Dec 06, 2010 at 21:32:39 +0100:
> On Mon, Dec 06, 2010 at 01:44:23PM -0600, David Dyer-Bennet wrote:
> > Subversion 1.6.12 running on Centos 5.5
> > 
> > If the value of the environment variable VISUAL contains a space,
> > subversion fails when attempting to invoke the editor to get the
> > comment.
> > 
> > sh-3.2$ export VISUAL="/home/spaces in name/bin/emacs"
> > sh-3.2$ svn commit
> > sh: /home/spaces: No such file or directory
> > svn: Commit failed (details follow):
> > svn: system('/home/spaces in name/bin/emacs svn-commit.tmp') returned 32512
> > 
> > As you see in the error, it's constructing a command string without
> > consideration of the possibility of spaces in various places.
> 
> [...]
> 
> > So, can somebody confirm this please?  And, ideally, submit the bug
> > (since I keep getting stuck trying to go through all the hoops they
> > want you to go through to become enabled to do that)?
> 
> Found this thread [1] that discusses spaces in the editor cmd string. Julian
> Foad confirms that it's a bug in a follow-up and even has some
> suggestions on how to tackle the problem if someone wants to write a
> patch.

Today we use system(), and thus things like EDITOR='gvim -f' and
EDITOR='emacs -nw' work.  If propedit-cmd.c just quoted all spaces in
the editor command, it would break this use case.

I suppose setting VISUAL="\"/path with spaces/to/editor/binary\"" is the
easiest solution --- it requires no code changes so it will work with
any svn binary out there.

> I haven't found any issues related to the problem in the
> tracker but I'm really loosy when it comes to using issue trackers.
> 
> Daniel
> 
> [1] http://svn.haxx.se/dev/archive-2010-02/0051.shtml
> 

Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by David Dyer-Bennet <dd...@dd-b.net>.
On Tue, December 7, 2010 09:58, Daniel Shahaf wrote:
> David Dyer-Bennet wrote on Tue, Dec 07, 2010 at 09:30:45 -0600:
>> On Tue, December 7, 2010 09:03, Daniel Shahaf wrote:
>> > David Dyer-Bennet wrote on Tue, Dec 07, 2010 at 08:44:28 -0600:
>> >> And, in any case, VISUAL is a public interface, and I wonder how many
>> >> other applications would break if I put that kind of thing into
>> >> VISUAL.
>> >
>> > Morale: don't use paths with spaces.
>>
>> Yes, that would be simpler.  But as I said in my initial post, I first
>> discovered this on Windows under Cygwin.  Avoiding "c:/Program Files"
>> and "c:/Documents and Settings/david.dyer-bennet/My Documents"
>> involves contortions and leaves you putting things in unusual places.
>>
>
> Just create a symlink in cygwin?

I keep forgetting Cygwin will do that.  Using shortnames has also been
suggested (which are ugly, but hey, this is all inside scripts, so I
shouldn't care).

>> Linux defines spaces as valid characters in paths and filenames, so
>> people are going to try to use them.  And Subversion does support
>> Windows.
>>
>
> Newline, tab, and colon are also valid in on linux, and I'm sure some
> parts of Subversion will not work properly if you use them.  (Do we
> URI-encode paths in svn:mergeinfo and 'svn diff'?)

And those are bugs too :-).

>> (Spelling nit, no flame intended:  "moral"; not "morale".  They're
>> both real words with quite different meanings, so it's of value to
>> keep them distinct.  And now I have no doubt guaranteed 3 basic
>> language mistakes of my own somewhere in this message :-) .)
>
> Thanks for the correction; appreciated.
>
> (Shouldn't you have spelled out 'three'?  Just yesterday I found a bug
> related to using 'two' instead of '2'.)

Yes.  Well, that's at the level of "style" and so is more arguable, but
nearly every authority calls for spelling out numbers up to about six when
used in sentences.   Good catch!

(Pruned back to users list and eliminated most of the direct CCs.)
-- 
David Dyer-Bennet, dd-b@dd-b.net; http://dd-b.net/
Snapshots: http://dd-b.net/dd-b/SnapshotAlbum/data/
Photos: http://dd-b.net/photography/gallery/
Dragaera: http://dragaera.info

Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
David Dyer-Bennet wrote on Tue, Dec 07, 2010 at 09:30:45 -0600:
> On Tue, December 7, 2010 09:03, Daniel Shahaf wrote:
> > David Dyer-Bennet wrote on Tue, Dec 07, 2010 at 08:44:28 -0600:
> >> And, in any case, VISUAL is a public interface, and I wonder how many
> >> other applications would break if I put that kind of thing into
> >> VISUAL.
> >
> > Morale: don't use paths with spaces.
> 
> Yes, that would be simpler.  But as I said in my initial post, I first
> discovered this on Windows under Cygwin.  Avoiding "c:/Program Files"
> and "c:/Documents and Settings/david.dyer-bennet/My Documents"
> involves contortions and leaves you putting things in unusual places.
> 

Just create a symlink in cygwin?

> Linux defines spaces as valid characters in paths and filenames, so
> people are going to try to use them.  And Subversion does support
> Windows.
> 

Newline, tab, and colon are also valid in on linux, and I'm sure some
parts of Subversion will not work properly if you use them.  (Do we
URI-encode paths in svn:mergeinfo and 'svn diff'?)

> (Spelling nit, no flame intended:  "moral"; not "morale".  They're
> both real words with quite different meanings, so it's of value to
> keep them distinct.  And now I have no doubt guaranteed 3 basic
> language mistakes of my own somewhere in this message :-) .)

Thanks for the correction; appreciated.

(Shouldn't you have spelled out 'three'?  Just yesterday I found a bug
related to using 'two' instead of '2'.)

Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by Peter Samuelson <pe...@p12n.org>.
[David Dyer-Bennet]
> as I said in my initial post, I first discovered this on Windows
> under Cygwin.  Avoiding "c:/Program Files" and "c:/Documents and
> Settings/david.dyer-bennet/My Documents" involves contortions and
> leaves you putting things in unusual places.

Well, or calling them C:/PROGRA~1 and C:/DOCUME~1.  Not necessarily
wise, since these days I hear it's possible to disable short name
generation in NTFS, but there you have it.
-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/

Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
David Dyer-Bennet wrote on Tue, Dec 07, 2010 at 09:30:45 -0600:
> On Tue, December 7, 2010 09:03, Daniel Shahaf wrote:
> > David Dyer-Bennet wrote on Tue, Dec 07, 2010 at 08:44:28 -0600:
> >> And, in any case, VISUAL is a public interface, and I wonder how many
> >> other applications would break if I put that kind of thing into
> >> VISUAL.
> >
> > Morale: don't use paths with spaces.
> 
> Yes, that would be simpler.  But as I said in my initial post, I first
> discovered this on Windows under Cygwin.  Avoiding "c:/Program Files"
> and "c:/Documents and Settings/david.dyer-bennet/My Documents"
> involves contortions and leaves you putting things in unusual places.
> 

Just create a symlink in cygwin?

> Linux defines spaces as valid characters in paths and filenames, so
> people are going to try to use them.  And Subversion does support
> Windows.
> 

Newline, tab, and colon are also valid in on linux, and I'm sure some
parts of Subversion will not work properly if you use them.  (Do we
URI-encode paths in svn:mergeinfo and 'svn diff'?)

> (Spelling nit, no flame intended:  "moral"; not "morale".  They're
> both real words with quite different meanings, so it's of value to
> keep them distinct.  And now I have no doubt guaranteed 3 basic
> language mistakes of my own somewhere in this message :-) .)

Thanks for the correction; appreciated.

(Shouldn't you have spelled out 'three'?  Just yesterday I found a bug
related to using 'two' instead of '2'.)

Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by Peter Samuelson <pe...@p12n.org>.
[David Dyer-Bennet]
> as I said in my initial post, I first discovered this on Windows
> under Cygwin.  Avoiding "c:/Program Files" and "c:/Documents and
> Settings/david.dyer-bennet/My Documents" involves contortions and
> leaves you putting things in unusual places.

Well, or calling them C:/PROGRA~1 and C:/DOCUME~1.  Not necessarily
wise, since these days I hear it's possible to disable short name
generation in NTFS, but there you have it.
-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/

Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by David Dyer-Bennet <dd...@dd-b.net>.
On Tue, December 7, 2010 09:03, Daniel Shahaf wrote:
> David Dyer-Bennet wrote on Tue, Dec 07, 2010 at 08:44:28 -0600:
>> On Tue, December 7, 2010 04:18, Julian Foad wrote:
>> > On Tue, 2010-12-07, Daniel Shahaf wrote:
>> >> I suppose setting VISUAL="\"/path with spaces/to/editor/binary\"" is
>> the
>> >> easiest solution --- it requires no code changes so it will work with
>> >> any svn binary out there.
>> >
>> > Yes, I think that's the best solution.
>>
>> That's an interesting idea.
>>
>> Unfortunately, it doesn't seem to work.  (Example from Centos 5.5
>> again):
>>
>
> The following syntaxes work for me:
>
> % SVN_EDITOR="'$HOME/pre fix/bin/vim' +%d" $svn mkdir
> file:///tmp/svn/r1/foo3
> % $svn mkdir file:///tmp/svn/r1/foo3 --editor-cmd "'$HOME/pre fix/bin/vim'
> +%d"

Having an application-specific env variable also makes this kind of
workaround more palatable -- setting SVN_EDITOR strangely shouldn't
break anything else, and if it does, it's the fault of the something
else.

> (the +%d is there both to prove to myself that it's not simply falling
> back to my default editor --- which is also vim --- and to show that
> passing argv[1] to the editor works)
>
>> sh-3.2$ export VISUAL='"/home/path with spaces/bin/emacs"'
>> sh-3.2$ echo $VISUAL
>> "/home/path with spaces/bin/emacs"
>> sh-3.2$ ls -l "/home/path with spaces/bin"
>> total 8
>> -rwxr-xr-x 1 root root 192 Dec  7 08:40 emacs
>> sh-3.2$ svn commit
>> basename: extra operand `spaces/bin/emacs'
>
> Why is basename(1) invoked here?
>                ^^^
>
> (That's almost certainly something in your configuration.)

I checked; turns out that /usr/bin/emacs is a script that invokes
basename (in the standard Centos 5.5 emacs install).  I copied
/usr/bin/emacs to /home/path with spaces/bin/emacs for this test.

It looks like the workaround of quoting the path with spaces in
SVN_EDITOR works, and I'm now encountering a bug in emacs (or perhaps
the Centos packaging of Emacs).  (The emacs script is easily fixed to
tolerate spaces in its path, just by doing the basic obvious things
you do when shell scripting.)

>> Try `basename --help' for more information.
>> Can't find
>> svn: Commit failed (details follow):
>> svn: system('"/home/path with spaces/bin/emacs" svn-commit.tmp')
>> returned 256
>
>
>> And, in any case, VISUAL is a public interface, and I wonder how many
>> other applications would break if I put that kind of thing into
>> VISUAL.
>
> Morale: don't use paths with spaces.

Yes, that would be simpler.  But as I said in my initial post, I first
discovered this on Windows under Cygwin.  Avoiding "c:/Program Files"
and "c:/Documents and Settings/david.dyer-bennet/My Documents"
involves contortions and leaves you putting things in unusual places.

Linux defines spaces as valid characters in paths and filenames, so
people are going to try to use them.  And Subversion does support
Windows.

(Spelling nit, no flame intended:  "moral"; not "morale".  They're
both real words with quite different meanings, so it's of value to
keep them distinct.  And now I have no doubt guaranteed 3 basic
language mistakes of my own somewhere in this message :-) .)
-- 
David Dyer-Bennet, dd-b@dd-b.net; http://dd-b.net/
Snapshots: http://dd-b.net/dd-b/SnapshotAlbum/data/
Photos: http://dd-b.net/photography/gallery/
Dragaera: http://dragaera.info


Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by David Dyer-Bennet <dd...@dd-b.net>.
On Tue, December 7, 2010 09:03, Daniel Shahaf wrote:
> David Dyer-Bennet wrote on Tue, Dec 07, 2010 at 08:44:28 -0600:
>> On Tue, December 7, 2010 04:18, Julian Foad wrote:
>> > On Tue, 2010-12-07, Daniel Shahaf wrote:
>> >> I suppose setting VISUAL="\"/path with spaces/to/editor/binary\"" is
>> the
>> >> easiest solution --- it requires no code changes so it will work with
>> >> any svn binary out there.
>> >
>> > Yes, I think that's the best solution.
>>
>> That's an interesting idea.
>>
>> Unfortunately, it doesn't seem to work.  (Example from Centos 5.5
>> again):
>>
>
> The following syntaxes work for me:
>
> % SVN_EDITOR="'$HOME/pre fix/bin/vim' +%d" $svn mkdir
> file:///tmp/svn/r1/foo3
> % $svn mkdir file:///tmp/svn/r1/foo3 --editor-cmd "'$HOME/pre fix/bin/vim'
> +%d"

Having an application-specific env variable also makes this kind of
workaround more palatable -- setting SVN_EDITOR strangely shouldn't
break anything else, and if it does, it's the fault of the something
else.

> (the +%d is there both to prove to myself that it's not simply falling
> back to my default editor --- which is also vim --- and to show that
> passing argv[1] to the editor works)
>
>> sh-3.2$ export VISUAL='"/home/path with spaces/bin/emacs"'
>> sh-3.2$ echo $VISUAL
>> "/home/path with spaces/bin/emacs"
>> sh-3.2$ ls -l "/home/path with spaces/bin"
>> total 8
>> -rwxr-xr-x 1 root root 192 Dec  7 08:40 emacs
>> sh-3.2$ svn commit
>> basename: extra operand `spaces/bin/emacs'
>
> Why is basename(1) invoked here?
>                ^^^
>
> (That's almost certainly something in your configuration.)

I checked; turns out that /usr/bin/emacs is a script that invokes
basename (in the standard Centos 5.5 emacs install).  I copied
/usr/bin/emacs to /home/path with spaces/bin/emacs for this test.

It looks like the workaround of quoting the path with spaces in
SVN_EDITOR works, and I'm now encountering a bug in emacs (or perhaps
the Centos packaging of Emacs).  (The emacs script is easily fixed to
tolerate spaces in its path, just by doing the basic obvious things
you do when shell scripting.)

>> Try `basename --help' for more information.
>> Can't find
>> svn: Commit failed (details follow):
>> svn: system('"/home/path with spaces/bin/emacs" svn-commit.tmp')
>> returned 256
>
>
>> And, in any case, VISUAL is a public interface, and I wonder how many
>> other applications would break if I put that kind of thing into
>> VISUAL.
>
> Morale: don't use paths with spaces.

Yes, that would be simpler.  But as I said in my initial post, I first
discovered this on Windows under Cygwin.  Avoiding "c:/Program Files"
and "c:/Documents and Settings/david.dyer-bennet/My Documents"
involves contortions and leaves you putting things in unusual places.

Linux defines spaces as valid characters in paths and filenames, so
people are going to try to use them.  And Subversion does support
Windows.

(Spelling nit, no flame intended:  "moral"; not "morale".  They're
both real words with quite different meanings, so it's of value to
keep them distinct.  And now I have no doubt guaranteed 3 basic
language mistakes of my own somewhere in this message :-) .)
-- 
David Dyer-Bennet, dd-b@dd-b.net; http://dd-b.net/
Snapshots: http://dd-b.net/dd-b/SnapshotAlbum/data/
Photos: http://dd-b.net/photography/gallery/
Dragaera: http://dragaera.info


Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
David Dyer-Bennet wrote on Tue, Dec 07, 2010 at 08:44:28 -0600:
> On Tue, December 7, 2010 04:18, Julian Foad wrote:
> > On Tue, 2010-12-07, Daniel Shahaf wrote:
> >> I suppose setting VISUAL="\"/path with spaces/to/editor/binary\"" is the
> >> easiest solution --- it requires no code changes so it will work with
> >> any svn binary out there.
> >
> > Yes, I think that's the best solution.
> 
> That's an interesting idea.
> 
> Unfortunately, it doesn't seem to work.  (Example from Centos 5.5 again):
> 

The following syntaxes work for me:

% SVN_EDITOR="'$HOME/pre fix/bin/vim' +%d" $svn mkdir file:///tmp/svn/r1/foo3
% $svn mkdir file:///tmp/svn/r1/foo3 --editor-cmd "'$HOME/pre fix/bin/vim' +%d"

(the +%d is there both to prove to myself that it's not simply falling
back to my default editor --- which is also vim --- and to show that
passing argv[1] to the editor works)

> sh-3.2$ export VISUAL='"/home/path with spaces/bin/emacs"'
> sh-3.2$ echo $VISUAL
> "/home/path with spaces/bin/emacs"
> sh-3.2$ ls -l "/home/path with spaces/bin"
> total 8
> -rwxr-xr-x 1 root root 192 Dec  7 08:40 emacs
> sh-3.2$ svn commit
> basename: extra operand `spaces/bin/emacs'

Why is basename(1) invoked here?
               ^^^

(That's almost certainly something in your configuration.)

> Try `basename --help' for more information.
> Can't find
> svn: Commit failed (details follow):
> svn: system('"/home/path with spaces/bin/emacs" svn-commit.tmp') returned 256


> And, in any case, VISUAL is a public interface, and I wonder how many
> other applications would break if I put that kind of thing into
> VISUAL.
> 

Morale: don't use paths with spaces.

Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
David Dyer-Bennet wrote on Tue, Dec 07, 2010 at 08:44:28 -0600:
> On Tue, December 7, 2010 04:18, Julian Foad wrote:
> > On Tue, 2010-12-07, Daniel Shahaf wrote:
> >> I suppose setting VISUAL="\"/path with spaces/to/editor/binary\"" is the
> >> easiest solution --- it requires no code changes so it will work with
> >> any svn binary out there.
> >
> > Yes, I think that's the best solution.
> 
> That's an interesting idea.
> 
> Unfortunately, it doesn't seem to work.  (Example from Centos 5.5 again):
> 

The following syntaxes work for me:

% SVN_EDITOR="'$HOME/pre fix/bin/vim' +%d" $svn mkdir file:///tmp/svn/r1/foo3
% $svn mkdir file:///tmp/svn/r1/foo3 --editor-cmd "'$HOME/pre fix/bin/vim' +%d"

(the +%d is there both to prove to myself that it's not simply falling
back to my default editor --- which is also vim --- and to show that
passing argv[1] to the editor works)

> sh-3.2$ export VISUAL='"/home/path with spaces/bin/emacs"'
> sh-3.2$ echo $VISUAL
> "/home/path with spaces/bin/emacs"
> sh-3.2$ ls -l "/home/path with spaces/bin"
> total 8
> -rwxr-xr-x 1 root root 192 Dec  7 08:40 emacs
> sh-3.2$ svn commit
> basename: extra operand `spaces/bin/emacs'

Why is basename(1) invoked here?
               ^^^

(That's almost certainly something in your configuration.)

> Try `basename --help' for more information.
> Can't find
> svn: Commit failed (details follow):
> svn: system('"/home/path with spaces/bin/emacs" svn-commit.tmp') returned 256


> And, in any case, VISUAL is a public interface, and I wonder how many
> other applications would break if I put that kind of thing into
> VISUAL.
> 

Morale: don't use paths with spaces.

Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by Peter Samuelson <pe...@p12n.org>.
[David Dyer-Bennet]
> >> I suppose setting VISUAL="\"/path with spaces/to/editor/binary\"" is the
> >> easiest solution --- it requires no code changes so it will work with
> >> any svn binary out there.
> >
> > Yes, I think that's the best solution.
> 
> And, in any case, VISUAL is a public interface, and I wonder how many
> other applications would break if I put that kind of thing into
> VISUAL.

A couple of random tests:

- vipw, vigr work
- visudo does not

I seem to recall the same question coming up about 'man' and 'PAGER'
some years ago.  At least the Debian version uses system() and
interprets PAGER='"/with spaces/pager" -with -args' correctly.
-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/

Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by Peter Samuelson <pe...@p12n.org>.
[David Dyer-Bennet]
> >> I suppose setting VISUAL="\"/path with spaces/to/editor/binary\"" is the
> >> easiest solution --- it requires no code changes so it will work with
> >> any svn binary out there.
> >
> > Yes, I think that's the best solution.
> 
> And, in any case, VISUAL is a public interface, and I wonder how many
> other applications would break if I put that kind of thing into
> VISUAL.

A couple of random tests:

- vipw, vigr work
- visudo does not

I seem to recall the same question coming up about 'man' and 'PAGER'
some years ago.  At least the Debian version uses system() and
interprets PAGER='"/with spaces/pager" -with -args' correctly.
-- 
Peter Samuelson | org-tld!p12n!peter | http://p12n.org/

Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by David Dyer-Bennet <dd...@dd-b.net>.
On Tue, December 7, 2010 04:18, Julian Foad wrote:
> On Tue, 2010-12-07, Daniel Shahaf wrote:

> I confirmed that there was a bug in that report, but that was on Windows
> and the evidence there was that the arguments were not being parsed
> correctly even when the space was escaped with the "^" character.
> That's a different problem from yours.

I'm also not sure what this concept of escaping with "^" is; if this is a
windows thing I've managed to avoid learning about it (and I still use
Windows systems, even today).

>> Today we use system(), and thus things like EDITOR='gvim -f' and
>> EDITOR='emacs -nw' work.  If propedit-cmd.c just quoted all spaces in
>> the editor command, it would break this use case.

Agreed, that would be too drastic a solution, and would break important
things.

>> I suppose setting VISUAL="\"/path with spaces/to/editor/binary\"" is the
>> easiest solution --- it requires no code changes so it will work with
>> any svn binary out there.
>
> Yes, I think that's the best solution.

That's an interesting idea.

Unfortunately, it doesn't seem to work.  (Example from Centos 5.5 again):

sh-3.2$ export VISUAL='"/home/path with spaces/bin/emacs"'
sh-3.2$ echo $VISUAL
"/home/path with spaces/bin/emacs"
sh-3.2$ ls -l "/home/path with spaces/bin"
total 8
-rwxr-xr-x 1 root root 192 Dec  7 08:40 emacs
sh-3.2$ svn commit
basename: extra operand `spaces/bin/emacs'
Try `basename --help' for more information.
Can't find
svn: Commit failed (details follow):
svn: system('"/home/path with spaces/bin/emacs" svn-commit.tmp') returned 256


And, in any case, VISUAL is a public interface, and I wonder how many
other applications would break if I put that kind of thing into
VISUAL.

-- 
David Dyer-Bennet, dd-b@dd-b.net; http://dd-b.net/
Snapshots: http://dd-b.net/dd-b/SnapshotAlbum/data/
Photos: http://dd-b.net/photography/gallery/
Dragaera: http://dragaera.info


Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by David Dyer-Bennet <dd...@dd-b.net>.
On Tue, December 7, 2010 04:18, Julian Foad wrote:
> On Tue, 2010-12-07, Daniel Shahaf wrote:

> I confirmed that there was a bug in that report, but that was on Windows
> and the evidence there was that the arguments were not being parsed
> correctly even when the space was escaped with the "^" character.
> That's a different problem from yours.

I'm also not sure what this concept of escaping with "^" is; if this is a
windows thing I've managed to avoid learning about it (and I still use
Windows systems, even today).

>> Today we use system(), and thus things like EDITOR='gvim -f' and
>> EDITOR='emacs -nw' work.  If propedit-cmd.c just quoted all spaces in
>> the editor command, it would break this use case.

Agreed, that would be too drastic a solution, and would break important
things.

>> I suppose setting VISUAL="\"/path with spaces/to/editor/binary\"" is the
>> easiest solution --- it requires no code changes so it will work with
>> any svn binary out there.
>
> Yes, I think that's the best solution.

That's an interesting idea.

Unfortunately, it doesn't seem to work.  (Example from Centos 5.5 again):

sh-3.2$ export VISUAL='"/home/path with spaces/bin/emacs"'
sh-3.2$ echo $VISUAL
"/home/path with spaces/bin/emacs"
sh-3.2$ ls -l "/home/path with spaces/bin"
total 8
-rwxr-xr-x 1 root root 192 Dec  7 08:40 emacs
sh-3.2$ svn commit
basename: extra operand `spaces/bin/emacs'
Try `basename --help' for more information.
Can't find
svn: Commit failed (details follow):
svn: system('"/home/path with spaces/bin/emacs" svn-commit.tmp') returned 256


And, in any case, VISUAL is a public interface, and I wonder how many
other applications would break if I put that kind of thing into
VISUAL.

-- 
David Dyer-Bennet, dd-b@dd-b.net; http://dd-b.net/
Snapshots: http://dd-b.net/dd-b/SnapshotAlbum/data/
Photos: http://dd-b.net/photography/gallery/
Dragaera: http://dragaera.info


Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by Julian Foad <ju...@wandisco.com>.
On Tue, 2010-12-07, Daniel Shahaf wrote:
> CC += dev@
> 
> Daniel Näslund wrote on Mon, Dec 06, 2010 at 21:32:39 +0100:
> > On Mon, Dec 06, 2010 at 01:44:23PM -0600, David Dyer-Bennet wrote:
> > > Subversion 1.6.12 running on Centos 5.5
> > > 
> > > If the value of the environment variable VISUAL contains a space,
> > > subversion fails when attempting to invoke the editor to get the
> > > comment.
> > > 
> > > sh-3.2$ export VISUAL="/home/spaces in name/bin/emacs"
> > > sh-3.2$ svn commit
> > > sh: /home/spaces: No such file or directory
> > > svn: Commit failed (details follow):
> > > svn: system('/home/spaces in name/bin/emacs svn-commit.tmp') returned 32512
> > > 
> > > As you see in the error, it's constructing a command string without
> > > consideration of the possibility of spaces in various places.
> > 
> > [...]
> > 
> > > So, can somebody confirm this please?  And, ideally, submit the bug
> > > (since I keep getting stuck trying to go through all the hoops they
> > > want you to go through to become enabled to do that)?
> > 
> > Found this thread [1] that discusses spaces in the editor cmd string. Julian
> > Foad confirms that it's a bug in a follow-up and even has some
> > suggestions on how to tackle the problem if someone wants to write a
> > patch.

I confirmed that there was a bug in that report, but that was on Windows
and the evidence there was that the arguments were not being parsed
correctly even when the space was escaped with the "^" character.
That's a different problem from yours.

> Today we use system(), and thus things like EDITOR='gvim -f' and
> EDITOR='emacs -nw' work.  If propedit-cmd.c just quoted all spaces in
> the editor command, it would break this use case.
> 
> I suppose setting VISUAL="\"/path with spaces/to/editor/binary\"" is the
> easiest solution --- it requires no code changes so it will work with
> any svn binary out there.

Yes, I think that's the best solution.

- Julian


> > [1] http://svn.haxx.se/dev/archive-2010-02/0051.shtml


Re: Bug report -- space in env. var. VISUAL causes commits to fail (needs confirmation)

Posted by Julian Foad <ju...@wandisco.com>.
On Tue, 2010-12-07, Daniel Shahaf wrote:
> CC += dev@
> 
> Daniel Näslund wrote on Mon, Dec 06, 2010 at 21:32:39 +0100:
> > On Mon, Dec 06, 2010 at 01:44:23PM -0600, David Dyer-Bennet wrote:
> > > Subversion 1.6.12 running on Centos 5.5
> > > 
> > > If the value of the environment variable VISUAL contains a space,
> > > subversion fails when attempting to invoke the editor to get the
> > > comment.
> > > 
> > > sh-3.2$ export VISUAL="/home/spaces in name/bin/emacs"
> > > sh-3.2$ svn commit
> > > sh: /home/spaces: No such file or directory
> > > svn: Commit failed (details follow):
> > > svn: system('/home/spaces in name/bin/emacs svn-commit.tmp') returned 32512
> > > 
> > > As you see in the error, it's constructing a command string without
> > > consideration of the possibility of spaces in various places.
> > 
> > [...]
> > 
> > > So, can somebody confirm this please?  And, ideally, submit the bug
> > > (since I keep getting stuck trying to go through all the hoops they
> > > want you to go through to become enabled to do that)?
> > 
> > Found this thread [1] that discusses spaces in the editor cmd string. Julian
> > Foad confirms that it's a bug in a follow-up and even has some
> > suggestions on how to tackle the problem if someone wants to write a
> > patch.

I confirmed that there was a bug in that report, but that was on Windows
and the evidence there was that the arguments were not being parsed
correctly even when the space was escaped with the "^" character.
That's a different problem from yours.

> Today we use system(), and thus things like EDITOR='gvim -f' and
> EDITOR='emacs -nw' work.  If propedit-cmd.c just quoted all spaces in
> the editor command, it would break this use case.
> 
> I suppose setting VISUAL="\"/path with spaces/to/editor/binary\"" is the
> easiest solution --- it requires no code changes so it will work with
> any svn binary out there.

Yes, I think that's the best solution.

- Julian


> > [1] http://svn.haxx.se/dev/archive-2010-02/0051.shtml