corysanin/tubestation
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Bug 1621322 - Implement mach lint --revset=REV r=ahal

Browse Source 
In the process, fixed a few bugs:
 - the template eg `{file_adds % "\n{file}"}` produced a leading blank line,
   which led to everything being linted.
 - 'd' was replaced with 'r' in diff_filters, but the replacement was discarded.
 - as a result of the above, `hg status -d` was being used ("show only deleted (but tracked) files") and now it is `hg status -r` ("show only removed files"). I *think* this is what was intended?

Differential Revision: https://phabricator.services.mozilla.com/D66714
This commit is contained in:
Steve Fink
2020-03-22 22:12:18 +00:00
parent 4a62f6a23c
commit d3913d9ab7
5 changed files with 122 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
python/mozlint/mozlint/cli.py
View File

@@ -77,6 +77,12 @@ class MozlintParser(ArgumentParser):
"can be used to only consider staged files. Works with "
"mercurial or git.",
}],
[['-r', '--rev'],
{'default': None,
'type': str,
'help': "Lint files touched by changes in revisions described by REV. "
"For mercurial, it may be any revset. For git, it is a single tree-ish.",
}],
[['--fix'],
{'action': 'store_true',
'default': False,
@@ -193,7 +199,7 @@ def find_linters(config_paths, linters=None):
return lints.values()
def run(paths, linters, formats, outgoing, workdir, edit,
def run(paths, linters, formats, outgoing, workdir, rev, edit,
setup=False, list_linters=False, num_procs=None, **lintargs):
from mozlint import LintRoller, formatters
from mozlint.editor import edit_issues
@@ -213,7 +219,11 @@ def run(paths, linters, formats, outgoing, workdir, edit,
return ret
# run all linters
result = lint.roll(paths, outgoing=outgoing, workdir=workdir, num_procs=num_procs)
result = lint.roll(
paths,
outgoing=outgoing, workdir=workdir, rev=rev,
num_procs=num_procs
)
if edit and result.issues:
edit_issues(result)

6
python/mozlint/mozlint/roller.py
View File

@@ -233,7 +233,9 @@ class LintRoller(object):
# Merge this job's results with our global ones.
self.result.update(future.result())
def roll(self, paths=None, outgoing=None, workdir=None, num_procs=None):
def roll(self, paths=None,
outgoing=None, workdir=None, rev=None,
num_procs=None):
"""Run all of the registered linters against the specified file paths.
:param paths: An iterable of files and/or directories to lint.
@@ -264,6 +266,8 @@ class LintRoller(object):
try:
if workdir:
vcs_paths.update(self.vcs.get_changed_files('AM', mode=workdir))
if rev:
vcs_paths.update(self.vcs.get_changed_files('AM', rev=rev))
if outgoing:
try:
vcs_paths.update(self.vcs.get_outgoing_files('AM', upstream=outgoing))

10
python/mozlint/test/test_roller.py
View File

@@ -74,6 +74,11 @@ def test_roll_from_subdir(lint, linters):
assert len(result.issues) == 1
assert len(result.failed) == 0
assert result.returncode == 1
result = lint.roll(rev='not public() and keyword("dummy revset expression")')
assert len(result.issues) == 1
assert len(result.failed) == 0
assert result.returncode == 1
finally:
os.chdir(oldcwd)
@@ -276,6 +281,11 @@ def test_support_files(lint, linters, filedir, monkeypatch, files):
actual_files = sorted(chain(*jobs))
assert actual_files == expected_files
jobs = []
lint.roll(path, rev='draft() and keyword("dummy revset expression")')
actual_files = sorted(chain(*jobs))
assert actual_files == expected_files
# Lint config file is implicitly added as a support file
lint.mock_vcs([linter_path])
jobs = []

67
python/mozversioncontrol/mozversioncontrol/__init__.py
View File

@@ -163,7 +163,7 @@ class Repository(object):
"""Reference to the upstream remote."""
@abc.abstractmethod
def get_changed_files(self, diff_filter, mode='unstaged'):
def get_changed_files(self, diff_filter, mode='unstaged', rev=None):
"""Return a list of files that are changed in this repository's
working copy.
@@ -177,7 +177,10 @@ class Repository(object):
By default, all three will be included.
``mode`` can be one of 'unstaged', 'staged' or 'all'. Only has an
affect on git. Defaults to 'unstaged'.
effect on git. Defaults to 'unstaged'.
``rev`` is a specifier for which changesets to consider for
changes. The exact meaning depends on the vcs system being used.
"""
@abc.abstractmethod
@@ -333,33 +336,37 @@ class HgRepository(Repository):
def get_upstream(self):
return 'default'
def _format_diff_filter(self, diff_filter):
def _format_diff_filter(self, diff_filter, for_status=False):
df = diff_filter.lower()
assert all(f in self._valid_diff_filter for f in df)
# Mercurial uses 'r' to denote removed files whereas git uses 'd'.
if 'd' in df:
df.replace('d', 'r')
# When looking at the changes in the working directory, the hg status
# command uses 'd' for files that have been deleted with a non-hg
# command, and 'r' for files that have been `hg rm`ed. Use both.
return df.replace('d', 'dr') if for_status else df
return df.lower()
def get_changed_files(self, diff_filter='ADM', mode='unstaged'):
def _files_template(self, diff_filter):
template = ''
df = self._format_diff_filter(diff_filter)
if 'a' in df:
template += "{file_adds % '{file}\\n'}"
if 'd' in df:
template += "{file_dels % '{file}\\n'}"
if 'm' in df:
template += "{file_mods % '{file}\\n'}"
return template
# Use --no-status to print just the filename.
return self._run('status', '--no-status', '-{}'.format(df)).splitlines()
def get_changed_files(self, diff_filter='ADM', mode='unstaged', rev=None):
if rev is None:
# Use --no-status to print just the filename.
df = self._format_diff_filter(diff_filter, for_status=True)
return self._run('status', '--no-status', '-{}'.format(df)).splitlines()
else:
template = self._files_template(diff_filter)
return self._run('log', '-r', rev, '-T', template).splitlines()
def get_outgoing_files(self, diff_filter='ADM', upstream='default'):
df = self._format_diff_filter(diff_filter)
template = ''
if 'a' in df:
template += "{file_adds % '\\n{file}'}"
if 'd' in df:
template += "{file_dels % '\\n{file}'}"
if 'm' in df:
template += "{file_mods % '\\n{file}'}"
template = self._files_template(diff_filter)
return self._run('outgoing', '-r', '.', '--quiet',
'--template', template, upstream, return_codes=(1,)).split()
@@ -458,14 +465,20 @@ class GitRepository(Repository):
return upstream
def get_changed_files(self, diff_filter='ADM', mode='unstaged'):
def get_changed_files(self, diff_filter='ADM', mode='unstaged', rev=None):
assert all(f.lower() in self._valid_diff_filter for f in diff_filter)
cmd = ['diff', '--diff-filter={}'.format(diff_filter.upper()), '--name-only']
if mode == 'staged':
cmd.append('--cached')
elif mode == 'all':
cmd.append('HEAD')
if rev is None:
cmd = ['diff']
if mode == 'staged':
cmd.append('--cached')
elif mode == 'all':
cmd.append('HEAD')
else:
cmd = ['diff-tree', '-r', '--no-commit-id', rev]
cmd.append('--name-only')
cmd.append('--diff-filter=' + diff_filter.upper())
return self._run(*cmd).splitlines()

62
python/mozversioncontrol/test/test_workdir_outgoing.py
View File

@@ -19,9 +19,20 @@ STEPS = {
hg add baz
hg rm foo
""",
"""
hg commit -m "Remove foo; modify bar; add baz"
""",
"""
echo ooka >> baz
echo newborn > baby
hg add baby
""",
"""
hg commit -m "Modify baz; add baby"
""",
],
'git': [
"""
@@ -30,9 +41,20 @@ STEPS = {
git add baz
git rm foo
""",
"""
git commit -am "Remove foo; modify bar; add baz"
""",
"""
echo ooka >> baz
echo newborn > baby
git add baz baby
""",
"""
git commit -m "Modify baz; add baby"
""",
]
}
@@ -47,22 +69,48 @@ def test_workdir_outgoing(repo):
remotepath = '../remoterepo' if repo.vcs == 'hg' else 'upstream/master'
# Mutate files.
next(repo.step)
assert_files(vcs.get_changed_files('A', 'all'), ['baz'])
assert_files(vcs.get_changed_files('AM', 'all'), ['bar', 'baz'])
assert_files(vcs.get_changed_files('D', 'all'), ['foo'])
if repo.vcs == 'git':
assert_files(vcs.get_changed_files('AM', mode='staged'), ['baz'])
elif repo.vcs == 'hg':
assert_files(vcs.get_changed_files('AM', 'staged'), ['bar', 'baz'])
assert_files(vcs.get_outgoing_files('AM'), [])
assert_files(vcs.get_outgoing_files('AM', remotepath), [])
# Mercurial does not use a staging area (and ignores the mode parameter.)
assert_files(vcs.get_changed_files('AM', 'unstaged'), ['bar', 'baz'])
assert_files(vcs.get_outgoing_files('AMD'), [])
assert_files(vcs.get_outgoing_files('AMD', remotepath), [])
# Create a commit.
next(repo.step)
assert_files(vcs.get_changed_files('AM', 'all'), [])
assert_files(vcs.get_changed_files('AM', 'staged'), [])
assert_files(vcs.get_outgoing_files('AM'), ['bar', 'baz'])
assert_files(vcs.get_outgoing_files('AM', remotepath), ['bar', 'baz'])
assert_files(vcs.get_changed_files('AMD', 'all'), [])
assert_files(vcs.get_changed_files('AMD', 'staged'), [])
assert_files(vcs.get_outgoing_files('AMD'), ['bar', 'baz', 'foo'])
assert_files(vcs.get_outgoing_files('AMD', remotepath), ['bar', 'baz', 'foo'])
# Mutate again.
next(repo.step)
assert_files(vcs.get_changed_files('A', 'all'), ['baby'])
assert_files(vcs.get_changed_files('AM', 'all'), ['baby', 'baz'])
assert_files(vcs.get_changed_files('D', 'all'), [])
# Create a second commit.
next(repo.step)
assert_files(vcs.get_outgoing_files('AM'), ['bar', 'baz', 'baby'])
assert_files(vcs.get_outgoing_files('AM', remotepath), ['bar', 'baz', 'baby'])
if repo.vcs == 'git':
assert_files(vcs.get_changed_files('AM', rev='HEAD~1'), ['bar', 'baz'])
assert_files(vcs.get_changed_files('AM', rev='HEAD'), ['baby', 'baz'])
else:
assert_files(vcs.get_changed_files('AM', rev='.^'), ['bar', 'baz'])
assert_files(vcs.get_changed_files('AM', rev='.'), ['baby', 'baz'])
assert_files(vcs.get_changed_files('AM', rev='.^::'), ['bar', 'baz', 'baby'])
assert_files(vcs.get_changed_files('AM', rev='modifies(baz)'), ['baz', 'baby'])
if __name__ == '__main__':