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

Going to tag v2.0.0 #383

Closed
rtfb opened this issue Jul 29, 2017 · 20 comments
Closed

Going to tag v2.0.0 #383

rtfb opened this issue Jul 29, 2017 · 20 comments

Comments

@rtfb
Copy link
Collaborator

rtfb commented Jul 29, 2017

Hey folks. I've submitted what I hope to be the last API-affecting PRs #381 and #382. Aftery they're in, I'm going to tag the tip of v2 branch with v2.0.0, migrate the relevant README changes to master and call v2 done.

So this is the last call to shout out whatever API-affecting issues, concerns or questions you might have.

I plan to do that some time within two weeks if nothing comes up.

@rtfb rtfb mentioned this issue Jul 29, 2017
@rtfb
Copy link
Collaborator Author

rtfb commented Aug 11, 2017

@rtfb
Copy link
Collaborator Author

rtfb commented Aug 11, 2017

Master readme sync'ed with v2.0.0: #354

@dmitshur
Copy link
Collaborator

dmitshur commented Aug 12, 2017

@rtfb, this tag is causing issues in projects that use dep, and import libraries that import github.com/russross/blackfriday (expecting v1 API).

See shurcooL/github_flavored_markdown#12.

/cc @sdboyer It's probably our fault for doing the wrong thing here, but FYI anyway.

@dmitshur
Copy link
Collaborator

@sdboyer wrote a detailed and informative analysis at shurcooL/github_flavored_markdown#12 (comment).

@rtfb, I think that should be enough information for us to at least resolve the issue. But we can also use it to consider alternative plan for how to offer v1 and v2 of blackfriday in the future.

@rtfb
Copy link
Collaborator Author

rtfb commented Aug 13, 2017

Uhh... Okay, so with current state of affairs, Blackfriday's v1 is unusable with dep, at least not in an intuitive way. Even for non-transitive users, the intuitive usage produces broken output:

$ cat main.go 
package main

import (
	"fmt"

	blackfriday "github.com/russross/blackfriday"
)

func main() {
	html := blackfriday.MarkdownCommon([]byte("*foo*"))
	fmt.Println(string(html))
}
$ dep init
  Using ^2.0.0 as constraint for direct dep github.com/russross/blackfriday
  Locking in v2.0.0 (cadec56) for direct dep github.com/russross/blackfriday
  Locking in master (541ff5e) for transitive dep github.com/shurcooL/sanitized_anchor_name
$ cat Gopkg.toml 
[...]
[[constraint]]
  name = "github.com/russross/blackfriday"
  version = "2.0.0"

For those who use Blackfriday v1 transitively it's even worse, because it's difficult and non-obvious how to convince dep to pick up v1.

Having read all the material, it seems to me that golang/dep#902 would save our day here. It would allow us to continue with the plan that we developed in pre-dep days (namely, keep v1 dormant on master and maintain an active v2 development on v2 branch). Other than that, I do not see any good ways out of it.

One easy way to get out of it is to remove the v2.0.0 tag and retag with v2.0, i.e. avoid semantic versioning. That would be a pity, though, I wouldn't like to take this course if possible.

@dmitshur
Copy link
Collaborator

One easy way to get out of it is to remove the v2.0.0 tag and retag with v2.0, i.e. avoid semantic versioning. That would be a pity, though, I wouldn't like to take this course if possible.

If we're going to try to avoid semantic versioning from interfering with v1, I would suggest/consider doing it by adding some prefix to a valid semver. The prefix indicates that the semver tag applies to a different import path. Something like gopkgin-v2.0.0, blackfriday2-v2.0.0 or so.

It would allow us to continue with the plan that we developed in pre-dep days (namely, keep v1 dormant on master and maintain an active v2 development on v2 branch).

I'm beginning to worry more that it's a plan we came up with before dep, and it may not scale well to the future. We're trying to have one repository contain different versions at different import paths, and still do versioning in between.

A radical approach to consider, not necessarily do, but to think through the cons/pros of is to rely on dep for offering blackfriday v2 at same github.com/russross/blackfriday import path.

@rtfb
Copy link
Collaborator Author

rtfb commented Aug 13, 2017

A radical approach to consider, not necessarily do, but to think through the cons/pros of is to rely on dep for offering blackfriday v2 at same github.com/russross/blackfriday import path.

Well, apparently, dep can perfectly do that today, at least for first-order importers. The only problem is that it defaults to v2 and thus breaks existing code: cd to a project that's been merrily using blackfriday v1 at same old github.com/russross/blackfriday, type go build and it works; then type dep init, go build and it stops working.

Breaking existing code just by typing dep init is really bad. It upsets users, goes against our reasoning for maintaining v1 import path at github.com/russross/blackfriday, and gives bad press for dep. Everyone loses :-(

If I'm reading things correctly, golang/dep#902 will give us best of all worlds:

  1. For somebody who's been using v1 at github.com/russross/blackfriday and starts using dep, it will default to v1.4 and code remains working.
  2. This will also solve the transitive dependency problem that we have at hand with Doesn't build in projects that use dep Go dependency management tool due to issue with blackfriday v2.0.0 tag. shurcooL/github_flavored_markdown#12.
  3. When the somebody from scenario 1 becomes aware of Blackfriday's v2, they can edit Gopkg.toml and upgrade to v2, leaving the import path intact.
  4. Somebody who already uses v2 via gopkg.in/russross/blackfriday.v2, but doesn't use dep yet, dep init will resolve to v2.0.0; the code will remain working.

@dmitshur
Copy link
Collaborator

dmitshur commented Aug 13, 2017

Well, apparently, dep can perfectly do that today, at least for first-order importers.

I know it can, but the problem is that we do not support/endorse it. Let's discuss only the supported/valid uses for now. So, our current plan is:

  • If you want v1, you must use github.com/russross/blackfriday import path (and, orthogonally to that, you could vendor or not vendor).
  • If you want v2, you must use gopkg.in/russross/blackfriday.v2 import path (and, orthogonally to that, you could vendor or not vendor).

The only problem is that it defaults to v2 and thus breaks existing code: cd to a project that's been merrily using blackfriday v1 at same old github.com/russross/blackfriday, type go build and it works; then type dep init, go build and it stops working.

Breaking existing code just by typing dep init is really bad. It upsets users, goes against our reasoning for maintaining v1 import path at github.com/russross/blackfriday, and gives bad press for dep. Everyone loses :-(

I don't think it's a problem. Let me explain why. In the "radical proposal" of using github.com/russross/blackfriday import path for both v1 and v2, this would be the situation:

  • If you want v1, you must use github.com/russross/blackfriday import path and you must vendor v1 via dependency management tool like dep or another vendoring tool.
  • If you want v2, you must use github.com/russross/blackfriday import path (and, orthogonally to that, you could vendor or not vendor).

So, with that approach, if someone wants to use blackfriday v1, they must vendor and we would provide semver tags for v1.x.x releases.

If someone wants to use v2, they don't have to vendor, or they could, it doesn't matter.

Does that make sense? Again, I realize it's pretty radical, I just wanted to describe it so we can fully consider it and the tradeoffs/implications.

But yes, the most obvious implication is that it means as soon as we release v2, all v1 users must have vendored blackfriday v1 or face build failures due to breaking API changes. This would be their Gopkg.toml file effectively:

[[constraint]]
  name = "github.com/russross/blackfriday"
  version = "1.x.x"

@sdboyer
Copy link

sdboyer commented Aug 14, 2017

hi folks - awesome to see you guys working through these things! i'm sure i'll end up referring people to this thread in the future as an example of people chewing through possible approaches.

then type dep init, go build and it stops working.

a minor nit on this, but this should probably happen a rather small portion of the time - and primarily if the user is not using an existing tool for managing their dependencies; we do pretty well getting things right in the automated conversions. (that also may not be a panacea, though, because...reasons)

still, though, not an outcome we should accept, for all the reasons you outline!

If I'm reading things correctly, golang/dep#902 will give us best of all worlds:

i do tend to think so, although it would require moving the v2.0.0 tag, which is obviously 😢. if you were to simply issue a v2.0.1, then, once that issue gets fixed in dep, dep would only skip the new release but would still find v2.0.0 acceptable.

@rtfb
Copy link
Collaborator Author

rtfb commented Aug 15, 2017

If I'm reading things correctly, golang/dep#902 will give us best of all worlds:
[...]
3. When the somebody from scenario 1 becomes aware of Blackfriday's v2, they can edit Gopkg.toml and upgrade to v2, leaving the import path intact.

OK, I'm no longer sure this is how things would work. While working on dep#903 I realized that once it's done, gopkg.in/russross/blackfriday.v2 would become the only accepted import path for v2. That's in line with what we came up with when we planned v2, but a bit less rosy than what I got excited over couple days back.

@shurcooL, are you saying that in your radical proposal we would also merge v2 to master?

@sdboyer
Copy link

sdboyer commented Aug 15, 2017

  1. When the somebody from scenario 1 becomes aware of Blackfriday's v2, they can edit Gopkg.toml and upgrade to v2, leaving the import path intact.

OK, I'm no longer sure this is how things would work.

ack, i missed that bullet. yes, that wouldn't work - upgrading to v2 would entail changing import paths in all of their source code.

@dmitshur
Copy link
Collaborator

dmitshur commented Aug 15, 2017

@shurcooL, are you saying that in your radical proposal we would also merge v2 to master?

In that radical proposal, yes, I implied that v2 would become master.

However, I just realized that it doesn't have to! We could flip that around, which would make the proposal a lot less radical. Here's the less radical alternative:

  • If you want v1, you must use github.com/russross/blackfriday import path (and, orthogonally to that, you could vendor or not vendor... but you should vendor in case enough time passes and we decide to merge v2 or v3 or v4 into master).
  • If you want v2, you must use github.com/russross/blackfriday import path and you must vendor v2 via dependency management tool like dep or another vendoring tool.

Basically, in this alternative, blackfriday v2 would be available at the same import path as v1, but it would only be accessible via vendoring (with a tool like dep or another). At least until we decide to merge v2 (or later) into master.

@rtfb
Copy link
Collaborator Author

rtfb commented Aug 16, 2017

Basically, in this alternative, blackfriday v2 would be available at the same import path as v1, but it would only be accessible via vendoring (with a tool like dep or another). At least until we decide to merge v2 (or later) into master.

Even better, in addition to what you listed above, v2 would also be available via (non-enforced) gopkg.in/russross/blackfriday.v2 for those who do not (yet) want to vendor or just want to tinker with it. The more we talk about it, the more I like this (no longer really that) radical proposal.

If only we could find a solution to the transitive dependency problem of shurcooL/github_flavored_markdown#12... And I suppose we ought to, because it's super unobvious what's hit you when it does. And the victims would be far from us, we can't even help them effectively by documenting things.

@zevdg
Copy link

zevdg commented Aug 24, 2017

Sorta tangentially related. It looks like the latest release of v1 (1.4) is pretty far behind the latest master. Specifically, it's 51 commits and almost 2 years behind. Anyone using v1 and switching over to dep right about now (e.g. cpuguy83/go-md2man#28 ) will have to either downgrade to this relatively old release or just abandon semver and track the master branch itself. Neither of those options are particularly appealing.
A new v1 release would be very helpful for these clients.

@rtfb
Copy link
Collaborator Author

rtfb commented Aug 26, 2017

You're right, v1 got quite neglected while we were focusing on v2. And my initial reaction was a bit of a squirm, mumbling in my mind along the lines of "yeah, I know, but there wasn't much happening on master, is it worth having a new release at all...". Then I set out to collect the release notes, and, well, check it out!

Thanks for a timely kick in the butt, @zevdg! ;-)

@rtfb
Copy link
Collaborator Author

rtfb commented Aug 26, 2017

Furthermore, this might be more tightly related to the discussion above -- what if we also have a v1 branch that can be imported via gopkg.in? It seems like it should solve the transitional dependency problem that was brought up in golang/dep#999. All one would have to do in order to use a v1 first-level Blackfriday dependency that has not adopted vendoring yet, is to specify that it constrains Blackfriday to v1:

[[constraint]]
  branch = "v1"
  name = "github.com/russross/blackfriday"

(or possibly with name = "gopkg.in/russross/blackfriday.v1").

I'm not entirely certain this would work, but it should be quite easy to set up a Guinea pig repo for testing that, which I'll do.

@rtfb
Copy link
Collaborator Author

rtfb commented Aug 26, 2017

Nah, it's not helping... Nothing but an [[override]] clause works for a transitive dependency.

@zevdg
Copy link

zevdg commented Aug 26, 2017

I applaud your effort to minimize disruption to downstream packages but I don't see any option that is totally painless for them. Making people switch to a gopkg.in v1 import path is actually one of the most disruptive options. Once all the libraries that directly import blackfriday v1 to switch to dep and add the appropriate [[constraint]] this problem will go away, so I think the best we can do is to simply encourage anyone who is affected by this to contibute a PR to whatever package is actually pulling blackfriday into their project.

In the meantime, this seems like one of the very rare cases where the [[override]] sledgehammer is actually appropriate. The only downside of encouraging this is that if/when the intermediate package does update to blackfriday v2 using dep (not gopkg.in), the override will need to be removed.

@rtfb
Copy link
Collaborator Author

rtfb commented Aug 26, 2017

Making people switch to a gopkg.in v1 import path is actually one of the most disruptive options.

Oh yeah, I didn't mean that as an approach to be forced on people. I was hoping that would be an optional extra to help the transitive users of Blackfriday to trick dep into picking the right version without cooperation of 1st order users (an abandoned or very vaguely maintained library would be a prime example), but it wasn't.

So yeah, so far I'm leaning towards the "less radical" @shurcooL's suggestion, plus a well documented [[override]] for those who will need it.

@dmitshur
Copy link
Collaborator

dmitshur commented Aug 29, 2017

Another observation. If we go with having same import path for both v1 and v2, then it becomes impossible to be able to view documentation for both via https://godoc.org, at least given its current implementation (it only shows documentation for the latest version any given import path).

But that might be on godoc.org. Still, it's worth taking into account.

For reference, I'm considering adding support for seeing godoc on gotools.org (see details here), which supports viewing any import path at any revision, so that could be helpful.

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