Max Bowsher wrote:
>
> I've examined the patch. Many comments:
>
> First, re the slash problem - what do you think of this:
> --- cvs2svn.py (revision 7052)
> +++ cvs2svn.py (working copy)
> @@ -310,13 +310,19 @@
> else:
> return ctx.trunk_base
>
> +if os.sep == '/':
> + def canonicalize_slashes(path):
> + return path
> +else:
> + def canonicalize_slashes(path):
> + path.replace(os.sep, '/')
>
> def relative_name(cvsroot, fname):
> l = len(cvsroot)
> if fname[:l] == cvsroot:
> if fname[l] == '/':
> - return fname[l+1:]
> - return fname[l:]
> + return canonicalize_slashes(fname[l+1:])
> + return canonicalize_slashes(fname[l:])
> sys.stderr.write('relative_path("%s", "%s"): fname is not a
> sub-path of' ' cvsroot\n' % (cvsroot, fname))
> sys.exit(1)
That's good. Cleaner and probably more efficient.
> Second, re escape_shell_arg:
> There is a different implementation of it in gen_win.py. Why is it
> different?
Actually both are really only half-assed attempts at escaping that will fail
in different circumstances. (For a more robust version see
subversion/bindings/swig/python/svn/fs.py). I wrote the implementation in
gen_win.py after doing a little more experimenting and I think it is
superior to the one used in this old patch.
> Third, re co -kb:
> I think we do want keyword expansion here, so I'm not keen to use -kb.
I added this because I thought it would be neccessary to work with binary
files. I'm not sure if I actually tested without -kb, though, so it might
work as is.
> Fourth, re this change:
> - output = open(ctx.log_fname_base + CLEAN_REVS_SUFFIX, 'w')
> + output = open(ctx.log_fname_base + CLEAN_REVS_SUFFIX, 'wt')
>
> It isn't in the log message. What is it for?
The file that is being opened is a text file. The 't' flag is not strictly
neccessary though since text mode is /usually/ the default mode on windows
and there's no difference between text and binary modes on unix.
> Fifth, in run-tests.py:
> Does anyone know *why* invoking cvs2svn as "python cvs2svn.py" makes
> sort work, when "cvs2svn.py" doesn't?
When I wrote the patch I experimented a little and found that when the
second invokation is used the command interpreter (cmd.exe) used to invoke
the sort command gets passed invalid standard input, output, and error
handles. I determined this by writing a little exe that attempts I/O on the
handles and setting the COMSPEC variable to point to the exe before calling
system(). I wasn't able to figure out what why this happens though.
- Russ
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon Sep 15 18:46:55 2003