Add script to create a local Chromium git repository....

Add script to create a local Chromium git repository.

BUG=none
TEST=creates a working Chromium git repository in src

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=24991

[chase, 2009-08-28 00:56:43]

After my recent Git experience, I wrote this script with the hope it might save
people interested in trying Git for Chromium some headaches.  Let me know what
you think.

If it lgty, I'll check it in and update our Git wiki pages to point to this
script as a starter.

[Evan Martin, 2009-08-28 01:08:20]

=> markus, shell script master

[M-A Ruel, 2009-08-28 01:10:21]

I know nothing about bash. lgtm for the idea.

http://codereview.chromium.org/173599/diff/1/2
File mkcrgit (right):

http://codereview.chromium.org/173599/diff/1/2#newcode2
Line 2: #
Can you add the standard copyright boilerplate?

http://codereview.chromium.org/173599/diff/1/2#newcode137
Line 137: # From what I can tell, the master branch is unnecessary.  Remove it.
I personally disagree, I like the master branch.

[chase, 2009-08-28 01:22:53]

On 2009/08/28 01:10:21, Marc-Antoine Ruel wrote:
> I know nothing about bash. lgtm for the idea.
> 
> http://codereview.chromium.org/173599/diff/1/2
> File mkcrgit (right):
> 
> http://codereview.chromium.org/173599/diff/1/2#newcode2
> Line 2: #
> Can you add the standard copyright boilerplate?

Done.

> http://codereview.chromium.org/173599/diff/1/2#newcode137
> Line 137: # From what I can tell, the master branch is unnecessary.  Remove
it.
> I personally disagree, I like the master branch.

Okay, if it's useful to someone, makes sense to leave it.  New version leaves it
alone.

[Markus (顧孟勤), 2009-08-28 02:31:51]

Just a bunch of comments on good coding practices when writing shell scripts. I
think, there are a few style guides for shell scripts floating around, if you
care to find even more suggestions on how to spruce up your script :-) These are
just the ones that I remember of the top of my head.

http://codereview.chromium.org/173599/diff/1/2
File mkcrgit (right):

http://codereview.chromium.org/173599/diff/1/2#newcode1
Line 1: #!/bin/bash
If you can make this code work with "/bin/bash -e" that would be nice.

http://codereview.chromium.org/173599/diff/1/2#newcode6
Line 6: GITSERVER=${GITSERVER:-git.chromium.org}
In general, you make things a lot more reliable, if you get into the habit of
quoting all variable expansions, and of always using curly braces.

So, you should always write

  "${x}"

when you reference a variable.

http://codereview.chromium.org/173599/diff/1/2#newcode7
Line 7: 
TMP=".mkcrgit.$$"

function cleanup {
  rm -rf "${TMP}"
}

trap 'cleanup; echo Failure!; tput bel; exit 1' TERM QUIT HUP INT EXIT

http://codereview.chromium.org/173599/diff/1/2#newcode11
Line 11: while [ "x"$EMAIL = "x" ]; do
You definitely need to quote the variable here. Please write

  [ "x${EMAIL}" = "x" ]

http://codereview.chromium.org/173599/diff/1/2#newcode14
Line 14: if [ "x"`echo $EMAIL | grep @` = "x" ]; then
Again, you are missing double quotes. And since we are explicitly using
"/bin/bash", we might as well get rid of the backticks. Backticks are annoying
because they are difficult to nest safely and harder to read. Instead, write

  [ "x$(echo "${EMAIL}" | grep @)" = "x" ]

In fact, we can improve this even more. "grep" isn't a built-in, and thus a
little less efficient to call. So, how about:

  [ "x${EMAIL}" = "x${EMAIL%@*}" ]

http://codereview.chromium.org/173599/diff/1/2#newcode26
Line 26: if [ -d hello-world ]; then
Not sure, why you would need this

http://codereview.chromium.org/173599/diff/1/2#newcode39
Line 39: GITV=`git --version`
local GITV="$(git --version 2>/dev/null)"

http://codereview.chromium.org/173599/diff/1/2#newcode40
Line 40: if [ $? != 0 ]; then
Technically, if comparing numerical values you should use '-ne' instead of '!='

But an easier option would be

if GITV="$(git --version 2>&1)"; then
  echo OK
else
  echo "git isn't installed, please install it"
  exit 1
fi

http://codereview.chromium.org/173599/diff/1/2#newcode48
Line 48: if [ $GITV0 -lt 1 -o \( $GITV0 = 1 -a $GITV1 -lt 6 \) ]; then
GITV="${GITV##* }"          # Only examine last word (i.e. version number)
local GITD=( ${GITV//./ } ) # Split version number into decimals
if ((GITD[0] < 1 || (GITV[0] == 1 && GITV[1] < 6) )); then

http://codereview.chromium.org/173599/diff/1/2#newcode63
Line 63: echo "git-svn isn't installed, please install it"
rm -rf "${TMP}"
git clone git://github.com/git/hello-world.git "${TMP}" &>/dev/null &&
local GITV="$(cd "${TMP}" && git svn --version 2>/dev/null)" || {
  echo "git-svn isn't installed, please install it"
  exit 1
}

http://codereview.chromium.org/173599/diff/1/2#newcode66
Line 66: 
See above example on how to parse version numbers

http://codereview.chromium.org/173599/diff/1/2#newcode75
Line 75: 
You can remove these two instructions

http://codereview.chromium.org/173599/diff/1/2#newcode99
Line 99: fi
mkdir -p "${TMP}"
(cd "${TMP}" &&
 rm -rf .git .gitignore &&
 git init &&
 git remote add origin git://"${GITSERVER}"/chromium.git &&
 git remote show origin) >/dev/null &
local pid=$!
{ sleep 10; kill "$pid"; } &>/dev/null &
if wait "${pid}" &>/dev/null; then
  echo OK
else
  echo "Error accessing Chromium git URL, is \"${GITSERVER}\" correct?"
  exit 1
fi

http://codereview.chromium.org/173599/diff/1/2#newcode106
Line 106: if [ $? != 0 ]; then
git clone git://"${GITSERVER}"/chromium.git src || {
  echo "git clone exited with error"
  echo 'You should probably remove "src" before retrying'
  exit 1
}

http://codereview.chromium.org/173599/diff/1/2#newcode124
Line 124: cd ../
echo -n "Configuring upstream SVN..."
if (cd src && git svn init --prefix=origin/ -T trunk/src
svn://svn.chromium.org/chrome); then
  echo OK
else
  echo '"git svn init" exited with error'
  exit 1
fi

http://codereview.chromium.org/173599/diff/1/2#newcode136
Line 136: fi
echo -n "Fetching SVN history..."
if (cd src && git svn fetch); then
  echo OK
else
  echo '"git svn fetch" exited with error'
  exit 1
fi

http://codereview.chromium.org/173599/diff/1/2#newcode141
Line 141: cd ../
If you decide to keep this code, then rewrite it as

echo -n 'Removing "master" branch...'
if (cd src && git branch -D master && git pull); then
  echo OK
else
  echo 'Failed to remove the "master" branch'
  exit 1
fi

http://codereview.chromium.org/173599/diff/1/2#newcode149
Line 149: function git_config {
See above for examples on how to check errors and on how to never change the
global current directory.

http://codereview.chromium.org/173599/diff/1/2#newcode180
Line 180: echo "A Chromium Git repository was created in src/."
echo 'A Chromium Git repository was created in "src/".'

http://codereview.chromium.org/173599/diff/1/2#newcode203
Line 203: echo
trap cleanup EXIT

[tony, 2009-08-28 17:21:56]

Drive by nit: Please name the script something more descriptive than mkcrgit. 
Maybe create-chrome-git-checkout?

[chase, 2009-08-31 07:38:51]

On 2009/08/28 02:31:51, Markus (顧孟勤) wrote:
> Just a bunch of comments on good coding practices when writing shell scripts.
I
> think, there are a few style guides for shell scripts floating around, if you
> care to find even more suggestions on how to spruce up your script :-) These
are
> just the ones that I remember of the top of my head.

Thanks for the great feedback, it's much appreciated!

> http://codereview.chromium.org/173599/diff/1/2
> File mkcrgit (right):
> 
> http://codereview.chromium.org/173599/diff/1/2#newcode1
> Line 1: #!/bin/bash
> If you can make this code work with "/bin/bash -e" that would be nice.

Done.

> http://codereview.chromium.org/173599/diff/1/2#newcode6
> Line 6: GITSERVER=${GITSERVER:-git.chromium.org}
> In general, you make things a lot more reliable, if you get into the habit of
> quoting all variable expansions, and of always using curly braces.
> 
> So, you should always write
> 
>   "${x}"
> 
> when you reference a variable.
> 
> http://codereview.chromium.org/173599/diff/1/2#newcode7
> Line 7: 
> TMP=".mkcrgit.$$"
> 
> function cleanup {
>   rm -rf "${TMP}"
> }
> 
> trap 'cleanup; echo Failure!; tput bel; exit 1' TERM QUIT HUP INT EXIT

Done.

> http://codereview.chromium.org/173599/diff/1/2#newcode11
> Line 11: while [ "x"$EMAIL = "x" ]; do
> You definitely need to quote the variable here. Please write
> 
>   [ "x${EMAIL}" = "x" ]
> 
> http://codereview.chromium.org/173599/diff/1/2#newcode14
> Line 14: if [ "x"`echo $EMAIL | grep @` = "x" ]; then
> Again, you are missing double quotes. And since we are explicitly using
> "/bin/bash", we might as well get rid of the backticks. Backticks are annoying
> because they are difficult to nest safely and harder to read. Instead, write
> 
>   [ "x$(echo "${EMAIL}" | grep @)" = "x" ]
> 
> In fact, we can improve this even more. "grep" isn't a built-in, and thus a
> little less efficient to call. So, how about:
> 
>   [ "x${EMAIL}" = "x${EMAIL%@*}" ]

Done.

> http://codereview.chromium.org/173599/diff/1/2#newcode26
> Line 26: if [ -d hello-world ]; then
> Not sure, why you would need this

Removed.

> http://codereview.chromium.org/173599/diff/1/2#newcode39
> Line 39: GITV=`git --version`
> local GITV="$(git --version 2>/dev/null)"

Done.

> http://codereview.chromium.org/173599/diff/1/2#newcode40
> Line 40: if [ $? != 0 ]; then
> Technically, if comparing numerical values you should use '-ne' instead of
'!='
> 
> But an easier option would be
> 
> if GITV="$(git --version 2>&1)"; then
>   echo OK
> else
>   echo "git isn't installed, please install it"
>   exit 1
> fi

In a version between #3 and #4 I introduced this pattern.  Soon after I modified
some parts to use a different pattern you point out later.  Exciting feedback.

> http://codereview.chromium.org/173599/diff/1/2#newcode48
> Line 48: if [ $GITV0 -lt 1 -o \( $GITV0 = 1 -a $GITV1 -lt 6 \) ]; then
> GITV="${GITV##* }"          # Only examine last word (i.e. version number)
> local GITD=( ${GITV//./ } ) # Split version number into decimals
> if ((GITD[0] < 1 || (GITV[0] == 1 && GITV[1] < 6) )); then

Presumably using GITV in the conditional was a typo.  Assuming that's right,
done.

> http://codereview.chromium.org/173599/diff/1/2#newcode63
> Line 63: echo "git-svn isn't installed, please install it"
> rm -rf "${TMP}"
> git clone git://github.com/git/hello-world.git "${TMP}" &>/dev/null &&
> local GITV="$(cd "${TMP}" && git svn --version 2>/dev/null)" || {
>   echo "git-svn isn't installed, please install it"
>   exit 1
> }

Done.

> http://codereview.chromium.org/173599/diff/1/2#newcode66
> Line 66: 
> See above example on how to parse version numbers

I used something similar to the above.  git svn --version provides a different
output format compared to git --version.

> http://codereview.chromium.org/173599/diff/1/2#newcode75
> Line 75: 
> You can remove these two instructions

Done.

> http://codereview.chromium.org/173599/diff/1/2#newcode99
> Line 99: fi
> mkdir -p "${TMP}"
> (cd "${TMP}" &&
>  rm -rf .git .gitignore &&
>  git init &&
>  git remote add origin git://"${GITSERVER}"/chromium.git &&
>  git remote show origin) >/dev/null &
> local pid=$!
> { sleep 10; kill "$pid"; } &>/dev/null &
> if wait "${pid}" &>/dev/null; then
>   echo OK
> else
>   echo "Error accessing Chromium git URL, is \"${GITSERVER}\" correct?"
>   exit 1
> fi

Adapted.

> http://codereview.chromium.org/173599/diff/1/2#newcode106
> Line 106: if [ $? != 0 ]; then
> git clone git://"${GITSERVER}"/chromium.git src || {
>   echo "git clone exited with error"
>   echo 'You should probably remove "src" before retrying'
>   exit 1
> }

Done.

> http://codereview.chromium.org/173599/diff/1/2#newcode124
> Line 124: cd ../
> echo -n "Configuring upstream SVN..."
> if (cd src && git svn init --prefix=origin/ -T trunk/src
> svn://svn.chromium.org/chrome); then
>   echo OK
> else
>   echo '"git svn init" exited with error'
>   exit 1
> fi

Adapted.

> http://codereview.chromium.org/173599/diff/1/2#newcode136
> Line 136: fi
> echo -n "Fetching SVN history..."
> if (cd src && git svn fetch); then
>   echo OK
> else
>   echo '"git svn fetch" exited with error'
>   exit 1
> fi

Adapted.

> http://codereview.chromium.org/173599/diff/1/2#newcode149
> Line 149: function git_config {
> See above for examples on how to check errors and on how to never change the
> global current directory.

Done.

> http://codereview.chromium.org/173599/diff/1/2#newcode180
> Line 180: echo "A Chromium Git repository was created in src/."
> echo 'A Chromium Git repository was created in "src/".'

Done.

> http://codereview.chromium.org/173599/diff/1/2#newcode203
> Line 203: echo
> trap cleanup EXIT

Done.

[chase, 2009-08-31 07:40:05]

On 2009/08/28 17:21:56, tony wrote:
> Drive by nit: Please name the script something more descriptive than mkcrgit. 
> Maybe create-chrome-git-checkout?

Okay, I renamed the script to 'create-chromium-git-src'.  How's that?

[tony, 2009-08-31 15:07:15]

That sounds fine to me.

On Mon, Aug 31, 2009 at 12:40 AM,  <chase@chromium.org> wrote:
> On 2009/08/28 17:21:56, tony wrote:
>>
>> Drive by nit: Please name the script something more descriptive than
>
> mkcrgit.
>>
>> Maybe create-chrome-git-checkout?
>
> Okay, I renamed the script to 'create-chromium-git-src'. =A0How's that?
>
> http://codereview.chromium.org/173599
>

[chase, 2009-08-31 18:25:50]

On Monday, Markus wrote:
> I think, both GITSERVER and EMAIL were the once that I
> spotted. Possibly others. Can you make another pass and
> just verify you caught all the places where this should
> be done?

Thanks, I believe I've caught all the places where this occurred.