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

Build / dependencies broken #48

Closed
andig opened this issue Aug 13, 2019 · 13 comments
Closed

Build / dependencies broken #48

andig opened this issue Aug 13, 2019 · 13 comments

Comments

@andig
Copy link

andig commented Aug 13, 2019

I'm using spf13/cobra/doc which depends on this module. Running go get -u I'm seing the following errors:

# github.com/cpuguy83/go-md2man/md2man
../go/pkg/mod/github.com/cpuguy83/[email protected]/md2man/md2man.go:11:16: undefined: blackfriday.EXTENSION_NO_INTRA_EMPHASIS
../go/pkg/mod/github.com/cpuguy83/[email protected]/md2man/md2man.go:12:16: undefined: blackfriday.EXTENSION_TABLES
../go/pkg/mod/github.com/cpuguy83/[email protected]/md2man/md2man.go:13:16: undefined: blackfriday.EXTENSION_FENCED_CODE
../go/pkg/mod/github.com/cpuguy83/[email protected]/md2man/md2man.go:14:16: undefined: blackfriday.EXTENSION_AUTOLINK
../go/pkg/mod/github.com/cpuguy83/[email protected]/md2man/md2man.go:15:16: undefined: blackfriday.EXTENSION_SPACE_HEADERS
../go/pkg/mod/github.com/cpuguy83/[email protected]/md2man/md2man.go:16:16: undefined: blackfriday.EXTENSION_FOOTNOTES
../go/pkg/mod/github.com/cpuguy83/[email protected]/md2man/md2man.go:17:16: undefined: blackfriday.EXTENSION_TITLEBLOCK
../go/pkg/mod/github.com/cpuguy83/[email protected]/md2man/md2man.go:19:29: too many arguments to conversion to blackfriday.Markdown: blackfriday.Markdown(doc, renderer, extensions)
../go/pkg/mod/github.com/cpuguy83/[email protected]/md2man/roff.go:19:9: cannot use &roffRenderer literal (type *roffRenderer) as type blackfriday.Renderer in return argument:
        *roffRenderer does not implement blackfriday.Renderer (missing RenderFooter method)
../go/pkg/mod/github.com/cpuguy83/[email protected]/md2man/roff.go:102:11: undefined: blackfriday.LIST_TYPE_ORDERED
../go/pkg/mod/github.com/cpuguy83/[email protected]/md2man/roff.go:102:11: too many errors
@lszucs
Copy link

lszucs commented Aug 13, 2019

Same issue here, go-md2man is a transitive dependency and build fails.

The reason might be, that transitive dependencies for go-md2man are not downloaded.

If I clone go-md2man and try to build it, it succeeds, both on master and on the v1.0.10 tag. In both cases the vendor folder is present.

@cpuguy83
Copy link
Owner

Nothing has changed here.
Did cobra switch to blackfriday v2?

@cpuguy83
Copy link
Owner

cpuguy83 commented Aug 15, 2019 via email

@andig
Copy link
Author

andig commented Aug 15, 2019

That's only on master.

Sorry, noticed that as well. Maybe this is the expected go get -u behaviour?

@epicchewy
Copy link

epicchewy commented Aug 23, 2019

I also came across this issue, so I ran go mod graph to look at the package's dependencies and found

go mod graph | grep blackfriday
github.com/cpuguy83/[email protected] github.com/russross/[email protected]

For that package, you might need to run
go mod edit -replace github.com/russross/blackfriday=github.com/russross/[email protected] because it is still pulling version blackfriday (v1.5.2 as in the release notes).

@cpuguy83
Copy link
Owner

cpuguy83 commented Sep 7, 2019

Yeah go get -u updates the packges, I'm not sure how modules might come into play here.
But if using modules definitely pin blackfriday v1.5.2.

@andig
Copy link
Author

andig commented Sep 7, 2019

I‘ve opened above go issue as I find the update behavior perplexing. Imho it must not update to a v2 version as that breaks import path equality.

@andig
Copy link
Author

andig commented Sep 13, 2019

@cpuguy83 citing from golang/go#34165 (comment):

The go.mod file was added in v2.0.1. IMO that is not the appropriate tag according to the rules at https://semver.org — adding a go.mod file is “adding functionality”, not a “backwards compatible bug fix”, so it should be at least a minor release — but it is what it is.

The reason this happens is that a v2.0.0 tag was added to blackfriday before they migrated to modules, along with the breaking changes that a major version bump implies. Normally, the go command requires v2.0.0 to be associated with modules whose paths end with a /v2 suffix (github.com/russross/blackfriday/v2), but that's only mandatory for repositories that have added a go.mod file. Without a go.mod file, v2.0.0 is considered part of the original module, and go get -u upgrades over the breaking change.

Given that cpuguy83/go-md2man depends on blackfriday v1 and hence go get -u will inadvertently pull in the incompatible v2.0.0.
The only way out of this that I see is to release a new version here that works against blackfriday>=v2.0.0, preferably as a v1 point release so it gets picked up by go get -u.

Could you kindly reopen this issue to track this?

@cpuguy83
Copy link
Owner

I cannot release a 1.x based on blackfriday 2.x, they are completely incompatible.
See that master is on blackfriday v2.

@andig
Copy link
Author

andig commented Sep 13, 2019

I cannot release a 1.x based on blackfriday 2.x, they are completely incompatible.

I haven't checked the code. If you cannot hide the incompatibility inside md2man's API then we're stuck until 2.0 is released and downstream projects habe upgraded. If you can keep your API constant (or wrap incompatibilities) then a point release should be acceptable.

@cpuguy83
Copy link
Owner

I think the only thing I could do is instead of having a go.mod specify the module as /v2, have a /v2 directory.

@andig
Copy link
Author

andig commented Sep 13, 2019

I think the only thing I could do is instead of having a go.mod specify the module as /v2, have a /v2 directory.

As I understood the discussion that wouldn't help. The problem is that blackfriday's v2.0.0 is treated as compatible to blackfriday 1.0 since go.mod was only added later at blackfriday.

So if a backwards-compatible fix isn't possible then a 2.0 release of md2man would allow clients to upgrade to a stable 2.0.

@cpuguy83
Copy link
Owner

Ok, I hope this works... I tagged v2.0.0 with a /v2 module path.

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

4 participants