-
Notifications
You must be signed in to change notification settings - Fork 43
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
Doesn't build in projects that use dep
Go dependency management tool due to issue with blackfriday v2.0.0 tag.
#12
Comments
I can easily reproduce this using
Gopkg.toml
[[constraint]]
branch = "master"
name = "github.com/bryanl/webbrowser"
[[constraint]]
branch = "development"
name = "github.com/gobuffalo/buffalo"
[[constraint]]
branch = "master"
name = "github.com/gobuffalo/makr"
[[constraint]]
branch = "master"
name = "github.com/gobuffalo/packr"
[[constraint]]
branch = "master"
name = "github.com/gobuffalo/plush"
[[constraint]]
branch = "master"
name = "github.com/gopherguides/remark"
[[constraint]]
branch = "master"
name = "github.com/markbates/inflect"
[[constraint]]
name = "github.com/pkg/errors"
version = "0.8.0"
[[constraint]]
name = "github.com/sirupsen/logrus"
version = "1.0.2"
[[constraint]]
branch = "master"
name = "github.com/spf13/cobra"
Gopkg.lock:
|
I believe I've tracked down the problem, sort of. Here is the release tag for blackfriday, v2 https://github.com/russross/blackfriday/releases/tag/v2.0.0 The Not really sure what the "correct" thing to do here is. It's kind of a big mess, mostly because of the way blackfriday has been tagged. |
Thanks for filing. We should report this at blackfriday (done in russross/blackfriday#383 (comment)) and/or dep (already done at golang/dep#999) and come up with a solution. The v2 version is supposed to be available at a different import path ( Note, I'm not closely familiar with the inner workings of dep yet. So I can't say if this is a problem dep should fix, or if dep is doing the right thing in this situation, and we should change blackfriday. I'm guessing the latter... but it needs discussion. |
I've opened an issue with dep, here golang/dep#999. The problem, is in how dep does transitive dependencies. What it basically boils down to is, either this repo needs to update to v2 of blackfriday, or it needs to vendor it. Because I was importing this repo, and it doesn't set a constraint on a version of blackfriday to use, I can't constrain it. My "workaround" is I have to use a blank import to import blackfriday, before this package, and then I have set the constraint and vendor so dep doesn't pick up v2 of blackfriday. It's a real mess, unfortunately. |
I want to update to v2, but it's not a priority, so it won't be done very soon. Until then, this package is using the latest "v1" version of blackfriday. Vendoring is not neccessary because it has little benefits—not enough to outweigh the downsides. The problem is that the |
If they removed the |
Another possible fix, if I understand things correctly, would be for my That would fix the issue for you @markbates, right? The problem with this solution is that it means every project that imports |
Yeah, right now everyone who uses this package and dep is going to have problems, unless they do a manual import of blackfriday and then lock the constraint down themselves. Take this application: package main
import _ "github.com/shurcooL/github_flavored_markdown"
func main() {
} If you run [[projects]]
name = "github.com/russross/blackfriday"
packages = ["."]
revision = "cadec560ec52d93835bf2f15bd794700d3a2473b"
version = "v2.0.0" Then, of course, it won't build:
I believe, and I might be mistaken, if you add a required = ["github.com/russross/blackfriday"]
[[constraint]]
branch = "master"
name = "github.com/russross/blackfriday" at the root of this repo, it should fix the problem. You don't want to specify the |
@markbates Drive-by comment: Would it help if you define an override in
|
@bpicode I haven't tried, it might. The bigger problem is everyone using this project and dep will run into this problem, which is probably not the best experience for everyone. |
@markbates I agree that it is not a good experience. However the even bigger problem is that most projects do not have dependency management at all, leaving tools like |
To clarify, this project has a specific form of dependency management in place (which is supported by me and working well, as can be seen here). My intention is to make it work with |
dep
Go dependency management tool due to issue with blackfriday v2.0.0 tag.
Makes sense, thanks. @markbates, if this is time-sensitive for you, I can apply the suggested solution temporary to fix |
@shurcooL I've patched Buffalo to fix this, so my "immediate" need is gone, but it definitely feels like a hack to fix it there. I love this project, and just want to make sure no one else runs into this problem. |
hi folks! @bpicode is definitely right - we can't cover everything. we have to define some general rules, and one of those is preferring semver tags to branches. this is causing some growing pains now, but we expect that pain should largely subside once the community starts generating semver-tagged releases as a matter of course. the two cases it tends to hurt in are:
an override would solve @markbates' problem, but we do try to discourage their use if possible. the ideal solution, from dep's perspective, is for this project to specify the a version constraint on
this would be an appropriate alternative to overrides for @markbates to exercise in his project. adding the however, because this project DOES import there are two other alternatives, as well. if this project starts importing blackfriday could also start using package import comments to enforce that its v2 is imported via gopkg.in - e.g. package blackfriday // import "gopkg.in/russross/blackfriday.v2" now, dep hasn't actually added the support for this yet (golang/dep#902), because people largely haven't reported needing it - but, i'd count this as such a report. anyway, what it will do is treat those package import comments as another satisfiability condition that must be met for a version to be acceptable. the problem with this approach is that the |
Thanks a lot for the detailed and informative response @sdboyer! I think that should be sufficient information for us to find a good upstream solution in blackfriday. |
always happy to help! |
Thanks everybody for reporting the issue and figuring out the details while I was away! Having read all the material, I can see only one solution: retag v2 with Or we could simply wait for golang/dep#902 to get done. Or I can start hacking on golang/dep#902 right away, but that would probably end up being a busier equivalent of waiting :-) |
this sounds reasonable to me (as i noted over on the other issue) 👍
🎉 🎉 honestly, i don't think it'd be that bad to do 😄 at least, not the additional parsing bits that #902 is focused on. incorporating a new satisfiability condition into the solver is trickier, but still probably pretty isolated - i'm happy to guide, or will do it myself if necessary. |
The intention is to make it so that it's possible to import and use this package from within Go modules (in addition to the well-supported GOPATH workspace mode), and have it build without users having to manually specify that github_flavored_markdown requires blackfriday v1. Use the recently created blackfriday v1.5.2 tag. Only specify the blackfriday version in go.mod and go.sum; others are unspecified to avoid the added burden of maintaining and updating them. Fixes #12. Closes #20.
The intention is to make it so that it's possible to import and use this package from within Go modules (in addition to the well-supported GOPATH workspace mode), and have it build without users having to manually specify that github_flavored_markdown requires blackfriday v1. Use the recently created blackfriday v1.5.2 tag. Only specify the blackfriday version in go.mod and go.sum; others are unspecified to avoid the added burden of maintaining and updating them. Updates #12. Updates #20.
Hello team, any update here? I am currently facing this issue with GFM and Dep. Any tips? |
I had to change dep to glide to work it |
@victorshinya There are no updates on this as far as I know. My suggested workaround would be to use Go modules that were added in Go 1.11. This package fully supports them as of PR #22. |
Dep is deprecated by now, so there's nothing more to do here. |
They released blackfriday v2, about 20 hours ago, and now this project doesn't build anymore. :(
The text was updated successfully, but these errors were encountered: