Éric Araujo | a0e92a8 | 2011-07-26 18:01:08 +0200 | [diff] [blame] | 1 | #!/usr/bin/env python3 |
Brett Cannon | 70cb187 | 2017-06-24 16:51:23 -0700 | [diff] [blame] | 2 | """Check proposed changes for common issues.""" |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 3 | import re |
| 4 | import sys |
| 5 | import shutil |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 6 | import os.path |
| 7 | import subprocess |
Éric Araujo | a3e072b | 2011-07-30 21:34:04 +0200 | [diff] [blame] | 8 | import sysconfig |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 9 | |
| 10 | import reindent |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 11 | import untabify |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 12 | |
| 13 | |
Victor Stinner | 4a347ce | 2017-08-17 16:29:15 +0200 | [diff] [blame] | 14 | # Excluded directories which are copies of external libraries: |
| 15 | # don't check their coding style |
| 16 | EXCLUDE_DIRS = [os.path.join('Modules', '_ctypes', 'libffi_osx'), |
| 17 | os.path.join('Modules', '_ctypes', 'libffi_msvc'), |
| 18 | os.path.join('Modules', '_decimal', 'libmpdec'), |
| 19 | os.path.join('Modules', 'expat'), |
| 20 | os.path.join('Modules', 'zlib')] |
Éric Araujo | a3e072b | 2011-07-30 21:34:04 +0200 | [diff] [blame] | 21 | SRCDIR = sysconfig.get_config_var('srcdir') |
| 22 | |
Victor Stinner | 4a347ce | 2017-08-17 16:29:15 +0200 | [diff] [blame] | 23 | |
Brett Cannon | 058173e | 2010-07-04 22:05:34 +0000 | [diff] [blame] | 24 | def n_files_str(count): |
| 25 | """Return 'N file(s)' with the proper plurality on 'file'.""" |
| 26 | return "{} file{}".format(count, "s" if count != 1 else "") |
| 27 | |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 28 | |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 29 | def status(message, modal=False, info=None): |
| 30 | """Decorator to output status info to stdout.""" |
| 31 | def decorated_fxn(fxn): |
| 32 | def call_fxn(*args, **kwargs): |
| 33 | sys.stdout.write(message + ' ... ') |
| 34 | sys.stdout.flush() |
| 35 | result = fxn(*args, **kwargs) |
| 36 | if not modal and not info: |
| 37 | print("done") |
| 38 | elif info: |
| 39 | print(info(result)) |
| 40 | else: |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 41 | print("yes" if result else "NO") |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 42 | return result |
| 43 | return call_fxn |
| 44 | return decorated_fxn |
| 45 | |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 46 | |
Nick Coghlan | 482f7a2 | 2017-03-12 13:19:08 +1000 | [diff] [blame] | 47 | def get_git_branch(): |
| 48 | """Get the symbolic name for the current git branch""" |
| 49 | cmd = "git rev-parse --abbrev-ref HEAD".split() |
| 50 | try: |
| 51 | return subprocess.check_output(cmd, stderr=subprocess.DEVNULL) |
| 52 | except subprocess.CalledProcessError: |
| 53 | return None |
| 54 | |
| 55 | |
| 56 | def get_git_upstream_remote(): |
| 57 | """Get the remote name to use for upstream branches |
| 58 | |
| 59 | Uses "upstream" if it exists, "origin" otherwise |
| 60 | """ |
| 61 | cmd = "git remote get-url upstream".split() |
| 62 | try: |
| 63 | subprocess.check_output(cmd, stderr=subprocess.DEVNULL) |
| 64 | except subprocess.CalledProcessError: |
| 65 | return "origin" |
| 66 | return "upstream" |
| 67 | |
| 68 | |
| 69 | @status("Getting base branch for PR", |
| 70 | info=lambda x: x if x is not None else "not a PR branch") |
| 71 | def get_base_branch(): |
Nick Coghlan | 2abfdf5 | 2017-04-09 18:33:03 +1000 | [diff] [blame] | 72 | if not os.path.exists(os.path.join(SRCDIR, '.git')): |
Nick Coghlan | 482f7a2 | 2017-03-12 13:19:08 +1000 | [diff] [blame] | 73 | # Not a git checkout, so there's no base branch |
| 74 | return None |
| 75 | version = sys.version_info |
| 76 | if version.releaselevel == 'alpha': |
| 77 | base_branch = "master" |
| 78 | else: |
| 79 | base_branch = "{0.major}.{0.minor}".format(version) |
| 80 | this_branch = get_git_branch() |
| 81 | if this_branch is None or this_branch == base_branch: |
| 82 | # Not on a git PR branch, so there's no base branch |
| 83 | return None |
| 84 | upstream_remote = get_git_upstream_remote() |
| 85 | return upstream_remote + "/" + base_branch |
| 86 | |
| 87 | |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 88 | @status("Getting the list of files that have been added/changed", |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 89 | info=lambda x: n_files_str(len(x))) |
Nick Coghlan | 482f7a2 | 2017-03-12 13:19:08 +1000 | [diff] [blame] | 90 | def changed_files(base_branch=None): |
Benjamin Peterson | b8c0845 | 2018-06-05 22:40:12 -0700 | [diff] [blame] | 91 | """Get the list of changed or added files from git.""" |
| 92 | if os.path.exists(os.path.join(SRCDIR, '.git')): |
Nick Coghlan | 6a6d090 | 2017-03-12 19:37:09 +1000 | [diff] [blame] | 93 | # We just use an existence check here as: |
| 94 | # directory = normal git checkout/clone |
| 95 | # file = git worktree directory |
Nick Coghlan | 482f7a2 | 2017-03-12 13:19:08 +1000 | [diff] [blame] | 96 | if base_branch: |
| 97 | cmd = 'git diff --name-status ' + base_branch |
| 98 | else: |
| 99 | cmd = 'git status --porcelain' |
Christian Heimes | d98c677 | 2015-04-23 11:24:14 +0200 | [diff] [blame] | 100 | filenames = [] |
| 101 | with subprocess.Popen(cmd.split(), stdout=subprocess.PIPE) as st: |
| 102 | for line in st.stdout: |
| 103 | line = line.decode().rstrip() |
Nick Coghlan | 482f7a2 | 2017-03-12 13:19:08 +1000 | [diff] [blame] | 104 | status_text, filename = line.split(maxsplit=1) |
| 105 | status = set(status_text) |
Christian Heimes | d98c677 | 2015-04-23 11:24:14 +0200 | [diff] [blame] | 106 | # modified, added or unmerged files |
| 107 | if not status.intersection('MAU'): |
| 108 | continue |
Christian Heimes | d98c677 | 2015-04-23 11:24:14 +0200 | [diff] [blame] | 109 | if ' -> ' in filename: |
| 110 | # file is renamed |
| 111 | filename = filename.split(' -> ', 2)[1].strip() |
| 112 | filenames.append(filename) |
Christian Heimes | d98c677 | 2015-04-23 11:24:14 +0200 | [diff] [blame] | 113 | else: |
Benjamin Peterson | b8c0845 | 2018-06-05 22:40:12 -0700 | [diff] [blame] | 114 | sys.exit('need a git checkout to get modified files') |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 115 | |
Victor Stinner | 4a347ce | 2017-08-17 16:29:15 +0200 | [diff] [blame] | 116 | filenames2 = [] |
| 117 | for filename in filenames: |
| 118 | # Normalize the path to be able to match using .startswith() |
| 119 | filename = os.path.normpath(filename) |
| 120 | if any(filename.startswith(path) for path in EXCLUDE_DIRS): |
| 121 | # Exclude the file |
| 122 | continue |
| 123 | filenames2.append(filename) |
| 124 | |
| 125 | return filenames2 |
| 126 | |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 127 | |
Brett Cannon | 058173e | 2010-07-04 22:05:34 +0000 | [diff] [blame] | 128 | def report_modified_files(file_paths): |
| 129 | count = len(file_paths) |
| 130 | if count == 0: |
| 131 | return n_files_str(count) |
| 132 | else: |
| 133 | lines = ["{}:".format(n_files_str(count))] |
| 134 | for path in file_paths: |
| 135 | lines.append(" {}".format(path)) |
| 136 | return "\n".join(lines) |
| 137 | |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 138 | |
Brett Cannon | 70cb187 | 2017-06-24 16:51:23 -0700 | [diff] [blame] | 139 | @status("Fixing Python file whitespace", info=report_modified_files) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 140 | def normalize_whitespace(file_paths): |
| 141 | """Make sure that the whitespace for .py files have been normalized.""" |
| 142 | reindent.makebackup = False # No need to create backups. |
Benjamin Peterson | 4177eff | 2011-06-27 18:25:06 -0500 | [diff] [blame] | 143 | fixed = [path for path in file_paths if path.endswith('.py') and |
Éric Araujo | ad548b8 | 2011-07-31 18:33:00 +0200 | [diff] [blame] | 144 | reindent.check(os.path.join(SRCDIR, path))] |
Brett Cannon | 058173e | 2010-07-04 22:05:34 +0000 | [diff] [blame] | 145 | return fixed |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 146 | |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 147 | |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 148 | @status("Fixing C file whitespace", info=report_modified_files) |
| 149 | def normalize_c_whitespace(file_paths): |
| 150 | """Report if any C files """ |
| 151 | fixed = [] |
| 152 | for path in file_paths: |
Éric Araujo | a3e072b | 2011-07-30 21:34:04 +0200 | [diff] [blame] | 153 | abspath = os.path.join(SRCDIR, path) |
| 154 | with open(abspath, 'r') as f: |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 155 | if '\t' not in f.read(): |
| 156 | continue |
Éric Araujo | a3e072b | 2011-07-30 21:34:04 +0200 | [diff] [blame] | 157 | untabify.process(abspath, 8, verbose=False) |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 158 | fixed.append(path) |
| 159 | return fixed |
| 160 | |
| 161 | |
| 162 | ws_re = re.compile(br'\s+(\r?\n)$') |
| 163 | |
| 164 | @status("Fixing docs whitespace", info=report_modified_files) |
| 165 | def normalize_docs_whitespace(file_paths): |
| 166 | fixed = [] |
| 167 | for path in file_paths: |
Éric Araujo | a3e072b | 2011-07-30 21:34:04 +0200 | [diff] [blame] | 168 | abspath = os.path.join(SRCDIR, path) |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 169 | try: |
Éric Araujo | a3e072b | 2011-07-30 21:34:04 +0200 | [diff] [blame] | 170 | with open(abspath, 'rb') as f: |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 171 | lines = f.readlines() |
| 172 | new_lines = [ws_re.sub(br'\1', line) for line in lines] |
| 173 | if new_lines != lines: |
Éric Araujo | a3e072b | 2011-07-30 21:34:04 +0200 | [diff] [blame] | 174 | shutil.copyfile(abspath, abspath + '.bak') |
| 175 | with open(abspath, 'wb') as f: |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 176 | f.writelines(new_lines) |
| 177 | fixed.append(path) |
| 178 | except Exception as err: |
| 179 | print('Cannot fix %s: %s' % (path, err)) |
| 180 | return fixed |
| 181 | |
| 182 | |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 183 | @status("Docs modified", modal=True) |
| 184 | def docs_modified(file_paths): |
Brett Cannon | 058173e | 2010-07-04 22:05:34 +0000 | [diff] [blame] | 185 | """Report if any file in the Doc directory has been changed.""" |
| 186 | return bool(file_paths) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 187 | |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 188 | |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 189 | @status("Misc/ACKS updated", modal=True) |
| 190 | def credit_given(file_paths): |
| 191 | """Check if Misc/ACKS has been changed.""" |
Terry Jan Reedy | 6e2711b | 2013-07-21 20:57:44 -0400 | [diff] [blame] | 192 | return os.path.join('Misc', 'ACKS') in file_paths |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 193 | |
Florent Xicluna | e4a3380 | 2010-08-09 12:24:20 +0000 | [diff] [blame] | 194 | |
Antoine Pitrou | 1ba9469 | 2017-06-25 03:21:49 +0200 | [diff] [blame] | 195 | @status("Misc/NEWS.d updated with `blurb`", modal=True) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 196 | def reported_news(file_paths): |
Antoine Pitrou | 1ba9469 | 2017-06-25 03:21:49 +0200 | [diff] [blame] | 197 | """Check if Misc/NEWS.d has been changed.""" |
| 198 | return any(p.startswith(os.path.join('Misc', 'NEWS.d', 'next')) |
| 199 | for p in file_paths) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 200 | |
Ross Lagerwall | 6c52c57 | 2012-03-11 19:21:07 +0200 | [diff] [blame] | 201 | @status("configure regenerated", modal=True, info=str) |
| 202 | def regenerated_configure(file_paths): |
| 203 | """Check if configure has been regenerated.""" |
Matthias Klose | 5ce31cc | 2012-03-14 23:17:31 +0100 | [diff] [blame] | 204 | if 'configure.ac' in file_paths: |
Ross Lagerwall | 6c52c57 | 2012-03-11 19:21:07 +0200 | [diff] [blame] | 205 | return "yes" if 'configure' in file_paths else "no" |
| 206 | else: |
| 207 | return "not needed" |
| 208 | |
| 209 | @status("pyconfig.h.in regenerated", modal=True, info=str) |
| 210 | def regenerated_pyconfig_h_in(file_paths): |
| 211 | """Check if pyconfig.h.in has been regenerated.""" |
Matthias Klose | 5ce31cc | 2012-03-14 23:17:31 +0100 | [diff] [blame] | 212 | if 'configure.ac' in file_paths: |
Ross Lagerwall | 6c52c57 | 2012-03-11 19:21:07 +0200 | [diff] [blame] | 213 | return "yes" if 'pyconfig.h.in' in file_paths else "no" |
| 214 | else: |
| 215 | return "not needed" |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 216 | |
Brett Cannon | 70cb187 | 2017-06-24 16:51:23 -0700 | [diff] [blame] | 217 | def travis(pull_request): |
| 218 | if pull_request == 'false': |
| 219 | print('Not a pull request; skipping') |
| 220 | return |
| 221 | base_branch = get_base_branch() |
| 222 | file_paths = changed_files(base_branch) |
| 223 | python_files = [fn for fn in file_paths if fn.endswith('.py')] |
| 224 | c_files = [fn for fn in file_paths if fn.endswith(('.c', '.h'))] |
| 225 | doc_files = [fn for fn in file_paths if fn.startswith('Doc') and |
| 226 | fn.endswith(('.rst', '.inc'))] |
| 227 | fixed = [] |
| 228 | fixed.extend(normalize_whitespace(python_files)) |
| 229 | fixed.extend(normalize_c_whitespace(c_files)) |
| 230 | fixed.extend(normalize_docs_whitespace(doc_files)) |
| 231 | if not fixed: |
| 232 | print('No whitespace issues found') |
| 233 | else: |
| 234 | print(f'Please fix the {len(fixed)} file(s) with whitespace issues') |
| 235 | print('(on UNIX you can run `make patchcheck` to make the fixes)') |
| 236 | sys.exit(1) |
| 237 | |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 238 | def main(): |
Nick Coghlan | 482f7a2 | 2017-03-12 13:19:08 +1000 | [diff] [blame] | 239 | base_branch = get_base_branch() |
| 240 | file_paths = changed_files(base_branch) |
Brett Cannon | 058173e | 2010-07-04 22:05:34 +0000 | [diff] [blame] | 241 | python_files = [fn for fn in file_paths if fn.endswith('.py')] |
| 242 | c_files = [fn for fn in file_paths if fn.endswith(('.c', '.h'))] |
Georg Brandl | 24f0717 | 2014-10-19 11:54:08 +0200 | [diff] [blame] | 243 | doc_files = [fn for fn in file_paths if fn.startswith('Doc') and |
| 244 | fn.endswith(('.rst', '.inc'))] |
Antoine Pitrou | 1ba9469 | 2017-06-25 03:21:49 +0200 | [diff] [blame] | 245 | misc_files = {p for p in file_paths if p.startswith('Misc')} |
Brett Cannon | 058173e | 2010-07-04 22:05:34 +0000 | [diff] [blame] | 246 | # PEP 8 whitespace rules enforcement. |
| 247 | normalize_whitespace(python_files) |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 248 | # C rules enforcement. |
| 249 | normalize_c_whitespace(c_files) |
| 250 | # Doc whitespace enforcement. |
| 251 | normalize_docs_whitespace(doc_files) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 252 | # Docs updated. |
Georg Brandl | a9afb68 | 2010-10-21 12:49:28 +0000 | [diff] [blame] | 253 | docs_modified(doc_files) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 254 | # Misc/ACKS changed. |
Terry Jan Reedy | 6e2711b | 2013-07-21 20:57:44 -0400 | [diff] [blame] | 255 | credit_given(misc_files) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 256 | # Misc/NEWS changed. |
Terry Jan Reedy | 6e2711b | 2013-07-21 20:57:44 -0400 | [diff] [blame] | 257 | reported_news(misc_files) |
Ross Lagerwall | 6c52c57 | 2012-03-11 19:21:07 +0200 | [diff] [blame] | 258 | # Regenerated configure, if necessary. |
| 259 | regenerated_configure(file_paths) |
| 260 | # Regenerated pyconfig.h.in, if necessary. |
| 261 | regenerated_pyconfig_h_in(file_paths) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 262 | |
| 263 | # Test suite run and passed. |
Éric Araujo | fbc5ff6 | 2011-08-12 17:50:08 +0200 | [diff] [blame] | 264 | if python_files or c_files: |
Ezio Melotti | 5e12bb7 | 2013-01-11 14:07:47 +0200 | [diff] [blame] | 265 | end = " and check for refleaks?" if c_files else "?" |
Éric Araujo | fbc5ff6 | 2011-08-12 17:50:08 +0200 | [diff] [blame] | 266 | print() |
Ezio Melotti | 5e12bb7 | 2013-01-11 14:07:47 +0200 | [diff] [blame] | 267 | print("Did you run the test suite" + end) |
Christian Heimes | ada8c3b | 2008-03-18 18:26:33 +0000 | [diff] [blame] | 268 | |
| 269 | |
| 270 | if __name__ == '__main__': |
Brett Cannon | 70cb187 | 2017-06-24 16:51:23 -0700 | [diff] [blame] | 271 | import argparse |
| 272 | parser = argparse.ArgumentParser(description=__doc__) |
| 273 | parser.add_argument('--travis', |
| 274 | help='Perform pass/fail checks') |
| 275 | args = parser.parse_args() |
| 276 | if args.travis: |
| 277 | travis(args.travis) |
| 278 | else: |
| 279 | main() |