Replace "git svn" with "git footers" commands to get commit position or svn revision.

Replace "git svn" with "git footers" commands to get commit position or svn revision.

BUG=391109
NOTRY=true

Committed: https://crrev.com/19c283b3e53587b78825eb192de3f9187ee55a77
Cr-Commit-Position: refs/heads/master@{#294190}

[prasadv, 2014-09-09 23:53:12]

prasadv@chromium.org changed reviewers:
+ qyearsley@chromium.org

[qyearsley, 2014-09-10 01:04:14]

https://codereview.chromium.org/552013008/diff/1/tools/auto_bisect/source_con...
File tools/auto_bisect/source_control.py (right):

https://codereview.chromium.org/552013008/diff/1/tools/auto_bisect/source_con...
tools/auto_bisect/source_control.py:208: def SVNFindRev(self, git_revision,
cwd=None):
This should be renamed now -- maybe "GetCommitPosition"?

https://codereview.chromium.org/552013008/diff/1/tools/auto_bisect/source_con...
tools/auto_bisect/source_control.py:209: """Finds a SVN revision OR
Commit-Postion number for the given git hash.
We could just say git commit position, instead of SVN revision OR commit
position, since before the migration, commit position was equal to SVN revision.

https://codereview.chromium.org/552013008/diff/1/tools/auto_bisect/source_con...
tools/auto_bisect/source_control.py:215: git_revision: The git SHA1 to use.
Should add a line for "cwd" here -- its purpose is to allow us to get commit
positions for other repositories, right?

https://codereview.chromium.org/552013008/diff/1/tools/auto_bisect/source_con...
tools/auto_bisect/source_control.py:218: Git number (aka git commit position) OR
an SVN revision as integer,
You could just say "A git commit position number or None".

https://codereview.chromium.org/552013008/diff/1/tools/auto_bisect/source_con...
tools/auto_bisect/source_control.py:223: svn_revision = output.strip()
Call this commit_position instead of svn_revision.

https://codereview.chromium.org/552013008/diff/1/tools/bisect-perf-regression...
File tools/bisect-perf-regression_test.py (right):

https://codereview.chromium.org/552013008/diff/1/tools/bisect-perf-regression...
tools/bisect-perf-regression_test.py:259: """Determine numerical SVN revision or
Commit Position."""
Note, this docstring doesn't really describe what the test does, it describes
what the test verifies... maybe this docstring isn't necessary?

Also, this test relies on being run in a git repo with a certain state, since it
actually runs the git commands, right?

https://codereview.chromium.org/552013008/diff/1/tools/bisect-perf-regression...
tools/bisect-perf-regression_test.py:267:
bisect_instance.source_control.SVNFindRev(svn_git_rev))
What about SVN revs from before the Cr-Commit-Position footer started getting
added to the commit messages?

For example, crrev.com/280000?

Since you're not running "git svn find-rev" anymore, I'd expect this to fail
now. (Which may be fine, since it will only fail for relatively old revisions.)

https://codereview.chromium.org/552013008/diff/1/tools/bisect-perf-regression...
tools/bisect-perf-regression_test.py:280:
bisect_instance.source_control.SVNFindRev(svn_wk_rev, depot_path))
Each of these could be separate test methods -- 

for example,

testSVNFindRev_Webkit(self):
  svn_wk_rev = 'a94d028e0f2c77f159b3dac95eb90c3b4cf48c61'
  depot_path = os.path.join(bisect_instance.src_cwd, 'src', 'third_party',
      'WebKit')
  self.assertEqual(
      181660,
      bisect_instance.source_control.SVNFindRev(svn_wk_rev, depot_path))

https://codereview.chromium.org/552013008/diff/1/tools/bisect-perf-regression...
tools/bisect-perf-regression_test.py:283: 
Formatting: One blank line between methods.

[prasadv, 2014-09-10 17:16:33]

I created this cl to replace git svn command to git footers.
which still gives SVN revision for revisions before Git migration. I think we
can replace all reference to "SVN" to "commit position" in codes later.

https://codereview.chromium.org/552013008/diff/1/tools/auto_bisect/source_con...
File tools/auto_bisect/source_control.py (right):

https://codereview.chromium.org/552013008/diff/1/tools/auto_bisect/source_con...
tools/auto_bisect/source_control.py:208: def SVNFindRev(self, git_revision,
cwd=None):
On 2014/09/10 01:04:14, qyearsley wrote:
> This should be renamed now -- maybe "GetCommitPosition"?
Right now git footers command get commit position if available else gets svn
revision. But eventually SVN will vanish, so changing it accordingly.

https://codereview.chromium.org/552013008/diff/1/tools/auto_bisect/source_con...
tools/auto_bisect/source_control.py:209: """Finds a SVN revision OR
Commit-Postion number for the given git hash.
On 2014/09/10 01:04:14, qyearsley wrote:
> We could just say git commit position, instead of SVN revision OR commit
> position, since before the migration, commit position was equal to SVN
revision.

Done.

https://codereview.chromium.org/552013008/diff/1/tools/auto_bisect/source_con...
tools/auto_bisect/source_control.py:215: git_revision: The git SHA1 to use.
On 2014/09/10 01:04:14, qyearsley wrote:
> Should add a line for "cwd" here -- its purpose is to allow us to get commit
> positions for other repositories, right?

Done.

https://codereview.chromium.org/552013008/diff/1/tools/auto_bisect/source_con...
tools/auto_bisect/source_control.py:218: Git number (aka git commit position) OR
an SVN revision as integer,
On 2014/09/10 01:04:14, qyearsley wrote:
> You could just say "A git commit position number or None".

Done.

https://codereview.chromium.org/552013008/diff/1/tools/auto_bisect/source_con...
tools/auto_bisect/source_control.py:223: svn_revision = output.strip()
On 2014/09/10 01:04:13, qyearsley wrote:
> Call this commit_position instead of svn_revision.

Done.

https://codereview.chromium.org/552013008/diff/1/tools/bisect-perf-regression...
File tools/bisect-perf-regression_test.py (right):

https://codereview.chromium.org/552013008/diff/1/tools/bisect-perf-regression...
tools/bisect-perf-regression_test.py:259: """Determine numerical SVN revision or
Commit Position."""
On 2014/09/10 01:04:14, qyearsley wrote:
> Note, this docstring doesn't really describe what the test does, it describes
> what the test verifies... maybe this docstring isn't necessary?
> 
> Also, this test relies on being run in a git repo with a certain state, since
it
> actually runs the git commands, right?
It should run in a directory where that git hash belongs, eg, run in src for
chromium sha, in src/v8 directory for V8 git hashes.
I don't think we need any special state other than running theses test on git
hash revision greater than the ones used in this test.

https://codereview.chromium.org/552013008/diff/1/tools/bisect-perf-regression...
tools/bisect-perf-regression_test.py:267:
bisect_instance.source_control.SVNFindRev(svn_git_rev))
On 2014/09/10 01:04:14, qyearsley wrote:
> What about SVN revs from before the Cr-Commit-Position footer started getting
> added to the commit messages?
> 
> For example, crrev.com/280000?
> 
> Since you're not running "git svn find-rev" anymore, I'd expect this to fail
> now. (Which may be fine, since it will only fail for relatively old
revisions.)

git footers will find both svn and commit position.

https://codereview.chromium.org/552013008/diff/1/tools/bisect-perf-regression...
tools/bisect-perf-regression_test.py:280:
bisect_instance.source_control.SVNFindRev(svn_wk_rev, depot_path))
On 2014/09/10 01:04:14, qyearsley wrote:
> Each of these could be separate test methods -- 
> 
> for example,
> 
> testSVNFindRev_Webkit(self):
>   svn_wk_rev = 'a94d028e0f2c77f159b3dac95eb90c3b4cf48c61'
>   depot_path = os.path.join(bisect_instance.src_cwd, 'src', 'third_party',
>       'WebKit')
>   self.assertEqual(
>       181660,
>       bisect_instance.source_control.SVNFindRev(svn_wk_rev, depot_path))

Done.

https://codereview.chromium.org/552013008/diff/1/tools/bisect-perf-regression...
tools/bisect-perf-regression_test.py:283: 
On 2014/09/10 01:04:14, qyearsley wrote:
> Formatting: One blank line between methods.

Done.

[qyearsley, 2014-09-10 17:49:53]

lgtm

https://codereview.chromium.org/552013008/diff/1/tools/bisect-perf-regression...
File tools/bisect-perf-regression_test.py (right):

https://codereview.chromium.org/552013008/diff/1/tools/bisect-perf-regression...
tools/bisect-perf-regression_test.py:259: """Determine numerical SVN revision or
Commit Position."""
On 2014/09/10 17:16:33, prasadv wrote:
> On 2014/09/10 01:04:14, qyearsley wrote:
> > Note, this docstring doesn't really describe what the test does, it
describes
> > what the test verifies... maybe this docstring isn't necessary?
> > 
> > Also, this test relies on being run in a git repo with a certain state,
since
> it
> > actually runs the git commands, right?
> It should run in a directory where that git hash belongs, eg, run in src for
> chromium sha, in src/v8 directory for V8 git hashes.
> I don't think we need any special state other than running theses test on git
> hash revision greater than the ones used in this test.

Alright, sounds good.

https://codereview.chromium.org/552013008/diff/1/tools/bisect-perf-regression...
tools/bisect-perf-regression_test.py:267:
bisect_instance.source_control.SVNFindRev(svn_git_rev))
On 2014/09/10 17:16:33, prasadv wrote:
> On 2014/09/10 01:04:14, qyearsley wrote:
> > What about SVN revs from before the Cr-Commit-Position footer started
getting
> > added to the commit messages?
> > 
> > For example, crrev.com/280000?
> > 
> > Since you're not running "git svn find-rev" anymore, I'd expect this to fail
> > now. (Which may be fine, since it will only fail for relatively old
> revisions.)
> 
> git footers will find both svn and commit position.

Nice, I just tested this, it works.
$ git footers --position-num 89aa0c99
280000

[prasadv, 2014-09-10 17:57:12]

The CQ bit was checked by prasadv@chromium.org

[commit-bot: I haz the power, 2014-09-10 18:02:35]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/prasadv@chromium.org/552013008/20001

[commit-bot: I haz the power, 2014-09-10 18:06:05]

Committed patchset #2 (id:20001) as 9222907fc40ff84eabb41e30dde102864a47f351

[commit-bot: I haz the power, 2014-09-10 18:14:09]

Patchset 2 (id:??) landed as
https://crrev.com/19c283b3e53587b78825eb192de3f9187ee55a77
Cr-Commit-Position: refs/heads/master@{#294190}