russross / blackfriday

Blackfriday: a markdown processor for Go
Other
5.44k stars 600 forks source link

go.mod in 1.5.2 is broken #491

Closed F21 closed 1 year ago

F21 commented 6 years ago

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/blackfriday.v1@v1.5.2: go.mod has non-....v1 module path "github.com/russross/blackfriday" at revision v1.5.2
dmitshur commented 6 years ago

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/blackfriday.v1@v1.5.2: 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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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/blackfriday.v1@v1.5.2: go.mod has non-....v1 module path "github.com/russross/blackfriday" at revision v1.5.2.

markbates commented 6 years ago

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

F21 commented 6 years ago

It seems to be required through:

thepudds commented 6 years ago

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/release@v1.0.11 github.com/gobuffalo/genny@v0.0.0-20180905180744-ca4140706556

github.com/gobuffalo/genny@v0.0.0-20180905180744-ca4140706556 github.com/gobuffalo/github_flavored_markdown@v1.0.0

github.com/gobuffalo/github_flavored_markdown@v1.0.0 gopkg.in/russross/blackfriday.v1@v1.5.1

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Here's a ticket that helps explain the back story a bit more https://github.com/shurcooL/github_flavored_markdown/issues/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 commented 6 years ago

@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. https://github.com/gobuffalo/github_flavored_markdown/commit/86eae9c06273300ebac5e41897bbdcf11b5ca7df

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

flibustenet commented 6 years ago

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

rtfb commented 6 years ago

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 commented 6 years ago

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 https://github.com/golang/go/commit/05604d7450b7cfcd946762039ee46e9ab85297d6 So it's safe to recommend this import path now.

@thepudds you confirm ?

thepudds commented 6 years ago

Here is my personal understanding:

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 commented 6 years ago

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 commented 6 years ago

And sorry again for the multiple posts here, but regarding the comment https://github.com/russross/blackfriday/issues/491#issuecomment-424233586 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 commented 6 years ago

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 commented 6 years ago

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: https://github.com/matcornic/hermes/pull/44

hwangjr commented 6 years ago

Here is the command:

go mod edit -replace=gopkg.in/russross/blackfriday.v2@v2.0.1=github.com/russross/blackfriday/v2@v2.0.1
carlwgeorge commented 5 years ago

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 commented 5 years ago

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.