[Svnmerge] [PATCH] uninit merge tracking (version 2)

[Raman Gupta]

Daniel Rall wrote:
> On Mon, 24 Apr 2006, Raman Gupta wrote:
> 
>> Patch for uninitialization of merge tracking given a head URL.
> 
> Raman, can you describe the use case for this feature, and why it's
> not satisfied by 'svn propdel'?

Sure. "svn propdel" does not work if you are tracking merge information
for multiple heads -- clearly, you would end up deleting more info than
desired. The uninit command removes merge tracking information for one
head only without having the user manipulate the property and manually
execute "svn propset".

> (Personally, I do prefer having real commands do provide a layer of
> insulation between the user and the properties where svnmerge stores
> its meta data.)

Absolutely.

> A few comments inline below.
> 
>> Add the uninit command, which removes merge tracking information for a
>> given head URL.
>>
>> * svnmerge.py: Added uninit to command table.
>>   (action_uninit): New method for uninitialization of merge tracking.
> 
> ...info.

Ok.

>> * svnmerge_test.py
>>   (TestCase_TestRepo.testUninit): New test case.
>>
>> Patch by: Raman Gupta <rocketraman at fastmail.fm>
>> Review by: ??
> 
> Index: svnmerge_test.py
> ===================================================================
> --- svnmerge_test.py	(revision 6073)
> +++ svnmerge_test.py	(working copy)
> @@ -562,6 +562,25 @@
>          p = self.getproperty()
>          self.assertEqual("/trunk:1-6", p)
>  
> +    def testUninit(self):
> +        self.svnmerge2(["init", self.test_repo_url + "/branches/test-branch"])
> +
> +        self.launch("svn commit -F svnmerge-commit-message.txt",
> +                    match=r"Committed revision")
> +
> +        self.svnmerge2(["init", self.test_repo_url + "/branches/testYYY-branch"])
> +
> +        self.launch("svn commit -F svnmerge-commit-message.txt",
> +                    match=r"Committed revision")
> +
> +        p = self.getproperty()
> +        self.assertEqual("/branches/test-branch:1-13 /branches/testYYY-branch:1-14", p)
> +
> +        self.svnmerge2(["uninit", "--head", self.test_repo_url + "/branches/testYYY-branch"])
> 
> --source or -S is preferred to the deprecated terminology "head".

Done.

> +
> +        p = self.getproperty()
> +        self.assertEqual("/branches/test-branch:1-13", p)
> +
> 
> Should there also be a test which checks whether all merge information
> can be cleared?

Yes, added. I have also added a testUninitForce test to mirror the 
testInitForce test.

>      def testCheckInitializeEverything(self):
>          self.svnmerge2(["init", self.test_repo_url + "/trunk"])
>          p = self.getproperty()
> Index: svnmerge.py
> ===================================================================
> --- svnmerge.py	(revision 6073)
> +++ svnmerge.py	(working copy)
> @@ -1186,7 +1186,30 @@
>          f.close()
>          report('wrote commit message to "%s"' % opts["commit-file"])
>  
> +def action_uninit(branch_dir, branch_props):
> +    """Uninit HEAD URL."""
> 
> Again, "head" terminology.

I am simply being consistent with the help and pydocs for most, if not
all, the other actions. I would suggest we leave this patch as is, and
change all uses of "head" to "source" in a separate commit.

> +    # Check branch directory is ready for being modified
> +    check_dir_clean(branch_dir)
>  
> +    # If the head-path already has an entry in the svnmerge-integrated
> +    # property, simply error out.
> +    if not branch_props.has_key(opts["head-path"]):
> +        error('%s does not contain merge tracking information for %s' \
> +                % (opts["head-path"], branch_dir))
> 
> Both those format strings should probably be quoted (double quotes
> seem to be commonly used throughout svnmerge.py).

Done.

> +
> +    branch_props.pop(opts["head-path"])
> +
> +    # Set property
> +    set_merge_props(branch_dir, branch_props)
> +
> +    # Write out commit message if desired
> +    if opts["commit-file"]:
> +        f = open(opts["commit-file"], "w")
> +        print >>f, 'Removed merge tracking for "%s" for ' % NAME
> +        print >>f, '%s' % opts["head-url"]
> +        f.close()
> +        report('wrote commit message to "%s"' % opts["commit-file"])
> +
>  ###############################################################################
>  # Command line parsing -- options and commands management
>  ###############################################################################
> @@ -1635,6 +1658,14 @@
>      [
>          "-f", "-r", "-S", # import common opts
>      ]),
> +
> +    "uninit": (action_uninit,
> +    "uninit [OPTION...] [HEAD]",
> 
> This should be "URL" rather than "HEAD".

Using "HEAD" is consistent with the documentation for the init command. Though
I agree with the change to "URL", as before, I would suggest that the change
be done in a separate commit.

> +    """Remove merge tracking information for HEAD on the current working
> +    directory.""",
> +    [
> +        "-f", "-S", # import common opts
> +    ]),
>  }

Patch v2 based on your comments attached, and applied to the latest sources.
Here is the new commit log message:

Add the uninit command, which removes merge tracking information for a given
head URL. This is useful if multiple heads are being tracked -- without 
uninit, this situation requires the new property value to be manually set by
the user via svn propset.

* svnmerge.py: Added uninit to command table.
  (action_uninit): New method for uninitialization of merge tracking info.

* svnmerge_test.py
  (TestCase_TestRepo.testUninit): New test case.

Patch by: Raman Gupta <rocketraman at fastmail.fm>
Review by: Daniel Rall <dlr at collab.net>


Cheers,
Raman
-------------- next part --------------
A non-text attachment was scrubbed...
Name: uninit_v2.patch
Type: text/x-patch
Size: 4363 bytes
Desc: not available
Url : /pipermail/svnmerge/attachments/20060510/43bd1bf5/attachment.bin