Bug 16053 - prepend git branch with build-webkit and friends
Summary: prepend git branch with build-webkit and friends
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: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-11-19 11:49 PST by Adam Treat
Modified: 2007-11-20 12:49 PST (History)
3 users (show)

See Also:


Attachments
prepend git branch (2.97 KB, patch)
2007-11-19 11:50 PST, Adam Treat
ddkilzer: review-
Details | Formatted Diff | Diff
prepend git branch V2 (2.67 KB, patch)
2007-11-20 01:26 PST, Adam Treat
ddkilzer: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Treat 2007-11-19 11:49:01 PST
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...
Comment 1 Adam Treat 2007-11-19 11:50:03 PST
Created attachment 17404 [details]
prepend git branch
Comment 2 Mark Rowe (bdash) 2007-11-19 12:12:40 PST
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).
Comment 3 David Kilzer (:ddkilzer) 2007-11-19 22:52:05 PST
(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

Comment 4 David Kilzer (:ddkilzer) 2007-11-19 23:04:34 PST
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.
Comment 5 Adam Treat 2007-11-20 01:26:06 PST
Created attachment 17413 [details]
prepend git branch V2
Comment 6 David Kilzer (:ddkilzer) 2007-11-20 09:31:42 PST
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.
Comment 7 David Kilzer (:ddkilzer) 2007-11-20 12:49:33 PST
Committed r27930
http://trac.webkit.org/projects/webkit/changeset/27930