Summary

Change 'rbt diff' and 'rbt post' to take revisions as arguments.

Review Request #5226 — Created Jan. 9, 2014 and submitted Jan. 14, 2014, 12:13 a.m.

Information

Owner

david

Repository

RBTools

Branch

master

Bugs

Depends On

Reviewers

Groups

rbtools

People

Description

Change 'rbt diff' and 'rbt post' to take revisions as arguments.

This change updates the 'diff' and 'post' commands to treat extra arguments as
the revision spec, and adds a new -I/--include flag to specify any files to
limit the diff to.

This is done for all SCMs except ClearCase. I need to work through all the
pending ClearCase patches and read through the proposal on how to fit it into
this new world order.

This also includes some changes to the way that changenums are pulled out for
perforce and plastic. Ideally I'd like to get rid of the server-side
summary/description fetch and implement the --guess-* options, but that would
likely piss off a lot of users. Instead, I've just moved the changenum
extraction into the diff methods, so that we don't duplicate the work of
parsing revisions in two different ways. This also returns 'None' for submitted
changelists (for now) because the server-side code is broken at the moment.

Whether or not files are supported is currently client-dependent. Most already
did, and I've added it for git. It's still not done for Plastic, since my
Plastic expertise is exactly zero.

Testing Done


Ran unit tests
Checked diffs and extracted summaries for:
bzr backend with zero, one, and two-arg forms.
cvs backend with zero arguments. I haven't tested the two-arg form because

    after trying four different public CVS repos, I have yet to find one that

    (a) has a CVSROOT, (b) allows me to check it out, (c) allows me to do a

    diff between two tags, and (d) have that diff not contain broken text

    encodings.
hg backend with zero, one, and two-arg forms.
hgsubversion backend with zero, one, and two-arg forms.
git backend with zero, one, and two-arg forms.
git-p4 backend with zero, one, and two-arg forms.
git-svn backend with zero, one, and two-arg forms.
perforce backend with zero, one, and two-arg forms, including submitted and

    shelved changesets.
svn backend with zero, one, and two-arg forms, including changelists.

Issues

Description	From	Last Updated
I think RBTools' current approach for Mercurial is wrong. This patch (along with the previous code) is very happy about …	IN indygreg	Jan. 9, 2014, 3:13 p.m.
"Subversion"	chipx86	Jan. 9, 2014, 1:33 p.m.
Should add a trailing period.	chipx86	Jan. 9, 2014, 1:33 p.m.
Same comments as with diff.	chipx86	Jan. 9, 2014, 1:33 p.m.

rbtools/clients/tests.py (Diff revision 1)

What's this addition for?

david Jan. 9, 2014, 1:59 p.m.

It's needed because I moved the call to _depot_to_local into the diff routines themselves. We now get the local filename even for deleted files, so that it can be tested against the -I parameters.

rbtools/commands/diff.py (Diff revision 1)
The issue has been resolved. Show all issues
```
"Subversion"
```
rbtools/commands/diff.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Should add a trailing period.
```
rbtools/commands/post.py (Diff revision 1)
The issue has been resolved. Show all issues
```
Same comments as with diff.
```

rbtools/clients/mercurial.py (Diff revision 1)

The issue has been dropped. Show all issues

I think RBTools' current approach for Mercurial is wrong. This patch (along with the previous code) is very happy about using 'tip' and the revlog revision range to construct outgoing changesets and diffs.

Please note that the revlog revision order is different from the DAG. The only guaranteed similarity is that a single revision's parent must come before the child in the revlog.

I have a set of outstanding patches (which will conflict significantly with this patch) that do away with blanket hg out (without -r). This ensures that every changeset under consideration by RBTools chains up through the DAG to a specific parent. Otherwise, we may have changesets belonging to other heads creaping in.

FWIW, your refactoring of adding revspec to all the functions is very similar to what I've done. I think we share many of the same opinions on what is needed to improve Mercurial support. There's just a few issues to work out.

Perhaps the Mercurial refactoring should be done as a standalone patch before the API change on rbt diff and post?

david Jan. 9, 2014, 3:19 p.m.

I wasn't aware that revlog (or even hg out) wasn't traversing the DAG. I guess I've been in the bowels of git so much that I make assumptions that don't hold true. I agree that traversing the DAG up from the desired head is much better.

I anticipate that this change will be pending for a while, since there's really a ton of testing that needs to be done with all the different backends (plus I haven't even tried to do anything with clearcase, yet). Feel free to post your changes as you think they're ready, and we can figure out the right time to merge our work together.

indygreg Jan. 9, 2014, 3:46 p.m.

Will do!

FWIW, I posted more comments on the hackpad on what I think Mercurial's behavior should be. Not sure if they are reviewed/posted yet.

david Jan. 9, 2014, 3:48 p.m.

I haven't received a moderation email about it yet. I think I'll just unmoderate it for now.

I have a few patches that conflict with this and your other patch. This patch makes me very happy and is larger in scope than just Mercurial. I'm totally fine with waiting for this patch to land and rebasing my work on top of it.

Change Summary:


Make requested changes
Fix arguments to get_diff in 'rbt post'

Description:

		Change 'rbt diff' and 'rbt post' to take revisions as arguments.

		This is a preliminary post that will probably change as I test things more
		thoroughly. I just wanted to get feedback on what I have.

		This change updates the 'diff' and 'post' commands to treat extra arguments as
		the revision spec, and adds a new -I/--include flag to specify any files to
		limit the diff to.

		This is done for all SCMs except ClearCase. I need to work through all the
		pending ClearCase patches and read through the proposal on how to fit it into
		this new world order.

		Whether or not files are supported is currently client-dependent. Most already
		did, and I've added it for git. It's still not done for Plastic, since my
		Plastic expertise is exactly zero.

Testing Done:

~		Ran unit tests.
~		Need to do a ton more with each SCM.
	~	Ran unit tests
	~	Tested zero, one, and two-arg posting with the Git backend. Verified that the resulting diffs and guessed summaries/descriptions were correct.

Diff:

Revision 2 (+265 -272)

Show changes

	rbtools/clients/__init__.py
	rbtools/clients/bazaar.py
	rbtools/clients/cvs.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/clients/perforce.py
	rbtools/clients/plastic.py
	rbtools/clients/svn.py
	4 more

Change Summary:

Fix some issues I encountered while testing the SVN backend.

Description:

		Change 'rbt diff' and 'rbt post' to take revisions as arguments.

		This is a preliminary post that will probably change as I test things more
		thoroughly. I just wanted to get feedback on what I have.

		This change updates the 'diff' and 'post' commands to treat extra arguments as
		the revision spec, and adds a new -I/--include flag to specify any files to
		limit the diff to.

		This is done for all SCMs except ClearCase. I need to work through all the
		pending ClearCase patches and read through the proposal on how to fit it into
		this new world order.

		Whether or not files are supported is currently client-dependent. Most already
		did, and I've added it for git. It's still not done for Plastic, since my
		Plastic expertise is exactly zero.

Testing Done:

		Ran unit tests
~		Tested zero, one, and two-arg posting with the Git backend. Verified that the resulting diffs and guessed summaries/descriptions were correct.
	~	Tested the Git backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
	+	Tested the SVN backend with zero, one, and two revisions, as well as changelists. Verified that the resulting diffs were correct.

Diff:

Revision 3 (+268 -274)

Show changes

	rbtools/clients/__init__.py
	rbtools/clients/bazaar.py
	rbtools/clients/cvs.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/clients/perforce.py
	rbtools/clients/plastic.py
	rbtools/clients/svn.py
	4 more

Change Summary:


Fix issues with perforce backend.
Change the way that changenums are extracted.
Remove check for p4d 2002.2. I think it's safe to say that people aren't running a version > 10 years old.

Description:

		Change 'rbt diff' and 'rbt post' to take revisions as arguments.

		This is a preliminary post that will probably change as I test things more
		thoroughly. I just wanted to get feedback on what I have.

		This change updates the 'diff' and 'post' commands to treat extra arguments as
		the revision spec, and adds a new -I/--include flag to specify any files to
		limit the diff to.

		This is done for all SCMs except ClearCase. I need to work through all the
		pending ClearCase patches and read through the proposal on how to fit it into
		this new world order.

	+	This also includes some changes to the way that changenums are pulled out for
	+	perforce and plastic. Ideally I'd like to get rid of the server-side
	+	summary/description fetch and implement the --guess-* options, but that would
	+	likely piss off a lot of users. Instead, I've just moved the changenum
	+	extraction into the diff methods, so that we don't duplicate the work of
	+	parsing revisions in two different ways. This also returns 'None' for submitted
	+	changelists (for now) because the server-side code is broken at the moment.
	+
		Whether or not files are supported is currently client-dependent. Most already
		did, and I've added it for git. It's still not done for Plastic, since my
		Plastic expertise is exactly zero.

Testing Done:

		Ran unit tests
		Tested the Git backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
~		Tested the SVN backend with zero, one, and two revisions, as well as changelists. Verified that the resulting diffs were correct.
	~	Testing the Perforce backend with zero, one, and two revisions, including submitted and shelved changesets. Verified that the resulting diffs and summaries/descriptions were correct.
	+	Tested the SVN backend with zero, one, and two revisions, as well as changelists. Verified that the resulting diffs were correct.

Diff:

Revision 4 (+290 -348)

Show changes

	rbtools/clients/__init__.py
	rbtools/clients/bazaar.py
	rbtools/clients/cvs.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/clients/perforce.py
	rbtools/clients/plastic.py
	rbtools/clients/svn.py
	4 more

Change Summary:

Updated testing to list git-svn backend.

Testing Done:

		Ran unit tests
		Tested the Git backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
~		Testing the Perforce backend with zero, one, and two revisions, including submitted and shelved changesets. Verified that the resulting diffs and summaries/descriptions were correct.
~		Tested the SVN backend with zero, one, and two revisions, as well as changelists. Verified that the resulting diffs were correct.
	~	Tested the Git-SVN backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
	~	Testing the Perforce backend with zero, one, and two revisions, including submitted and shelved changesets. Verified that the resulting diffs and summaries/descriptions were correct.
	+	Tested the SVN backend with zero, one, and two revisions, as well as changelists. Verified that the resulting diffs were correct.

Change Summary:

Fix the parameters to extract_description for mercurial.

Testing Done:

		Ran unit tests
~		Tested the Git backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
~		Tested the Git-SVN backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
~		Testing the Perforce backend with zero, one, and two revisions, including submitted and shelved changesets. Verified that the resulting diffs and summaries/descriptions were correct.
~		Tested the SVN backend with zero, one, and two revisions, as well as changelists. Verified that the resulting diffs were correct.
	~	Tested the hgsubversion backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
	~	Tested the Git backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
	~	Tested the Git-SVN backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
	~	Testing the Perforce backend with zero, one, and two revisions, including submitted and shelved changesets. Verified that the resulting diffs and summaries/descriptions were correct.
	+	Tested the SVN backend with zero, one, and two revisions, as well as changelists. Verified that the resulting diffs were correct.

Diff:

Revision 5 (+291 -349)

Show changes

	rbtools/clients/__init__.py
	rbtools/clients/bazaar.py
	rbtools/clients/cvs.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/clients/perforce.py
	rbtools/clients/plastic.py
	rbtools/clients/svn.py
	4 more

Change Summary:

git-p4 testing details

Testing Done:

		Ran unit tests
		Tested the hgsubversion backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
		Tested the Git backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
~		Tested the Git-SVN backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
~		Testing the Perforce backend with zero, one, and two revisions, including submitted and shelved changesets. Verified that the resulting diffs and summaries/descriptions were correct.
~		Tested the SVN backend with zero, one, and two revisions, as well as changelists. Verified that the resulting diffs were correct.
	~	Tested the Git-P4 backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
	~	Tested the Git-SVN backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
	~	Testing the Perforce backend with zero, one, and two revisions, including submitted and shelved changesets. Verified that the resulting diffs and summaries/descriptions were correct.
	+	Tested the SVN backend with zero, one, and two revisions, as well as changelists. Verified that the resulting diffs were correct.

Change Summary:

Update testing info to include hg, reformat for readability.

Testing Done:

		Ran unit tests
~		Tested the hgsubversion backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
~		Tested the Git backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
~		Tested the Git-P4 backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
~		Tested the Git-SVN backend with zero, one, and two-arg posting. Verified that the resulting diffs and guessed summaries/descriptions were correct.
~		Testing the Perforce backend with zero, one, and two revisions, including submitted and shelved changesets. Verified that the resulting diffs and summaries/descriptions were correct.
~		Tested the SVN backend with zero, one, and two revisions, as well as changelists. Verified that the resulting diffs were correct.
	~	Checked diffs and extracted summaries for:
	~	hg backend with zero, one, and two-arg forms.
	~	hgsubversion backend with zero, one, and two-arg forms.
	~	git backend with zero, one, and two-arg forms.
	~	git-p4 backend with zero, one, and two-arg forms.
	~	git-svn backend with zero, one, and two-arg forms.
	+	perforce backend with zero, one, and two-arg forms, including submitted and shelved changesets.
	+	svn backend with zero, one, and two-arg forms, including changelists.

Change Summary:

CVS testing info

Testing Done:

		Ran unit tests
		Checked diffs and extracted summaries for:
~		hg backend with zero, one, and two-arg forms.
~		hgsubversion backend with zero, one, and two-arg forms.
~		git backend with zero, one, and two-arg forms.
~		git-p4 backend with zero, one, and two-arg forms.
~		git-svn backend with zero, one, and two-arg forms.
~		perforce backend with zero, one, and two-arg forms, including submitted and shelved changesets.
~		svn backend with zero, one, and two-arg forms, including changelists.
	~	cvs backend with zero arguments. I haven't tested the two-arg form because after trying four different public CVS repos, I have yet to find one that (a) has a CVSROOT, (b) allows me to check it out, (c) allows me to do a diff between two tags, and (d) have that diff not contain broken text encodings.
	~	hg backend with zero, one, and two-arg forms.
	~	hgsubversion backend with zero, one, and two-arg forms.
	~	git backend with zero, one, and two-arg forms.
	~	git-p4 backend with zero, one, and two-arg forms.
	~	git-svn backend with zero, one, and two-arg forms.
	~	perforce backend with zero, one, and two-arg forms, including submitted and shelved changesets.
	+	svn backend with zero, one, and two-arg forms, including changelists.

Change Summary:

Final update: ready for review!

Description:

		Change 'rbt diff' and 'rbt post' to take revisions as arguments.

-		This is a preliminary post that will probably change as I test things more
-		thoroughly. I just wanted to get feedback on what I have.
-
		This change updates the 'diff' and 'post' commands to treat extra arguments as
		the revision spec, and adds a new -I/--include flag to specify any files to
		limit the diff to.

		This is done for all SCMs except ClearCase. I need to work through all the
		pending ClearCase patches and read through the proposal on how to fit it into
		this new world order.

		This also includes some changes to the way that changenums are pulled out for
		perforce and plastic. Ideally I'd like to get rid of the server-side
		summary/description fetch and implement the --guess-* options, but that would
		likely piss off a lot of users. Instead, I've just moved the changenum
		extraction into the diff methods, so that we don't duplicate the work of
		parsing revisions in two different ways. This also returns 'None' for submitted
		changelists (for now) because the server-side code is broken at the moment.

		Whether or not files are supported is currently client-dependent. Most already
		did, and I've added it for git. It's still not done for Plastic, since my
		Plastic expertise is exactly zero.

Testing Done:

		Ran unit tests
		Checked diffs and extracted summaries for:
~		cvs backend with zero arguments. I haven't tested the two-arg form because after trying four different public CVS repos, I have yet to find one that (a) has a CVSROOT, (b) allows me to check it out, (c) allows me to do a diff between two tags, and (d) have that diff not contain broken text encodings.
~		hg backend with zero, one, and two-arg forms.
~		hgsubversion backend with zero, one, and two-arg forms.
~		git backend with zero, one, and two-arg forms.
~		git-p4 backend with zero, one, and two-arg forms.
~		git-svn backend with zero, one, and two-arg forms.
~		perforce backend with zero, one, and two-arg forms, including submitted and shelved changesets.
~		svn backend with zero, one, and two-arg forms, including changelists.
	~	bzr backend with zero, one, and two-arg forms.
	~	cvs backend with zero arguments. I haven't tested the two-arg form because after trying four different public CVS repos, I have yet to find one that (a) has a CVSROOT, (b) allows me to check it out, (c) allows me to do a diff between two tags, and (d) have that diff not contain broken text encodings.
	~	hg backend with zero, one, and two-arg forms.
	~	hgsubversion backend with zero, one, and two-arg forms.
	~	git backend with zero, one, and two-arg forms.
	~	git-p4 backend with zero, one, and two-arg forms.
	~	git-svn backend with zero, one, and two-arg forms.
	~	perforce backend with zero, one, and two-arg forms, including submitted and shelved changesets.
	+	svn backend with zero, one, and two-arg forms, including changelists.

Diff:

Revision 6 (+295 -353)

Show changes

	rbtools/clients/__init__.py
	rbtools/clients/bazaar.py
	rbtools/clients/cvs.py
	rbtools/clients/git.py
	rbtools/clients/mercurial.py
	rbtools/clients/perforce.py
	rbtools/clients/plastic.py
	rbtools/clients/svn.py
	4 more

Ship it!

Looks good!

Was there really nothing more to do with the Bazaar support?

Status: Closed (submitted)

Change Summary:

Pushed to master (cf70a4c)