Bug 151570 – Add an API for getting the branch identifier from a Git or SVN checkout.

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

[Matthew Hanson]

VCSUtils.pm knows how to extract the remote URL from SVN and Git repositories, but it does not expose this function publicly. In fact, the logic for doing this is embedded in another function called pathRelativeToSVNRepositoryRootForPath. This makes it difficult for VCSUtils clients to determine the branch identifier (e.g. "trunk", "Safari-601.1.46.1", "safari-601.1.46-branch") of a repository.

This bug tracks exposing an API for getting such an identifier, given a path to a repository.

[Matthew Hanson]

Created attachment 266095 [details]
Patch

[Matthew Hanson]

Need to update my checkout. Resubmitting patch shortly.

[Csaba Osztrogonác]

Comment on attachment 266095 [details]
Patch

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

Please don't fix the style on unrelated part of the code base.

> Tools/ChangeLog:152
> -        
> +

Please don't change other changelog entries.

> Tools/Scripts/VCSUtils.pm:11
> -#     notice, this list of conditions and the following disclaimer. 
> +#     notice, this list of conditions and the following disclaimer.

unrelated typo fix

[Matthew Hanson]

Created attachment 266142 [details]
Patch

[Matthew Hanson]

(In reply to comment #3)
> Comment on attachment 266095 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=266095&action=review
> 
> Please don't fix the style on unrelated part of the code base.
> 
> > Tools/ChangeLog:152
> > -        
> > +
> 
> Please don't change other changelog entries.

Fixed.

> 
> > Tools/Scripts/VCSUtils.pm:11
> > -#     notice, this list of conditions and the following disclaimer. 
> > +#     notice, this list of conditions and the following disclaimer.
> 
> unrelated typo fix

Fixed.

I've turned off the automatic whitespace trimming in my editor.

[Gyuyoung Kim]

Comment on attachment 266142 [details]
Patch

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

> Tools/Scripts/VCSUtils.pm:88
> +        svnIdentifierForPath

& missing ?

[Matthew Hanson]

(In reply to comment #6)
> Comment on attachment 266142 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=266142&action=review
> 
> > Tools/Scripts/VCSUtils.pm:88
> > +        svnIdentifierForPath
> 
> & missing ?

Indeed. Thank you, Gyuyoung.

[Matthew Hanson]

Created attachment 266282 [details]
Patch

[Matthew Hanson]

Created attachment 266288 [details]
Patch

[Matthew Hanson]

The final patch makes svnInfoForPath safe to use with paths that are not the current working directory.

[David Kilzer (:ddkilzer)]

Comment on attachment 266288 [details]
Patch

r=me

Sorry this took so long to review!

[David Kilzer (:ddkilzer)]

BTW, there are unit tests for VCSUtils.pm here:

  Tools/Scripts/webkitperl/VCSUtils_unittest/

For the subroutines that just extract data from command output, it might be nice to write a couple tests with output from actual SVN or git WebKit repository checkouts.

Testing the methods that run shell commands would be much more difficult since we don't have a way to mock shell commands to run.

[WebKit Commit Bot]

Comment on attachment 266288 [details]
Patch

Clearing flags on attachment: 266288

Committed r206283: <http://trac.webkit.org/changeset/206283>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.