You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Alexis Huxley <ah...@gmx.net> on 2002/08/08 09:22:05 UTC

svn commit invokes $EDITOR not where svn run from

Hi, ok, it's clear that svn is cd'ing to the root of the working copy,
to do some stuff, and then directly invoking $EDITOR from there.

In another SCM product ;-) from a subdirectory I often do the 
equivalent of 'svn diff > diff.out', then 'svn commit', and read
the diff.out file into the editor, and edit it down to a change list. 

Ok, it is possible with svn but by specifying paths relative to
the WC root, not to the directory in which I invoked svn.

Is it possible for svn to cd back to the original directory immediately 
before invoking EDITOR?

Alexis

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Justin Erenkrantz <je...@apache.org>.
On Thu, Aug 08, 2002 at 05:52:35PM +0200, Marcus Comstedt wrote:
> For AmigaOS, T: can be used.  Don't know about VMS or MacOS 9.

I'm sure MacOS 9 isn't supported and VMS probably wouldn't be
either.  -- justin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Philip Martin <ph...@codematters.co.uk>.
Philip Martin <ph...@codematters.co.uk> writes:

> $ mkdir /tmp/\;rm\ -rf\ /
> $ TMPDIR="/tmp/;rm -rf /" svn ci

On second thoughts, this is just "user error" :)  We deliberately don't
check SVN_EDITOR which could contain similar things.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Marcus Comstedt <ma...@mc.pp.se>.
Philip Martin <ph...@codematters.co.uk> writes:

> Having said it might be easy, now I'm not so sure.  Suppose I try
> 
> $ mkdir /tmp/\;rm\ -rf\ /
> $ TMPDIR="/tmp/;rm -rf /" svn ci

The environment variable case was for W*ndows, and as I understood
Mike you would be passing

  editor.exe %TMPDIR%\foobar.txt

and not the expansion of the variable.  The UNIX equivalent would be
to call system on the string

  emacs "$TMPDIR"/foobar.txt

which works regardless of which characters are inside $TMPDIR.  (At
least from a quoting standpoint.  Emacs could still interpret its
argument as an option instead of as a path to open, if it starts with
-.)

If this was _not_ what Mike meant, he probably misunderstood my
original question, which was essentially "can't the path to the
tempdir contain unsafe characters on W*ndows".  On UNIX, we can just
use "/tmp", which is known to contain no unsafe characters.

The way we get away with using system() is that we _know_ that the
path contains no characters that need quoting.  Prepending it with an
expanded tempdir path that could contain unsafe characters would bring
us right back to square one.


  // Marcus



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Philip Martin <ph...@codematters.co.uk>.
Marcus Comstedt <ma...@mc.pp.se> writes:

> cmpilato@collab.net writes:
> 
> > Windows has environmental variables for answering this question.  I
> > think the ordering is WINTMP, TMP, TEMP, but I can't recall with
> > certainty.
> 
> And you can use them in what you pass to system(), having them
> automagically expanded somewhere along the way.  Ok, then no problem
> there, except possibly an API one.  If APR should generate the path to
> a tempdir, can it return a path which contains environment variable
> references?  Even if it works with system(), will it work with other
> things you might want to do with a tempdir path?

Having said it might be easy, now I'm not so sure.  Suppose I try

$ mkdir /tmp/\;rm\ -rf\ /
$ TMPDIR="/tmp/;rm -rf /" svn ci

I don't want that passed straight to system.  APR would need to sanity
check the value which could be hard and is platform dependent.  I
suppose APR could impose a limited set of acceptable characters.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Marcus Comstedt <ma...@mc.pp.se>.
cmpilato@collab.net writes:

> Windows has environmental variables for answering this question.  I
> think the ordering is WINTMP, TMP, TEMP, but I can't recall with
> certainty.

And you can use them in what you pass to system(), having them
automagically expanded somewhere along the way.  Ok, then no problem
there, except possibly an API one.  If APR should generate the path to
a tempdir, can it return a path which contains environment variable
references?  Even if it works with system(), will it work with other
things you might want to do with a tempdir path?


  // Marcus



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Marcus Comstedt <ma...@mc.pp.se> writes:
> If you want to do something right away, what Karl and Philip talked
> about should be the way to go; first try using the cwd if it's
> writable, and then possibly the temp area of the current directory if
> it's a wc dir, and then fall back to the current behaviour.  That is,
> the file should still be referenced relative to the current directory,
> but the current directory set by the code could be chosen differently
> from now.

+1

In fact, the difference would be that the code wouldn't need to "set"
the current directory -- now it can just use cwd right where it is.

And the paths inserted *into* the tempfile (the ones that appear in
the log message template in the editor) should be relative to that
directory.

Go for it, Alexis!

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Marcus Comstedt <ma...@mc.pp.se>.
Alexis Huxley <ah...@gmx.net> writes:

> Blimey, I had no idea it would be so complicated :-) Still, I'd like
> to try writing a patch - I'm using SVN a lot and this would be a
> small payback. I printed HACKING out today :-)
> 
> The easiest to implement I guess is use /tmp (Unix) or use
> getenv("WINTMP") || getenv("TMP") || ... (Windows). Ok, on Unix
> /tmp can be relied upon, but can somebody confirm that the env.var +
> fallback env.vars is the way on Windows? Alternatively, Karl's way
> daunts me, and I think I would have to chicken out.
> 
> Is TMPDIR support in APR imminent?  Should I just grit my teeth -
> very gently - and type ":r <subdir>/diffs.out" in vi instead of
> ":r diffs.out". ?
> 
> My question then is simply: what is the correct solution? Subject to
> how easy it is I'll start trying to code it (and the 'commit 
> message file not left around on commit error bug' at the same time).

If you want to do something right away, what Karl and Philip talked
about should be the way to go; first try using the cwd if it's
writable, and then possibly the temp area of the current directory if
it's a wc dir, and then fall back to the current behaviour.  That is,
the file should still be referenced relative to the current directory,
but the current directory set by the code could be chosen differently
from now.

The path passed to the editor must not contain components that
originate from an expanded environment variable, a repository entry,
or the expanded cwd, since that would mean letting arbitrary
characters in, which would require quoting.  (And we can't do quoting,
since we're not sure which shell will be interpreting the command.)


  // Marcus



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Alexis Huxley <ah...@gmx.net>.
Alexis wrote:

> What's wrong with a normal fork() + exec()? The child does a chdir()
> and uses probably execvp() on a sprintf() of getenv("EDITOR") and the

Karl wrote:

>    If P is also an svn working copy directory, then we can use P's
>    temp area.  In other words, the target paths should always be

Philip wrote:

> > > If subversion had an interface to /tmp and $TMPDIR (or whatever any
> > > given platform uses) we could use that, such a path should be safe and
> > > so we could avoid the cd.  It is likely that such an interface would

cmpilato wrote:

> Windows has environmental variables for answering this question.  I
> think the ordering is WINTMP, TMP, TEMP, but I can't recall with
> certainty.

Blimey, I had no idea it would be so complicated :-) Still, I'd like
to try writing a patch - I'm using SVN a lot and this would be a
small payback. I printed HACKING out today :-)

The easiest to implement I guess is use /tmp (Unix) or use
getenv("WINTMP") || getenv("TMP") || ... (Windows). Ok, on Unix
/tmp can be relied upon, but can somebody confirm that the env.var +
fallback env.vars is the way on Windows? Alternatively, Karl's way
daunts me, and I think I would have to chicken out.

Is TMPDIR support in APR imminent?  Should I just grit my teeth -
very gently - and type ":r <subdir>/diffs.out" in vi instead of
":r diffs.out". ?

My question then is simply: what is the correct solution? Subject to
how easy it is I'll start trying to code it (and the 'commit 
message file not left around on commit error bug' at the same time).

Alexis

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by cm...@collab.net.
Marcus Comstedt <ma...@mc.pp.se> writes:

> Philip Martin <ph...@codematters.co.uk> writes:
> 
> > If subversion had an interface to /tmp and $TMPDIR (or whatever any
> > given platform uses) we could use that, such a path should be safe and
> > so we could avoid the cd.  It is likely that such an interface would
> > need APR support for the platform dependent bits; probably not very
> > hard to do but nobody has done it.
> 
> Does W*ndows have a tempdir that is always a "safe" path?  Isn't the
> normal tempdir relative to the WINDOWS directory, which can
> (theoretically) have any path?

Windows has environmental variables for answering this question.  I
think the ordering is WINTMP, TMP, TEMP, but I can't recall with
certainty.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Marcus Comstedt <ma...@mc.pp.se>.
Philip Martin <ph...@codematters.co.uk> writes:

> If subversion had an interface to /tmp and $TMPDIR (or whatever any
> given platform uses) we could use that, such a path should be safe and
> so we could avoid the cd.  It is likely that such an interface would
> need APR support for the platform dependent bits; probably not very
> hard to do but nobody has done it.

Does W*ndows have a tempdir that is always a "safe" path?  Isn't the
normal tempdir relative to the WINDOWS directory, which can
(theoretically) have any path?

For AmigaOS, T: can be used.  Don't know about VMS or MacOS 9.


   // Marcus



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Alexis Huxley <ah...@gmx.net>.
> Ok, it's taking me a bit longer to get round to writing the test
> than I'd hoped. In the mean time here's the first version of the
> patch ...

A couple of questions regarding tests:

sbox.build creates a working copy, right?  If I want to create a 
directory *in which* to call sbox.build, then where should I be 
doing it? Presumably I need to do this under the test directory,
but then what variables/interface does the test harness offer me?

(I want to create a writable and unwritable directory to run the 
commit from.)

Is '<envvar>=<value> <command>' portable? 

(Rather than looking at how to integrate a pseudo-editor into the
test directories/PATH/etc, I thought I might invoke

	EDITOR="python -c 'import os; print os.getcwd()'" svn commit

but perhaps that only works on Unix-like OSs.)

Can anyone advise? Thanks!

Alexis

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Alexis Huxley <ah...@gmx.net>.
> > So my question is: should I start learning Python so I can code
> > this, or is it sufficiently un-cross-platformy or integratable
> > to warrant not bothering? :-)
> 
> Well, "cat >" is certainly not portable enough to put in the test
> suite.  I'm not sure what a portable solution to the editor problem
> would be, unless we want to write a special Python program, that our
> test suite just uses to *be* the editor.  Hmmm, you know, that might
> just be the trick.  Want to try that way?

Ok, it's taking me a bit longer to get round to writing the test
than I'd hoped. In the mean time here's the first version of the
patch ...

Alexis

Index: subversion/clients/cmdline/util.c
===================================================================
--- subversion/clients/cmdline/util.c
+++ subversion/clients/cmdline/util.c	2002-08-20 21:02:20.000000000 +0200
@@ -382,31 +382,49 @@
   /* Convert file contents from UTF-8 */
   SVN_ERR (svn_utf_cstring_from_utf8 (&contents_native, contents, pool));
 
-  /* Move to BASE_DIR to avoid getting characters that need quoting
-     into tmpfile_name */
+  /* Get cwd - will be needed later if we cd anywhere searching for writable dir */
   apr_err = apr_filepath_get (&old_cwd, APR_FILEPATH_NATIVE, pool);
   if (apr_err)
     {
       return svn_error_create
-        (apr_err, 0, NULL, pool, "failed to get current working directory");
-    }
-  SVN_ERR (svn_utf_cstring_from_utf8 (&base_dir_native, base_dir, pool));
-  apr_err = apr_filepath_set (base_dir_native, pool);
-  if (apr_err)
-    {
-      return svn_error_createf
-        (apr_err, 0, NULL, pool,
-         "failed to change working directory to '%s'", base_dir);
+          (apr_err, 0, NULL, pool, "failed to get current working directory");
     }
 
-  /*** From here on, any problems that occur require us to cd back!! ***/
+  /*** From here on, if problems occur we cd back unconditionally, even if superfluous ***/
 
-  /* Ask the working copy for a temporary file */
-  err = svn_io_open_unique_file 
-    (&tmp_file, &tmpfile_name,
-     "msg", ".tmp", FALSE, pool);
-  if (err)
+  do {						/* 1 loop only; 'do' facilitates 'break' */
+
+    /* first attempt: stay in cwd, and try opening here */
+    err = svn_io_open_unique_file (&tmp_file, &tmpfile_name, "msg", ".tmp", FALSE, pool);
+    if (!err)
+      break;
+    else if (err && err->apr_err != APR_EACCES) 
+      goto cleanup2;
+   
+    /*** Additional cd+open's go here (copy and edit the 'last attempt' block) ***/
+
+    /* last attempt: go to base of WC, and try opening there */
+    err = svn_utf_cstring_from_utf8 (&base_dir_native, base_dir, pool);
+    if (err)
+        goto cleanup2;
+    apr_err = apr_filepath_set (base_dir_native, pool);
+    if (apr_err)
+      {
+        err = svn_error_createf (apr_err, 0, NULL, pool,
+           "failed to change working directory to '%s'", base_dir);
+        goto cleanup2;
+      }
+    err = svn_io_open_unique_file (&tmp_file, &tmpfile_name, "msg", ".tmp", FALSE, pool);
+    if (!err)
+      break;
+    else if (err && err->apr_err != APR_EACCES) 
+      goto cleanup2;
+  
+    /* finally: give up and generate an error using last EACCESS error */
     goto cleanup2;
+    
+  } while (0);					/* facilitate goto without goto */
+
 
   /*** From here on, any problems that occur require us to cleanup
        the file we just created!! ***/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Alexis Huxley <ah...@gmx.net> writes:
> So my question is: should I start learning Python so I can code
> this, or is it sufficiently un-cross-platformy or integratable
> to warrant not bothering? :-)

Well, "cat >" is certainly not portable enough to put in the test
suite.  I'm not sure what a portable solution to the editor problem
would be, unless we want to write a special Python program, that our
test suite just uses to *be* the editor.  Hmmm, you know, that might
just be the trick.  Want to try that way?

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Alexis Huxley <ah...@gmx.net>.
> > >    If P is also an svn working copy directory, then we can use P's
> > >    temp area.  In other words, the target paths should always be
> > >    constructed relative to the directory the editor is running in, and
> > >    that directory should be the one from which the editor was invoked
> > >    when possible (i.e., when it's an svn-controlled directory), else
> > >    the lowest common root of all the targets when P is not an
> > >    svn-controlled directory.
> > 
> > That sounds reasonable.  I suppose we could also try the current
> > directory, and only fall back on the working copy temp. area if the
> > current directory is not writeable.
> 
> Even better, yeah!

Ok, I have something coded. I ran the commit_tests.py, but I
suspect that it doesn't invoke $EDITOR at all. ('grep -i editor'
doesn't find anything).

I'd like to submit a test with it if possible. But since this 
change is specifically to do with the invocation of an interactive 
program, I was thinking of setting:

	EDITOR="cat >" 

in order to be able to pipe the log message 'through' svn. Or
even to write a little script:

	#!/bin/sh
	cat > $1
	echo "cwd=`pwd`, dircmpt=`dirname $1`" >&2

in order to get $EDITOR's opinion of cwd, and check that the tmp 
file was relative to cwd.

So my question is: should I start learning Python so I can code
this, or is it sufficiently un-cross-platformy or integratable
to warrant not bothering? :-)

Alexis

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Philip Martin <ph...@codematters.co.uk> writes:
> > The solution I thought he was proposing, and which I liked, is simply:
> > 
> >    If P is also an svn working copy directory, then we can use P's
> >    temp area.  In other words, the target paths should always be
> >    constructed relative to the directory the editor is running in, and
> >    that directory should be the one from which the editor was invoked
> >    when possible (i.e., when it's an svn-controlled directory), else
> >    the lowest common root of all the targets when P is not an
> >    svn-controlled directory.
> 
> That sounds reasonable.  I suppose we could also try the current
> directory, and only fall back on the working copy temp. area if the
> current directory is not writeable.

Even better, yeah!

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Philip Martin <ph...@codematters.co.uk>.
Karl Fogel <kf...@newton.ch.collab.net> writes:

> The solution I thought he was proposing, and which I liked, is simply:
> 
>    If P is also an svn working copy directory, then we can use P's
>    temp area.  In other words, the target paths should always be
>    constructed relative to the directory the editor is running in, and
>    that directory should be the one from which the editor was invoked
>    when possible (i.e., when it's an svn-controlled directory), else
>    the lowest common root of all the targets when P is not an
>    svn-controlled directory.

That sounds reasonable.  I suppose we could also try the current
directory, and only fall back on the working copy temp. area if the
current directory is not writeable.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Philip Martin <ph...@codematters.co.uk> writes:
> > Is it possible for svn to cd back to the original directory immediately 
> > before invoking EDITOR?
> 
> No, it was a deliberate decision to cd.  The temporary file is created
> in the temp. area of the path to be committed.  By cd'ing we don't
> need to put the path into a system() call, which means that the shell
> cannot interpret characters in the path as "special".

Oops, I'm a bit confused here.

Alexis is trying to solve this problem:

   When you run "svn ci" in directory P, the editor is not necessarily
   invoked in directory P, but rather in the lowest common root of all
   the targets being committed, which may or may not be P.

The solution I thought he was proposing, and which I liked, is simply:

   If P is also an svn working copy directory, then we can use P's
   temp area.  In other words, the target paths should always be
   constructed relative to the directory the editor is running in, and
   that directory should be the one from which the editor was invoked
   when possible (i.e., when it's an svn-controlled directory), else
   the lowest common root of all the targets when P is not an
   svn-controlled directory.

This is still not a perfect solution, but until we have TMPDIR support
from APR, we're not going to get a perfect solution.  At least this
one behaves "more reasonably, more often" :-).  Right now, we *always*
use the lowest common root (the "anchor") of the committed targets,
even when we *could* do what Alexis wants and use P instead.

Is this at least an accurate description, or am I just missing some
big aspect of the problem?

-Karl

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Philip Martin <ph...@codematters.co.uk>.
Alexis Huxley <ah...@gmx.net> writes:

> I thought system() was bad anyway?

Check the mailing list archives, or see issue 809.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Daniel Stenberg <da...@haxx.se>.
On Thu, 8 Aug 2002, Alexis Huxley wrote:

> I thought system() was bad anyway?
>
> What's wrong with a normal fork() + exec()? The child does a chdir() and
> uses probably execvp() on a sprintf() of getenv("EDITOR") and the name of
> the temp file? Ok, I'm sure SVN has it's own way of calling these
> functions, but ... did I miss something?

Now now, we've been in the "system() is bad" loop several laps already,
please spare us another round. ;-)

system() is fine for this.

-- 
      Daniel Stenberg - http://daniel.haxx.se - +46-705-44 31 77
   ech`echo xiun|tr nu oc|sed 'sx\([sx]\)\([xoi]\)xo un\2\1 is xg'`ol


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Alexis Huxley <ah...@gmx.net>.
Karl wrote:

> Yeah -- that seems very reasonable.  Want to write the patch?
> Shouldn't be too complex (therefore the review and apply cycle will
> probably also be quick).

In light of other people's nagative comments I wasn't sure if
you're joking, but I'll do it if it's simple :-)

Philip wrote:

> No, it was a deliberate decision to cd.  The temporary file is created
> in the temp. area of the path to be committed.  By cd'ing we don't
> need to put the path into a system() call, which means that the shell
> cannot interpret characters in the path as "special".

I thought system() was bad anyway?

What's wrong with a normal fork() + exec()? The child does a chdir()
and uses probably execvp() on a sprintf() of getenv("EDITOR") and the
name of the temp file? Ok, I'm sure SVN has it's own way of calling
these functions, but ... did I miss something?

Alexis

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Philip Martin <ph...@codematters.co.uk>.
Alexis Huxley <ah...@gmx.net> writes:

> Hi, ok, it's clear that svn is cd'ing to the root of the working copy,
> to do some stuff, and then directly invoking $EDITOR from there.
> 
> In another SCM product ;-) from a subdirectory I often do the 
> equivalent of 'svn diff > diff.out', then 'svn commit', and read
> the diff.out file into the editor, and edit it down to a change list. 
>
> Ok, it is possible with svn but by specifying paths relative to
> the WC root, not to the directory in which I invoked svn.

You could edit diff.out prior to the commit, and then use the commit
option '-F diff.out' to cause the file contents to be used as the log
message.

> Is it possible for svn to cd back to the original directory immediately 
> before invoking EDITOR?

No, it was a deliberate decision to cd.  The temporary file is created
in the temp. area of the path to be committed.  By cd'ing we don't
need to put the path into a system() call, which means that the shell
cannot interpret characters in the path as "special".

If subversion had an interface to /tmp and $TMPDIR (or whatever any
given platform uses) we could use that, such a path should be safe and
so we could avoid the cd.  It is likely that such an interface would
need APR support for the platform dependent bits; probably not very
hard to do but nobody has done it.

-- 
Philip Martin

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit invokes $EDITOR not where svn run from

Posted by Karl Fogel <kf...@newton.ch.collab.net>.
Alexis Huxley <ah...@gmx.net> writes:
> Hi, ok, it's clear that svn is cd'ing to the root of the working copy,
> to do some stuff, and then directly invoking $EDITOR from there.
> 
> In another SCM product ;-) from a subdirectory I often do the 
> equivalent of 'svn diff > diff.out', then 'svn commit', and read
> the diff.out file into the editor, and edit it down to a change list. 
> 
> Ok, it is possible with svn but by specifying paths relative to
> the WC root, not to the directory in which I invoked svn.
> 
> Is it possible for svn to cd back to the original directory immediately 
> before invoking EDITOR?

Yeah -- that seems very reasonable.  Want to write the patch?
Shouldn't be too complex (therefore the review and apply cycle will
probably also be quick).

-K

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org