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

2010-12-06 Thread David Dyer-Bennet
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.

This also occurs when running subvrsion under Cygwin on Windows XP.  I
discovered it there, because there the natural place to put my
personal editor script (which attaches to an existing emacs if there
is one, and otherwise starts emacs itself, with various other personal
parameters and condition checking) ends up having the path

/cygdrive/c/Documents and Settings/david.bennet/My Documents/bin/ew

which has three spaces in two path components, neither name chosen by
me.

The workaround, obviously, is to place your editor (or editor script)
in a place that doesn't have spaces in the path.

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)?
-- 
David Dyer-Bennet, d...@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)

2010-12-07 Thread David Dyer-Bennet

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, d...@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)

2010-12-07 Thread David Dyer-Bennet

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, d...@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)

2010-12-07 Thread David Dyer-Bennet

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, d...@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