Add support for new repo topology, with submodule-specific merge commits

Add support for new repo topology, with submodule-specific merge commits
on origin/master.

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=141958

[cmp, 2012-06-12 16:05:46]

https://chromiumcodereview.appspot.com/10537117/diff/3001/git_cl.py
File git_cl.py (right):

https://chromiumcodereview.appspot.com/10537117/diff/3001/git_cl.py#newcode1275
git_cl.py:1275: # Delete the branches if they exist
nit: append a period to make this a first class sentence.

https://chromiumcodereview.appspot.com/10537117/diff/3001/tests/git_cl_test.py
File tests/git_cl_test.py (right):

https://chromiumcodereview.appspot.com/10537117/diff/3001/tests/git_cl_test.p...
tests/git_cl_test.py:213: ((['git', 'branch', '-D', 'git-cl-commit'],), ''),
Presumably this test is one without the git submodule merge commit.  Could you
add a test that tests the git submodule commit path, as well?  I think that
makes sense since it will be the common case after src.git is updated to create
that merge commit.

[M-A Ruel, 2012-06-12 16:27:36]

https://chromiumcodereview.appspot.com/10537117/diff/3001/git_cl.py
File git_cl.py (right):

https://chromiumcodereview.appspot.com/10537117/diff/3001/git_cl.py#newcode76
git_cl.py:76: return 1, ''
Can you add a comment when it's happening?

https://chromiumcodereview.appspot.com/10537117/diff/3001/git_cl.py#newcode1203
git_cl.py:1203: # If the base_head is a submodule merge commit, the second
parent of the
I still think it should be the primary parent (and not the second parent) in the
repository. e.g.

# On every new commit in trunk, run all of the following:
# Update refs/heads/trunk
git svn fetch
# Checkout detached HEAD
git checkout $(git rev-parse refs/heads/trunk)
git merge refs/heads/master
# Note NEW_HASH
git checkout refs/heads/master
git reset --hard NEW_HASH

Not tested, but I think it can fast forward even if it is not the primary
parent. I don't think it's exactly the right flow of commands either, especially
that it requires a full checkout and wouldn't work on a bare checkout.

[szager1, 2012-06-13 13:54:46]

https://chromiumcodereview.appspot.com/10537117/diff/3001/git_cl.py
File git_cl.py (right):

https://chromiumcodereview.appspot.com/10537117/diff/3001/git_cl.py#newcode76
git_cl.py:76: return 1, ''
On 2012/06/12 16:27:36, Marc-Antoine Ruel wrote:
> Can you add a comment when it's happening?

When the subprocess fails, communicate() returns None.  That triggers a
ValueError when trying to unpack the return value into (out, code).

I added the above comment to the code.

https://chromiumcodereview.appspot.com/10537117/diff/3001/git_cl.py#newcode1203
git_cl.py:1203: # If the base_head is a submodule merge commit, the second
parent of the
On 2012/06/12 16:27:36, Marc-Antoine Ruel wrote:
> I still think it should be the primary parent (and not the second parent) in
the
> repository. e.g.
> 
> # On every new commit in trunk, run all of the following:
> # Update refs/heads/trunk
> git svn fetch
> # Checkout detached HEAD
> git checkout $(git rev-parse refs/heads/trunk)
> git merge refs/heads/master
> # Note NEW_HASH
> git checkout refs/heads/master
> git reset --hard NEW_HASH
> 
> Not tested, but I think it can fast forward even if it is not the primary
> parent. I don't think it's exactly the right flow of commands either,
especially
> that it requires a full checkout and wouldn't work on a bare checkout.

Done.

https://chromiumcodereview.appspot.com/10537117/diff/3001/git_cl.py#newcode1275
git_cl.py:1275: # Delete the branches if they exist
On 2012/06/12 16:05:46, cmp wrote:
> nit: append a period to make this a first class sentence.

Done.

https://chromiumcodereview.appspot.com/10537117/diff/3001/tests/git_cl_test.py
File tests/git_cl_test.py (right):

https://chromiumcodereview.appspot.com/10537117/diff/3001/tests/git_cl_test.p...
tests/git_cl_test.py:213: ((['git', 'branch', '-D', 'git-cl-commit'],), ''),
On 2012/06/12 16:05:46, cmp wrote:
> Presumably this test is one without the git submodule merge commit.  Could you
> add a test that tests the git submodule commit path, as well?  I think that
> makes sense since it will be the common case after src.git is updated to
create
> that merge commit.

Yep, I'm working on putting tests in this file.

[cmp, 2012-06-13 17:21:32]

lgtm

(not sure if this is the version you want to commit and commit another test
separately or if you're planning to add another test to it before you land it,
lgtm either way and happy to look at another patchset here if needed)

[szager, 2012-06-13 17:31:54]

I'll add the tests in a forthcoming CL.

[commit-bot: I haz the power, 2012-06-13 17:32:18]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/szager@chromium.org/10537117/5002

[commit-bot: I haz the power, 2012-06-13 17:39:36]

Presubmit check for 10537117-5002 failed and returned exit status 1.

warning: code.google.com certificate with fingerprint
c3:fe:90:ab:92:55:ac:c9:d8:63:37:d2:8b:25:67:8a:68:94:f9:05 not verified (check
hostfingerprints or web.cacerts config setting)
Running presubmit commit checks ...
************* Module git_cl
W0612: 75,21:RunGitWithCode: Unused variable 'e'
Checking out rietveld...
Running push-basic.sh
Running save-description-on-failure.sh
Running basic.sh
Running upload-stale.sh
Running upload-local-tracking-branch.sh
Running patch.sh
Running submit-from-new-dir.sh
Running post-dcommit-hook-test.sh
Running hooks.sh
Running abandon.sh
Running upstream.sh

** Presubmit ERRORS **
Fix pylint errors first.

push-basic.sh failed
Command /mnt/data/b/commit-queue/workdir/tools/depot_tools/tests/push-basic.sh
returned non-zero exit status 1 in
/mnt/data/b/commit-queue/workdir/tools/depot_tools/tests
Setting up test upstream git repo...
Setting up test git repo...
Branch work set up to track remote branch master from origin.
TESTING: git-cl upload wants a server
TESTING: git-cl status has no issue
TESTING: upload succeeds (needs a server running on localhost)

WARNING: Use -t or --title to set the title of the patchset.
In the near future, -m or --message will send a message instead.
See http://goo.gl/JGg0Z for details.

TESTING: git-cl status now knows the issue
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0    79      0   102k --:--:-- --:--:-- --:--:--  102k
 0     0    0     0    0    79      0   1550 --:--:-- --:--:-- --:--:--     0
TESTING: Base URL contains branch name
TESTING: git-cl push ok
Migrating from a git-svn v1 layout...
Data from a previous version of git-svn exists, but
	.git/svn
	(required for this version (1.7.0.4) of git-svn) does not exist.
Done migrating from a git-svn v1 layout
Unable to determine upstream SVN information from HEAD history.
Perhaps the repository is empty. at /usr/lib/git-core/git-svn line 517.

Loaded authentication cookies from /home/chrome-bot/.codereview_upload_cookies
Running presubmit commit checks ...

Presubmit checks passed.
Description: 'foo-quux\n\nReview URL: http://localhost:8080/1'
FAILURE: git-cl push ok


basic.sh failed
Command /mnt/data/b/commit-queue/workdir/tools/depot_tools/tests/basic.sh
returned non-zero exit status 1 in
/mnt/data/b/commit-queue/workdir/tools/depot_tools/tests
Setting up test SVN repo...
Setting up test git-svn repo...
TESTING: git-cl upload wants a server
TESTING: git-cl status has no issue
TESTING: upload succeeds (needs a server running on localhost)

WARNING: Use -t or --title to set the title of the patchset.
In the near future, -m or --message will send a message instead.
See http://goo.gl/JGg0Z for details.

TESTING: git-cl status now knows the issue
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed

  0     0    0     0    0    79      0   103k --:--:-- --:--:-- --:--:--  103k
 0     0    0     0    0    79      0   1532 --:--:-- --:--:-- --:--:--     0
TESTING: git-cl dcommits ok
Loaded authentication cookies from /home/chrome-bot/.codereview_upload_cookies
Running presubmit commit checks ...

Presubmit checks passed.
Description: 'foo-quux\n\nReview URL: http://localhost:8080/5'
FAILURE: git-cl dcommits ok


submit-from-new-dir.sh failed
Command
/mnt/data/b/commit-queue/workdir/tools/depot_tools/tests/submit-from-new-dir.sh
returned non-zero exit status 1 in
/mnt/data/b/commit-queue/workdir/tools/depot_tools/tests
Setting up test SVN repo...
Setting up test git-svn repo...
TESTING: upload succeeds

WARNING: Use -t or --title to set the title of the patchset.
In the near future, -m or --message will send a message instead.
See http://goo.gl/JGg0Z for details.

TESTING: git-cl dcommits ok
Loaded authentication cookies from /home/chrome-bot/.codereview_upload_cookies
Running presubmit commit checks ...

Presubmit checks passed.
Description: 'test\nBUG=\nTEST=\n\nReview URL: http://localhost:8080/21'
FAILURE: git-cl dcommits ok


post-dcommit-hook-test.sh failed
Command
/mnt/data/b/commit-queue/workdir/tools/depot_tools/tests/post-dcommit-hook-test.sh
returned non-zero exit status 1 in
/mnt/data/b/commit-queue/workdir/tools/depot_tools/tests
Setting up test SVN repo...
Setting up test git-svn repo...
Branch work set up to track local branch master.
TESTING: dcommitted code
Description: 'dcommit'
TESTING: post-cl-dcommit hook executed
FAILURE: post-cl-dcommit hook executed


Presubmit checks took 436.3s to calculate.

[M-A Ruel, 2012-06-13 17:45:47]

On 2012/06/13 17:39:36, I haz the power (commit-bot) wrote:
(...)
> TESTING: git-cl push ok
> Migrating from a git-svn v1 layout...
> Data from a previous version of git-svn exists, but
> 	.git/svn
> 	(required for this version (1.7.0.4) of git-svn) does not exist.
> Done migrating from a git-svn v1 layout
> Unable to determine upstream SVN information from HEAD history.
> Perhaps the repository is empty. at /usr/lib/git-core/git-svn line 517.

That's weird. I thought we wouldn't run git svn commands unless we found a git
svn remote.

I'm not sure about the rest.

[commit-bot: I haz the power, 2012-06-13 20:20:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/szager@chromium.org/10537117/2002

[commit-bot: I haz the power, 2012-06-13 20:28:25]

Change committed as 141958