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

Don't modify tags! #904

Closed
Kubuxu opened this issue Jun 27, 2019 · 15 comments

Comments

Projects
None yet
5 participants
@Kubuxu
Copy link

commented Jun 27, 2019

Hi,
the badger is great KV-store, it really is, but I have to say after two build breaking event I'm getting tired by it.

The first one was the removal of the original 2.0.0-rc.2. Then you've pushed master to it (that was incompatible).
Now, again, you modified v2.0.0-rc2 to point to #896, which again breaks our builds (and builds for a lot of people who depend on us).

Please, stop modifying tags. All of this could have been avoided by just pushing v2.0.0-rc3 and -rc4.

What is even worst, nobody should be using v2.0.0-x tags from this repo.
By messing with them, you've polluted the namespace at proxy.golang.com which will be default in next release of Go (1.13). I challenge you to show me a go.mod file that uses tags for depending on badger and builds with both:

go build .

and

GOPROXY='https://proxy.golang.org` go build .

Characteristic builds errors:

 missing github.com/dgraph-io/badger/go.mod and .../v2/go.mod at revision v2.0.0-rc2
go: github.com/dgraph-io/badger@v2.0.0-rc.2+incompatible: go.mod has post-v2 module path "github.com/dgraph-io/badger/v2" at revision v2.0.0-rc.2
go: error loading module requirements
@campoy

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2019

Hi there,

First of all apologies for the inconvenience. We have not published any version of Badger for a while and getting the repo back in shape for v2 is proving harder than we expected.

I personally deleted this tag because there had never been any v2.0.0-rc.1 at all, so the tag itself didn't really make sense from a semantic versioning point of view. We quickly realized with #886 that some (ipfs-go) depended on this mysterious tag so we put it back on but clearly on the wrong commit.

I'm curious about how you ended up depending on this tag? Was it an automated choice made by your dependency management tool? Was there any other reasoning behind?
Also, could you point me to what commit hash that tag pointed to?

Again, I am very sorry for the inconvenience. We promise this won't happen again.

@Kubuxu

This comment has been minimized.

Copy link
Author

commented Jun 27, 2019

Hi, It was an automatic choice made by Go when doing: go get github.com/dgraph-io/badger@latest.

I think it was pointing to https://github.com/Kubuxu/badger/releases/tag/v2.0-rc1

@campoy

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2019

Oh, that's bad indeed because that's actually really far from being the latest tag.

I will be tagging a v2.0.0-rc3 later today which points to the right commit and will be seen as latest, but in the meantime, I'd like to provide a fix.

I'm wondering whether it's better to modify the tag once more and just set it back to what it was pointing to, or if that could cause even more issues for those depending on that tag.

You could also use v1.5.5 which is the actual version that contains around the same commits as v2.0-rc1.

@campoy

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2019

OK, I've decided to leave this tag as is.

If you need to go back to what the code looked like before any backwards incompatible change was made I recommend using v1.5.5 which is the actual latest release we've done.

v1.6.0 will be coming up soon, followed by v2.0.0 (and their corresponding rcX).

We will directly jump to v2.0.0-rc3 to avoid confusion with the existing rc2 and rc.2 (yes, they both exist).

Again, sorry for the inconvenience, it won't happen again.

@campoy campoy closed this Jun 27, 2019

@brandonwestcott

This comment has been minimized.

Copy link

commented Jun 27, 2019

Sorry to jump in late here after this is already closed, but our team is also running into this. As far as I can tell, leaving the tag pinned as is really doesn't help anyone. Those of use who are using this renegade tag (largely the ipfs community) have chains of projects depending on that tag at the original commit. Our team did start the process of changing to v1.5.5, but there are a couple of interface changes that make this more than find and replace in go.mod.

That being said, I understand that pointing to the old tag breaks semver, but that was the state of the repo earlier this week. As soon as v2.0.0-rc3 is released, I doubt anyone coming in will care about the rc2 tags. If thats a concern, maybe consider bumping the semver and skipping v2.0.0 all together.

Now, I'm not sure what effects changing the tag back will have on proxy.golang.org, but it doesn't seem like it can get any worse than the broken state it's in right now.

And I second what @Kubuxu said, badger itself is truly awesome 👍

@campoy

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2019

We will not skip v2.0.0, since it would just make it more confusing for everyone in the future.

If the consensus is to point v2.0.0-rc.2 back to that old comit, I'm happy doing so but that might cause extra breakages for others.

Please @Kubuxu and @brandonwestcott, could you assess whether that would help you?

@Kubuxu

This comment has been minimized.

Copy link
Author

commented Jun 27, 2019

Let's not mangle the tags anymore, what is done is done. I think it will be just worst if we keep poking at it.

GOPROXY is a workaround we can use for the moment.
I would suggest releasing -rc3 just so there is a fresh tag with all the code in a stable state between github and proxy.golang.org.

@campoy

This comment has been minimized.

Copy link
Contributor

commented Jun 27, 2019

I just released a new version v1.6.0-rc1 which corresponds to the old v2.0.0-rc2.

Also sent PR ipfs/go-ds-badger#65 to ipfs to fix their build issues.

@postables

This comment has been minimized.

Copy link

commented Jun 28, 2019

I dont think this actually fixed anything, I'm getting an error that I haven't seen before:

go: github.com/dgraph-io/badger/v2@v2.0.0-rc2: missing github.com/dgraph-io/badger/go.mod and .../v2/go.mod at revision v2.0.0-rc2

whoever was responsible for this decision, really needs to have a stern talking too. The amount of man hours being collectively burnt by everyone effected by this, is probably worth tens of thousands of dollars. This is a royal screwup

edit:

I managed to fix this with some janky replace directives

replace github.com/dgraph-io/badger v2.0.0-rc.2+incompatible => github.com/dgraph-io/badger/v2 v2.0.0-rc.2

replace github.com/dgraph-io/badger/v2 v2.0.0-rc2 => github.com/dgraph-io/badger v1.6.0-rc1
@Kubuxu

This comment has been minimized.

Copy link
Author

commented Jun 28, 2019

@postables try cleaning the modcache: go clean -modcache.
You can also try running all of your builds with: export GOPROXY='https://proxy.golang.org'
And if you are using it, updating go-ds-badger to v0.0.5.

@postables

This comment has been minimized.

Copy link

commented Jun 28, 2019

@Kubuxu Interesting, I'll try poking around with those settings but still no dice. Loads of errors all across the board.

@campoy

This comment has been minimized.

Copy link
Contributor

commented Jun 28, 2019

Hi @postables,

I am the person responsible for the changes done and I apologized profusely.
Please, stop depending on v2.0.0-rc2 as this tag was not intended to be used by most people.

Unfortunately, Go Modules surprisingly provided a tag that we had never released officially (and of which I wasn't even aware) as the latest branch, driving loads of our users into a situation where semantic versioning was violated.

In order to fix your code, depending on v1.6.0-rc1 should be the easiest way. Next week we will release v1.6.0 if all of our tests on the release candidate are successful.

Again, sorry for the inconvenience - I sincerely am doing my best to avoid impacting our community of users while also taking the repository back to a more maintainable situation.

PS: please reach directly to me if there's any way in which I can help you debug the issues you're facing.

@thepudds

This comment has been minimized.

Copy link

commented Jun 29, 2019

Please, stop depending on v2.0.0-rc2 as this tag was not intended to be used by most people.

I haven't fully followed the history here, so big, big apologies in advance if I create any confusion.

Some quick observations:

  1. The current go.mod on master of ipfs/ipfs-cluster is still depending on the v2.0.0-rc2 of badger, and ipfs/ipfs-cluster is still using the /v2 in the path for badger (in particular, as you can see in the ipfs/ipfs-cluster go.mod, it is using github.com/dgraph-io/badger/v2 v2.0.0-rc2). That is probably a problem.

  2. As a quick example of how it might be a problem, if you try to go get master of ipfs/ipfs-cluster (without setting a GOPROXY), as of right now it generates what looks to be the same error message that was reported about 16 hours ago a few comments up on this issue:

$ cd $(mktemp -d)
$ go mod init tempmod
$ GO111MODULE=on go get github.com/ipfs/ipfs-cluster@master   # currently: 4c16675a39b38
...
go: github.com/dgraph-io/badger/v2@v2.0.0-rc2: missing github.com/dgraph-io/badger/go.mod
 and .../v2/go.mod at revision v2.0.0-rc2
  1. That error message is saying that the go command is rejecting the use of the /v2 in the path for badger because the v2.0.0-rc2 version of badger on github right now does not have a go.mod and is not a module. (Based spot checking the v2.0.0-rc2 version of badger on github, as far as I understand things, it seems that error message is correct to complain, even if the error message might not be 100% obvious).

  2. Until roughly 6 days ago, it seems ipfs/ipfs-cluster depended on a different badger tag v2.0.0-rc.2 which at least at one point very likely pointed to a badger commit that was not a module. (The +incompatible bit in v2.0.0-rc.2+incompatible is how the go command records a version when depending on a package that has v2+ semver tags but has not opted in to modules).

  3. I'm not sure what the solution is regarding ipfs/ipfs-cluster, but based on the prior conversation here, it seems there was some consensus to try to fix things on the ipfs side and it looks like ipfs/go-ds-badger#65 was sent, so maybe that would make sense to try to adjust ipfs/ipfs-cluster too? Not sure.

Finally, sorry if any of this is off base, and I am definitely not sure of all of the history here, but the TLDR is it seems there might still be a problem that happens if you get master for ipfs/ipfs-cluster.

@Kubuxu

This comment has been minimized.

Copy link
Author

commented Jun 29, 2019

Yes, ipfs-cluster has to update to go-ds-badger v0.0.5.

@campoy

This comment has been minimized.

Copy link
Contributor

commented Jul 1, 2019

Hey @Kubuxu, let me know if there's any way we can help you with the migration.

Also, thanks @thepudds for the compilation of facts, I think you got all the details right and we'll be out of this situation soon!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.