Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

go/build: honour GO15VENDOREXPERIMENT #12278

Closed
brknstrngz opened this issue Aug 23, 2015 · 33 comments
Closed

go/build: honour GO15VENDOREXPERIMENT #12278

brknstrngz opened this issue Aug 23, 2015 · 33 comments
Labels
FrozenDueToAge
Milestone
Go1.6

Comments

@brknstrngz
Copy link

Hi,

Any chance of goimports being taught about GO15VENDOREXPERIMENT? With go 1.5 it rewrites import paths (to $CWD/vendor/path/to/3rd/party/pkg) even when GO15VENDOREXPERIMENT is set.

Thanks
Vlad
@mikioh mikioh changed the title goimports does not honour GO15VENDOREXPERIMENT x/tools/cmd/goimports: does not honour GO15VENDOREXPERIMENT Aug 23, 2015
@mikioh mikioh added this to the Unreleased milestone Aug 23, 2015
@bradfitz
Copy link
Contributor

Yes, this needs to happen. Now that my projects are also using vendor directories, I've also been running into this.

/cc @adg @Sajmani @josharian

@ereyes01
Copy link

ereyes01 commented Sep 2, 2015

I also have issues with oracle and GO15VENDOREXPERIMENT and oracle, which I've initially reported here: fatih/vim-go#525

Is this correct place to raise an issue for oracle? If so should I open a new one, or report in this one?

Thanks, Eddy

@ianlancetaylor
Copy link
Contributor

@ereyes01 Assuming you are talking about x/tools/cmd/oracle, then filing an issue on this issue tracker is the right thing to do. This particular issue is about goimports.

@minux
Copy link
Member

minux commented Sep 2, 2015 via email

@kylewolfe
Copy link

@minux, this is what I've been trying to get my head around the last few days. Tools like goimports and gocode would need a significant refactor and very temporary workaround to implement support for the vendor experiment.

Is there a reason that the vendor experiment was implemented only in the go tool rather than in go/build?

@ereyes01 ereyes01 mentioned this issue Sep 2, 2015
Closed
@kylewolfe
Copy link

@rsc, Is it possible and even worth it at this point to move the vendor functionality from cmd/go to go/build to enable third party tools to piggy back on the existing vendor functionality?

@minux
Copy link
Member

minux commented Sep 4, 2015

On Sep 4, 2015 11:29 AM, "Kyle Wolfe" notifications@github.com wrote:

@rsc, Is it possible and even worth it at this point to move the vendor
functionality from cmd/go to go/build to enable third party tools to piggy
back on the existing vendor functionality?

One issue is that the new go/build package will only be available in Go
1.6. I don't know if that's acceptable.

@pwaller
Copy link
Contributor

pwaller commented Sep 22, 2015

Could this be put onto a near-term milestone? (1.6?) I'm happily using the new vendoring mechanism and not having these source manipulation tools at our disposal is unfortunate. I'm also happy to have a crack at doing something if there is a consensus about what is a reasonable approach to fix it. Waiting until 1.6 is annoying, but waiting until after 1.6 would be worse.

@bradfitz
Copy link
Contributor

Goimports is not distributed with Go so we don't use the Go 1.N milestones for it. This can be fixed whenever.

@pwaller
Copy link
Contributor

pwaller commented Sep 22, 2015

@bradfitz I was referring to @minux's suggestion that the fix might involve fixing go/build. There are actually lots of things in need of fixing, and many of them are using go/build, so it is a logical thing that might need attention.

@pwaller
Copy link
Contributor

pwaller commented Sep 22, 2015

For example, goimports uses go/build here: https://github.com/golang/tools/blob/5a90b2958f12ca0f046ffe2eb1523c5896af5dae/imports/fix.go#L158 - which implies either the stdlib needs fixing or the whole importing logic needs to move out of the stdlib.

This was referenced Sep 22, 2015
Closed
Open
@minux minux changed the title x/tools/cmd/goimports: does not honour GO15VENDOREXPERIMENT go/build: honour GO15VENDOREXPERIMENT Sep 22, 2015
@minux minux modified the milestones: Go1.6Early, Unreleased Sep 22, 2015
@minux
Copy link
Member

minux commented Sep 22, 2015 via email

@kylewolfe kylewolfe mentioned this issue Sep 23, 2015
Closed
This was referenced Sep 24, 2015
Closed
Closed
@fatih fatih mentioned this issue Oct 1, 2015
Closed
@minux minux mentioned this issue Oct 4, 2015
Closed
@cespare cespare mentioned this issue Oct 9, 2015
Closed
@pwaller pwaller mentioned this issue Oct 19, 2015
Closed
@pwaller
Copy link
Contributor

pwaller commented Oct 19, 2015

Ping.

I note that the 1.6Early deadline (according to the github milestone) is October the 31st, which is now ~12 days away. Is this issue on anybody's list, and is there anything I can do to help? Has the deadline already been missed, so to say? I would really like to be able to use gorename and friends in Go 1.6. It is already hurting.

@adg
Copy link
Contributor

adg commented Oct 20, 2015

The deadline hasn't been missed.
But we should get moving on this.
If someone could investigate what's involved in making this work, that
would be great.

On 19 October 2015 at 21:39, Peter Waller notifications@github.com wrote:

Ping.

I note that the 1.6Early deadline (according to the github milestone) is
October the 31st, which is now ~12 days away. Is this issue on anybody's
list, and is there anything I can do to help? Has the deadline already been
missed, so to say? I would really like to be able to use gorename and
friends in Go 1.6. It is already hurting.

—
Reply to this email directly or view it on GitHub
#12278 (comment).

@gonzaloserrano
Copy link

Me too! Waiting to be able to pass all these awesome linters with the vendors' code instead of the global one.

@bradfitz
Copy link
Contributor

bradfitz commented Nov 4, 2015

Many people want this. No need to enumerate the list of them here.

@kylewolfe
Copy link

I sent an email to golang-dev to solicit more ideas about go/build.

@cespare, can you link to discussion, please?

@cespare
Copy link
Contributor

cespare commented Nov 4, 2015

@kylewolfe my post is

https://groups.google.com/d/msg/golang-dev/kyckwHWpKQg/9nHp6kPzCwAJ

but it hasn't yielded any responses.

@kylewolfe
Copy link

Should we also discuss how internal is handled? Currently internal packages are resolved at cmd/go as well. Should this be handled in go/build? If not, are we ok with internal continuing to be resolved by cmd/go while vendor is handled by go/build?

@davecheney
Copy link
Contributor

@kylewolfe vendor is also implemented in cmd/go, not go/build

https://go-review.googlesource.com/#/c/10923/

@kylewolfe
Copy link

@davecheney, Yes. The discussion is around moving vendor from cmd/go to go/build.

@davecheney
Copy link
Contributor

@kylewolfe I'm sorry I misunderstood what you mean by "If not, are we ok with internal continuing to be resolved by cmd/go while vendor is handled by go/build?"

@cespare
Copy link
Contributor

cespare commented Nov 4, 2015

Please, this issue is already far too muddled without talking about internal packages as well. If you wish to discuss internal packages, please use the mailing list (or open a new issue if you feel there's a bug).

@kylewolfe
Copy link

@cespare, I'm sorry, but I think we should discuss where this lives, since it's doing almost exactly the same thing.

@dsymonds
Copy link
Contributor

dsymonds commented Nov 4, 2015

internal is easy to implement, and doesn't impact import paths. It doesn't need to be discussed on this issue, which is specifically about vendor.

@kylewolfe
Copy link

@davecheney, I could have phrased my entire post a little better :)

Here is the internal implementation review: https://codereview.appspot.com/120600043/

@bradfitz bradfitz mentioned this issue Nov 26, 2015
Closed
@laher laher mentioned this issue Nov 27, 2015
Closed
@keegancsmith keegancsmith mentioned this issue Dec 3, 2015
Merged
@broady broady mentioned this issue Dec 9, 2015
Closed
@ianlancetaylor ianlancetaylor modified the milestones: Go1.6, Go1.6Early Dec 11, 2015
@gopherbot
Copy link

CL https://golang.org/cl/17726 mentions this issue.

@gopherbot
Copy link

CL https://golang.org/cl/17727 mentions this issue.

@gopherbot
Copy link

CL https://golang.org/cl/17728 mentions this issue.

@rsc rsc closed this as completed in 0c428a5 Dec 16, 2015
@cespare cespare mentioned this issue Dec 16, 2015
Closed
rsc added a commit to golang/tools that referenced this issue Dec 17, 2015
@rsc
go/loader: use build.AllowVendor during Import on Go 1.6 and later
34ae141 
Makes programs like ssadump work on packages using vendored code,
for example net/http.

For golang/go#12278.
Depends on CL 17726 in main repository.

Change-Id: Ibabf564e397044a0f449087124dd96161081baaf
Reviewed-on: https://go-review.googlesource.com/17727
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>
@FiloSottile
Copy link
Contributor

Many tools now work out of the box just rebuilding them with 1.6, thanks!

gopherbot pushed a commit to golang/tools that referenced this issue Feb 18, 2016
@rsc
imports: add support for vendor directories
bf084ef 
Editor modes that invoke the goimports command on temporary copies
of actual source files will need to invoke goimports -srcdir now to say
where the real source directory is. Otherwise goimports will not consider
vendored or internal packages when looking for new imports.

In lieu of a test for cmd/goimports (because it has no tests),
a command transcript:

	$ cd /tmp
	$ cat x.go
	package p
	var _ = hpack.HuffmanDecode
	$

	$ GOPATH= goimports < x.go
	package p

	var _ = hpack.HuffmanDecode
	$ GOPATH= goimports x.go
	package p

	var _ = hpack.HuffmanDecode
	$

But with the new flag:

	$ GOPATH= goimports -srcdir $GOROOT/src/math < x.go
	package p

	import "golang.org/x/net/http2/hpack"

	var _ = hpack.HuffmanDecode
	$ GOPATH= goimports -srcdir $GOROOT/src/math x.go
	package p

	import "golang.org/x/net/http2/hpack"

	var _ = hpack.HuffmanDecode
	$

The tests in this CL and the above transcript assume that
$GOROOT/src/vendor/golang.org/x/net/http2/hpack exists.
It did in 40a26c9, but it does not today.
It will again soon (once Go 1.7 opens).

For golang/go#12278 (original request).

Change-Id: I27b136041f54edcde4bf474215b48ebb0417f34d
Reviewed-on: https://go-review.googlesource.com/17728
Run-TryBot: Russ Cox <rsc@golang.org>
Reviewed-by: Andrew Gerrand <adg@golang.org>
ashwanthkumar added a commit to ashwanthkumar/marathon-alerts that referenced this issue Mar 7, 2016
@ashwanthkumar
Not building the project on SnapCI because of golang/go#12278
5e8f8fa
@golang golang locked and limited conversation to collaborators Dec 29, 2016
@rsc rsc removed their assignment Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Go1.6
Development

No branches or pull requests