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

[Raman Gupta]

Daniel Rall wrote:
> Comments inline, all the way through.  Nearly ready for commit.
> 
> On Wed, 10 May 2006, Raman Gupta wrote:
> ...
>> Attached v3 of the patch, and new 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.
>>   (TestCase_TestRepo.testUninitForce): New test case.
> 
> Missing a change log for this guy:
> 
>     (TestCase_TestRepo.getproperty): ...

Oops, I'll add it depending on your response to my comments below.

> More on him below.
> 
>> Patch by: Raman Gupta <rocketraman at fastmail.fm>
>> Review by: Daniel Rall <dlr at collab.net>
>>            Giovanni Bajo <rasky at develer.com>
> 
> You can add Madan in here, too:
> 
>              Madan U Sreenivasan <madan at collab.net>

When I send the final patch I'll do that.

> Index: svnmerge.py
> ===================================================================
> --- svnmerge.py	(revision 19599)
> +++ svnmerge.py	(working copy)
> @@ -1191,7 +1191,30 @@
>          f.close()
>          report('wrote commit message to "%s"' % opts["commit-file"])
>  
> +def action_uninit(branch_dir, branch_props):
> +    """Uninit HEAD URL."""
> +    # 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))
> +
> +    del branch_props[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
>  ###############################################################################
> @@ -1640,6 +1663,14 @@
>      [
>          "-f", "-r", "-S", # import common opts
>      ]),
> +
> +    "uninit": (action_uninit,
> +    "uninit [OPTION...] [HEAD]",
> +    """Remove merge tracking information for HEAD on the current working
> +    directory.""",
> +    [
> +        "-f", "-S", # import common opts
> +    ]),
>  }
>  
>  
> Index: svnmerge_test.py
> ===================================================================
> --- svnmerge_test.py	(revision 19599)
> +++ svnmerge_test.py	(working copy)
> @@ -456,7 +456,10 @@
>  
>      def getproperty(self):
>          out = svnmerge.launch("svn pg %s ." % svnmerge.opts["prop"])
> -        return out[0].strip()
> +        if len(out) == 0:
> +            return None
> +        else:
> +            return out[0].strip()
>  
> 
> I can't say that I understand the reasoning behind this change.  Let's
> look at the usage below...

The reason is that if svn pg is called and the property does not exist,
then out[0] throws an array index exception. This has only come up now
because only my new test calls this when the property does not exist.

>      def testNoWc(self):
>          os.mkdir("foo")
> @@ -562,6 +565,56 @@
>          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", "--source", self.test_repo_url + "/branches/testYYY-branch"])
> +
> +        p = self.getproperty()
> +        self.assertEqual("/branches/test-branch:1-13", p)
> +
> +        self.launch("svn commit -F svnmerge-commit-message.txt",
> +                    match=r"Committed revision")
> +
> +        self.svnmerge2(["uninit", "--source", self.test_repo_url + "/branches/test-branch"])
> +
> +        p = self.getproperty()
> +        self.assertEqual(None, p)
> 
> So, it's allowing you to test for None instead of "" in this one
> place.  Can you describe the benefits of this?

See above.

> +
> +    def testUninitForce(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)
> +
> +        open("test1", "a").write("foo")
> +
> +        self.svnmerge("uninit --source " + self.test_repo_url + "/branches/testYYY-branch",
> +                      error=True, match=r"clean")
> +
> +        self.svnmerge("uninit -F --source " + self.test_repo_url + "/branches/testYYY-branch")
> +        p = self.getproperty()
> +        self.assertEqual("/branches/test-branch:1-13", p)
> +
>      def testCheckInitializeEverything(self):
>          self.svnmerge2(["init", self.test_repo_url + "/trunk"])
>          p = self.getproperty()
> 

Cheers,
Raman