Make WebKitPy's git_commit_from_svn_revision works for pure Git repository
Created attachment 190192 [details] Patch
Comment on attachment 190192 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190192&action=review > Tools/Scripts/webkitpy/common/checkout/scm/git.py:311 > + for i in xrange(25): > + git_header = self._run_git(['show', '-s', '--pretty=format:%H%n%b', 'HEAD~%s' % i]) Why 25? And isn't the other support better when available?
> Why 25? I did that because Git.svn_revision also use the arbitrary limit of 25 (and that's how we find revisions in the first place). I can change to "while True:", there is really no point at limiting to 25. > And isn't the other support better when available? I just don't know a good way to detect if a Git repository is synced with SVN. Any idea?
I believe git-svn sets a config. I am on a non-dev machine at the moment, but git config -l should show us.
I'm not sure what you're trying to accomplish here (in the sense of which commands are broken that you're trying to fix) (and the fact that there are no tests being modified doesn't help). You can check to see if git-svn is configured by looking for 'svn-remote.svn.url' in the git config. the svn_revision() command is intended to find the most recent svn revision in the checkout. It is not a command I would recommend be used widely, since it'll fail if you're on branches. I think it was added so rniwa could pull something for the perf tests. git_commit_from_svn_revision(), on the other hand, can probably be used to refer to *any* svn revision in the tree. Your patch would make it work only for the most recent 25 (git) revisions, and so this probably isn't correct.
Comment on attachment 190192 [details] Patch r-'ing until I can understand better what's going on ...
For later then, I don't really have time to fix git.
(In reply to comment #5) > I'm not sure what you're trying to accomplish here (in the sense of which commands are broken that you're trying to fix) (and the fact that there are no tests being modified doesn't help). We're trying to fix run-perf-tests on a pure git clone (no svn git).
Elliot tells me this is blocking him from being able to run-perf-tests. :(
I'll take a look and suggest a better fix for this particular case. Likely run-perf-tests should just bail out if it can tell if there's no svn checkout, rather than trying to grep the history.
reopening to fix :).
Created attachment 191898 [details] Patch
Comment on attachment 191898 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=191898&action=review LGTM. Thank you for taking this on. > Tools/Scripts/webkitpy/common/checkout/scm/scm.py:170 > + """Returns the latest svn revision found in the checkout.""" This may be the first time I've found docstrings helpful. :p > Tools/Scripts/webkitpy/common/system/outputcapture.py:32 > +import unittest # Don't use unittest2 here as the autoinstaller may not have it yet. Assuming you tested this with py2.5/2.7 sgtm.
Attachment 191898 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy/common/checkout/scm/git.py', u'Tools/Scripts/webkitpy/common/checkout/scm/scm.py', u'Tools/Scripts/webkitpy/common/checkout/scm/scm_mock.py', u'Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py', u'Tools/Scripts/webkitpy/common/checkout/scm/svn.py', u'Tools/Scripts/webkitpy/common/system/outputcapture.py', u'Tools/Scripts/webkitpy/performance_tests/perftestsrunner.py']" exit_code: 1 Traceback (most recent call last): File "Tools/Scripts/check-webkit-style", line 48, in <module> sys.exit(CheckWebKitStyle().main()) File "/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/style/main.py", line 152, in main patch = host.scm().create_patch(options.git_commit, changed_files=changed_files) File "/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/common/checkout/scm/git.py", line 295, in create_patch return self.prepend_svn_revision(self.run(command, decode_output=False, cwd=self.checkout_root)) File "/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/common/checkout/scm/git.py", line 273, in prepend_svn_revision revision = self.head_svn_revision() File "/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/common/checkout/scm/scm.py", line 167, in head_svn_revision return self.svn_revision(self.checkout_root) File "/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/common/checkout/scm/git.py", line 251, in svn_revision git_log = self._run_git(['log', '-1', '--grep', 'git-svn-id:', self.find_checkout_root(path)]) File "/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/common/checkout/scm/git.py", line 101, in _run_git return self.run(full_command_args, **full_kwargs) File "/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/common/checkout/scm/scm.py", line 78, in run decode_output=decode_output) File "/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 432, in run_command (error_handler or self.default_error_handler)(script_error) File "/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/common/system/executive.py", line 350, in default_error_handler raise error webkitpy.common.system.executive.ScriptError: Failed to run "['git', 'log', '-1', '--grep', 'git-svn-id:', u'/mnt/git/webkit-style-queue']" exit_code: 128 cwd: /mnt/git/webkit-style-queue If any of these errors are false positives, please file a bug against check-webkit-style.
Looks like this patch may have caused the style-queue to barf on itself?
Comment on attachment 191898 [details] Patch style-queue error seems like this may break things.
check-webkit-style is clean locally. there must be something wrong with that command in that checkout itself. any chance you can log into the bot and run the command interactively and see what's wrong?
Alan Cutter is your man.
git svn is not set up by default on the style queue bot. We have a script to set this up for the bots: http://trac.webkit.org/browser/trunk/Tools/EWSTools/configure-git-svn.sh You'll want to add this command to the style queue build script: http://trac.webkit.org/browser/trunk/Tools/EWSTools/GoogleComputeEngine/build-feeder-style-sheriffbot.sh Add the line bash configure-git-svn.sh style-queue && next to bash configure-git-svn.sh sheriff-bot && in the build script. I have run the script on the style-queue bot for you already so it should be safe to resubmit the patch now.
I thought the whole point of this patch was to handle the case where git-svn wasn't setup? Or is that a later patch?
(In reply to comment #20) > I thought the whole point of this patch was to handle the case where git-svn wasn't setup? Or is that a later patch? Sorry, was jumping to conclusions based on the error. This is what I get on the style-queue: alancutter@gce-feeder-01:/mnt/git/webkit-style-queue$ git log -1 --grep git-svn-id: fatal: Invalid object name 'git-svn-id'. alancutter@gce-feeder-01:/mnt/git/webkit-style-queue$ git --version git version 1.7.0.4 While this is what I get locally: [17:33:05]~/repos/WebKit$ git log -1 --grep git-svn-id: commit a7db26f5eab9b33abef5dc6244bf09e0de48fc90 Author: rniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc> Date: Thu Mar 7 03:03:05 2013 +0000 Build fix: PageAllocationAligned no longer has executable flag https://bugs.webkit.org/show_bug.cgi?id=111659 Patch by Adenilson Cavalcanti <cavalcantii@gmail.com> on 2013-03-06 Reviewed by Kentaro Hara. Build fix: use false as parameter to execution flag. * wtf/PageAllocationAligned.cpp: (WTF::PageAllocationAligned::allocate): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@145022 268f45cc-cd09-0410-ab3c-d52691b4dbfc [17:33:07]~/repos/WebKit$ git --version git version 1.8.1.3 Looks like a version problem with git. This could be due to the bots still running Lucid. Please let me know what should be done next with the bots.
Hmm. Yeah, looks like --grep is totally broken in 1.7.0.4.
ok, turns out --grep isn't totally broken, you just need to use "--grep=foo" rather than "--grep" "foo". handling separate arguments wasn't implemented until 8/21/10, and 1.7.0.4 was released in 3/10. New patch coming, no need to change the bots (although we should probably upgrade them to a precise-ish version soon).
Created attachment 192076 [details] use --grep=foo rather than --grep foo
Comment on attachment 192076 [details] use --grep=foo rather than --grep foo View in context: https://bugs.webkit.org/attachment.cgi?id=192076&action=review > Tools/Scripts/webkitpy/common/checkout/scm/git.py:251 > + git_log = self._run_git(['log', '-1', '--grep=git-svn-id:', self.find_checkout_root(path)]) You might add a comment about why we use grep= instead of the standard separate arguments. Or heck, we could even share these two calls in a helper function.
Comment on attachment 192076 [details] use --grep=foo rather than --grep foo View in context: https://bugs.webkit.org/attachment.cgi?id=192076&action=review >> Tools/Scripts/webkitpy/common/checkout/scm/git.py:251 >> + git_log = self._run_git(['log', '-1', '--grep=git-svn-id:', self.find_checkout_root(path)]) > > You might add a comment about why we use grep= instead of the standard separate arguments. Or heck, we could even share these two calls in a helper function. Will do.
Created attachment 192084 [details] consolidate log --grep code
Comment on attachment 192084 [details] consolidate log --grep code View in context: https://bugs.webkit.org/attachment.cgi?id=192084&action=review LGTM. > Tools/Scripts/webkitpy/common/checkout/scm/git.py:252 > + return self._run_git(['log', '-1', '--grep=' + grep_str, '--date=iso', self.find_checkout_root(path)]) You may want to quote grep_str, but this is OK. > Tools/Scripts/webkitpy/common/checkout/scm/git.py:315 > + # FIXME: https://bugs.webkit.org/show_bug.cgi?id=111668 > + # We should change this to run git log --grep 'git-svn-id' instead > + # so that we don't require git+svn to be set up. You might mention _most_recent_log_matching, but you can also do that in the bug.
Committed r145141: <http://trac.webkit.org/changeset/145141>