rooneg@gmail.com wrote on 06/05/2006 01:20:19 PM:
> On 6/5/06, Paul Burba <paulb@softlanding.com> wrote:
> > Hi All,
> >
> > Came across this tidbit this morning:
> >
> > copy_tests.py has two tests in it's test_list named
"mv_unversioned_file".
> > There are also two definitions for mv_unversioned_file, so only the
> > second is used. I was going to simply rename the second instance and
> > commit it, but then I looked into things a bit further...never look
into
> > things a bit further it just causes pain :-D
> >
> > Both tests try to move unversioned files and were added to test fixes
that
> > prevented seg faults. The only significant difference between the two
is
> > that the second test uses the --force option.
> >
> > The first mv_unversioned_file is from was back in the day, r1106. This
> > revision also modified copy.c's copy_file_administratively() to check
if
> > the source of a move is unversioned.
> >
> > The second mv_unversioned_file is from r17242 & r17243 and was added
as a
> > test for issue #2436. r17243 checks if the source of a move is
versioned
> > in svn_wc_copy2().
> >
> > It appears that the check of the move source in svn_wc_copy2() makes
the
> > check in copy_file_administratively() redundant, since the check in
the
> > former is made before calling the latter and svn_wc_copy2() is the
only
> > caller of copy_file_administratively(). FWIW, the comment for
> > copy_file_administratively() states clearly that we *shouldn't* have
to
> > check for this error:
> >
> > /* This function effectively creates and schedules a file for
> > addition, but does extra administrative things to allow it to
> > function as a 'copy'.
> >
> > ASSUMPTIONS:
> >
> > - src_path points to a file under version control
> >
> > Given the above I have two questions, should I:
> >
> > Q1) Just rename the second instance of mv_unversioned_file to
> > mv_unversioned_file2 and call it a day? Or combine the two instances
of
> > mv_unversioned_file into one test?
>
> If it's easy to combine them into one test, I'd do it. Saving on test
> framework setup time is a good thing.
Done r19963. I made one mv_unversioned_file test that tries to move an
unversioned file without --force and then tries to move another
unversioned file with --force. That is all the other two tests did.
> > Q2) Just leave copy.c alone, change the comment for
> > copy_file_administratively, or remove the check for "versioned-ness"
in
> > copy_file_administratively():
> >
> > Index: libsvn_wc/copy.c
> > ===================================================================
> > --- copy.c (revision 19933)
> > +++ copy.c (working copy)
> > @@ -94,11 +94,6 @@
> > in the repository. See comment at the bottom of this file for
an
> > explanation. */
> > SVN_ERR(svn_wc_entry(&src_entry, src_path, src_access, FALSE,
pool));
> > - if (! src_entry)
> > - return svn_error_createf
> > - (SVN_ERR_UNVERSIONED_RESOURCE, NULL,
> > - _("Cannot copy or move '%s': it's not under version control"),
> > - svn_path_local_style(src_path, pool));
> > if ((src_entry->schedule == svn_wc_schedule_add)
> > || (! src_entry->url)
> > || (src_entry->copied))
> >
> > Thoughts?
>
> If we're already retrieving that entry and checking for it to be NULL
> further up the stack, then I imagine we can get away with removing
> that check, although I'm not sure if we really should. If we do, then
> the doc comment for copy_file_administratively should be more strict,
> right now it says that src_path must point to a file under version
> control, which isn't quite the same as saying "it must point to
> something we've already called svn_wc_entry on so that it'll be
> certain to be in the entries cache, cause we're not going to check
> that it's not NULL"...
>
> Or maybe i'm just being paranoid...
No, I'm just being too trusting/stupid - I'll leave the code as-is.
Paul B.
_____________________________________________________________________________
Scanned for SoftLanding Systems, Inc. and SoftLanding Europe Plc by IBM Email Security Management Services powered by MessageLabs.
_____________________________________________________________________________
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jun 6 16:33:03 2006