Make move_source_file.py work in blink again.

Make move_source_file.py work in blink again.

It got busted after the repository merge. You still have to run it from
third_party/WebKit. It doesn't work if you run it from src/.

Also update comment to note that it now does reorder gyp(i) entries.

Tested locally by doing a dummy move in each of chromium and blink.

Committed: https://crrev.com/cbc1e379d2cdd15075312be77c769ab646ad4607
Cr-Commit-Position: refs/heads/master@{#370778}

[dgrogan, 2016-01-21 04:15:11]

Description was changed from

==========
Make move_source_file.py work in blink again.

It got busted after the repository merge.

Also update comment to note that it now does reorder gyp(i) entries.
==========

to

==========
Make move_source_file.py work in blink again.

It got busted after the repository merge. You still have to run it from
third_party/WebKit. It doesn't work if you run it from src/.

Also update comment to note that it now does reorder gyp(i) entries.
==========

[dgrogan, 2016-01-21 04:15:56]

Description was changed from

==========
Make move_source_file.py work in blink again.

It got busted after the repository merge. You still have to run it from
third_party/WebKit. It doesn't work if you run it from src/.

Also update comment to note that it now does reorder gyp(i) entries.
==========

to

==========
Make move_source_file.py work in blink again.

It got busted after the repository merge. You still have to run it from
third_party/WebKit. It doesn't work if you run it from src/.

Also update comment to note that it now does reorder gyp(i) entries.

Tested locally by doing a dummy move in each of chromium and blink.
==========

[dgrogan, 2016-01-21 04:20:47]

dgrogan@chromium.org changed reviewers:
+ satorux@chromium.org

[dgrogan, 2016-01-21 04:20:47]

Satoru, can you review this as de-facto owner of move_source_file.py?

Would you like me to add you to the OWNERS file for it?

[satorux1, 2016-01-21 05:52:07]

I don't actively work on it but I'm totally fine with becoming a per-file OWNER
of this script. :)

https://codereview.chromium.org/1614633002/diff/1/tools/git/move_source_file.py
File tools/git/move_source_file.py (right):

https://codereview.chromium.org/1614633002/diff/1/tools/git/move_source_file....
tools/git/move_source_file.py:215: # git rev-parse returns 0 even when run in
the .git directory. We don't want
how about commenting that we use 'git rev-parse' to check if the script is run
inside of a git checkout.

https://codereview.chromium.org/1614633002/diff/1/tools/git/move_source_file....
tools/git/move_source_file.py:217: if not os.system('git rev-parse') == 0 or
os.getcwd().endswith(".git"):
This probably matches "foo/bar.git".


how about os.path.basename(os.getcwd()) == '.git' ?

https://codereview.chromium.org/1614633002/diff/1/tools/git/move_source_file....
tools/git/move_source_file.py:221: in_blink =
os.getcwd().endswith("third_party/WebKit")
'/' may not match on Windows? while you are at it, could you make it friendly
for Windows? something like below would work:

  cwd = os.getcwd()
  parent = os.path.dirname(cwd)
  os.path.basename(parent) == 'third_party' and
  os.path.basename(cwd) == 'WebKit'

[dgrogan, 2016-01-21 19:50:53]

dgrogan@chromium.org changed reviewers:
+ scottmg@chromium.org

[dgrogan, 2016-01-21 19:50:53]

scottmg, can you review the OWNERS change? I realize it's suboptimal to not have
more OWNERS for the git/ directory but I don't want to saddle satorux with even
more responsibility. At least this is a marginal step in the right direction.

https://codereview.chromium.org/1614633002/diff/1/tools/git/move_source_file.py
File tools/git/move_source_file.py (right):

https://codereview.chromium.org/1614633002/diff/1/tools/git/move_source_file....
tools/git/move_source_file.py:215: # git rev-parse returns 0 even when run in
the .git directory. We don't want
On 2016/01/21 05:52:07, satorux1 wrote:
> how about commenting that we use 'git rev-parse' to check if the script is run
> inside of a git checkout.

Done.

https://codereview.chromium.org/1614633002/diff/1/tools/git/move_source_file....
tools/git/move_source_file.py:217: if not os.system('git rev-parse') == 0 or
os.getcwd().endswith(".git"):
On 2016/01/21 05:52:07, satorux1 wrote:
> This probably matches "foo/bar.git".
> 
> 
> how about os.path.basename(os.getcwd()) == '.git' ?

Good point. Fixed.

https://codereview.chromium.org/1614633002/diff/1/tools/git/move_source_file....
tools/git/move_source_file.py:221: in_blink =
os.getcwd().endswith("third_party/WebKit")
On 2016/01/21 05:52:07, satorux1 wrote:
> '/' may not match on Windows? while you are at it, could you make it friendly
> for Windows? something like below would work:
> 
>   cwd = os.getcwd()
>   parent = os.path.dirname(cwd)
>   os.path.basename(parent) == 'third_party' and
>   os.path.basename(cwd) == 'WebKit'

Done.

[scottmg, 2016-01-21 19:57:20]

lgtm

https://codereview.chromium.org/1614633002/diff/20001/tools/git/move_source_f...
File tools/git/move_source_file.py (right):

https://codereview.chromium.org/1614633002/diff/20001/tools/git/move_source_f...
tools/git/move_source_file.py:218: if (not os.system('git rev-parse') == 0 or
os.system() != 0 seems more natural

https://codereview.chromium.org/1614633002/diff/20001/tools/git/move_source_f...
tools/git/move_source_file.py:225: in_blink = (os.path.basename(parent) ==
'third_party' and
I might make this a --blink option with a default of this value. But it's fine
as is if that doesn't seem useful to you.

[dgrogan, 2016-01-21 20:05:49]

Thanks for the review.

https://codereview.chromium.org/1614633002/diff/20001/tools/git/move_source_f...
File tools/git/move_source_file.py (right):

https://codereview.chromium.org/1614633002/diff/20001/tools/git/move_source_f...
tools/git/move_source_file.py:218: if (not os.system('git rev-parse') == 0 or
On 2016/01/21 19:57:20, scottmg wrote:
> os.system() != 0 seems more natural

Done.

https://codereview.chromium.org/1614633002/diff/20001/tools/git/move_source_f...
tools/git/move_source_file.py:225: in_blink = (os.path.basename(parent) ==
'third_party' and
On 2016/01/21 19:57:20, scottmg wrote:
> I might make this a --blink option with a default of this value. But it's fine
> as is if that doesn't seem useful to you.

You're thinking of the move from third_party/WebKit/ to blink/?

[scottmg, 2016-01-21 20:14:39]

https://codereview.chromium.org/1614633002/diff/20001/tools/git/move_source_f...
File tools/git/move_source_file.py (right):

https://codereview.chromium.org/1614633002/diff/20001/tools/git/move_source_f...
tools/git/move_source_file.py:225: in_blink = (os.path.basename(parent) ==
'third_party' and
On 2016/01/21 20:05:49, dgrogan wrote:
> On 2016/01/21 19:57:20, scottmg wrote:
> > I might make this a --blink option with a default of this value. But it's
fine
> > as is if that doesn't seem useful to you.
> 
> You're thinking of the move from third_party/WebKit/ to blink/?

Yeah, or running from src, but moving a file in third_party/WebKit. (Though I
don't know if that even works... So, meh. :)

[dgrogan, 2016-01-21 20:18:30]

https://codereview.chromium.org/1614633002/diff/20001/tools/git/move_source_f...
File tools/git/move_source_file.py (right):

https://codereview.chromium.org/1614633002/diff/20001/tools/git/move_source_f...
tools/git/move_source_file.py:225: in_blink = (os.path.basename(parent) ==
'third_party' and
On 2016/01/21 20:14:39, scottmg wrote:
> On 2016/01/21 20:05:49, dgrogan wrote:
> > On 2016/01/21 19:57:20, scottmg wrote:
> > > I might make this a --blink option with a default of this value. But it's
> fine
> > > as is if that doesn't seem useful to you.
> > 
> > You're thinking of the move from third_party/WebKit/ to blink/?
> 
> Yeah, or running from src, but moving a file in third_party/WebKit. (Though I
> don't know if that even works... So, meh. :)

Sure doesn't :)

[dgrogan, 2016-01-21 20:18:40]

The CQ bit was checked by dgrogan@chromium.org

[dgrogan, 2016-01-21 20:18:40]

The patchset sent to the CQ was uploaded after l-g-t-m from scottmg@chromium.org
Link to the patchset: https://codereview.chromium.org/1614633002/#ps40001
(title: "change from 'not... == 0' to '!= 0'")

[commit-bot: I haz the power, 2016-01-21 20:19:28]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1614633002/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1614633002/40001

[commit-bot: I haz the power, 2016-01-21 20:39:07]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-01-21 20:41:15]

Description was changed from

==========
Make move_source_file.py work in blink again.

It got busted after the repository merge. You still have to run it from
third_party/WebKit. It doesn't work if you run it from src/.

Also update comment to note that it now does reorder gyp(i) entries.

Tested locally by doing a dummy move in each of chromium and blink.
==========

to

==========
Make move_source_file.py work in blink again.

It got busted after the repository merge. You still have to run it from
third_party/WebKit. It doesn't work if you run it from src/.

Also update comment to note that it now does reorder gyp(i) entries.

Tested locally by doing a dummy move in each of chromium and blink.

Committed: https://crrev.com/cbc1e379d2cdd15075312be77c769ab646ad4607
Cr-Commit-Position: refs/heads/master@{#370778}
==========

[commit-bot: I haz the power, 2016-01-21 20:41:15]

Patchset 3 (id:??) landed as
https://crrev.com/cbc1e379d2cdd15075312be77c769ab646ad4607
Cr-Commit-Position: refs/heads/master@{#370778}