code review 10893044: dist: fix postinstall script for Darwin

dist: fix postinstall script for Darwin

The postinstall script causes the installation to fail because the last
step that copies files for Xcode is broken. Two details can cause the
command to fail:
  1. The XCODE_MISC_DIR value has a space. Without quotes in the cp
     command, cp will just complain that this is an invalid syntax.
  2. The source of the cp is a directory with two subdirectories.
     We actually want the files for either Xcode 3 or Xcode 4 to be copied. 
     Using xcodebuild -version, we check for the Xcode version and 
     select which of xcode/3/* or xcode/4/* should be the source 
     of the copy.

Fixes issue 5874.

[Alexandre Normand, 2013-07-12 04:57:51]

Hello golang-dev@googlegroups.com (cc: shenghou ma <minux.ma@gmail.com>),

I'd like you to review this change to
https://code.google.com/p/go/

[minux1, 2013-07-12 05:57:26]

the change looks good, just one question.

we also have specification files for Xcode 3, do you think we should
try to install that if people are using Xcode 3?

btw, Xcode 3 is usable on Mac OS X 10.6, so i think we'd better
support it.

[Alexandre Normand, 2013-07-12 14:47:37]

On 2013/07/12 05:57:26, minux wrote:
> the change looks good, just one question.
> 
> we also have specification files for Xcode 3, do you think we should
> try to install that if people are using Xcode 3?
> 
> btw, Xcode 3 is usable on Mac OS X 10.6, so i think we'd better
> support it.

You're right, we should support xcode 3 for people that use it. Here's what I'm
thinking: 
I can use xcodebuild -version to get the version number and select the source of
the copy accordingly. Since xcode 4 made the command-line tools optional though,
I'll assume that an error executing that command means that xcode 4 is the
version. Otherwise, if the version number is 3.x, the source is going to be
xcode/3/*.

If anyone has a suggestion for a more robust solution, I'm all ears. I actually
don't know much about xcode myself aside from needin it for the command-line
tools.

[Alexandre Normand, 2013-07-13 03:41:05]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[minux1, 2013-07-14 16:58:59]

LGTM. thank you.

btw, /Library/Application Support/Developer/Shared/Xcode/ doesn't exist
on my OS X 10.6 machine, should we try to mkdir -p?

[Alexandre Normand, 2013-07-14 17:16:55]

On 2013/07/14 16:58:59, minux wrote:
> LGTM. thank you.
> 
> btw, /Library/Application Support/Developer/Shared/Xcode/ doesn't exist
> on my OS X 10.6 machine, should we try to mkdir -p?

Thanks for reviewing. 

We check the existence of that directory to determine if we need to copy the
Xcode files or not and I had assumed that the absence of that directory meant
that Xcode wasn't installed and we didn't need to do anything. Is that
assumption incorrect? It would certainly be easy to mkdir -p that directory and
always do the copy.

[minux1, 2013-07-14 17:24:39]

On Mon, Jul 15, 2013 at 1:16 AM, <alexandre.normand@gmail.com> wrote:

> We check the existence of that directory to determine if we need to copy
> the Xcode files or not and I had assumed that the absence of that
> directory meant that Xcode wasn't installed and we didn't need to do
> anything. Is that assumption incorrect? It would certainly be easy to
> mkdir -p that directory and always do the copy.
>
could we check the existence of /Library/Application
Support/Developer/Shared/Xcode/
and then mkdir?
(i only verified i got that directory with Xcode 3, what about xcode 4?)

[Alexandre Normand, 2013-07-14 18:02:03]

Hello golang-dev@googlegroups.com, minux.ma@gmail.com (cc:
golang-dev@googlegroups.com),

Please take another look.

[Alexandre Normand, 2013-07-14 18:02:33]

On 2013/07/14 17:24:39, minux wrote:
> On Mon, Jul 15, 2013 at 1:16 AM, <mailto:alexandre.normand@gmail.com> wrote:
> 
> > We check the existence of that directory to determine if we need to copy
> > the Xcode files or not and I had assumed that the absence of that
> > directory meant that Xcode wasn't installed and we didn't need to do
> > anything. Is that assumption incorrect? It would certainly be easy to
> > mkdir -p that directory and always do the copy.
> >
> could we check the existence of /Library/Application
> Support/Developer/Shared/Xcode/
> and then mkdir?
> (i only verified i got that directory with Xcode 3, what about xcode 4?)

I have the same with Xcode 4 (actually I had the /Library/Application
Support/Developer/Shared/Xcode/Specifications directory already with Xcode
4...maybe something else had created it).
I think checking for the shared directory and creating the misc directory if it
doesn't exist is a good idea. I made the changes.

[minux1, 2013-07-14 18:10:11]

LGTM.

Thank you very much, i'm completely satisfied.

Will wait some time before submitting in case others
have comments.

[minux1, 2013-07-14 18:10:33]

and, please update the description, it's no longer accurate.

[Alexandre Normand, 2013-07-14 18:14:09]

On 2013/07/14 18:10:33, minux wrote:
> and, please update the description, it's no longer accurate.

Thanks, I just updated the description.

[adg, 2013-07-15 00:45:51]

LGTM

https://codereview.appspot.com/10893044/diff/22001/misc/dist/darwin/scripts/p...
File misc/dist/darwin/scripts/postinstall (right):

https://codereview.appspot.com/10893044/diff/22001/misc/dist/darwin/scripts/p...
misc/dist/darwin/scripts/postinstall:25: version=4
trailing tab

[adg, 2013-07-15 00:52:42]

*** Submitted as https://code.google.com/p/go/source/detail?r=dcc14ba7a1e5 ***

dist: fix postinstall script for Darwin

The postinstall script causes the installation to fail because the last
step that copies files for Xcode is broken. Two details can cause the
command to fail:
  1. The XCODE_MISC_DIR value has a space. Without quotes in the cp
     command, cp will just complain that this is an invalid syntax.
  2. The source of the cp is a directory with two subdirectories.
     We actually want the files for either Xcode 3 or Xcode 4 to be copied.
     Using xcodebuild -version, we check for the Xcode version and
     select which of xcode/3/* or xcode/4/* should be the source
     of the copy.

Fixes issue 5874.

R=golang-dev, minux.ma, adg
CC=golang-dev
https://codereview.appspot.com/10893044

Committer: Andrew Gerrand <adg@golang.org>