svn_load_dirs.pl bug when specifying a tag that already exists, and other bugs that led to it

[Ryan Schmidt <su...@ryandesign.com>]

I have used svn_load_dirs.pl for years now and have run into a minor  
but unfortunate problem which occurs if you specify a tag name that  
already exists in the repository. I ran it like this [1]:

$ svn_load_dirs.pl -t 1.2.3 $REPO/vendor/foo current ./foo-1.2.3

The problem arose because the tag 1.2.3 already existed in $REPO/ 
vendor/foo. svn_load_dirs apparently does not check for this, and  
just executes its svn cp command anyway. The result was that the  
1.2.3 tag in the repository now contains a "current" directory with  
another copy of the code, which is not desired. The script should  
instead print an error message and exit if the specified tag already  
exists. A complete reproduction recipe is below [3].

I could not see an issue in the issue tracker about this, and would  
like to report it.

---

You could say that this was user error, and in a way it is. But a)  
the tool should prevent user error where possible, and b) the user  
error occurred because of further problems in svn_load_dirs. A file  
"bar.baz" in foo-1.2.3 had DOS line endings, but I have set up my  
Subversion config to set svn:eol-style to LF on all *.baz files.  
After importing version 1.2.3, svn_load_dirs checks out the new code  
and runs a diff to make sure everything worked correctly. In this  
case, it printed an error that there was an unexpected difference,  
and showed a difference across every line of this file. When I saw  
the error message, I assumed the import had not succeeded. I changed  
the line endings of bar.baz to LF in a text editor and reran the  
svn_load_dirs command, which unexpectedly created the "current"  
directory in the existing tag as I explained above. The additional  
svn_load_dirs bugs here would be:

* svn_load_dirs should accomodate having svn:eol-style set, in cases  
where this results in the line endings changing. It should also  
support any other properties that would change the files (this might  
include svn:keywords).

* When svn_load_dirs exits with an error, but the import was  
nevertheless successful, a message should be printed stating that the  
import was successful, so that the user does not think he needs to  
repeat the import.

I would like to report these as bugs as well.



[1] In fact, it's not 1.2.3, it's 2.9.1; it's not foo, it's  
phpMyAdmin [2]; and it's not bar.baz, it's config.sample.inc.php. But  
you get the idea.

[2] http://www.phpmyadmin.net/



[3] Reproduction recipe:

Make sure ~/.subversion/config contains these lines:

[miscellany]
enable-auto-props = yes
[auto-props]
*.php = svn:eol-style=LF

Then:

$ cd /tmp
$ curl -O http://umn.dl.sf.net/sourceforge/phpmyadmin/ 
phpMyAdmin-2.9.1-english.tar.bz2
$ svnadmin create testrepo
$ svn mkdir file://`pwd`/testrepo/vendor -m ""
$ svn mkdir file://`pwd`/testrepo/vendor/pma -m ""
$ svn_load_dirs.pl -t 2.9.1 file://`pwd`/testrepo/vendor/pma  
current ./phpMyAdmin-2.9.1-english

Result:

svn_load_dirs.pl: diff -u -x .svn -r ./phpMyAdmin-2.9.1-english /tmp/ 
svn_load_dirs_hS7TrNSsom/my_tag_wc_named_2.9.1 failed with this output:
[snipped diff of config.sample.inc.php]
Press return to quit and clean up svn working directory:

This sounds like an error, but the repository in fact contains 2.9.1  
now. But the user repeats the command because he thinks it was a  
fatal error:

$ svn_load_dirs.pl -t 2.9.1 file://`pwd`/testrepo/vendor/pma  
current ./phpMyAdmin-2.9.1-english

Result:

svn_load_dirs.pl: diff -u -x .svn -r ./phpMyAdmin-2.9.1-english /tmp/ 
svn_load_dirs_mKTdL2Rh0u/my_tag_wc_named_2.9.1 failed with this output:
[snipped diff of config.sample.inc.php]
Only in /tmp/svn_load_dirs_mKTdL2Rh0u/my_tag_wc_named_2.9.1: current
Press return to quit and clean up svn working directory:

And now the 2.9.1 tag also contains a "current" directory.


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

Re: svn_load_dirs.pl bug when specifying a tag that already exists, and other bugs that led to it

[Marc Girod <ma...@iona.com>]

Blair Zajac <blair <at> orcaware.com> writes:

> If you put a patch together, I can review and shepherd it through and commit
> it.

Would you care to comment also the issue I submitted last week, and help to make
a better patch than the one I submitted?

It was about supporting changes in the type of some objects, e.g. from a file to
a directory.

The problems I saw were:
- the same thing is tested two times, before and after the renaming operation,
which couples code written in different places in the script
- as I understood it, one needs to perform two distinct commits: 1) to remove
the previous object, 2) to create the new
- of course one should collect together all the cases, so as not to do multiple
commits if there happens to be multiple type changes, as my simple minded patch
does
- there is already a patch concerning the support for soft links which seems to
have been waiting for a year or so without much attention of anybody; it should
be either committed, or clearly rejected, but not left unattended like that...
- a lot of attention has been spent as it seems to detect renames or moves, and
show them to the user; I didn't look in detail, but the result was verbose and
confusing to me, so that I ended up accepting a default behaviour without
clearly understanding what had happened.

Marc


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

Re: svn_load_dirs.pl bug when specifying a tag that already exists, and other bugs that led to it

[Blair Zajac <bl...@orcaware.com>]

Ryan Schmidt wrote:
> I have used svn_load_dirs.pl for years now and have run into a minor but 
> unfortunate problem which occurs if you specify a tag name that already 
> exists in the repository. I ran it like this [1]:
> 
> $ svn_load_dirs.pl -t 1.2.3 $REPO/vendor/foo current ./foo-1.2.3
> 
> The problem arose because the tag 1.2.3 already existed in 
> $REPO/vendor/foo. svn_load_dirs apparently does not check for this, and 
> just executes its svn cp command anyway. The result was that the 1.2.3 
> tag in the repository now contains a "current" directory with another 
> copy of the code, which is not desired. The script should instead print 
> an error message and exit if the specified tag already exists. A 
> complete reproduction recipe is below [3].
> 
> I could not see an issue in the issue tracker about this, and would like 
> to report it.

Hi Ryan,

Yes, it would be good for the script to check for it.

If you put a patch together, I can review and shepherd it through and commit it.

Regards,
Blair

-- 
Blair Zajac, Ph.D.
http://www.orcaware.com/svn/

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