Bug 146082 – Make webkitdirs::runGitUpdate() work when invoked in more than one Git checkout

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Daniel Bates]

For Apple internal scripts it would be beneficial to be able to call the convenience function webkitdirs::runGitUpdate() to update an arbitrary Git- and Git SVN- checkouts. Currently webkitdirs::runGitUpdate() cannot be used to update both Git and Git SVN- checkout because it calls isGitSVN() to determine whether the current working directory is a Git SVN checkout. And isGitSVN() caches its result to speed up subsequent queries. We should support running webkitdirs::runGitUpdate() from inside an arbitrary Git/Git SVN checkout to update it.

[Daniel Bates]

Created attachment 255044 [details]
Patch

[Daniel Bates]

Created attachment 255046 [details]
Patch

[David Kilzer (:ddkilzer)]

Comment on attachment 255046 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=255046&action=review

> Tools/ChangeLog:18
> +        different than the current working directory). Instead runGitUpdate() should check
> +        whether the current working directory is a Git SVN checkout on each invocation.

If we find isGitSVNDirectory() is too slow, we could memoize the result instead.  (Not necessary for this patch; something to keep in mind for the future.)

> Tools/Scripts/VCSUtils.pm:244
> +sub isGitSVN()

Should we add a comment here that this is deprecated?  I assume that's the long-term intention by introducing isGitSVNDirectory().

[Daniel Bates]

(In reply to comment #3)
> Comment on attachment 255046 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=255046&action=review
> 
> > Tools/ChangeLog:18
> > +        different than the current working directory). Instead runGitUpdate() should check
> > +        whether the current working directory is a Git SVN checkout on each invocation.
> 
> If we find isGitSVNDirectory() is too slow, we could memoize the result
> instead.  (Not necessary for this patch; something to keep in mind for the
> future.)
> 

Yes, we could use memoization should a script(s) make repeated calls to isGitSVNDirectory() with the same directory. Currently runGitUpdate() is the only caller of isGitSVNDirectory() and scripts that use runGitUpdate() either invoke it once or invoke it more than once with a different directory on each invocation.

> > Tools/Scripts/VCSUtils.pm:244
> > +sub isGitSVN()
> 
> Should we add a comment here that this is deprecated?  I assume that's the
> long-term intention by introducing isGitSVNDirectory().

No, I was not planning to deprecate isGitSVN(). isGitSVN() and isGitSVNDirectory() serve two different audience. The former is for the majority of scripts that operate on exactly one SCM repository (usually WebKit). Similar to the purpose of VCSUtils::is{Git, SVN}(), isGitSVN() is the preferred function to use to determine if a WebKit checkout is a Git SVN checkout because it caches its value to speed up subsequent calls. For the few scripts that operate on multiple repositories VCSUtils::is{Git, GitSVN, SVN}Directory() are provided.

[Daniel Bates]

Comment on attachment 255046 [details]
Patch

Clearing flags on attachment: 255046

Committed r185710: <http://trac.webkit.org/changeset/185710>

[Daniel Bates]

All reviewed patches have been landed.  Closing bug.