Summary

Add -X support for the svn SCMClient.

Review Request #6345 — Created Sept. 20, 2014 and submitted Sept. 20, 2014, 10:57 a.m.

Information

Owner

brennie

Repository

RBTools

Branch

master

Bugs

Depends On

~~6339~~

Commit

5a8fae4...

Reviewers

Groups

rbtools, students

People

Description

The svn backend now supports excluding files through a
filterdiff-esque method: SVNClient._filter_diff. This
method looks through the output of svn diff for lines
that begin with Index: that mark the beginning of a
new file in the diff. If the file matches any given
patterns, it will not yield the lines corresponding to
that file's diff.

Testing Done

Ran rbt diff -X ... in an SVN repository and successfully
excluded files and empty files.

Issues

Description	From	Last Updated
If for some reason there's text at the beginning of the diff before the first index line (which is valid), …	david	Sept. 20, 2014, 8:30 a.m.
Should have a trailing period. Can you add more details to this comment to describe what's happening in the change? …	chipx86	Sept. 20, 2014, 9:24 a.m.
Blank line between statements and blocks.	chipx86	Sept. 20, 2014, 9:25 a.m.
Here too. Also, minor thing, but we usually just use m for this variable. Not a big deal, but doesn't …	chipx86	Sept. 20, 2014, 9:25 a.m.
And here.	chipx86	Sept. 20, 2014, 9:25 a.m.
What about on Windows?	chipx86	Sept. 20, 2014, 9:37 a.m.
Summary should be on the same line as the """.	chipx86	Sept. 20, 2014, 10:49 a.m.

Change Summary:

Add students group

Groups:

+	students

Tool: Pyflakes
Processed Files:
    rbtools/clients/svn.py

Ignored Files:
    docs/rbtools/rbt/commands/post.rst
    docs/rbtools/rbt/commands/diff.rst



Tool: PEP8 Style Checker
Processed Files:
    rbtools/clients/svn.py

Ignored Files:
    docs/rbtools/rbt/commands/post.rst
    docs/rbtools/rbt/commands/diff.rst

Change Summary:

Depend on -X support for git (for documentation changes).

Depends On:

+	~~6339 - Add support for -X to git SCMClient~~

Change Summary:

Newlines no longer included in matched filename.
Fix including self in call to _filter_diff twice.

Diff:

Revision 2 (+25 -2)

Show changes

	docs/rbtools/rbt/commands/diff.rst
	docs/rbtools/rbt/commands/post.rst
	rbtools/clients/svn.py

Tool: Pyflakes
Processed Files:
    rbtools/clients/svn.py

Ignored Files:
    docs/rbtools/rbt/commands/post.rst
    docs/rbtools/rbt/commands/diff.rst



Tool: PEP8 Style Checker
Processed Files:
    rbtools/clients/svn.py

Ignored Files:
    docs/rbtools/rbt/commands/post.rst
    docs/rbtools/rbt/commands/diff.rst

rbtools/clients/svn.py (Diff revision 2)

Show all issues

If for some reason there's text at the beginning of the diff before the first index line (which is valid), it will cause a NameError here, since include_file hasn't yet been set. Can you set include_file = True at the top of this function?

Change Summary:

Avoid NameError when diff includes lines before an Index: line

Diff:

Revision 3 (+26 -2)

Show changes

	docs/rbtools/rbt/commands/diff.rst
	docs/rbtools/rbt/commands/post.rst
	rbtools/clients/svn.py

Tool: Pyflakes
Processed Files:
    rbtools/clients/svn.py

Ignored Files:
    docs/rbtools/rbt/commands/post.rst
    docs/rbtools/rbt/commands/diff.rst



Tool: PEP8 Style Checker
Processed Files:
    rbtools/clients/svn.py

Ignored Files:
    docs/rbtools/rbt/commands/post.rst
    docs/rbtools/rbt/commands/diff.rst

rbtools/clients/svn.py (Diff revision 3)

Show all issues

Should have a trailing period.

Can you add more details to this comment to describe what's happening in the change?

I'm also interested in why we're filtering and not using -x. Did it turn out that we couldn't pass that to SVN?

brennie Sept. 20, 2014, 9:37 a.m.

SVN calls out to external diff for each file, so if we run svn diff --diff-cmd=diff '-u -x pattern, then it runs a command like:
diff -L "f (rev x)" -L "f (rev y)" old new -x pattern f. However, if f matches the given pattern, it will still spit out the diff becuase it was called on that specific file (not with, e.g., diff -x pattern -r old/ new/).

rbtools/clients/svn.py (Diff revision 3)
Show all issues
```
Blank line between statements and blocks.
```

rbtools/clients/svn.py (Diff revision 3)

Show all issues

Here too.

Also, minor thing, but we usually just use m for this variable. Not a big deal, but doesn't hurt to be consistent.

rbtools/clients/svn.py (Diff revision 3)
Show all issues
```
And here.
```

rbtools/clients/svn.py (Diff revision 3)

Show all issues

What about on Windows?

brennie Sept. 20, 2014, 9:37 a.m.

It turns out its not from svn diff. Its from SCMClient.convert_to_absolute_files.
It will always prepend the slash.

Change Summary:

Fix issues as per chipx86

Description:

		The svn backend now supports excluding files through a
~		filterdiff-esque method: `SVNClient._filter_diff`.
	~	filterdiff-esque method: `SVNClient._filter_diff`. This
	+	method looks through the output of `svn diff` for lines
	+	that begin with `Index:` that mark the beginning of a
	+	new file in the diff. If the file matches any given
	+	patterns, it will not yield the lines corresponding to
	+	that file's diff.

Commit:

+	5a8fae44e49b65ac912ccb1417ed58f6c99d0e59

Diff:

Revision 4 (+38 -2)

Show changes

	docs/rbtools/rbt/commands/diff.rst
	docs/rbtools/rbt/commands/post.rst
	rbtools/clients/svn.py

Tool: Pyflakes
Processed Files:
    rbtools/clients/svn.py

Ignored Files:
    docs/rbtools/rbt/commands/post.rst
    docs/rbtools/rbt/commands/diff.rst



Tool: PEP8 Style Checker
Processed Files:
    rbtools/clients/svn.py

Ignored Files:
    docs/rbtools/rbt/commands/post.rst
    docs/rbtools/rbt/commands/diff.rst

```
One last thing!
```

rbtools/clients/svn.py (Diff revision 4)

Show all issues

Summary should be on the same line as the """.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to release-0.6.x (f4b3936)