Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
478843002:
Script that checks git credentials and attempts to push to a test repo. (Closed)
|
Created:
6 years, 4 months ago by Vadim Sh. Modified:
6 years, 4 months ago Reviewers:
iannucci CC:
chromium-reviews Base URL:
svn://svn.chromium.org/chrome/trunk/src Project:
chromium Visibility:
Public. |
DescriptionScript that checks git credentials and attempts to push to a test repo.
R=iannucci@chromium.org
BUG=399054
NOTRY=true
Patch Set 1 #
Total comments: 14
Patch Set 2 : fixes #Patch Set 3 : add confirmation #
Total comments: 2
Patch Set 4 : check git version #Patch Set 5 : also rename to check_git_push_access.py #
Total comments: 6
Patch Set 6 : #Messages
Total messages: 11 (0 generated)
ptal
added confirmation prompt please check the message it prints
talked with chase, let's ONLY upload information from google-owned computers (corp.google.com). https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py File tools/check_git_access.py (right): https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py#ne... tools/check_git_access.py:98: '_netrc' if sys.platform.startswith('win') else '.netrc') os.path.expanduser works on win + *nix, AFAIK. https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py#ne... tools/check_git_access.py:115: 'checker_version': CHECKER_VERSION, git version too? https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py#ne... tools/check_git_access.py:136: return os.path.join(REPO_ROOT, '.check_git_access_conf.json') should set the svn:ignore property for this https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py#ne... tools/check_git_access.py:209: # Ref to push to, each user has its own ref. I'm assuming this is enforced by acls? https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py#ne... tools/check_git_access.py:261: req = urllib2.Request( we should prompt users who run this manually (e.g. not googlers), if they want to upload this stuff. https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py#ne... tools/check_git_access.py:281: parser = optparse.OptionParser(description=sys.modules[__name__].__doc__) why not argparse :( python in depot_tools is 2.7 for win, and the only supported mac and linux dev platforms are 2.7... The only place where 2.6 is around is on a (small) handful of master machines. https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py#ne... tools/check_git_access.py:288: default=MOTHERSHIP_URL, I'm assuming we prompt the first time this happens https://codereview.chromium.org/478843002/diff/40001/tools/check_git_access.py File tools/check_git_access.py (right): https://codereview.chromium.org/478843002/diff/40001/tools/check_git_access.p... tools/check_git_access.py:285: 'Chrome Infrastructure Team and no one else:') Let's just report for corp.google.com, and others will just not upload any data at all. They'll need to report problems via email.
https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py File tools/check_git_access.py (right): https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py#ne... tools/check_git_access.py:98: '_netrc' if sys.platform.startswith('win') else '.netrc') On 2014/08/16 00:09:35, iannucci wrote: > os.path.expanduser works on win + *nix, AFAIK. Yeah, but the check for HOME is still required, and _netrc vs .netrc check too. So whatever. https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py#ne... tools/check_git_access.py:115: 'checker_version': CHECKER_VERSION, On 2014/08/16 00:09:35, iannucci wrote: > git version too? Done. https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py#ne... tools/check_git_access.py:136: return os.path.join(REPO_ROOT, '.check_git_access_conf.json') On 2014/08/16 00:09:35, iannucci wrote: > should set the svn:ignore property for this I'd rather store it in .svn then. Will it work that way? https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py#ne... tools/check_git_access.py:209: # Ref to push to, each user has its own ref. On 2014/08/16 00:09:35, iannucci wrote: > I'm assuming this is enforced by acls? Only down to committers level, e.g. chromium-committers can push to refs/push-test/* (and anyone else can't). That also mean that evil committers can push to somebody else's tags. But evil committers can do pretty much anything now anyway. Also refs/push-test/* is readable only by committers. https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py#ne... tools/check_git_access.py:261: req = urllib2.Request( On 2014/08/16 00:09:35, iannucci wrote: > we should prompt users who run this manually (e.g. not googlers), if they want > to upload this stuff. Done. https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py#ne... tools/check_git_access.py:281: parser = optparse.OptionParser(description=sys.modules[__name__].__doc__) On 2014/08/16 00:09:35, iannucci wrote: > why not argparse :( > > python in depot_tools is 2.7 for win, and the only supported mac and linux dev > platforms are 2.7... The only place where 2.6 is around is on a (small) handful > of master machines. What's the difference for such simple script? I'm more used to optparse. https://codereview.chromium.org/478843002/diff/1/tools/check_git_access.py#ne... tools/check_git_access.py:288: default=MOTHERSHIP_URL, On 2014/08/16 00:09:35, iannucci wrote: > I'm assuming we prompt the first time this happens If running as hook on *.corp.google.com machine, then submit silently. If invoked manually (e.g. not as hook), always prompt.
https://codereview.chromium.org/478843002/diff/40001/tools/check_git_access.py File tools/check_git_access.py (right): https://codereview.chromium.org/478843002/diff/40001/tools/check_git_access.p... tools/check_git_access.py:285: 'Chrome Infrastructure Team and no one else:') On 2014/08/16 00:09:35, iannucci wrote: > Let's just report for http://corp.google.com, and others will just not upload any data > at all. They'll need to report problems via email. Done.
https://codereview.chromium.org/478843002/diff/70001/tools/check_git_push_acc... File tools/check_git_push_access.py (right): https://codereview.chromium.org/478843002/diff/70001/tools/check_git_push_acc... tools/check_git_push_access.py:302: # Do not upload it outside of corp. It still is not ran at all outside corp, since attempt to push also sort of gives us some information... https://codereview.chromium.org/478843002/diff/70001/tools/check_git_push_acc... tools/check_git_push_access.py:306: 'You can send it to chrome-git-migration@google.com if you need help ' Is this a good email?
lgtm https://codereview.chromium.org/478843002/diff/70001/tools/check_git_push_acc... File tools/check_git_push_access.py (right): https://codereview.chromium.org/478843002/diff/70001/tools/check_git_push_acc... tools/check_git_push_access.py:306: 'You can send it to chrome-git-migration@google.com if you need help ' s/it/the above report https://codereview.chromium.org/478843002/diff/70001/tools/check_git_push_acc... tools/check_git_push_access.py:307: 'with setting up you committer git account.') to set up your
https://codereview.chromium.org/478843002/diff/70001/tools/check_git_push_acc... File tools/check_git_push_access.py (right): https://codereview.chromium.org/478843002/diff/70001/tools/check_git_push_acc... tools/check_git_push_access.py:306: 'You can send it to chrome-git-migration@google.com if you need help ' On 2014/08/16 01:55:19, iannucci wrote: > s/it/the above report Done. https://codereview.chromium.org/478843002/diff/70001/tools/check_git_push_acc... tools/check_git_push_access.py:307: 'with setting up you committer git account.') On 2014/08/16 01:55:19, iannucci wrote: > to set up your Done.
The CQ bit was checked by vadimsh@chromium.org
The CQ bit was unchecked by vadimsh@chromium.org
Committed manually as r290083. |
