add git skia-verify tool

add git skia-verify tool

I'm using git svn to check skia upstream, and during my development, I find
this little script is quite handy. So I would like to share with others. To be
honest, this bash script is kind of hack, is there any Google shell style
guides I can follow? :)

BUG=
TEST=

[guanqun, 2012-04-24 07:16:13]

Some handy script...

[epoger, 2012-04-24 13:56:21]

Thanks for sharing!!!!

I made lots of comments, but they are all small and optional.

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify
File tools/git-skia-verify (right):

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode7
tools/git-skia-verify:7: #
Maybe add here a one-sentence description, like:

Script that builds/runs gm in current workspace, and at another Skia rev,
comparing results.

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode10
tools/git-skia-verify:10: #   And then invoke $ git skia-verify
[sha1-to-compare-default-is-HEAD^]
should say "git-skia-verify", not "git skia-verify"

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode36
tools/git-skia-verify:36: tput setaf 1
maybe make another function like say() that encapsulates this tput/echo/tput
combination

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode44
tools/git-skia-verify:44: tput setaf 1
maybe make another function like say() that encapsulates this tput/echo/tput
combination

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode52
tools/git-skia-verify:52: rm -rf after
maybe nicer to combine these?
rm -rf before after diff

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode55
tools/git-skia-verify:55: mkdir after
see above comment

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode61
tools/git-skia-verify:61: ./gyp_skia
FYI: Right now, you don't need to run gyp_skia manually because the top-level
makefile does it for you automatically.  Although that may change, so there's no
harm in leaving this as-is.  Just wanted to make sure you knew.

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode65
tools/git-skia-verify:65: tput setaf 1
maybe make another function like say() that encapsulates this tput/echo/tput
combination

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode74
tools/git-skia-verify:74: git checkout --detach "$REVISION"
I don't know much about git, but it looks like you are overwriting the local
copy with one from the repo... maybe it would be safer to create a temporary
directory and check out rev $REVISION into that temp dir?  (Although you already
made sure there were no uncommitted changes, so I guess this is already safe.)

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode79
tools/git-skia-verify:79: tput setaf 1
maybe make another function like say() that encapsulates this tput/echo/tput
combination

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode93
tools/git-skia-verify:93: tput setaf 1
maybe make another function like say() that encapsulates this tput/echo/tput
combination

[guanqun, 2012-04-24 14:56:06]

I've updated this file, please review it again. Thanks.

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify
File tools/git-skia-verify (right):

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode7
tools/git-skia-verify:7: #
On 2012/04/24 13:56:21, epoger wrote:
> Maybe add here a one-sentence description, like:
> 
> Script that builds/runs gm in current workspace, and at another Skia rev,
> comparing results.

Done.

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode10
tools/git-skia-verify:10: #   And then invoke $ git skia-verify
[sha1-to-compare-default-is-HEAD^]
On 2012/04/24 13:56:21, epoger wrote:
> should say "git-skia-verify", not "git skia-verify"

git can handle it carefully, so just use git skia-verify is ok. ;)

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode36
tools/git-skia-verify:36: tput setaf 1
On 2012/04/24 13:56:21, epoger wrote:
> maybe make another function like say() that encapsulates this tput/echo/tput
> combination

I've added another function called "warn", because these are all kinds of
warning messages.

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode44
tools/git-skia-verify:44: tput setaf 1
On 2012/04/24 13:56:21, epoger wrote:
> maybe make another function like say() that encapsulates this tput/echo/tput
> combination

Done.

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode52
tools/git-skia-verify:52: rm -rf after
On 2012/04/24 13:56:21, epoger wrote:
> maybe nicer to combine these?
> rm -rf before after diff

Done.

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode55
tools/git-skia-verify:55: mkdir after
On 2012/04/24 13:56:21, epoger wrote:
> see above comment

Done.

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode61
tools/git-skia-verify:61: ./gyp_skia
On 2012/04/24 13:56:21, epoger wrote:
> FYI: Right now, you don't need to run gyp_skia manually because the top-level
> makefile does it for you automatically.  Although that may change, so there's
no
> harm in leaving this as-is.  Just wanted to make sure you knew.

Thanks for your info.

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode74
tools/git-skia-verify:74: git checkout --detach "$REVISION"
On 2012/04/24 13:56:21, epoger wrote:
> I don't know much about git, but it looks like you are overwriting the local
> copy with one from the repo... maybe it would be safer to create a temporary
> directory and check out rev $REVISION into that temp dir?  (Although you
already
> made sure there were no uncommitted changes, so I guess this is already safe.)

Hmm, that's an option, but I like to keep this script simple, and using a
detached checkout in git is quite normal. git bisect does this detached checkout
all the times. Regaring the safety, I think you're right, I'm making sure all
changes are committed.

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode79
tools/git-skia-verify:79: tput setaf 1
On 2012/04/24 13:56:21, epoger wrote:
> maybe make another function like say() that encapsulates this tput/echo/tput
> combination

Done.

http://codereview.appspot.com/6117046/diff/1/tools/git-skia-verify#newcode93
tools/git-skia-verify:93: tput setaf 1
On 2012/04/24 13:56:21, epoger wrote:
> maybe make another function like say() that encapsulates this tput/echo/tput
> combination

Here, using the newly added "warn" or "say" function is not enough, because the
following line is an explicit command call.

So in order not to make this tput spread everywhere, I decided to delete these
last two tput lines. The reason is the output of skdiff is already on the latest
output of the screen, users could see it without any problems.

[epoger, 2012-04-24 15:11:58]

LGTM

Thanks again for your contributions!

Would you like me to land this for you?

[guanqun, 2012-04-24 15:14:56]

On 2012/04/24 15:11:58, epoger wrote:
> LGTM
> 
> Thanks again for your contributions!
> 
> Would you like me to land this for you?

Yes, please, I don't have the commit permission. :)

[epoger, 2012-04-24 15:20:08]

Committed as http://code.google.com/p/skia/source/detail?r=3758