Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(317)

Issue 9416047: code review 9416047: cmd/go: fix LDFLAGS handling, enable misc/cgo/testso on... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
11 years ago by minux1
Modified:
10 years, 11 months ago
Reviewers:
capnm, dave
CC:
golang-dev, dave_cheney.net
Visibility:
Public.

Description

cmd/go: fix LDFLAGS handling, enable misc/cgo/testso on Darwin Fixes issue 5479.

Patch Set 1 #

Patch Set 2 : diff -r a23a2a12cd36 https://code.google.com/p/go #

Patch Set 3 : diff -r d8aeb43d6127 https://code.google.com/p/go #

Patch Set 4 : diff -r d8aeb43d6127 https://code.google.com/p/go #

Patch Set 5 : diff -r 75602c1fb7cc https://code.google.com/p/go/ #

Patch Set 6 : diff -r 75602c1fb7cc https://code.google.com/p/go/ #

Patch Set 7 : diff -r 75602c1fb7cc https://code.google.com/p/go/ #

Patch Set 8 : diff -r 75602c1fb7cc https://code.google.com/p/go/ #

Patch Set 9 : diff -r b63f423f8f4d https://code.google.com/p/go/ #

Patch Set 10 : diff -r b63f423f8f4d https://code.google.com/p/go/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+39 lines, -10 lines) Patch
M misc/cgo/testso/cgoso.go View 1 2 3 4 5 6 1 chunk +9 lines, -1 line 0 comments Download
M misc/cgo/testso/test.bash View 1 2 3 4 5 1 chunk +15 lines, -3 lines 0 comments Download
M src/cmd/go/build.go View 1 2 3 1 chunk +15 lines, -5 lines 0 comments Download
M src/run.bash View 1 2 3 4 1 chunk +0 lines, -1 line 0 comments Download

Messages

Total messages: 9
minux1
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://code.google.com/p/go/
10 years, 12 months ago (2013-05-20 22:10:30 UTC) #1
dave_cheney.net
LGTM. Tested on darwin/amd64, 10.7.4.
10 years, 12 months ago (2013-05-21 11:02:16 UTC) #2
minux1
*** Submitted as https://code.google.com/p/go/source/detail?r=75123d9d5b96 *** cmd/go: fix LDFLAGS handling, enable misc/cgo/testso on Darwin Fixes issue ...
10 years, 12 months ago (2013-05-21 16:32:17 UTC) #3
capnm
On 2013/05/21 16:32:17, minux wrote: > *** Submitted as https://code.google.com/p/go/source/detail?r=75123d9d5b96 *** > > cmd/go: fix ...
10 years, 11 months ago (2013-05-24 01:04:09 UTC) #4
dave_cheney.net
Given that file name ends in .bash, the simplest solution would be to upgrade the ...
10 years, 11 months ago (2013-05-24 01:46:56 UTC) #5
minux1
On Friday, May 24, 2013, wrote: > On 2013/05/21 16:32:17, minux wrote: > >> *** ...
10 years, 11 months ago (2013-05-24 07:43:07 UTC) #6
dave_cheney.net
https://codereview.appspot.com/9717043 It was hard to find a machine where /bin/sh was not bash (or dash, ...
10 years, 11 months ago (2013-05-24 08:07:41 UTC) #7
capnm
On 24 May 2013 10:07, Dave Cheney <dave@cheney.net> wrote: > https://codereview.appspot.com/9717043 > > It was ...
10 years, 11 months ago (2013-05-24 08:55:23 UTC) #8
dave_cheney.net
10 years, 11 months ago (2013-05-24 09:04:21 UTC) #9
Interestingly I could _not_ reproduce the problem in amd64 Ubuntu 13.04.  

But could reproduce the problem on 386 Ubuntu 12.04.1

Either way, the sh env is now consistent with the file extension. 

On 24/05/2013, at 18:55, Martin Capitanio <capnm9@gmail.com> wrote:

> On 24 May 2013 10:07, Dave Cheney <dave@cheney.net> wrote:
>> https://codereview.appspot.com/9717043
>> 
>> It was hard to find a machine where /bin/sh was not bash (or dash, or
>> whatever the problem child was)
> 
> cat /etc/lsb-release 
> DISTRIB_DESCRIPTION="Ubuntu 13.04"
> 
> ls -all /bin/sh
> lrwxrwxrwx 1 root root 4 Mai 13 16:39 /bin/sh -> dash
> 
> pi@rpi $ ls -all /bin/sh
> lrwxrwxrwx 1 root root 4 Mär 30  2012 /bin/sh -> dash
> 
> It seems to be the default on debian based distributions.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b