Bug 16053 – prepend git branch with build-webkit and friends

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

[Adam Treat]

This patch will prepend the git branch name if you are in one to the 
$baseProductDir if you have the appropriate key/value pair in your git config 
file.

The use case is a developer who is switching between branches and testing but 
doesn't want to clobber his build everytime.  The default is off and you can 
override the global setting on a branch by branch basis.

Comments and review welcome...

[Adam Treat]

Created attachment 17404 [details]
prepend git branch

[Mark Rowe (bdash)]

This would look to do the wrong thing if my branch name contains the word "master" anywhere, or if I check out a revision that is not the head of the branch (eg, I check out HEAD^ to compare performance with HEAD).

[David Kilzer (:ddkilzer)]

(In reply to comment #2)
> This would look to do the wrong thing if my branch name contains the word
> "master" anywhere, or if I check out a revision that is not the head of the
> branch (eg, I check out HEAD^ to compare performance with HEAD).

$ git checkout HEAD^
Note: moving to "HEAD^" which isn't a local branch
If you want to create a new branch from this checkout, you may do so
(now or later) by using -b with the checkout command again. Example:
  git checkout -b <new_branch_name>
HEAD is now at ca0684e... Restoring a binary svn:mime-type on the test.
$ git branch
* (no branch)
  master
$ git symbolic-ref HEAD
fatal: ref HEAD is not a symbolic ref

[David Kilzer (:ddkilzer)]

Comment on attachment 17404 [details]
prepend git branch

>+sub gitBranch()
>+{
>+    unless (defined $gitBranch) {
>+        chomp($gitBranch = `git symbolic-ref HEAD`);
>+        $gitBranch =~ s/refs\/heads\///;

Please (1) anchor this using "^" and (2) use another character besides '/' to make the regular expression more readable, e.g.:

$gitBranch =~ s#^refs/heads/##;

You may want to do something like this instead:

$gitBranch = pop @{[split('/', $gitBranch)]}

(Or maybe not if that's not readable by most folks. :)

>+        $gitBranch =~ s/master//;

Should probably test for equality rather than using a non-anchored regex:

$gitBranch = "" if $gitBranch eq "master";

Otherwise the regex should be anchored with both "^" and "$".

Also, the floating head "(no branch)" case mentioned in Comment #2 and Comment #3 needs to be addressed.  (You may want to test the exit status of the git command using exitStatus() from webkitdirs.pm.)

>+sub isGitBranchBuild()
>+{
>+    my $branch = gitBranch();
>+    chomp(my $override = `git config branch.$branch.webkitbranchbuild`);
>+    return 1 if $override eq "true";
>+    return 0 if $override eq "false";
>+
>+    unless (defined $isGitBranchBuild) {
>+        chomp(my $gitBranchBuild = `git config --bool core.webkitbranchbuild`);
>+        $isGitBranchBuild = $gitBranchBuild eq "true";
>+    }

Why is "--bool" used in the second git-config call but not the first?  I think the first command should also have a "--bool" switch.

r- to fix gitBranch() issues.

[Adam Treat]

Created attachment 17413 [details]
prepend git branch V2

[David Kilzer (:ddkilzer)]

Comment on attachment 17413 [details]
prepend git branch V2

>+    unless (defined $gitBranch) {
>+        chomp($gitBranch = `git symbolic-ref -q HEAD`);
>+        $gitBranch =~ s#^refs/heads/##;
>+        $gitBranch = "" if $gitBranch eq "master";
>+        $gitBranch = "" if exitStatus($?);
>+    }

I think it would make more sense to check the exit status immediately after the command is run.

>branch.$branch.webkitbranchbuild
>core.webkitbranchbuild

Do you have a preference between "webkitbranchbuild" or "webKitBranchBuild" (or "webkitBranchBuild")?  I see both all-lowercase and camelCase config options in "man git-config", but I think it's much easier to read the camelCase version.

r=me, although I'd like to see the above changes made when committing.

[David Kilzer (:ddkilzer)]

Committed r27930
http://trac.webkit.org/projects/webkit/changeset/27930