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

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:
2 years, 10 months ago by Joe Poirier
Modified:
2 years, 10 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/
2 years, 10 months ago #1
brainman
I don't know enough about make to review. But test on linux/386 completes successfully. Alex
2 years, 10 months ago #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 ...
2 years, 10 months ago #3
Joe Poirier
Hello golang-dev@googlegroups.com, alex.brainman@gmail.com, dave@cheney.net (cc: golang-dev@googlegroups.com), Please take another look.
2 years, 10 months ago #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, ...
2 years, 10 months ago #5
rsc
$PWD is there so that gcc gets the full path to the file, so that ...
2 years, 10 months ago #6
Joe Poirier
On Jun 21, 2011, at Jun 21 - 11:07 AM, Russ Cox wrote: > $PWD ...
2 years, 10 months ago #7
rsc
> AFAIK the compiler resolves the absolute path by default. It does include the path ...
2 years, 10 months ago #8
Joe Poirier
On Jun 21, 2011, at Jun 21 - 2:11 PM, Russ Cox wrote: >> AFAIK ...
2 years, 10 months ago #9
rsc
> But isn't it more common to explicitly set the source path > when starting ...
2 years, 10 months ago #10
Joe Poirier
On Jun 21, 2011, at Jun 21 - 2:53 PM, Russ Cox wrote: >> But ...
2 years, 10 months ago #11
rsc
>> Why do that when you can make gdb just do the right thing automatically? ...
2 years, 10 months ago #12
Joe Poirier
On Jun 21, 2011, at Jun 21 - 3:05 PM, Russ Cox wrote: >>> Why ...
2 years, 10 months ago #13
Joe Poirier
2 years, 10 months ago #14
*** Abandoned ***
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 1278:e6ce13d99bf5