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>
This reverts commit 4abf8e6ef8.
The curl process for updating the cookie file is not atomic. When
fetching many bundles in parallel, we can sometimes corrupt the file
causing it to be cleared. Since users should manage gitcookies on
their own, leave it read-only.
Bug: https://crbug.com/gerrit/12300
Change-Id: Id472c99b197bc4cf8533c649f8881509f38643c1
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254092
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
Tested-by: Mike Frysinger <vapier@google.com>
(cherry picked from commit dc1d0e0c7f)
This reverts commit 4abf8e6ef8.
The curl process for updating the cookie file is not atomic. When
fetching many bundles in parallel, we can sometimes corrupt the file
causing it to be cleared. Since users should manage gitcookies on
their own, leave it read-only.
Bug: https://crbug.com/gerrit/12300
Change-Id: Id472c99b197bc4cf8533c649f8881509f38643c1
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254092
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
Tested-by: Mike Frysinger <vapier@google.com>
We were perhaps a bit too hasty to jump to git-2.10. Existing LTS
releases of Ubuntu are quite old still: Trusty has 1.9 while Xenial
has 2.5. While we plan on dropping support for those eventually as
we migrate to Python 3.6, we don't need to be so strict just yet on
the git versions.
We also want to disconnect the version the repo launcher requires
from the version the rest of the source tree requires. The repo
launcher doesn't need as many features, and being flexible there
allows us more freedom to upgrade & rollback as needed.
So we'll allow git-1.7 again, but start warning on any users older
than git-1.9. This aligns better with existing LTS releases, and
gives users a chance to start upgrading before we cut them off.
Change-Id: I140305dd8e42c9719c84e2aee0dc6a5c5b18da25
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254573
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
Tested-by: Mike Frysinger <vapier@google.com>
Going purely on upstream package release cycles doesn't tell the whole
story: a lot of people run LTS distros which will have older versions
of software we want to support.
Build out a table for us to quickly reference when making decisions as
to what versions of git/python we want to support, and when we can drop
them. This will also help to refer users to as why we made a specific
decision that might be affecting them.
Change-Id: I7aea24bbefd50e358aeacf11e8c15a346c8fb8a9
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254572
Tested-by: Mike Frysinger <vapier@google.com>
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
This way we can push out the updated stable branch change.
Change-Id: I72d5dab4523a10dfeb6529796892096aa80eba3c
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254492
Reviewed-by: David Pursehouse <dpursehouse@collab.net>
Tested-by: Mike Frysinger <vapier@google.com>
Some automatic git operations will prune objects on us, and not just
the gc step. Normally we don't care, but with shared projects, we
will have multiple git checkouts with refs that the others cannot
see, but with a shared object dir. Any pruning of objects based on
refs in just one repo can easily break the others.
git-2.7.0 introduced a preciousObjects setting which tells git to
never prune objects for this exact scenario: there might be refs in
some location that git is unable to see.
Change-Id: I781de27c5bbe1d4c70f0187566141c9cce088bd8
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254392
Reviewed-by: Nasser Grainawi <nasser@codeaurora.org>
Reviewed-by: David Riley <davidriley@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: Mike Frysinger <vapier@google.com>
Since tempfile.mkstemp() returns a file handle in binary mode,
make sure we turn our strings into bytes before writing.
Bug: https://crbug.com/gerrit/12043
Change-Id: I3e84d595e84b8bc12a1fbc7fd0bb3ea0ba2832b0
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254393
Reviewed-by: Michael Mortensen <mmortensen@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: Mike Frysinger <vapier@google.com>
(cherry picked from commit 163d42eb43)
Since tempfile.mkstemp() returns a file handle in binary mode,
make sure we turn our strings into bytes before writing.
Bug: https://crbug.com/gerrit/12043
Change-Id: I3e84d595e84b8bc12a1fbc7fd0bb3ea0ba2832b0
Reviewed-on: https://gerrit-review.googlesource.com/c/git-repo/+/254393
Reviewed-by: Michael Mortensen <mmortensen@google.com>
Reviewed-by: Mike Frysinger <vapier@google.com>
Tested-by: Mike Frysinger <vapier@google.com>
