- flake8 is a wrapper around pyflakes, so it's redundant to mention
both of them. Roll the explicit sections about coding errors and
coding style violations into a single section.
- After recent cleanups the project now has zero warnings or errors
from flake8. Reword the requirements so that it is now mandatory
to not introduce new warnings.
- Expand the section on suppression of warnings to differentiate
between suppressing inline individually and globally suppressing
for the whole project.
- Properly capitalize "Python Style Guide".
Change-Id: I4b333d013e985db252873441b16cb719ed5be5b5
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/255040
Tested-by: David Pursehouse <dpursehouse@collab.net>
Reviewed-by: Mike Frysinger <vapier@google.com>
If programs emit non-UTF-8 output, we currently throw a fatal error.
We largely only care about the exit status of programs, and even the
output we do parse is a bit minimal. Lets make it into a warning and
mangle the invalid bytes into U+FFFD. This should complain enough to
annoy but not to break when it's not necessary.
Bug: https://crbug.com/gerrit/12337#c2
Change-Id: Idbc94f19ff4d84d2e47e01960dd17d5b492d4a8a
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/255272
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
Tested-by: Mike Frysinger <vapier@google.com>
All of the instances of this are related to Python 2 names that
don't exist in Python 3, and the warnings are raised when running
flake8 on Python 3.
All of these will go away once we completely remove support for
Python 2, so just suppress them inline. We don't globally suppress
the check so that we will still see legitimate errors if/when they
occur in new code.
Change-Id: Iccf955f50abfc9f83b371fc0af6cceb51037456f
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/255039
Tested-by: David Pursehouse <dpursehouse@collab.net>
Reviewed-by: Mike Frysinger <vapier@google.com>
Some versions of gpg on Windows mishandle native paths with homedir.
It manifests itself like:
gpg: keybox 'C:\Users\.../.repoconfig\gnupg/pubring.kbx' created
gpg: C:\Users\.../.repoconfig\gnupg/trustdb.gpg: trustdb created
gpg: key 16530D5E920F5C65: public key "Repo Maintainer <repo@android.kernel.org>" imported
gpg: can't connect to the agent: Invalid value passed to IPC
gpg: Total number processed: 1
gpg: imported: 1
fatal: registering repo maintainer keys failed
It seems gpg (at least version 2.2.17) needs paths to be specified
in cygwin form (e.g. "/c/Users/.../.repoconfig/gnupg") otherwise
it fails to talk to its own processes. We can work around this
with a minor trick: we cd to the right path and then invoke gpg
with --homedir . and let gpg itself resolve . to whatever form it
really wants.
This is a bit hacky, but we don't control gpg, and this allows us
to avoid having to muck with the environment. Since --homedir has
been around since at least gpg-1.4.x from 2004, backwards compat
shouldn't be an issue.
While we're here, touch up the output a bit: there's no need to
dump all the chatty gpg output if things don't fail, so always
swallow the output. If things do fail, our exception handler
takes care of dumping the full stdout & stderr.
Change-Id: I74ab98e1e61e95318fda6faf57c6a8699f775935
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/255120
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
Tested-by: Mike Frysinger <vapier@google.com>
We added support for `repo init -c` to main.py, but not to the
launcher, so the -c option only works after the first init has
run which kind of defeats its purpose. Rework the parser setup
so that we can tell it whether it's for "init" or "gitc-init"
and then add the -c option in the same way we do in main.py.
This has the benefit of getting the parser entirely out of the
module scope which makes it a lot easier to reason about, and
it means we can write some unittests.
Change-Id: Icbc2ec3aceb938d5a8f941d5fbce1548553dc5f7
Test: repo help init
Test: repo help gitc-init
Test: repo init -u https://android.googlesource.com/platform/manifest -c
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/255113
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
Tested-by: Mike Frysinger <vapier@google.com>
We've been using a DSA/1024 key to sign our tags. Time to update to
something a bit newer. We'll include RSA & ECC keys, but only use
RSA keys initially for backwards compatibility and see how it goes
with our user base.
Change-Id: I683c97b6fbd860f220ed4ddc7b21f07db279a916
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/255112
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
Tested-by: Mike Frysinger <vapier@google.com>
repo:342:5: E306 expected 1 blank line before a nested definition, found 0
repo:617:5: F841 local variable 'ret' is assigned to but never used
Change-Id: I364fdb5dac8ebaff398b848935fe8356cb9ed2d3
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/255035
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: David Pursehouse <dpursehouse@collab.net>
Now that we have a central run_command point, we can easily add
tracing support to the launcher script.
Change-Id: I9e0335c196cafd6263ff501925abfe835f036c5e
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254755
Tested-by: Mike Frysinger <vapier@google.com>
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
We can't rely on subprocess.run yet as that requires Python 3.6,
but we can clean up the code we have with some ad-hoc replacement.
This unifies all the inconsistent subprocess.Popen usage we have.
Change-Id: I56af40a3df988ee47b299105d692ff419d07ad6b
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254754
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
Tested-by: Mike Frysinger <vapier@google.com>
We increased the max line length to 100 columns which got rid of
the majority of these warnings, but there were still a few lines
that exceeded 100 columns.
Change-Id: Ib3372868ca2297f83073a14f91c8ae3df9d0d0e6
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254699
Tested-by: David Pursehouse <dpursehouse@collab.net>
Reviewed-by: Mike Frysinger <vapier@google.com>
Some people find the existing output to be a bit confusing. It spews
a lot of git output before exiting, but it's not exactly clear what
the final state is when things pass. Add an explicit message.
Bug: https://crbug.com/gerrit/10501
Change-Id: I9de83b595d3185feb820005b8fc81c6adc55b357
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254732
Reviewed-by: Michael Mortensen <mmortensen@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: Mike Frysinger <vapier@google.com>
We can get version info when in a checkout, but it'd be helpful
to show that info at all times.
Change-Id: Ieeb44a503c9d7d8c487db4810bdcf3d5f6656c82
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254712
Reviewed-by: Michael Mortensen <mmortensen@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: Mike Frysinger <vapier@google.com>
This helps with people tracing repo/git execution. We use a similar
format to git, but a little simpler since we always initialize the
env var setting, and we want to avoid too much overhead.
Bug: https://crbug.com/gerrit/12314
Change-Id: I75675b6cc4c6f7c4f5e09f54128eba9456364d04
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254331
Reviewed-by: Josh Steadmon <steadmon@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: Mike Frysinger <vapier@google.com>
We import the wrapper on the fly, so minimize how much code we run
in module scope. It's pointless/wasted when importing.
Change-Id: I4a71c2030325d0a639585671cd7ebe8f22687ecd
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254072
Tested-by: Mike Frysinger <vapier@google.com>
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
Reviewed-by: Mike Frysinger <vapier@google.com>
We've been requiring git-1.7.2 since Oct 2012, so we can safely drop
the individual checks sprinkled throughout the code base for older.
Change-Id: I1737fff7b3f27f475960b0bff9cb300aefd5d108
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/253135
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
Tested-by: Mike Frysinger <vapier@google.com>
- W503 line break before binary operator
- W504 line break after binary operator
There doesn't seem to be a nice way of fixing all of these without
replacing W503 with W504 or vice-versa, or unwrapping them resulting
in excessively long lines. Let's just suppress them.
Change-Id: I7846d0124054f58e1cb480d4459cd9c86b737a50
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254608
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: David Pursehouse <dpursehouse@collab.net>
Unwrap one unnecessarily wrapped line, and use parentheses on
a wrapped condition instead of wrapping with backslashes.
Change-Id: I12679a0547dd822b15a6551e0f6c308239ff7b2d
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254607
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: David Pursehouse <dpursehouse@collab.net>
flake8 reports:
E722 do not use bare 'except'
Replace them with 'except Exception' per [1] which says:
Bare except will catch exceptions you almost certainly don't want
to catch, including KeyboardInterrupt (the user hitting Ctrl+C) and
Python-raised errors like SystemExit
If you don't have a specific exception you're expecting, at least
except Exception, which is the base type for all "Regular" exceptions.
[1] https://stackoverflow.com/a/54948581
Change-Id: Ic555ea9482645899f5b04040ddb6b24eadbf9062
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254606
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: David Pursehouse <dpursehouse@collab.net>
- E301 expected 1 blank line
- E302 expected 2 blank lines
- E303 too many blank lines
- E305 expected 2 blank lines after class or function definition
- E306 expected 1 blank line before a nested definition
Fixed automatically with autopep8:
git ls-files | grep py$ | xargs autopep8 --in-place \
--select E301,E302,E303,E305,E306
Manually fix issues in project.py caused by misuse of block comments.
Change-Id: Iee840fcaff48aae504ddac9c3e76d2acd484f6a9
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254599
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: David Pursehouse <dpursehouse@collab.net>
- E121 continuation line under-indented for hanging indent
- E122 continuation line missing indentation or outdented
- E125 continuation line with same indent as next logical line
- E126 continuation line over-indented for hanging indent
- E127 continuation line over-indented for visual indent
- E128 continuation line under-indented for visual indent
- E129 visually indented line with same indent as next logical line
- E131 continuation line unaligned for hanging indent
Fixed automatically with autopep8:
git ls-files | grep py$ | xargs autopep8 --in-place \
--select E121,E122,E125,E126,E127,E128,E129,E131
Change-Id: Ifd95fb8e6a1a4d6e9de187b5787d64a6326dd249
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254605
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: David Pursehouse <dpursehouse@collab.net>
Surround the condition with parentheses rather than using
backslashes. This prevents confusion about indentation when
running flake8/autoflake8.
Change-Id: I01775b96f817ee616f545b55369a4864fa1d6712
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254603
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: David Pursehouse <dpursehouse@collab.net>
flake8 reports:
E703 statement ends with a semicolon
Change-Id: Ia63fc9efb04425e425c0f289272db76ff1ceeb34
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254600
Tested-by: David Pursehouse <dpursehouse@collab.net>
Reviewed-by: Mike Frysinger <vapier@google.com>
flake8 reports:
E713 test for membership should be 'not in'
Change-Id: I4446be67c431b7267105b53478d2ceba2af758d7
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254451
Tested-by: David Pursehouse <dpursehouse@collab.net>
Reviewed-by: Mike Frysinger <vapier@google.com>
flake8 reports:
E714 test for object identity should be 'is not'
Change-Id: Ib8c4100babaf952bbfe65fd56555ece8a958e4b0
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254450
Tested-by: David Pursehouse <dpursehouse@collab.net>
Reviewed-by: Mike Frysinger <vapier@google.com>
The Google style guide for python [1] says the maximum line length
should be 80, but there are several lines in the code base that
exceed it:
git ls-files | grep py$ | xargs flake8 | grep E501 | wc -l
64
I don't think it's worth going through and re-wrapping all those,
so just increase the limit to 100 which seems to be a reasonable
compromise:
git ls-files | grep py$ | xargs flake8 | grep E501 | wc -l
6
Leave the re-rewrapping of those lines for a follow-up commit,
though.
[1] http://google.github.io/styleguide/pyguide.html#32-line-length
Change-Id: Ia37c34301163431fd1fb4fb6697a4a482d6be077
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254595
Tested-by: David Pursehouse <dpursehouse@collab.net>
Reviewed-by: Mike Frysinger <vapier@google.com>
We have a few files that we optionally symlink from the work tree
.git/ to the .repo/projects/ path. If they don't exist when we
first initialize, then we skip creating symlinks. If the files
are created later on under the work tree .git/, repo gets upset.
This can happen with the packed-refs file: if we don't have any
packed refs initially, we don't symlink it. But if git tries to
pack refs later on and creates the file, the project gets wedged.
We could create an empty file initially and then symlink it, but
for some files, it's not clear we want to always do that (e.g.
the .git/shallow setting). Instead, lets make handling of these
paths more dynamic. If they show up later on in the work tree
.git/ only, we'll take care of relocating & symlinking. This
also makes repo a little more robust and autorecovers incase a
path goes missing in one of the dirs.
Ideally we wouldn't monkey around at all here, but considering
the only option we give to users currently is to blow things
away with --force-sync, this seems a bit better.
Bug: https://crbug.com/gerrit/12324
Change-Id: Ia6960f1896ac6d890c762d7d053684a1c6ab2c87
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254632
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
Tested-by: Mike Frysinger <vapier@google.com>
flake8 reports:
E265 block comment should start with '# '
While we're at it, add a period at the end of the comment sentence.
Change-Id: Icb7119079a1d64e6defafc3f6d24e99dbf16139d
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254596
Tested-by: David Pursehouse <dpursehouse@collab.net>
Reviewed-by: Mike Frysinger <vapier@google.com>
When upload hooks fail, people are forced to use --no-verify to upload
CLs anyways. When projects have flaky hooks, this trains people to
always use that option. This is obviously bad: hooks might get fixed,
or some of the hooks are always good & people should review.
Lets add an --ignore-hooks option. This still runs the hooks, but any
failures will be ignored and allow the user to upload anyways.
Bug: https://crbug.com/gerrit/12230
Change-Id: Ide2ac8a40a656bfcd6aae20c3ce8118e06bf909b
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254452
Tested-by: Mike Frysinger <vapier@google.com>
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
flake8 reports:
W391 blank line at end of file
Change-Id: I5498b2de2d1268d4f1f4b9e1760f9fa93a6da4cd
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254594
Tested-by: David Pursehouse <dpursehouse@collab.net>
Reviewed-by: Mike Frysinger <vapier@google.com>
flake8 reports:
F841 local variable 'f' is assigned to but never used
Change-Id: If808eb381ee44c7da71e6281615a06a6723cf945
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254593
Tested-by: David Pursehouse <dpursehouse@collab.net>
Reviewed-by: Mike Frysinger <vapier@google.com>
flake8 reports:
F811 redefinition of unused 'test_src_block_dir' from line 259
which is caused by having two methods with the same name. Rename
them both to better desribe their purpose.
Change-Id: If7612a42001776d71bb1a6a80fc631d3d262e6ce
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254449
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: David Pursehouse <dpursehouse@collab.net>
