Skip to content

go build supports import ./pkg; go get does not #33129

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

Closed
ppbrown opened this issue Jul 16, 2019 · 14 comments
Closed

go build supports import ./pkg; go get does not #33129

ppbrown opened this issue Jul 16, 2019 · 14 comments

Comments

@ppbrown
Copy link

ppbrown commented Jul 16, 2019

$ go version
go version go1.11.5 linux/amd64

If you have code, that normally would do:

import "you.com/yourmodule/sub"

You CAN do

import "./sub"

and "go build" will work.

However, "go get" will not.

This is important for the following reasons:

  1. if you fork something on github, and make changes, then want to test your code before submitting a pull request...
    If you DONT change the imports to "github/you" instead of "github/origin"... you wont actually use your new code! It will use the OLD code, and you will get false results!!

  2. if you DO change the imports to "github/you... you've then just screwed the diffs for a pull request

So you have to change it to something, and ideally you want it to work for both the original, AND your fork.

import ./sub

does work, if people do
$ git clone blah
$ go build

but it does NOT work, if people just do
$ go get blah

Seems like it's a partially implemented feature, if it works for go build, but not go get.

So, please round out the feature, and make it work for "go get" as well?

Sample error output:

$ go get github.com/ppbrown/gdrive
go/src/github.com/ppbrown/gdrive/handlers_drive.go:15:2: local import "./auth" in non-local package
go/src/github.com/ppbrown/gdrive/gdrive.go:7:2: local import "./cli" in non-local package
go/src/github.com/ppbrown/gdrive/compare.go:7:2: local import "./drive" in non-local package

HOWEVER...

$ git clone http://github.com/ppbrown/gdrive
$ cd gdrive
/tmp/gdrive$ go build
/tmp/gdrive$

@FiloSottile
Copy link
Contributor

I'm not sure what you mean with "does not work". Does it print an error? What error? Please fill the issue template.

BTW, how the fork-and-modify flow usually works is that you clone the forked repository in its original import path in GOPATH mode, or you just don't change its name in go.mod in modules mode, so that everything still works without changing the full import paths.

@FiloSottile FiloSottile added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 17, 2019
@ppbrown
Copy link
Author

ppbrown commented Jul 17, 2019

Sorry. I updated the issue with sample outputs.
Thanks for mentioning "the usual" flow. However.. that seems rather non-optimal.
If you REALLY want to do a full test of code, seems like you would want to actually test it fully, in the same way the original is.

The original is built with
"go get XXX"

So for validation before doing a pull request, seems like most appropriate thing to do, is likewise

"go get newXXX"

Right now, thats just not cleanly possible. Please fix.

@thepudds
Copy link
Contributor

Note that modules do not allow relative imports like import "./subdir".

See #26645 (comment), which includes:

In modules, there finally is a name for the subdirectory. If the parent directory says "module m" then the subdirectory is imported as "m/subdir", no longer "./subdir".

@ppbrown
Copy link
Author

ppbrown commented Jul 18, 2019 via email

@ppbrown
Copy link
Author

ppbrown commented Jul 18, 2019 via email

@FiloSottile
Copy link
Contributor

Looks like this was considered and the decision was made to phase out relative imports as modules are introduced.

If you REALLY want to do a full test of code, seems like you would want to actually test it fully, in the same way the original is.

With GOPATH, if you are sending a patch to github.com/original/module, the most full test is to test it in $GOPATH/src/github.com/original/module, with github.com/original/module import paths.

With modules, the import path is already relative to the name in go.mod, so there's no need for ./. Simply put module foo in go.mod, and then use foo/bar instead of ./bar. foo can also be github.com/original/module of course, even if it's cloned at ~/github.com/you/module or wherever, it doesn't matter.

@FiloSottile FiloSottile added modules Question and removed WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. labels Jul 18, 2019
@thepudds
Copy link
Contributor

@ppbrown in case this helps, this is a commonly recommended blog post on working with forks in GOPATH:

https://blog.sgmansfield.com/2016/06/working-with-forks-in-go/

From your initial error message, I’m guessing you might not yet be using modules and instead are working in the traditional GOPATH, but I am not sure.

(Sorry for brief comment; on mobile).

@ppbrown
Copy link
Author

ppbrown commented Jul 18, 2019

I just read through
https://github.com/golang/go/wiki/Modules#gomod
it doesnt seem to support what is needed in the way that you claim.
Might you please give me a specific working example?

ie: could you give me specific syntax for a "go.mod" file, that should make
"go get github.com/ppbrown/gdrive" work...
but ALSO allow
"go get github.com/gdrive-org/gdrive" to
work, if a pull is accepted to there?

Seems to me that go.mod requires full github.com paths. which will then work for ONE of the repos, but not both of them.
I say that because the official github wiki referenced above, only documents use of FULL PATHS.

If relative pathing is allowed, (and that would be great), then at minimum, seems like the wiki needs to be updated to clearly say, that both

module github.com/gdrive-org/gdrive
AND
module gdrive

are acceptible use, for go.mod

@ppbrown
Copy link
Author

ppbrown commented Jul 18, 2019

to "thepuds":
I just read the page you mentioned. There are two things wrong with that approach IMO.

  1. it seems to be a hack that is almost identical to basically equivalent to doing a separate "git clone".
    Except IMO it is dangerous, because it then LIES about where the source is from.
    YOu end up with
    $GOPATH/src/github.com/original
    but the ACTUAl code is from
    http://github.com/forkbranch.

To me, that is HORIFYING. In the future, you could be doing work in that path, and FORGET that it has been messed with.
IMO it is far safer, and therefore better, to have a clean mechanism, to work with
http://github.com/forkbranch
as actually in
$GOPATH/src/github.com/forkbranch.

  1. It still doesnt let you FULLY test the branch, by allowing you to do
    go get github.com/forkbranch

As I've shown, "go get" and "go build" do not actually function 100% identically.
The most thorough testing before submitting a pull, should idealy be done by running a "go get".
which currently you cant do.

@thepudds
Copy link
Contributor

Could I ask again if you are using modules here?

The repository you mentioned https://github.com/ppbrown/gdrive does not seem to be using modules, as far as I could see from a quick look.

If you are using modules, worth reading this if you haven’t already:
https://github.com/golang/go/wiki/Modules#when-should-i-use-the-replace-directive

(Still on mobile; still brief).

@thepudds
Copy link
Contributor

thepudds commented Jul 18, 2019

If you are not using modules, but are interested in a greater diversity of opinions on working with forks and Go, you could see for example the community discussions here, here, and here.

If you would like to solicit other people’s opinions, the forums listed on the Questions wiki page are very helpful. (Unlike many projects, the Go project does not currently use GitHub Issues for general discussion or asking questions. GitHub Issues are used for tracking bugs and proposals).

@ppbrown
Copy link
Author

ppbrown commented Jul 18, 2019

Thank you for your continued input.
To first officially answer your question: no I am not currently using go.mod (I think thats what you mean by "are you using modules", since its kinda unavoidable to use modules with go? :)
I'm happy to start using it if it helps. Unfortunately, going by the docs, it doesnt seem it would.

I checked out those links. I learned something new about git, which is cool (git add remote).
However... it still doesnt actually fix the problem. which is "go get github.com/myforkproject" doesnt work!
It merely hacks around the problem, which as I mentioned, is unsafe.
If a directory path is github.com/foo, it should contain code FROM github.com/foo !

So, how do i properly raise this as a policy issue?
I saw that the "relative module support was (kind of) removed". but I didnt see writeup as to why this was done. And, just because it seemed a good idea at the time, doesnt mean it's a good idea forever.

I opened this issue, because I found what I thought was a bug (and in actuality, it STILL IS A BUG.
Behaviour is inconsistent)
How should I officially approach this as a policy problem?

@FiloSottile
Copy link
Contributor

I realize this does not cover all cases, but there are usually two kinds of forks: GitHub forks made to contribute back (which should just keep using the original import paths because that's how they will look like when merged) and real forks meant to diverge (which should just update all import paths to the new location).

The former kind you don't usually "go get" because it only exists to make a PR, and with modules you have "replace" directives to use it, which in fact require the import paths to stay the same.

@ppbrown
Copy link
Author

ppbrown commented Jul 18, 2019 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants