You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Ben Collins-Sussman <su...@collab.net> on 2003/07/06 14:15:57 UTC

Re: [PATCH] merge fails when target's directory of changed file removed

Chia-liang Kao <cl...@clkao.org> writes:

> Hi,
> 
> attached is a test case.

Thanks!  Next time, can you put a [PATCH] is the subject next time?  I
just added it, so this thread will be filtered properly.  This is
documented in HACKING.

> I'm not sure what behaviour should it be, maybe print a warning 
> or so.  but it shouldn't fail IMHO.

I agree; this is only one example where 'svn merge' is too fragile.
It should print warnings whenever a change can't be applied, rather
than throwing an error and quitting.  I've seen other examples of this
kind of behavior.

> I tried to fix it in merge_file_changed, because svn_wc_merge
> requires target to exist. but svn_repos_dir_delta only gets in 
> merge_file_changed when dry-run is turned on. 
> 
> maybe someone familiar with the reporter/update_editor could 
> fix this easily.

Can you file this recipe/patch as an issue?  We have a number of 'svn
merge' bugs.


> Index: merge_tests.py
> ===================================================================
> --- merge_tests.py	(revision 6392)
> +++ merge_tests.py	(working copy)
> @@ -843,6 +843,52 @@
>      os.chdir(saved_cwd)

I'm thrilled that you're writing regression tests... but let me give
you a few pointers.  :-)

> +  outlines,errlines = svntest.main.run_svn(None, 'ci', '-m', 'rev 3', B_path)
> +  if errlines:
> +    raise svntest.Failure
[...]
> +  outlines,errlines = svntest.main.run_svn(None, 'up', os.path.join(wc_dir,'A'))
> +  if errlines:
> +    raise svntest.Failure


If you look throughout our test suite, you'll notice that this isn't
the way we perform commits or updates.  The "proper" way is to build
three trees: an expected output tree ("what output paths will the
command print?"), an expected status tree ("what will 'svn st -v' say
after the command finishes?"), and an expected disk tree ("what will
the wc file contents be after the command finishes?") That's why we
have a python tree-class.  Once you build the expected trees, you pass
them to svntest.actions.run_and_verify_[commit|update](), which runs
the command and compares actual trees with expected trees.

If you simply check for error, you don't really know if the command
behaved the way you expected; you just know that it didn't bomb out. :-)

> +  saved_cwd = os.getcwd()
> +  try:
> +    os.chdir(I_path)
> +    out, err = svntest.main.run_svn(0, 'merge', '-r', '2:3', B_url)
> +    if err:
> +      raise svntest.Failure
> +  finally:
> +    os.chdir(saved_cwd)

Yikes!  Why do you have to chdir() here?  Why not just run 

   'svn merge -r 2:3 B_url I_path'

The last argument certainly doesn't have to be '.' !

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

Re: [PATCH] merge fails when target's directory of changed file removed

Posted by Chia-liang Kao <cl...@clkao.org>.
#1399.

On Mon, Jul 14, 2003 at 04:13:38PM -0400, Paul L Lussier wrote:
> >I'll attach the updated patch to the issue I file. thanks for the hints!
> 
> Was this issue ever filed?  If so, what is the issue# ?

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

Re: [PATCH] merge fails when target's directory of changed file removed

Posted by Paul L Lussier <pl...@lanminds.com>.
In a message dated: Sun, 06 Jul 2003 23:07:22 +0800
Chia-liang Kao said:

>I'll attach the updated patch to the issue I file. thanks for the hints!

Was this issue ever filed?  If so, what is the issue# ?

Thanks,
-- 

Seeya,
Paul
--
Key fingerprint = 1660 FECC 5D21 D286 F853  E808 BB07 9239 53F1 28EE

	It may look like I'm just sitting here doing nothing,
   but I'm really actively waiting for all my problems to go away.

	 If you're not having fun, you're not doing it right!



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

Re: [PATCH] merge fails when target's directory of changed file removed

Posted by Chia-liang Kao <cl...@clkao.org>.
On Sun, Jul 06, 2003 at 09:15:57AM -0500, Ben Collins-Sussman wrote:
> Can you file this recipe/patch as an issue?  We have a number of 'svn
> merge' bugs.

I found this related to #1176 in synopsis. for #1176 it deals with file
targets, while this one is targets in deleted directory. #1176 is fixed
in svn_merge_wc, while the directory thing seems to be problem of reporter
or delta driver. So I'll file a new issue instead of reopening #1176

> I'm thrilled that you're writing regression tests... but let me give
> you a few pointers.  :-)

heh, just found it a better way describing the problem and also have
additional benefits (such as thrilling people)

> If you look throughout our test suite, you'll notice that this isn't
> the way we perform commits or updates.  The "proper" way is to build
> three trees: an expected output tree ("what output paths will the
> command print?"), an expected status tree ("what will 'svn st -v' say
> after the command finishes?"), and an expected disk tree ("what will
> the wc file contents be after the command finishes?") That's why we
> have a python tree-class.  Once you build the expected trees, you pass
> them to svntest.actions.run_and_verify_[commit|update](), which runs
> the command and compares actual trees with expected trees.
>
> If you simply check for error, you don't really know if the command
> behaved the way you expected; you just know that it didn't bomb out. :-)

ya, that's what i'm trying to do. since the result of ignored entry is still
undetermined (a warning or maybe some flag in merge result status).

and I was just copying from a test "merge_catches_nonexistent_target" above.
my python skill is still copy+edit.

I'll attach the updated patch to the issue I file. thanks for the hints!

Cheers,
CLK

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