Add --transitive flag.

Add --transitive flag.
When specifying a revision (with -r) propagates the date of the revision to its children.

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

[M-A Ruel, 2011-04-20 15:43:28]

http://codereview.chromium.org/6873110/diff/1008/gclient.py
File gclient.py (right):

http://codereview.chromium.org/6873110/diff/1008/gclient.py#newcode365
gclient.py:365: check-ini of revision 101."""
ini?

http://codereview.chromium.org/6873110/diff/1008/gclient_scm.py
File gclient_scm.py (right):

http://codereview.chromium.org/6873110/diff/1008/gclient_scm.py#newcode130
gclient_scm.py:130: """Returns the current revision's date in the form
"{<date>}"""""
Document the time zone used, local or UTC?

Also, format is ISO 8601.

I'm not sure about adding {} here. I don't feel like it's the right place to do
that.

http://codereview.chromium.org/6873110/diff/1008/gclient_scm.py#newcode687
gclient_scm.py:687: return "{" + date.strip() + "}"
Same as 133.

http://codereview.chromium.org/6873110/diff/1008/gclient_utils.py
File gclient_utils.py (right):

http://codereview.chromium.org/6873110/diff/1008/gclient_utils.py#newcode117
gclient_utils.py:117: if revision is None: return False
keep on two lines by style guide.

http://codereview.chromium.org/6873110/diff/1008/gclient_utils.py#newcode120
gclient_utils.py:120: return str_revision[0] == '{' and str_revision[-1] == '}'
return bool(re.match(r'^\{.+\}$', str(revision)))
and remove 118-119

http://codereview.chromium.org/6873110/diff/1008/tests/fake_repos.py
File tests/fake_repos.py (right):

http://codereview.chromium.org/6873110/diff/1008/tests/fake_repos.py#newcode598
tests/fake_repos.py:598: self._commit_git('repo_1', {
If you don't mind, I'd almost prefer you to do this change separately to make
the test diff simpler and make sure nothing is regressed in the process with
hidden assumption.

You may want to try this:

cd ..
git svn clone svn://svn.chromium.org/chrome -T trunk/tools/depot_tools
depot_tools2
# will take > 5 minutes
cd depot_tools2
svn diff ../depot_tools | patch -p0
git commit -a -m base
git cl issue 6873110
# then use my technique to separate 2 commits as described at
# http://maruel.github.com/landslide/maruel_git.html#slide20

http://codereview.chromium.org/6873110/diff/1008/tests/gclient_smoketest.py
File tests/gclient_smoketest.py (right):

http://codereview.chromium.org/6873110/diff/1008/tests/gclient_smoketest.py#n...
tests/gclient_smoketest.py:335: # Test unversioned checkout.
This whole part is unneeded since is already tested. Just do

self.gclient(['sync', '--deps', 'mac', '--jobs', '1'])

and skip up to 349.

Maybe it's time to put that code into an utility function.

http://codereview.chromium.org/6873110/diff/1008/tests/gclient_smoketest.py#n...
tests/gclient_smoketest.py:368: # Test incremental sync:
delete-unversioned_trees isn't there.
I don't think the remaining of the test adds any value to the test so you can
probably remove that too.

[Florian Loitsch, 2011-04-20 18:32:01]

http://codereview.chromium.org/6873110/diff/1008/gclient.py
File gclient.py (right):

http://codereview.chromium.org/6873110/diff/1008/gclient.py#newcode365
gclient.py:365: check-ini of revision 101."""
On 2011/04/20 15:43:28, Marc-Antoine Ruel wrote:
> ini?

Done.

http://codereview.chromium.org/6873110/diff/1008/gclient_scm.py
File gclient_scm.py (right):

http://codereview.chromium.org/6873110/diff/1008/gclient_scm.py#newcode130
gclient_scm.py:130: """Returns the current revision's date in the form
"{<date>}"""""
On 2011/04/20 15:43:28, Marc-Antoine Ruel wrote:
> Document the time zone used, local or UTC?
> 
> Also, format is ISO 8601.
> 
> I'm not sure about adding {} here. I don't feel like it's the right place to
do
> that.

changed to GetRevisionDate.
added helper function to make a date-revision out of it.
ISO 8601 contains timezone information. added comment to make this clear.

http://codereview.chromium.org/6873110/diff/1008/gclient_scm.py#newcode687
gclient_scm.py:687: return "{" + date.strip() + "}"
On 2011/04/20 15:43:28, Marc-Antoine Ruel wrote:
> Same as 133.

same as 133.

http://codereview.chromium.org/6873110/diff/1008/gclient_utils.py
File gclient_utils.py (right):

http://codereview.chromium.org/6873110/diff/1008/gclient_utils.py#newcode117
gclient_utils.py:117: if revision is None: return False
On 2011/04/20 15:43:28, Marc-Antoine Ruel wrote:
> keep on two lines by style guide.

Done.

http://codereview.chromium.org/6873110/diff/1008/gclient_utils.py#newcode120
gclient_utils.py:120: return str_revision[0] == '{' and str_revision[-1] == '}'
On 2011/04/20 15:43:28, Marc-Antoine Ruel wrote:
> return bool(re.match(r'^\{.+\}$', str(revision)))
> and remove 118-119

Done.

http://codereview.chromium.org/6873110/diff/1008/tests/fake_repos.py
File tests/fake_repos.py (right):

http://codereview.chromium.org/6873110/diff/1008/tests/fake_repos.py#newcode598
tests/fake_repos.py:598: self._commit_git('repo_1', {
On 2011/04/20 15:43:28, Marc-Antoine Ruel wrote:
> If you don't mind, I'd almost prefer you to do this change separately to make
> the test diff simpler and make sure nothing is regressed in the process with
> hidden assumption.
> 
> You may want to try this:
> 
> cd ..
> git svn clone svn://svn.chromium.org/chrome -T trunk/tools/depot_tools
> depot_tools2
> # will take > 5 minutes
> cd depot_tools2
> svn diff ../depot_tools | patch -p0
> git commit -a -m base
> git cl issue 6873110
> # then use my technique to separate 2 commits as described at
> # http://maruel.github.com/landslide/maruel_git.html#slide20
done.
I started differently (no git), so I did it differently. But I bookmarked your
presentation :)

http://codereview.chromium.org/6873110/diff/1008/tests/gclient_smoketest.py
File tests/gclient_smoketest.py (right):

http://codereview.chromium.org/6873110/diff/1008/tests/gclient_smoketest.py#n...
tests/gclient_smoketest.py:335: # Test unversioned checkout.
On 2011/04/20 15:43:28, Marc-Antoine Ruel wrote:
> This whole part is unneeded since is already tested. Just do
> 
> self.gclient(['sync', '--deps', 'mac', '--jobs', '1'])
> 
> and skip up to 349.
> 
> Maybe it's time to put that code into an utility function.

Done.

http://codereview.chromium.org/6873110/diff/1008/tests/gclient_smoketest.py#n...
tests/gclient_smoketest.py:368: # Test incremental sync:
delete-unversioned_trees isn't there.
On 2011/04/20 15:43:28, Marc-Antoine Ruel wrote:
> I don't think the remaining of the test adds any value to the test so you can
> probably remove that too.

Done.

[M-A Ruel, 2011-04-21 23:54:58]

http://codereview.chromium.org/6873110/diff/9001/gclient.py
File gclient.py (right):

http://codereview.chromium.org/6873110/diff/9001/gclient.py#newcode350
gclient.py:350: revision is None and
style nit: I think you can just do "and revision and not options.revision"

That's what we use in general. Same for following lines.

http://codereview.chromium.org/6873110/diff/9001/gclient.py#newcode363
gclient.py:363: -r 101 replace the revision with the date of 101 (e.g.
"{2011-18-04}").
s/date/time stamp/

http://codereview.chromium.org/6873110/diff/9001/gclient_utils.py
File gclient_utils.py (right):

http://codereview.chromium.org/6873110/diff/9001/gclient_utils.py#newcode117
gclient_utils.py:117: if revision is None:
return bool(revision and re.match(r'^\{.+\}$', str(revision)))

[Florian Loitsch, 2011-04-26 12:53:05]

http://codereview.chromium.org/6873110/diff/9001/gclient.py
File gclient.py (right):

http://codereview.chromium.org/6873110/diff/9001/gclient.py#newcode350
gclient.py:350: revision is None and
On 2011/04/21 23:54:58, Marc-Antoine Ruel wrote:
> style nit: I think you can just do "and revision and not options.revision"
> 
> That's what we use in general. Same for following lines.

Done.

http://codereview.chromium.org/6873110/diff/9001/gclient.py#newcode363
gclient.py:363: -r 101 replace the revision with the date of 101 (e.g.
"{2011-18-04}").
On 2011/04/21 23:54:58, Marc-Antoine Ruel wrote:
> s/date/time stamp/

Done.

http://codereview.chromium.org/6873110/diff/9001/gclient_utils.py
File gclient_utils.py (right):

http://codereview.chromium.org/6873110/diff/9001/gclient_utils.py#newcode117
gclient_utils.py:117: if revision is None:
On 2011/04/21 23:54:58, Marc-Antoine Ruel wrote:
> return bool(revision and re.match(r'^\{.+\}$', str(revision)))

Done.

[M-A Ruel, 2011-04-26 18:38:48]

lgtm with comments

http://codereview.chromium.org/6873110/diff/13001/gclient.py
File gclient.py (right):

http://codereview.chromium.org/6873110/diff/13001/gclient.py#newcode348
gclient.py:348: if (command == 'update' and
style nit: (optional) What about?

if (command == 'update' and options.transitive and not options.revision):
  _, revision = gclient_utils.SplitUrlRevision(self.parsed_url)
  if not revision:
    ...

I think it'd be slightly more readable.

http://codereview.chromium.org/6873110/diff/13001/gclient.py#newcode1087
gclient.py:1087: 'the dependencies to the date of the given revision.')
Can you note it's supported only for svn at the moment?

http://codereview.chromium.org/6873110/diff/13001/gclient_scm.py
File gclient_scm.py (right):

http://codereview.chromium.org/6873110/diff/13001/gclient_scm.py#newcode200
gclient_scm.py:200: '--before="' + revision[1:-1] + '"',
optional style nit: I prefer:
  '--before="%s"' % revision[1:-1],

[M-A Ruel, 2011-04-26 18:39:13]

Looks like you used git-cl config manually. You don't need to.

[Florian Loitsch, 2011-04-27 10:58:23]

Removed commented code and added TODO.
Added warning (in GIT).
Addressed comments.

http://codereview.chromium.org/6873110/diff/13001/gclient.py
File gclient.py (right):

http://codereview.chromium.org/6873110/diff/13001/gclient.py#newcode348
gclient.py:348: if (command == 'update' and
On 2011/04/26 18:38:49, Marc-Antoine Ruel wrote:
> style nit: (optional) What about?
> 
> if (command == 'update' and options.transitive and not options.revision):
>   _, revision = gclient_utils.SplitUrlRevision(self.parsed_url)
>   if not revision:
>     ...
> 
> I think it'd be slightly more readable.

Done.

http://codereview.chromium.org/6873110/diff/13001/gclient.py#newcode1087
gclient.py:1087: 'the dependencies to the date of the given revision.')
On 2011/04/26 18:38:49, Marc-Antoine Ruel wrote:
> Can you note it's supported only for svn at the moment?

Done.

http://codereview.chromium.org/6873110/diff/13001/gclient_scm.py
File gclient_scm.py (right):

http://codereview.chromium.org/6873110/diff/13001/gclient_scm.py#newcode200
gclient_scm.py:200: '--before="' + revision[1:-1] + '"',
On 2011/04/26 18:38:49, Marc-Antoine Ruel wrote:
> optional style nit: I prefer:
>   '--before="%s"' % revision[1:-1],

Since git is not working (and actually crashes, because the call to
_GetCurrentBranch fails) I replaced this code with a warning and a revision =
default_rev.

[M-A Ruel, 2011-04-27 14:10:36]

lgtm with nits.

(Note that the commit queue is broken for depot_tools, just svn co
svn://svn.chromium.org/chrome/trunk/tools/depot_tools and move your patch over
there)

http://codereview.chromium.org/6873110/diff/15003/gclient.py
File gclient.py (right):

http://codereview.chromium.org/6873110/diff/15003/gclient.py#newcode347
gclient.py:347: if (command == 'update' and options.transitive and not
options.revision):
Don't put parenthesis when not necessary:

if command == 'update' and options.transitive and not options.revision:

http://codereview.chromium.org/6873110/diff/15003/gclient.py#newcode1085
gclient.py:1085: 'the dependencies to the date of the given revision.'
Add a space, otherwise the text is collapsed:

... revision. '

[Florian Loitsch, 2011-04-28 08:53:28]

http://codereview.chromium.org/6873110/diff/15003/gclient.py
File gclient.py (right):

http://codereview.chromium.org/6873110/diff/15003/gclient.py#newcode347
gclient.py:347: if (command == 'update' and options.transitive and not
options.revision):
On 2011/04/27 14:10:36, Marc-Antoine Ruel wrote:
> Don't put parenthesis when not necessary:
> 
> if command == 'update' and options.transitive and not options.revision:
> 

Done.

http://codereview.chromium.org/6873110/diff/15003/gclient.py#newcode1085
gclient.py:1085: 'the dependencies to the date of the given revision.'
On 2011/04/27 14:10:36, Marc-Antoine Ruel wrote:
> Add a space, otherwise the text is collapsed:
> 
> ... revision. '

Done.