Hi all.
This is a little issue I came across while reusing 'svnlook diff' code
for a related project (an indexing hook script).
prepare_tmpfiles in svnlook/main.c will use the item's path in the
repository as part of the temporary file name it uses for diffing files.
This can result in attempted temporary file names that are valid in
the repository but don't work in the system's temp dir. For example
(Windows):
$ svnlook diff -r 1399 d:\svn\svn-depot
svnlook: Can't open file
'C:\DOCUME~1\Edmund\LOCALS~1\Temp\svnlook.1\project\svn\trunk\packages\
freebsd\subversion\files\patch-subversion::libsvn_ra_dav::session.c':
The filename, directory name, or volume label syntax is incorrect.
(My r1399 corresponds to r1398 on svn.collab.net.)
The easiest solution that I can see is to use plain old temporary file
names instead of full paths. Max Bowsher suggested I write a patch to
that effect (attached).
However I've noticed that building out the full diff tree is intentional
in svnlook (print_diff_tree). My patch seems to work (it gives
identical diffs for those revisions I've tried that don't contain
unusual paths), but I'm not 100% certain of the reason for doing it this
way...
FWIW, the temporary files are created, used, and deleted before the rest
of the tree is processed; and the only functions that use the names of
the temporary files are (all in print_diff_tree):
* prepare_tmpfiles (which creates them using tmpdir)
* svn_diff_file_diff
* svn_diff_file_output_unified2
* svn_io_remove_file (which gets rid of them)
tmpdir is $(TEMP)/svnlook.n (with n >= 1), which is created by
create_unique_tmpdir.
The best case scenario I can see is that svnlook diff doesn't need to
build the whole tree any more, and we can simplify things (and possibly
remove create_unique_tmpdir). Max noted that create_unique_tmpdir makes
an effort to set appropriate file permissions on tmpdir.
However, if it does need to build the whole tree then some other method
will be needed to make temporary file names that are compatible with the
OS but still fit into the diffing mechanism. *shrug*
This patch simply stops building the tree. I have also updated some
comments in print_diff_tree to reflect that. The use of tmpdir is retained.
Let me know if you want anything else in the way of a better patch, etc.
Edmund.
[[[
Use simple and safe temporary file names for svnlook diff, rather than
building out the tree as it exists in the repository -- this avoids
problems with paths that can't be used in the temporary directory.
Patch by: Edmund Horner <ejrh00@gmail.com>
Suggested by: Max Bowsher <maxb@ukf.net>
* subversion/svnlook/main.c
(prepare_tmpfiles): Create a normal unique file in the temporary
directory.
(print_diff_tree): Update comment to omit mention of the built-out
temporary directories, which don't get created any more.
]]]
Index: main.c
===================================================================
--- main.c (revision 18619)
+++ main.c (working copy)
@@ -694,8 +694,8 @@
/* Now, prepare the two temporary files, each of which will either
be empty, or will have real contents. The first file we will
make in our temporary directory. */
- *tmpfile2 = svn_path_join(tmpdir, path2, pool);
- SVN_ERR(open_writable_binary_file(&fh, *tmpfile2, pool));
+ SVN_ERR(svn_io_open_unique_file2(&fh, tmpfile2, apr_psprintf(pool, "%s/diff", tempdir),
+ ".tmp", svn_io_file_del_none, pool));
if (root2)
SVN_ERR(dump_contents(fh, root2, path2, pool));
apr_file_close(fh);
@@ -897,24 +897,18 @@
diff.
- Second, dump the contents of the new version of the file
- into the svnlook temporary directory, building out the
- actual directories that need to be created in order to
- fully represent the filesystem path inside the tmp
- directory.
+ into the svnlook temporary directory.
- Then, dump the contents of the old version of the file into
- the svnlook temporary directory, also building out intermediate
- directories as needed, using a unique temporary file name (we
- do this *after* putting the new version of the file
- there in case something actually versioned has a name
- that looks like one of our unique identifiers).
+ the svnlook temporary directory (we do this *after* putting
+ the new version of the file there in case something actually
+ versioned has a name that looks like one of our unique
+ identifiers).
- Next, we run 'diff', passing the repository paths as the
labels.
- - Finally, we delete the temporary files (but leave the
- built-out directories in place until after all diff
- handling has been finished). */
+ - Finally, we delete the temporary files. */
if (node->action == 'R' && node->text_mod)
{
do_diff = TRUE;
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Feb 25 14:57:30 2006