Bug 146082 - Make webkitdirs::runGitUpdate() work when invoked in more than one Git checkout
Summary: Make webkitdirs::runGitUpdate() work when invoked in more than one Git checkout
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: All All
: P2 Normal
Assignee: Daniel Bates
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-06-17 15:22 PDT by Daniel Bates
Modified: 2015-06-18 11:00 PDT (History)
0 users

See Also:


Attachments
Patch (3.36 KB, patch)
2015-06-17 15:42 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff
Patch (3.64 KB, patch)
2015-06-17 15:44 PDT, Daniel Bates
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Daniel Bates 2015-06-17 15:22:27 PDT
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.
Comment 1 Daniel Bates 2015-06-17 15:42:35 PDT
Created attachment 255044 [details]
Patch
Comment 2 Daniel Bates 2015-06-17 15:44:31 PDT
Created attachment 255046 [details]
Patch
Comment 3 David Kilzer (:ddkilzer) 2015-06-17 15:55:49 PDT
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().
Comment 4 Daniel Bates 2015-06-18 10:59:28 PDT
(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.
Comment 5 Daniel Bates 2015-06-18 11:00:51 PDT
Comment on attachment 255046 [details]
Patch

Clearing flags on attachment: 255046

Committed r185710: <http://trac.webkit.org/changeset/185710>
Comment 6 Daniel Bates 2015-06-18 11:00:55 PDT
All reviewed patches have been landed.  Closing bug.