Bug 1361172 - Rewrite code for finding files in VCS checkout; r=glandium

We're getting an intermittent failure running `hg manifest` in CI. I
have no clue why.

What I do know is that we now have the mozversioncontrol Python package
for containing utility code for interacting with version control. It is
slightly more robust and I'm willing to support it more than I am
check_utils.py.

This commit adds a new API to our abstract repository class to obtain the
files in the working directory by querying version control.

Since I suspect cwd was coming into play in automation, I've also
added a utility function to mozversioncontrol to attempt to find
a version control checkout from the current working directory. It
simply traces ancestor paths looking for a .hg or .git directory.

Finally, I've ported all callers of the now-deleted API to the new
one. The old code had some "../.." paths in it, meaning it only
worked when cwd was just right. Since we resolve the absolute path
to the checkout and store it on the repo object, I've updated the
code so it should work no matter what cwd is as long as a repo can
be found. I'm not 100% confident I found all consumers assuming cwd.
But it's a start.

I'm not 100% confident this will fix the intermittent issues in CI. But
at least we should get a better error message and at least we'll be
running less hacky code.

MozReview-Commit-ID: AmCraHXcTEX

--HG--
extra : rebase_source : 815ae369776577ad374333920fd645d412a55148
This commit is contained in:
Gregory Szorc 2017-05-18 16:06:49 -07:00
Родитель 9a8d52f7af
Коммит 2b1957d1e5
5 изменённых файлов: 65 добавлений и 45 удалений

Просмотреть файл

@ -13,7 +13,9 @@ from __future__ import print_function
import os import os
import sys import sys
from check_utils import get_all_toplevel_filenames
from mozversioncontrol import get_repository_from_env
scriptname = os.path.basename(__file__); scriptname = os.path.basename(__file__);
expected_encoding = 'ascii' expected_encoding = 'ascii'
@ -45,10 +47,13 @@ def check_single_file(filename):
def check_files(): def check_files():
result = True result = True
for filename in get_all_toplevel_filenames(): repo = get_repository_from_env()
root = repo.path
for filename in repo.get_files_in_working_directory():
if filename.endswith('.msg'): if filename.endswith('.msg'):
if filename not in ignore_files: if filename not in ignore_files:
if not check_single_file(filename): if not check_single_file(os.path.join(root, filename)):
result = False result = False
return result return result

Просмотреть файл

@ -25,9 +25,10 @@ from __future__ import print_function
import difflib import difflib
import os import os
import re import re
import subprocess
import sys import sys
from check_utils import get_all_toplevel_filenames
from mozversioncontrol import get_repository_from_env
architecture_independent = set([ 'generic' ]) architecture_independent = set([ 'generic' ])
all_architecture_names = set([ 'x86', 'x64', 'arm', 'arm64', 'mips32', 'mips64' ]) all_architecture_names = set([ 'x86', 'x64', 'arm', 'arm64', 'mips32', 'mips64' ])
@ -131,7 +132,7 @@ def get_macroassembler_definitions(filename):
code_section = False code_section = False
lines = '' lines = ''
signatures = [] signatures = []
with open(os.path.join('../..', filename)) as f: with open(filename) as f:
for line in f: for line in f:
if '//{{{ check_macroassembler_style' in line: if '//{{{ check_macroassembler_style' in line:
style_section = True style_section = True
@ -171,7 +172,7 @@ def get_macroassembler_declaration(filename):
style_section = False style_section = False
lines = '' lines = ''
signatures = [] signatures = []
with open(os.path.join('../..', filename)) as f: with open(filename) as f:
for line in f: for line in f:
if '//{{{ check_macroassembler_style' in line: if '//{{{ check_macroassembler_style' in line:
style_section = True style_section = True
@ -238,13 +239,17 @@ def check_style():
# We infer from each file the signature of each MacroAssembler function. # We infer from each file the signature of each MacroAssembler function.
defs = dict() # type: dict(signature => ['x86', 'x64']) defs = dict() # type: dict(signature => ['x86', 'x64'])
repo = get_repository_from_env()
# Select the appropriate files. # Select the appropriate files.
for filename in get_all_toplevel_filenames(): for filename in repo.get_files_in_working_directory():
if not filename.startswith('js/src/jit/'): if not filename.startswith('js/src/jit/'):
continue continue
if 'MacroAssembler' not in filename: if 'MacroAssembler' not in filename:
continue continue
filename = os.path.join(repo.path, filename)
if filename.endswith('MacroAssembler.h'): if filename.endswith('MacroAssembler.h'):
decls = append_signatures(decls, get_macroassembler_declaration(filename)) decls = append_signatures(decls, get_macroassembler_declaration(filename))
else: else:

Просмотреть файл

@ -40,10 +40,10 @@ from __future__ import print_function
import difflib import difflib
import os import os
import re import re
import subprocess
import sys import sys
import traceback
from check_utils import get_all_toplevel_filenames from mozversioncontrol import get_repository_from_env
# We don't bother checking files in these directories, because they're (a) auxiliary or (b) # We don't bother checking files in these directories, because they're (a) auxiliary or (b)
# imported code that doesn't follow our coding style. # imported code that doesn't follow our coding style.
@ -249,8 +249,10 @@ def check_style():
non_js_inclnames = set() # type: set(inclname) non_js_inclnames = set() # type: set(inclname)
js_names = dict() # type: dict(filename, inclname) js_names = dict() # type: dict(filename, inclname)
repo = get_repository_from_env()
# Select the appropriate files. # Select the appropriate files.
for filename in get_all_toplevel_filenames(): for filename in repo.get_files_in_working_directory():
for non_js_dir in non_js_dirnames: for non_js_dir in non_js_dirnames:
if filename.startswith(non_js_dir) and filename.endswith('.h'): if filename.startswith(non_js_dir) and filename.endswith('.h'):
inclname = 'mozilla/' + filename.split('/')[-1] inclname = 'mozilla/' + filename.split('/')[-1]
@ -285,7 +287,7 @@ def check_style():
# This script is run in js/src/, so prepend '../../' to get to the root of the Mozilla # This script is run in js/src/, so prepend '../../' to get to the root of the Mozilla
# source tree. # source tree.
with open(os.path.join('../..', filename)) as f: with open(os.path.join(repo.path, filename)) as f:
do_file(filename, inclname, file_kind, f, all_inclnames, included_h_inclnames) do_file(filename, inclname, file_kind, f, all_inclnames, included_h_inclnames)
edges[inclname] = included_h_inclnames edges[inclname] = included_h_inclnames

Просмотреть файл

@ -1,31 +0,0 @@
# vim: set ts=8 sts=4 et sw=4 tw=99:
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
import subprocess
def get_all_toplevel_filenames():
'''Get a list of all the files in the (Mercurial or Git) repository.'''
failed_cmds = []
try:
cmd = ['hg', 'manifest', '-q']
all_filenames = subprocess.check_output(cmd, universal_newlines=True,
stderr=subprocess.PIPE).split('\n')
return all_filenames
except:
failed_cmds.append(cmd)
try:
# Get the relative path to the top-level directory.
cmd = ['git', 'rev-parse', '--show-cdup']
top_level = subprocess.check_output(cmd, universal_newlines=True,
stderr=subprocess.PIPE).split('\n')[0]
cmd = ['git', 'ls-files', '--full-name', top_level]
all_filenames = subprocess.check_output(cmd, universal_newlines=True,
stderr=subprocess.PIPE).split('\n')
return all_filenames
except:
failed_cmds.append(cmd)
raise Exception('failed to run any of the repo manifest commands', failed_cmds)

Просмотреть файл

@ -76,6 +76,11 @@ class Repository(object):
''' '''
raise NotImplementedError raise NotImplementedError
def get_files_in_working_directory(self):
"""Obtain a list of managed files in the working directory."""
raise NotImplementedError
class HgRepository(Repository): class HgRepository(Repository):
'''An implementation of `Repository` for Mercurial repositories.''' '''An implementation of `Repository` for Mercurial repositories.'''
def __init__(self, path): def __init__(self, path):
@ -99,6 +104,12 @@ class HgRepository(Repository):
def forget_add_remove_files(self, path): def forget_add_remove_files(self, path):
self._run('forget', path) self._run('forget', path)
def get_files_in_working_directory(self):
# Can return backslashes on Windows. Normalize to forward slashes.
return list(p.replace('\\', '/') for p in
self._run('files', '-0').split('\0'))
class GitRepository(Repository): class GitRepository(Repository):
'''An implementation of `Repository` for Git repositories.''' '''An implementation of `Repository` for Git repositories.'''
def __init__(self, path): def __init__(self, path):
@ -116,6 +127,14 @@ class GitRepository(Repository):
def forget_add_remove_files(self, path): def forget_add_remove_files(self, path):
self._run('reset', path) self._run('reset', path)
def get_files_in_working_directory(self):
return self._run('ls-files', '-z').split('\0')
class InvalidRepoPath(Exception):
"""Represents a failure to find a VCS repo at a specified path."""
def get_repository_object(path): def get_repository_object(path):
'''Get a repository object for the repository at `path`. '''Get a repository object for the repository at `path`.
If `path` is not a known VCS repository, raise an exception. If `path` is not a known VCS repository, raise an exception.
@ -125,4 +144,24 @@ def get_repository_object(path):
elif os.path.exists(os.path.join(path, '.git')): elif os.path.exists(os.path.join(path, '.git')):
return GitRepository(path) return GitRepository(path)
else: else:
raise Exception('Unknown VCS, or not a source checkout: %s' % path) raise InvalidRepoPath('Unknown VCS, or not a source checkout: %s' %
path)
def get_repository_from_env():
"""Obtain a repository object by looking at the environment."""
def ancestors(path):
while path:
yield path
path, child = os.path.split(path)
if child == '':
break
for path in ancestors(os.getcwd()):
try:
return get_repository_object(path)
except InvalidRepoPath:
continue
raise Exception('Could not find Mercurial or Git checkout for %s' %
os.getcwd())