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

go.mod in 1.5.2 is broken #491

Closed
F21 opened this issue Sep 23, 2018 · 25 comments
Closed

go.mod in 1.5.2 is broken #491

F21 opened this issue Sep 23, 2018 · 25 comments

Comments

@F21
Copy link

F21 commented Sep 23, 2018

The go.mod in 1.5.2 has the suffix v1, however, modules v1 and below do not need any version suffixes.

I have a transitive dependency on v1.5.1, but go mod breaks:

go: gopkg.in/russross/[email protected]: go.mod has non-....v1 module path "github.com/russross/blackfriday" at revision v1.5.2
@dmitshur
Copy link
Collaborator

dmitshur commented Sep 23, 2018

The go.mod in 1.5.2 has the suffix v1,

Where? I don't see it:

https://github.com/russross/blackfriday/blob/v1.5.2/go.mod#L1

go: gopkg.in/russross/[email protected]: go.mod has non-....v1 module path "github.com/russross/blackfriday" at revision v1.5.2

The canonical import path of blackfriday v1 is github.com/russross/blackfriday. Where is gopkg.in/russross/blackfriday.v1 coming from?

@F21
Copy link
Author

F21 commented Sep 23, 2018

I must have gotten the go.mod files confused. I think the repo encourages the use of gopkg.in, see: https://github.com/russross/blackfriday#installation

I don't need blackfriday directly, but packr imports it: https://github.com/gobuffalo/packr/blob/master/go.sum#L105

I also tried go mod -m github.com/russross/blackfriday and go mod -m gopkg.in/russross/blackfriday.v1, but I get:

# github.com/russross/blackfriday
(main module does not need module github.com/russross/blackfriday)

@dmitshur
Copy link
Collaborator

Thanks for providing additional information @F21, it's helpful.

I think the repo encourages the use of gopkg.in, see: https://github.com/russross/blackfriday#installation

I know it does for v2, but I didn't think that applied for v1. It's possible that something about v1.5.2 is incompatible with using gopkg.in/russross/blackfriday.v1 as an import path.

I don't need the blackfriday directly, but packr imports it: https://github.com/gobuffalo/packr/blob/master/go.sum#L105

I've looked at packr source code, and I'm not seeing blackfriday imported anywhere in it. The only mention seems to be those 2 lines in go.sum file. @markbates, do you know why it's showing up there? Perhaps it was used before, but isn't now, and go mod tidy would get rid of it?

/cc @rtfb @markbates

@F21
Copy link
Author

F21 commented Sep 23, 2018

@dmitshur Thanks for investigating! I also went through packr's directly imported dependencies but was not able to find any references to blackfriday. However, even if it was not used, I am still baffled why go mod breaks: go: gopkg.in/russross/[email protected]: go.mod has non-....v1 module path "github.com/russross/blackfriday" at revision v1.5.2.

@markbates
Copy link

It’s not being used by packer but it could be coming in through a transitive dep somewhere.

@F21
Copy link
Author

F21 commented Sep 23, 2018

@thepudds
Copy link

thepudds commented Sep 24, 2018

I had posted this to @F21 in slack earlier, but also posting here.

It looks like blackfriday is coming in via github.com/gobuffalo/release.

The go.mod for packr includes a require for:

           github.com/gobuffalo/release v1.0.11 // indirect

From go mod graph (after cloning packr locally):

github.com/gobuffalo/[email protected] github.com/gobuffalo/[email protected]

github.com/gobuffalo/[email protected] github.com/gobuffalo/[email protected]

github.com/gobuffalo/[email protected] gopkg.in/russross/[email protected]

And github.com/gobuffalo/github_flavored_markdown seems to import blackfriday as "gopkg.in/russross/blackfriday.v1" here:

https://github.com/gobuffalo/github_flavored_markdown/blob/v1.0.0/main.go#L30

@thepudds
Copy link

So hopefully that answers the question from @dmitshur:

The canonical import path of blackfriday v1 is github.com/russross/blackfriday. Where is gopkg.in/russross/blackfriday.v1 coming from?

In general, as far as I understand, I think the go tooling in 1.11 will complain if there is a mis-match between the import path used in an import statement (e.g., gopkg.in/russross/blackfriday.v1) vs. the module path used in the module directive for the corresponding module (which from https://github.com/russross/blackfriday/blob/v1.5.2/go.mod, we can see it uses the module path module github.com/russross/blackfriday).

@thepudds
Copy link

thepudds commented Sep 24, 2018

And sorry for the multiple posts, but as far as I understand it, the module path (as used in the module directive in the go.mod of a module) is a declaration of how the code in a module wants to be known.

In this case, it sounds like at least v1.5.2 of blackfriday wants to be known as github.com/russross/blackfriday, so it seems like github.com/gobuffalo/github_flavored_markdown should likely be updated to use that as the import path for at least v1 of blackfriday (rather than importing blackfriday as gopkg.in/russross/blackfriday.v1, which is what github.com/gobuffalo/github_flavored_markdown currently does).

@markbates
Copy link

We only maintain a fork because this repo doesn’t build cleanly with go get, dep, and go modules. Using gopkin has solved the problem for us. I don’t understand why it became a problem suddenly. Things were working earlier today just fine. It’s Sunday so I don’t know if there’s been a release of this library that could’ve caused the problem or if it’s something else.

@markbates
Copy link

Sorry, apologies. We maintain a fork of Black Friday because it doesn’t build cleaning against this repo. We don’t use this repo directly. Apologies for the confusion. There are many issues talking about the same thing.

@thepudds
Copy link

thepudds commented Sep 24, 2018

One other tidbit.

Note that:

I suspect this is likely because @F21 did a go get -u github.com/gobuffalo/packr (note the -u), which would explain why @F21 is seeing the latest available v1 version of v1.5.2 for blackfriday.

@markbates on the other hand is not seeing any issue on v1.5.1 of blackfriday, as far as I understand the comments here so far.

v1.5.1 of blackfriday does not have a go.mod file, as far as I can see:
https://github.com/russross/blackfriday/tree/v1.5.1/

which means v1.5.1 would be more liberal about an unexpected import path used by a consumer.

In other words, the go.mod file that seems to have been added to v1.5.2 of blackfriday declared that v1.5.2 of blackfriday wants to known as github.com/russross/blackfriday, and Go 1.11 enforces that, and hence the errors reported at the top of this issue when @F21 upgraded to v1.5.2.

As far as I am understanding things, if those are correct observations, then that would explain all the behaviors reported here, I think?

@markbates
Copy link

Here's a ticket that helps explain the back story a bit more shurcooL/github_flavored_markdown#12

Basically it's this:

github.com/shurcooL/github_flavored_markdown is written against blackfriday v1. dep always pulls v2 of blackfriday, so GFM doesn't compile when using dep. We started a fork to force the gopkg.in/russross/blackfriday.v1 import because it would fix all the ways people were installing packages. Then came modules. :)

So we're importing gopkg.in/russross/blackfriday.v1 which is now declaring itself in it's go.mod file as github.com/russross/blackfriday. Conversely, I noticed that the go.mod for v2 declares github.com/russross/blackfriday/v2 but the install instructions say gopkg.in/russross/blackfriday.v2, so that's an issue people are going to hit immediately too.

@markbates
Copy link

@thepudds to solve the issue, for now, on our end we ventured blackfriday at the version that works as an internal library so Go modules doesn't try to control it. gobuffalo/github_flavored_markdown@86eae9c

That seems to fix the issue with the gobuffalo fork of GFM.

markbates added a commit to gobuffalo/buffalo that referenced this issue Sep 24, 2018
@flibustenet
Copy link

Maybe the doc should recommend to import github.com/russross/blackfriday/v2 with go>1.11 ?

markbates added a commit to gobuffalo/buffalo that referenced this issue Sep 24, 2018
* russross/blackfriday#491 (comment)

* updated deps

* udpated deps

* updated events

* fixed race condition
@rtfb
Copy link
Collaborator

rtfb commented Sep 25, 2018

Maybe the doc should recommend to import github.com/russross/blackfriday/v2 with go>1.11 ?

Definitely. Import path github.com/russross/blackfriday/v2 can not possibly work with Go versions prior to 1.11.

The docs surely need updating, with explicit instructions on what to do with which tool. But if I'm reading this tread correctly, go.mod is fine and there's no underlying technical problems, only a confusing usability issue?

@flibustenet
Copy link

With Go1.11 go.mod is fine when we import blackfriday with github.com/russross/blackfriday for v1 and github.com/russross/blackfriday/v2 for v2.

Before Go1.11 we could use import github.com/russross/blackfriday or import gopkg.in/russross.blackfriday like we want. Not more.

https://github.com/rsc/quote did the opposite, the import path must be rsc.io/quote, we cannot use github.com/rsc/quote even if could before.

I believe every app that use an import path with gopkg.in for blackfriday will have to change that with Go1.11 (it will be difficult if it's in a dependency !).

Last minor version of Go1.9 and Go1.10 can use github.com/russross/blackfriday/v2
golang/go@05604d7
So it's safe to recommend this import path now.

@thepudds you confirm ?

@thepudds
Copy link

Here is my personal understanding:

  • The module path in the module directive in your go.mod serves as a definitive declaration of the identify of your code (e.g., a go.mod file might have module github.com/proj/foo)
  • Go 1.11 enforces that the import path used in a consumer's import statement must match the module path in the consumed module's module path in its go.mod.
    • For example, if consumer's import statement says import "gopkg.in/foo.v1", but foo itself declares its identity in foo's go.mod as module github.com/proj/foo, then the consumer code will not compile in Go 1.11 because of the mismatch.
  • If a pre-existing public package previously documented that it should be imported via import "gopkg.in/foo.v1" before it adopted go.mod, then I believe the most common approach would be to have the go.mod file read module gopkg.in/foo.v1 so that pre-existing consumers do not need to change their code.
    • Different projects can always make different choices, but I think this would be the most common choice and probably the most consumer friendly choice
    • Alternatively, a project could document that any existing consumers must update their code so that import paths no longer use import "gopkg.in/foo.v1"
  • Also, it could be reasonable to have a different approach for v1 vs. v2 of a package (e.g., v1 could declare itself as module gopkg.in/foo.v1 in its go.mod, and v2 could declare itself as module github.com/proj/foo/v2 in its go.mod, which could be a way of transitioning off of gopkg.in).

All that said:

  1. Just trying to share my understanding. Happy to learn more.
  2. I don't know enough about the history of these specific projects to make any particular recommendation. I tried to outline what I think the typical choice might be for a project adopting modules if it was already using gopkg.in, but maybe a different choice makes sense here given a more complex history.

CC @myitcv

@thepudds
Copy link

Two other more general comments.

In general, there’s a special exemption in Go 1.11 modules system so that existing code that uses import paths starting with gopkg.in (such as gopkg.in/yaml.v1 and gopkg.in/yaml.v2) can continue to use those forms for their module paths and import paths even when opting in to modules.

Also related to this general discussion here is the pre-existing convention of “import comments” and their relationship to modules, which is spelled out at the end of this quote from the documentation here:

https://golang.org/cmd/go/#hdr-Import_path_checking

A package statement is said to have an "import comment" if it is immediately followed (before the next newline) by a comment of one of these two forms:

package math // import "path"

The go command will refuse to install a package with an import comment unless it is being referred to by that import path. In this way, import comments let package authors make sure the custom import path is used and not a direct path to the underlying code hosting site.
...
Import path checking is also disabled when using modules. Import path comments are obsoleted by the go.mod file's module statement.

@thepudds
Copy link

And sorry again for the multiple posts here, but regarding the comment #491 (comment) above from @rtfb:

Import path github.com/russross/blackfriday/v2 can not possibly work with Go versions prior to 1.11.

I just wanted to also mention that Go versions 1.9.7+, 1.10.3+ and 1.11 have been updated so that code built with those releases can properly consume v2+ modules without requiring modification of pre-existing code. (I'm making a general comment here, and am setting aside for the moment any particular considerations or exceptions for gopkg.in).

@rtfb
Copy link
Collaborator

rtfb commented Sep 29, 2018

I just wanted to also mention that Go versions 1.9.7+, 1.10.3+ and 1.11 have been updated so that code built with those releases can properly consume v2+ modules without requiring modification of pre-existing code. (I'm making a general comment here, and am setting aside for the moment any particular considerations or exceptions for gopkg.in).

Yeah, sorry for uneducated assertions. I wasn't aware that Go team did these compatibility changes, that comes as a very pleasant surprise.

@maxekman
Copy link

In my case I found that setting replace did it as a workaround:

replace gopkg.in/russross/blackfriday.v2 v2.0.1 => github.com/russross/blackfriday/v2 v2.0.1

My original build error was cannot find module for path gopkg.in/russross/blackfriday.v2 which I got where this package was imported indirectly by Hermes. They are working on using the correct path for Go modules: matcornic/hermes#44

@hwangjr
Copy link

hwangjr commented Oct 16, 2018

Here is the command:

go mod edit -replace=gopkg.in/russross/[email protected]=github.com/russross/blackfriday/[email protected]

@carlwgeorge
Copy link

Can a new v1 (1.5.3?) version be released to update go.mod to reference gopkg.in/russross/blackfriday.v1 and get this working?

@dmitshur
Copy link
Collaborator

Making that change would break far too many people. I don’t think it’s a change we can make.

We should update the README to reflect the current reality better.

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

No branches or pull requests

9 participants