Fix git depot_tools update flow.

Fix git depot_tools update flow.

In Windows (already worked correctly in posix): gclient wouldn't correctly update depot_tools when called.

Refactored away the bootstrap depot_tools update flow which only still existed in Windows.

Patch contributed by gab@chromium.org

R=maruel@chromium.org
BUG=
TEST=Running `gclient sync` from src/ and see depot_tools being correctly updated under a git depot_tools checkout.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=132191

[gab, 2012-03-20 14:52:18]

PTAL, thanks.

https://chromiumcodereview.appspot.com/9751009/diff/1/gclient.bat
File gclient.bat (right):

https://chromiumcodereview.appspot.com/9751009/diff/1/gclient.bat#newcode47
gclient.bat:47: call git svn rebase -q -q
I noticed update_depot_tools uses -q -q for extra quietness, but wasn't sure why
we are doing that as the rest of the sync process is fairly verbose.

(I added echos above to let the user know what is happening, let me know should
I need to proceed otherwise)

[M-A Ruel, 2012-03-20 15:19:13]

https://chromiumcodereview.appspot.com/9751009/diff/1/.gitignore
File .gitignore (right):

https://chromiumcodereview.appspot.com/9751009/diff/1/.gitignore#newcode1
.gitignore:1: .gitconfig
Why would this file be created?

https://chromiumcodereview.appspot.com/9751009/diff/1/.gitignore#newcode21
.gitignore:21: /.ssh
Same here.

https://chromiumcodereview.appspot.com/9751009/diff/1/bootstrap/update_depot_...
File bootstrap/update_depot_tools.bat (right):

https://chromiumcodereview.appspot.com/9751009/diff/1/bootstrap/update_depot_...
bootstrap/update_depot_tools.bat:7: :: This bootstrap is only used in the svn
workflow.
It'd be nice if it was used by the git workflow too since it'd be coherent with
update_depot_tools.

https://chromiumcodereview.appspot.com/9751009/diff/1/gclient.bat
File gclient.bat (right):

https://chromiumcodereview.appspot.com/9751009/diff/1/gclient.bat#newcode25
gclient.bat:25: IF EXIST "%~dp0.git\." GOTO :GIT_UPDATE
IF EXIST "%~dp0.git\." GOTO :<line 35>

https://chromiumcodereview.appspot.com/9751009/diff/1/gclient.bat#newcode47
gclient.bat:47: call git svn rebase -q -q
On 2012/03/20 14:52:19, gab wrote:
> I noticed update_depot_tools uses -q -q for extra quietness, but wasn't sure
why
> we are doing that as the rest of the sync process is fairly verbose.

What,s the rest of the sync process? You mean the initial one? This one is
verbose because it's much slower and it is a one-time process.

> (I added echos above to let the user know what is happening, let me know
should
> I need to proceed otherwise)

It should do the same than update_depot_tools, it hadn't been updated here when
update_depot_tools verbosity was adjusted.


Overall, I'd prefer this code to be in update_depot_tools.bat

[gab, 2012-03-20 20:10:52]

Removed the bootstrap completely, only the Windows update flow was still using
it. The Windows update flow is now much closer to its posix equivalent.

This can be a problem as people syncing with the current flow will get an error
when the current update script will try to call bootstrap\gclient.bat after
updating the bootstrap dir (since it won't be there anymore).

I could leave gclient.bat and modify it to echo a message mentioning to run the
command again to complete the update (or do something more complex to actually
have the script bootstrap the update itself... this will be legacy code soon
though as everyone will soon be passed the current revision)... I guess the only
option not leaving code behind is a post on chromium-dev...

https://chromiumcodereview.appspot.com/9751009/diff/1/.gitignore
File .gitignore (right):

https://chromiumcodereview.appspot.com/9751009/diff/1/.gitignore#newcode1
.gitignore:1: .gitconfig
On 2012/03/20 15:19:13, Marc-Antoine Ruel wrote:
> Why would this file be created?

Not sure why, but it was created in my Windows checkout and contains:
[core]
        autocrlf = false
        filemode = false

https://chromiumcodereview.appspot.com/9751009/diff/1/.gitignore#newcode21
.gitignore:21: /.ssh
On 2012/03/20 15:19:13, Marc-Antoine Ruel wrote:
> Same here.

This was also created in my Windows checkout and contains .ssh/config and
.ssh/known_hosts.

I think both .gitconfig and .ssh were created here originally as in Windows
depot_tools installs ssh and git. I now notice however that I also have these in
my homedir (i.e. %USERPROFILE% which for me is C:\Users\gab).

The files are identical, so I'll delete them for now and see if they later
reappear in depot_tools\

Actually... I recently set a HOME environment variable pointing to my homedir on
Windows to fix another problem I had (maybe that also forced .ssh\ and
.gitconfig to land in my home, but that wasn't the default (hence the legacy
files left behind))

https://chromiumcodereview.appspot.com/9751009/diff/5001/bootstrap/gclient.sh
File bootstrap/gclient.sh (left):

https://chromiumcodereview.appspot.com/9751009/diff/5001/bootstrap/gclient.sh...
bootstrap/gclient.sh:1: #!/bin/sh
Only Windows was still using the bootstrap.

No reference was left to this shell script in the posix flow.

[M-A Ruel, 2012-03-20 20:31:31]

On 2012/03/20 20:10:52, gab wrote:
> Removed the bootstrap completely, only the Windows update flow was still using
> it. The Windows update flow is now much closer to its posix equivalent.

That's awesome and should be the end goal. In the meantime...

> This can be a problem as people syncing with the current flow will get an
error
> when the current update script will try to call bootstrap\gclient.bat after
> updating the bootstrap dir (since it won't be there anymore).

Yep.
 
> I could leave gclient.bat and modify it to echo a message mentioning to run
the
> command again to complete the update (or do something more complex to actually
> have the script bootstrap the update itself... this will be legacy code soon
> though as everyone will soon be passed the current revision)... I guess the
only
> option not leaving code behind is a post on chromium-dev...

No, the slave won't read the comment. So you need to make the transition process
smooth.

https://chromiumcodereview.appspot.com/9751009/diff/6007/gclient.bat
File gclient.bat (left):

https://chromiumcodereview.appspot.com/9751009/diff/6007/gclient.bat#oldcode27
gclient.bat:27: call svn up -q "%~dp0bootstrap"
The problem is that this call in the previous code will trigger, then the next
line will fail since it doesn't exist anymore, causing all gclient users to fail
in their next call.

<3 batch files.

[gab, 2012-03-20 21:10:15]

Added logic to bootstrap\gclient.bat to "wrap" the update. So that it will
silently switch to the new workflow on the first update (done by the old
script).

https://chromiumcodereview.appspot.com/9751009/diff/6007/update_depot_tools.bat
File update_depot_tools.bat (right):

https://chromiumcodereview.appspot.com/9751009/diff/6007/update_depot_tools.b...
update_depot_tools.bat:35: set UPDATED_DIR=%CD%
This variable was no longer necessary now that I'm doing the update in its own
batch file.

[M-A Ruel, 2012-03-21 01:57:09]

looks good, will have to think a bit about it tomorrow just to make sure nothing
blows up.

https://chromiumcodereview.appspot.com/9751009/diff/10002/bootstrap/gclient.bat
File bootstrap/gclient.bat (right):

https://chromiumcodereview.appspot.com/9751009/diff/10002/bootstrap/gclient.b...
bootstrap/gclient.bat:2: :: Copyright (c) 2010 The Chromium Authors. All rights
reserved.
2012

https://chromiumcodereview.appspot.com/9751009/diff/10002/update_depot_tools.bat
File update_depot_tools.bat (right):

https://chromiumcodereview.appspot.com/9751009/diff/10002/update_depot_tools....
update_depot_tools.bat:2: :: Copyright (c) 2010 The Chromium Authors. All rights
reserved.
2012

https://chromiumcodereview.appspot.com/9751009/diff/10002/update_depot_tools....
update_depot_tools.bat:8: :: This is required with cygwin only.
Actually, cygwin should not get there. :/

[gab, 2012-03-21 13:31:46]

Fixed dates.

Is there a try server for this kind of depot_tools update?

https://chromiumcodereview.appspot.com/9751009/diff/10002/update_depot_tools.bat
File update_depot_tools.bat (right):

https://chromiumcodereview.appspot.com/9751009/diff/10002/update_depot_tools....
update_depot_tools.bat:8: :: This is required with cygwin only.
On 2012/03/21 01:57:09, Marc-Antoine Ruel wrote:
> Actually, cygwin should not get there. :/

This is coming straight from the old script. I think it's to make sure our
version of git/svn has priority over the cygwin one, by putting %~dp0 at the
start of the path.

[M-A Ruel, 2012-03-21 14:32:50]

https://chromiumcodereview.appspot.com/9751009/diff/10002/update_depot_tools.bat
File update_depot_tools.bat (right):

https://chromiumcodereview.appspot.com/9751009/diff/10002/update_depot_tools....
update_depot_tools.bat:8: :: This is required with cygwin only.
On 2012/03/21 13:31:46, gab wrote:
> On 2012/03/21 01:57:09, Marc-Antoine Ruel wrote:
> > Actually, cygwin should not get there. :/
> 
> This is coming straight from the old script. I think it's to make sure our
> version of git/svn has priority over the cygwin one, by putting %~dp0 at the
> start of the path.

Yeah and that's wrong. Remove lines 8 and 10.

[gab, 2012-03-21 14:37:52]

https://chromiumcodereview.appspot.com/9751009/diff/10002/update_depot_tools.bat
File update_depot_tools.bat (right):

https://chromiumcodereview.appspot.com/9751009/diff/10002/update_depot_tools....
update_depot_tools.bat:8: :: This is required with cygwin only.
On 2012/03/21 14:32:50, Marc-Antoine Ruel wrote:
> On 2012/03/21 13:31:46, gab wrote:
> > On 2012/03/21 01:57:09, Marc-Antoine Ruel wrote:
> > > Actually, cygwin should not get there. :/
> > 
> > This is coming straight from the old script. I think it's to make sure our
> > version of git/svn has priority over the cygwin one, by putting %~dp0 at the
> > start of the path.
> 
> Yeah and that's wrong. Remove lines 8 and 10. 

Ok, done.

[M-A Ruel, 2012-03-21 15:05:46]

https://chromiumcodereview.appspot.com/9751009/diff/18008/README
File README (right):

https://chromiumcodereview.appspot.com/9751009/diff/18008/README#newcode36
README:36: To update this distribution manually, run .\update_depot_tools.bat on
Windows,
Remove the .\, it's confusing on line 36 and wrong on line 37.

https://chromiumcodereview.appspot.com/9751009/diff/18008/update_depot_tools.bat
File update_depot_tools.bat (right):

https://chromiumcodereview.appspot.com/9751009/diff/18008/update_depot_tools....
update_depot_tools.bat:33: cd "%~dp0"
You should pop back to the current directory afterward, since this side effect
lasts after this script exits. Also, it's not guaranteed that you are on the
right drive letter. I'm not familiar with pushd/popd so you'll have to test a
bit.
You may also need to change line 8 to
setlocal enableextensions


pushd
cd /d "%~dp0"
call git svn rebase -q -q
popd

[gab, 2012-03-21 16:34:50]

https://chromiumcodereview.appspot.com/9751009/diff/18008/update_depot_tools.bat
File update_depot_tools.bat (right):

https://chromiumcodereview.appspot.com/9751009/diff/18008/update_depot_tools....
update_depot_tools.bat:33: cd "%~dp0"
On 2012/03/21 15:05:46, Marc-Antoine Ruel wrote:
> You should pop back to the current directory afterward, since this side effect
> lasts after this script exits. Also, it's not guaranteed that you are on the
> right drive letter. I'm not familiar with pushd/popd so you'll have to test a
> bit.
> You may also need to change line 8 to
> setlocal enableextensions
> 
> 
> pushd
> cd /d "%~dp0"
> call git svn rebase -q -q
> popd

Actually I did test this and it looks like, because this is in its own script
and "probably because" setlocal is on, this work, i.e. I call `gclient sync`
from my chromium/src directory and gclient.py correctly syncs that directory
after depot_tools update is done.

You are right about cd /d though as cd will fail otherwise if depot_tools is not
on the same drive as the source... Crazy DOS...

I added /d and just tested with a depot_tools on a separate drive and it works
(i.e. calling gclient sync from G:\google\chromium\src when PATH points to
C:\tmp\depot_tools).

enableextensions in the cd case would only cause it to adjust to bad character
case in the directory spelling (which I hope is correct given it comes from a
DOS variable...): http://www.computerhope.com/cdhlp.htm

[M-A Ruel, 2012-03-21 19:02:00]

lgtm with one comment nit

https://chromiumcodereview.appspot.com/9751009/diff/13005/README
File README (right):

https://chromiumcodereview.appspot.com/9751009/diff/13005/README#newcode37
README:37: or .\update_depot_tools on Linux or Mac.
still wrong.

[gab, 2012-03-21 19:08:34]

How should we go about committing this? I tested it on my Windows machine, but I
can't test the "wrap update" flow as I already have the new version...

https://chromiumcodereview.appspot.com/9751009/diff/13005/README
File README (right):

https://chromiumcodereview.appspot.com/9751009/diff/13005/README#newcode37
README:37: or .\update_depot_tools on Linux or Mac.
On 2012/03/21 19:02:00, Marc-Antoine Ruel wrote:
> still wrong.

Changed the '\' to '/' in Linux/Mac case. If that's not what you meant I'm not
sure what else is wrong..?

[M-A Ruel, 2012-03-21 19:32:48]

On 2012/03/21 19:08:34, gab wrote:
> How should we go about committing this? I tested it on my Windows machine, but
I
> can't test the "wrap update" flow as I already have the new version...

The only way would be to:

start svnserve somewhere; actually there's example code in
testing_support/fake_repos.py around line 312. 
1. Run the svnadmin/create conf/svnserver.conf, conf/passwd, svnserve -d
--foreground -r foo from a linux workstation to keep sanity.
2. svn co from your windows workstation.
3. svn commit the current version of depot_tools
4. then commit the new version of depot_tools.
5. Then delete your local checkout from your windows workstation
6. svn co svn://workstation/foo@1 to grab revision 1
7. Verify you have revision 1 by looking at the batch files.
8. foo\gclient
9. verify the update went flawlessly.


> Changed the '\' to '/' in Linux/Mac case. If that's not what you meant I'm not
> sure what else is wrong..?

That's what I had meant.


M-A

[gab, 2012-03-22 16:43:47]

Fully tested using an independent svn server as you suggested.

The only nit was the fix I uploaded in patch set 9 ("%~dp0" alone resolves to,
for example, C:\src\foo", where "%~dp0." resolves correctly to "C:\src\foo\." it
looks like... crazy DOS again).
Before I changed this the svn up -q "%~dp0" call would output an error =>
svn: error resolving case C:\src\foo".
(note the dangling ")

Ready to commit, let me know when you're ready... I'd rather have you here if
things go wrong for some reason!

[M-A Ruel, 2012-03-22 17:29:40]

lgtm anyway

https://chromiumcodereview.appspot.com/9751009/diff/23001/update_depot_tools.bat
File update_depot_tools.bat (right):

https://chromiumcodereview.appspot.com/9751009/diff/23001/update_depot_tools....
update_depot_tools.bat:33: cd /d "%~dp0."
Interesting. Maybe the quotes are not needed at all?

[gab, 2012-03-22 17:46:48]

https://chromiumcodereview.appspot.com/9751009/diff/23001/update_depot_tools.bat
File update_depot_tools.bat (right):

https://chromiumcodereview.appspot.com/9751009/diff/23001/update_depot_tools....
update_depot_tools.bat:33: cd /d "%~dp0."
On 2012/03/22 17:29:40, Marc-Antoine Ruel wrote:
> Interesting. Maybe the quotes are not needed at all?

I'm scared without quotes it might error out on paths with spaces...

[commit-bot: I haz the power, 2012-03-22 17:47:01]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/9751009/23001

[commit-bot: I haz the power, 2012-03-22 17:47:04]

Can't apply patch for file depot_tools/bootstrap/gclient.bat.
While running patch -p1 --forward --force;
patching file depot_tools/bootstrap/gclient.bat
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file
depot_tools/bootstrap/gclient.bat.rej

[gab, 2012-03-22 17:49:14]

On 2012/03/22 17:47:04, I haz the power (commit-bot) wrote:
> Can't apply patch for file depot_tools/bootstrap/gclient.bat.
> While running patch -p1 --forward --force;
> patching file depot_tools/bootstrap/gclient.bat
> Hunk #1 FAILED at 1.
> 1 out of 1 hunk FAILED -- saving rejects to file
> depot_tools/bootstrap/gclient.bat.rej

Hmmm.... could this be due to the fact that svn M/D status are wrong in patch
set 9 above(i.e. bootstrap\update_depot_tools.bat should be D and
bootstrap\gclient.bat should be M).

[M-A Ruel, 2012-03-22 17:53:52]

On 2012/03/22 17:49:14, gab wrote:
> On 2012/03/22 17:47:04, I haz the power (commit-bot) wrote:
> > Can't apply patch for file depot_tools/bootstrap/gclient.bat.
> > While running patch -p1 --forward --force;
> > patching file depot_tools/bootstrap/gclient.bat
> > Hunk #1 FAILED at 1.
> > 1 out of 1 hunk FAILED -- saving rejects to file
> > depot_tools/bootstrap/gclient.bat.rej
> 
> Hmmm.... could this be due to the fact that svn M/D status are wrong in patch
> set 9 above(i.e. bootstrap\update_depot_tools.bat should be D and
> bootstrap\gclient.bat should be M).

The CQ mostly ignores the file status for svn users since it often wrong. I'll
take a look in a few hours, at worst I'll commit it myself but I'd prefer to
figure out what's happening.

[gab, 2012-03-27 14:51:56]

On 2012/03/22 17:53:52, Marc-Antoine Ruel wrote:
> On 2012/03/22 17:49:14, gab wrote:
> > On 2012/03/22 17:47:04, I haz the power (commit-bot) wrote:
> > > Can't apply patch for file depot_tools/bootstrap/gclient.bat.
> > > While running patch -p1 --forward --force;
> > > patching file depot_tools/bootstrap/gclient.bat
> > > Hunk #1 FAILED at 1.
> > > 1 out of 1 hunk FAILED -- saving rejects to file
> > > depot_tools/bootstrap/gclient.bat.rej
> > 
> > Hmmm.... could this be due to the fact that svn M/D status are wrong in
patch
> > set 9 above(i.e. bootstrap\update_depot_tools.bat should be D and
> > bootstrap\gclient.bat should be M).
> 
> The CQ mostly ignores the file status for svn users since it often wrong. I'll
> take a look in a few hours, at worst I'll commit it myself but I'd prefer to
> figure out what's happening.

I tried to apply the patch to trunk locally by doing `git cl patch  9751009 -b
test` from my master branch and it also fails.

Looks like `patch` is having trouble with new files and deleted files (idk much
about `patch`... missing command-line param maybe?).

[M-A Ruel, 2012-03-27 14:53:10]

On 2012/03/27 14:51:56, gab wrote:
> On 2012/03/22 17:53:52, Marc-Antoine Ruel wrote:
> > The CQ mostly ignores the file status for svn users since it often wrong.
I'll
> > take a look in a few hours, at worst I'll commit it myself but I'd prefer to
> > figure out what's happening.
> 
> I tried to apply the patch to trunk locally by doing `git cl patch  9751009 -b
> test` from my master branch and it also fails.
> 
> Looks like `patch` is having trouble with new files and deleted files (idk
much
> about `patch`... missing command-line param maybe?).

Sorry for the delay, I'll get back to it within this week.

[gab, 2012-04-05 14:37:47]

Ping.

On Tue, Mar 27, 2012 at 10:53 AM, <maruel@chromium.org> wrote:

> On 2012/03/27 14:51:56, gab wrote:
>
>> On 2012/03/22 17:53:52, Marc-Antoine Ruel wrote:
>> > The CQ mostly ignores the file status for svn users since it often
>> wrong.
>>
> I'll
>
>> > take a look in a few hours, at worst I'll commit it myself but I'd
>> prefer to
>> > figure out what's happening.
>>
>
>  I tried to apply the patch to trunk locally by doing `git cl patch
>>  9751009 -b
>> test` from my master branch and it also fails.
>>
>
>  Looks like `patch` is having trouble with new files and deleted files (idk
>>
> much
>
>> about `patch`... missing command-line param maybe?).
>>
>
> Sorry for the delay, I'll get back to it within this week.
>
>
http://codereview.chromium.**org/9751009/<http://codereview.chromium.org/9751...
>

[gab, 2012-04-13 14:52:44]

Ping :)

[commit-bot: I haz the power, 2012-04-13 15:24:02]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/gab@chromium.org/9751009/33001

[commit-bot: I haz the power, 2012-04-13 15:24:05]

Can't apply patch for file depot_tools/bootstrap/gclient.bat.
While running patch -p1 --forward --force;
patching file depot_tools/bootstrap/gclient.bat
Hunk #1 FAILED at 1.
1 out of 1 hunk FAILED -- saving rejects to file
depot_tools/bootstrap/gclient.bat.rej