Bug 151570 - Add an API for getting the branch identifier from a Git or SVN checkout.
Summary: Add an API for getting the branch identifier from a Git or SVN checkout.
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Local Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-11-23 13:06 PST by Matthew Hanson
Modified: 2016-09-22 18:17 PDT (History)
5 users (show)

See Also:

Attachments
Patch (69.98 KB, patch)
2015-11-23 13:10 PST, Matthew Hanson 		no flags Details | Formatted Diff | Diff
Patch (3.82 KB, patch)
2015-11-24 12:23 PST, Matthew Hanson 		no flags Details | Formatted Diff | Diff
Patch (3.86 KB, patch)
2015-11-30 14:25 PST, Matthew Hanson 		no flags Details | Formatted Diff | Diff
Patch (4.40 KB, patch)
2015-11-30 15:31 PST, Matthew Hanson 		no flags Details | Formatted Diff | Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Matthew Hanson 2015-11-23 13:06:06 PST 
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.
Comment 1 Matthew Hanson 2015-11-23 13:10:12 PST 
Created attachment 266095 [details]
Patch
Comment 2 Matthew Hanson 2015-11-23 13:15:33 PST 
Need to update my checkout. Resubmitting patch shortly.
Comment 3 Csaba Osztrogonác 2015-11-24 10:52:05 PST 
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
Comment 4 Matthew Hanson 2015-11-24 12:23:00 PST 
Created attachment 266142 [details]
Patch
Comment 5 Matthew Hanson 2015-11-24 12:24:06 PST 
(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.
Comment 6 Gyuyoung Kim 2015-11-24 20:52:18 PST 
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 ?
Comment 7 Matthew Hanson 2015-11-25 09:57:40 PST 
(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.
Comment 8 Matthew Hanson 2015-11-30 14:25:20 PST 
Created attachment 266282 [details]
Patch
Comment 9 Matthew Hanson 2015-11-30 15:31:12 PST 
Created attachment 266288 [details]
Patch
Comment 10 Matthew Hanson 2015-11-30 15:32:20 PST 
The final patch makes svnInfoForPath safe to use with paths that are not the current working directory.
Comment 11 David Kilzer (:ddkilzer) 2016-03-15 11:42:15 PDT 
Comment on attachment 266288 [details]
Patch

r=me

Sorry this took so long to review!
Comment 12 David Kilzer (:ddkilzer) 2016-03-15 11:45:26 PDT 
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.
Comment 13 WebKit Commit Bot 2016-09-22 18:17:15 PDT 
Comment on attachment 266288 [details]
Patch

Clearing flags on attachment: 266288

Committed r206283: <http://trac.webkit.org/changeset/206283>
Comment 14 WebKit Commit Bot 2016-09-22 18:17:20 PDT 
All reviewed patches have been landed.  Closing bug.