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

Issue 5674043: code review 5674043: cmd/go: a raft of fixes (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
12 years, 1 month ago by rsc
Modified:
12 years, 1 month ago
Reviewers:
CC:
golang-dev, r, r2
Visibility:
Public.

Description

cmd/go: a raft of fixes * add -work option to save temporary files (Fixes issue 2980) * fix go test -i to work with cgo packages (Fixes issue 2936) * do not overwrite/remove empty directories or non-object files during build (Fixes issue 2829) * remove package main vs package non-main heuristic: a directory must contain only one package (Fixes issue 2864) * to make last item workable, ignore +build tags for files named on command line: go build x.go builds x.go even if it says // +build ignore. * add // +build ignore tags to helper programs

Patch Set 1 #

Patch Set 2 : diff -r 0158e2415ca5 https://go.googlecode.com/hg/ #

Total comments: 1

Patch Set 3 : diff -r 642c65adbfe6 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+113 lines, -28 lines) Patch
M src/cmd/go/build.go View 1 11 chunks +57 lines, -7 lines 0 comments Download
M src/cmd/go/main.go View 1 1 chunk +1 line, -1 line 0 comments Download
M src/cmd/go/pkg.go View 1 2 7 chunks +21 lines, -4 lines 0 comments Download
M src/cmd/go/test.go View 1 1 chunk +4 lines, -0 lines 0 comments Download
M src/pkg/crypto/tls/generate_cert.go View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/pkg/encoding/gob/dump.go View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/pkg/exp/norm/maketables.go View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/pkg/exp/norm/maketesttables.go View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/pkg/exp/norm/normregtest.go View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/pkg/exp/norm/triegen.go View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/pkg/go/build/dir.go View 1 6 chunks +12 lines, -16 lines 0 comments Download
M src/pkg/go/doc/headscan.go View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/pkg/net/http/triv.go View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/pkg/unicode/maketables.go View 1 1 chunk +2 lines, -0 lines 0 comments Download

Messages

Total messages: 8
rsc
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://go.googlecode.com/hg/
12 years, 1 month ago (2012-02-14 20:48:25 UTC) #1
r
LGTM http://codereview.appspot.com/5674043/diff/2001/src/cmd/go/pkg.go File src/cmd/go/pkg.go (right): http://codereview.appspot.com/5674043/diff/2001/src/cmd/go/pkg.go#newcode198 src/cmd/go/pkg.go:198: Err: fmt.Sprintf("expected package main in %s; found %s", ...
12 years, 1 month ago (2012-02-14 20:56:08 UTC) #2
rsc
On Tue, Feb 14, 2012 at 15:56, <r@golang.org> wrote: > probably should be done generally; ...
12 years, 1 month ago (2012-02-14 21:16:35 UTC) #3
rsc
changed to fmt.Sprintf("expected package main in %q; found package %s", dir, p.Name),
12 years, 1 month ago (2012-02-14 21:26:45 UTC) #4
r2
On Feb 15, 2012, at 8:16 AM, Russ Cox wrote: > On Tue, Feb 14, ...
12 years, 1 month ago (2012-02-14 21:29:44 UTC) #5
rsc
*** Submitted as http://code.google.com/p/go/source/detail?r=d36241352964 *** cmd/go: a raft of fixes * add -work option to ...
12 years, 1 month ago (2012-02-14 21:39:22 UTC) #6
rsc
On Tue, Feb 14, 2012 at 16:29, Rob 'Commander' Pike <r@google.com> wrote: > can you ...
12 years, 1 month ago (2012-02-14 21:41:01 UTC) #7
r2
12 years, 1 month ago (2012-02-14 21:47:49 UTC) #8
On Feb 15, 2012, at 8:40 AM, Russ Cox wrote:

> On Tue, Feb 14, 2012 at 16:29, Rob 'Commander' Pike <r@google.com> wrote:
>> can you reasonably exclude spaces? perhaps you can, but i know at some point
someone is going to want to put a space into a path name. every 'modern'
operating system lets you do it to file names. not sure which way that pushes
the argument.
> 
> I don't know.  We can exclude them now and relent if people complain.
> I propose to exclude control characters, spaces, invalid UTF-8, and
> backslash and see what happens.

sgtm

i would also consider prohibiting @#$%^&*(){}[]<>`~'";:, and such, characters
that creep in due to escaping and other mistakes and lead to ambiguity.

-rob


Sign in to reply to this message.

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