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

Issue 5554079: code review 5554079: go/build: add BuildTags to Context, allow !tag (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 10 months ago by rsc
Modified:
8 years, 10 months ago
Reviewers:
CC:
r, adg, golang-dev
Visibility:
Public.

Description

go/build: add BuildTags to Context, allow !tag This lets the client of go/build specify additional tags that can be recognized in a // +build directive. For example, a build for a custom environment like App Engine might include "appengine" in the BuildTags list, so that packages can be written with some files saying // +build appengine (build only on app engine) or // +build !appengine (build only when NOT on app engine) App Engine here is just a hypothetical context. I plan to use this in the cmd/go sources to distinguish the bootstrap version of cmd/go (which will not use networking) from the full version using a custom tag. It might also be useful in App Engine. Also, delete Build and Script, which we did not end up using for cmd/go and which never got turned on for real in goinstall.

Patch Set 1 #

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

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

Total comments: 3

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

Total comments: 12

Patch Set 5 : diff -r 802d4c3a4254 https://go.googlecode.com/hg/ #

Total comments: 2

Patch Set 6 : diff -r fff00f608958 https://go.googlecode.com/hg/ #

Patch Set 7 : diff -r d9358d10ab37 https://go.googlecode.com/hg/ #

Unified diffs Side-by-side diffs Delta from patch set Stats (+141 lines, -511 lines) Patch
M src/cmd/goinstall/main.go View 1 2 2 chunks +2 lines, -27 lines 0 comments Download
M src/pkg/crypto/tls/root_stub.go View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/go/build/build.go View 1 2 chunks +1 line, -424 lines 0 comments Download
M src/pkg/go/build/build_test.go View 1 2 3 4 5 3 chunks +27 lines, -37 lines 0 comments Download
M src/pkg/go/build/dir.go View 1 2 3 4 5 7 chunks +108 lines, -20 lines 0 comments Download
M src/pkg/net/cgo_stub.go View 1 2 1 chunk +1 line, -1 line 0 comments Download
M src/pkg/os/user/lookup_stubs.go View 1 2 1 chunk +1 line, -1 line 0 comments Download

Messages

Total messages: 13
rsc
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/
8 years, 10 months ago (2012-01-21 06:14:14 UTC) #1
r
code's fine. where is it documented? http://codereview.appspot.com/5554079/diff/3001/src/cmd/goinstall/main.go File src/cmd/goinstall/main.go (right): http://codereview.appspot.com/5554079/diff/3001/src/cmd/goinstall/main.go#newcode47 src/cmd/goinstall/main.go:47: useMake = flag.Bool("make", ...
8 years, 10 months ago (2012-01-21 22:45:05 UTC) #2
rsc
On Sat, Jan 21, 2012 at 17:45, <r@golang.org> wrote: > the words don't feel right ...
8 years, 10 months ago (2012-01-22 04:12:52 UTC) #3
adg
Can you please add docs for +build directives to go/build in this CL? http://codereview.appspot.com/5554079/diff/3001/src/pkg/go/build/dir.go File ...
8 years, 10 months ago (2012-01-22 23:39:13 UTC) #4
rsc
On Sun, Jan 22, 2012 at 18:39, <adg@golang.org> wrote: > Seems odd to reject this ...
8 years, 10 months ago (2012-01-23 17:16:03 UTC) #5
rsc
Hello golang-dev@googlegroups.com, r@golang.org, adg@golang.org (cc: golang-dev@googlegroups.com), Please take another look.
8 years, 10 months ago (2012-01-23 17:16:22 UTC) #6
rsc
Large comment added.
8 years, 10 months ago (2012-01-23 17:16:34 UTC) #7
r
http://codereview.appspot.com/5554079/diff/9001/src/pkg/go/build/dir.go File src/pkg/go/build/dir.go (right): http://codereview.appspot.com/5554079/diff/9001/src/pkg/go/build/dir.go#newcode155 src/pkg/go/build/dir.go:155: // Files ending in _test.go are treated as test ...
8 years, 10 months ago (2012-01-23 18:42:53 UTC) #8
rsc
http://codereview.appspot.com/5554079/diff/9001/src/pkg/go/build/dir.go File src/pkg/go/build/dir.go (right): http://codereview.appspot.com/5554079/diff/9001/src/pkg/go/build/dir.go#newcode155 src/pkg/go/build/dir.go:155: // Files ending in _test.go are treated as test ...
8 years, 10 months ago (2012-01-23 19:40:16 UTC) #9
rsc
PTAL
8 years, 10 months ago (2012-01-23 19:40:34 UTC) #10
r
LGTM for comment in dir.go after suggested rearrangement http://codereview.appspot.com/5554079/diff/13001/src/pkg/go/build/dir.go File src/pkg/go/build/dir.go (right): http://codereview.appspot.com/5554079/diff/13001/src/pkg/go/build/dir.go#newcode185 src/pkg/go/build/dir.go:185: // ...
8 years, 10 months ago (2012-01-23 19:50:50 UTC) #11
rsc
On Mon, Jan 23, 2012 at 14:50, <r@golang.org> wrote: > 1) define the syntax of ...
8 years, 10 months ago (2012-01-23 20:08:37 UTC) #12
rsc
8 years, 10 months ago (2012-01-23 20:16:41 UTC) #13
*** Submitted as http://code.google.com/p/go/source/detail?r=0b94f1e20038 ***

go/build: add BuildTags to Context, allow !tag

This lets the client of go/build specify additional tags that
can be recognized in a // +build directive.  For example,
a build for a custom environment like App Engine might
include "appengine" in the BuildTags list, so that packages
can be written with some files saying

        // +build appengine   (build only on app engine)

or

        // +build !appengine  (build only when NOT on app engine)

App Engine here is just a hypothetical context.  I plan to use
this in the cmd/go sources to distinguish the bootstrap version
of cmd/go (which will not use networking) from the full version
using a custom tag.  It might also be useful in App Engine.

Also, delete Build and Script, which we did not end up using for
cmd/go and which never got turned on for real in goinstall.

R=r, adg
CC=golang-dev
http://codereview.appspot.com/5554079
Sign in to reply to this message.

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