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

Issue 4631057: code review 4631057: Build enhancements: remove PWD variable and do an immed... (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
4 years, 2 months ago by Joe Poirier
Modified:
4 years, 2 months ago
Reviewers:
rsc, brainman, dfc, golang-dev
CC:
golang-dev
Visibility:
Public.

Description

Build enhancements: remove PWD variable and do an immediate assignment for QUOTED_GOBIN. PWD would have benefited from an immediate assignment, rather than a reference, to keep shell from being invoked multiple times in each make file. Tested on OSX and Windows; could someone try it on Linux and/or BSD?

Patch Set 1 #

Patch Set 2 : diff -r 134b5e56c87a https://go.googlecode.com/hg/ #

Patch Set 3 : diff -r 812bb69a9ec1 https://go.googlecode.com/hg/ #

Total comments: 4

Patch Set 4 : diff -r 812bb69a9ec1 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+3 lines, -4 lines) Patch
M src/Make.ccmd View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M src/Make.clib View 1 2 3 1 chunk +1 line, -1 line 0 comments Download
M src/Make.inc View 1 2 chunks +1 line, -2 lines 0 comments Download

Messages

Total messages: 14
Joe Poirier
Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com), I'd like you to review this change to https://go.googlecode.com/hg/
4 years, 2 months ago (2011-06-21 02:40:55 UTC) #1
brainman
I don't know enough about make to review. But test on linux/386 completes successfully. Alex
4 years, 2 months ago (2011-06-21 02:58:20 UTC) #2
dfc
LGTM freebsd/amd64 http://codereview.appspot.com/4631057/diff/1005/src/Make.ccmd File src/Make.ccmd (right): http://codereview.appspot.com/4631057/diff/1005/src/Make.ccmd#newcode40 src/Make.ccmd:40: $(HOST_CC) $(HOST_CFLAGS) -c "$*.c" Does "$*.c" need ...
4 years, 2 months ago (2011-06-21 03:42:56 UTC) #3
Joe Poirier
Hello golang-dev@googlegroups.com, alex.brainman@gmail.com, dave@cheney.net (cc: golang-dev@googlegroups.com), Please take another look.
4 years, 2 months ago (2011-06-21 04:14:23 UTC) #4
Joe Poirier
updates done http://codereview.appspot.com/4631057/diff/1005/src/Make.ccmd File src/Make.ccmd (right): http://codereview.appspot.com/4631057/diff/1005/src/Make.ccmd#newcode40 src/Make.ccmd:40: $(HOST_CC) $(HOST_CFLAGS) -c "$*.c" On 2011/06/21 03:42:56, ...
4 years, 2 months ago (2011-06-21 04:14:50 UTC) #5
rsc
$PWD is there so that gcc gets the full path to the file, so that ...
4 years, 2 months ago (2011-06-21 16:07:46 UTC) #6
Joe Poirier
On Jun 21, 2011, at Jun 21 - 11:07 AM, Russ Cox wrote: > $PWD ...
4 years, 2 months ago (2011-06-21 19:04:46 UTC) #7
rsc
> AFAIK the compiler resolves the absolute path by default. It does include the path ...
4 years, 2 months ago (2011-06-21 19:11:47 UTC) #8
Joe Poirier
On Jun 21, 2011, at Jun 21 - 2:11 PM, Russ Cox wrote: >> AFAIK ...
4 years, 2 months ago (2011-06-21 19:52:40 UTC) #9
rsc
> But isn't it more common to explicitly set the source path > when starting ...
4 years, 2 months ago (2011-06-21 19:53:35 UTC) #10
Joe Poirier
On Jun 21, 2011, at Jun 21 - 2:53 PM, Russ Cox wrote: >> But ...
4 years, 2 months ago (2011-06-21 19:56:47 UTC) #11
rsc
>> Why do that when you can make gdb just do the right thing automatically? ...
4 years, 2 months ago (2011-06-21 20:05:42 UTC) #12
Joe Poirier
On Jun 21, 2011, at Jun 21 - 3:05 PM, Russ Cox wrote: >>> Why ...
4 years, 2 months ago (2011-06-21 21:36:11 UTC) #13
Joe Poirier
4 years, 2 months ago (2011-06-21 23:51:22 UTC) #14
*** Abandoned ***
Sign in to reply to this message.

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