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

Issue 5732045: code review 5732045: cmd/go: fix relative imports again (Closed)

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

Description

cmd/go: fix relative imports again I tried before to make relative imports work by simply invoking the compiler in the right directory, so that an import of ./foo could be resolved by ./foo.a. This required creating a separate tree of package binaries that included the full path to the source directory, so that /home/gopher/bar.go would be compiled in tmpdir/work/local/home/gopher and perhaps find a ./foo.a in that directory. This model breaks on Windows because : appears in path names but cannot be used in subdirectory names, and I missed one or two places where it needed to be removed. The model breaks more fundamentally when compiling a test of a package that lives outside the Go path, because we effectively use a ./ import in the generated testmain, but there we want to be able to resolve the ./ import of the test package to one directory and all the other ./ imports to a different directory. Piggybacking on the compiler's current working directory is then no longer possible. Instead, introduce a new compiler option -D prefix that makes the compiler turn a ./ import into prefix+that, so that import "./foo" with -D a/b/c turns into import "a/b/c/foo". Then we can invent a package hierarchy "_/" with subdirectories named for file system paths: import "./foo" in the directory /home/gopher becomes import "_/home/gopher/foo", and since that final path is just an ordinary import now, all the ordinary processing works, without special cases. We will have to change the name of the hierarchy if we ever decide to introduce a standard package with import path "_", but that seems unlikely, and the detail is known only in temporary packages that get thrown away at the end of a build. Fixes issue 3169.

Patch Set 1 #

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

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

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

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

Unified diffs Side-by-side diffs Delta from patch set Stats (+171 lines, -92 lines) Patch
M src/cmd/gc/doc.go View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/cmd/gc/go.h View 1 1 chunk +1 line, -0 lines 0 comments Download
M src/cmd/gc/lex.c View 1 5 chunks +19 lines, -7 lines 0 comments Download
M src/cmd/go/build.go View 1 2 18 chunks +30 lines, -54 lines 0 comments Download
M src/cmd/go/pkg.go View 1 2 3 7 chunks +39 lines, -25 lines 0 comments Download
M src/cmd/go/run.go View 1 1 chunk +6 lines, -0 lines 0 comments Download
M src/cmd/go/test.bash View 1 2 chunks +21 lines, -0 lines 0 comments Download
M src/cmd/go/test.go View 1 2 2 chunks +7 lines, -6 lines 0 comments Download
A src/cmd/go/testdata/local/easysub/main.go View 1 2 3 4 1 chunk +9 lines, -0 lines 0 comments Download
A src/cmd/go/testdata/testimport/p.go View 1 1 chunk +3 lines, -0 lines 0 comments Download
A src/cmd/go/testdata/testimport/p1/p1.go View 1 1 chunk +3 lines, -0 lines 0 comments Download
A src/cmd/go/testdata/testimport/p2/p2.go View 1 1 chunk +3 lines, -0 lines 0 comments Download
A src/cmd/go/testdata/testimport/p_test.go View 1 1 chunk +13 lines, -0 lines 0 comments Download
A src/cmd/go/testdata/testimport/x_test.go View 1 1 chunk +15 lines, -0 lines 0 comments Download

Messages

Total messages: 3
rsc
Hello golang-dev@googlegroups.com, I'd like you to review this change to https://code.google.com/p/go/
13 years ago (2012-03-02 22:27:38 UTC) #1
r
LGTM
13 years ago (2012-03-02 22:37:38 UTC) #2
rsc
13 years ago (2012-03-03 03:16:06 UTC) #3
*** Submitted as 9dd9374109a9 ***

cmd/go: fix relative imports again

I tried before to make relative imports work by simply
invoking the compiler in the right directory, so that
an import of ./foo could be resolved by ./foo.a.
This required creating a separate tree of package binaries
that included the full path to the source directory, so that
/home/gopher/bar.go would be compiled in
tmpdir/work/local/home/gopher and perhaps find
a ./foo.a in that directory.

This model breaks on Windows because : appears in path
names but cannot be used in subdirectory names, and I
missed one or two places where it needed to be removed.

The model breaks more fundamentally when compiling
a test of a package that lives outside the Go path, because
we effectively use a ./ import in the generated testmain,
but there we want to be able to resolve the ./ import
of the test package to one directory and all the other ./
imports to a different directory.  Piggybacking on the compiler's
current working directory is then no longer possible.

Instead, introduce a new compiler option -D prefix that
makes the compiler turn a ./ import into prefix+that,
so that import "./foo" with -D a/b/c turns into import
"a/b/c/foo".  Then we can invent a package hierarchy
"_/" with subdirectories named for file system paths:
import "./foo" in the directory /home/gopher becomes
import "_/home/gopher/foo", and since that final path
is just an ordinary import now, all the ordinary processing
works, without special cases.

We will have to change the name of the hierarchy if we
ever decide to introduce a standard package with import
path "_", but that seems unlikely, and the detail is known
only in temporary packages that get thrown away at the
end of a build.

Fixes issue 3169.

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

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