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

Versioning is confusing when building master #3791

Closed
so7ow opened this issue May 27, 2024 · 16 comments · Fixed by #3812
Closed

Versioning is confusing when building master #3791

so7ow opened this issue May 27, 2024 · 16 comments · Fixed by #3812

Comments

@so7ow
Copy link

so7ow commented May 27, 2024

It seems to reflect the most recently released version of ord, which is very confusing to users, especially when master has a schema update and isn't compatible with the most recent release.

Example: at some point after 0.18.5 was released there was a schema update in the code. Now, if you build from master your binary will say it is version 0.18.5, but it won't read an index file created by the 0.18.5 release. ➡️ confusion

Any good way to change the version after a release, to reflect the unreleased nature of building from master?

@cryptoni9n
Copy link
Collaborator

Thanks for opening this issue @so7ow - I ran into it, also. I think that the address indexing may have had a schema update in it.

@raphjaph will know what to do.

@so7ow
Copy link
Author

so7ow commented May 27, 2024

The schema update is the most obvious manifestation of this issue, but even when there is no schema update it's still confusing for folks to build master and get an executable that lies about its version number.

@raphjaph
Copy link
Collaborator

I think that is just the nature of building from master. It should give you an error that the index.redb file needs to be upgraded. Something like this:

error: index at `/Users/raphael/Library/Application Support/ord/index.redb` appears to have been built with an older, incompatible version of ord, consider deleting and rebuilding the index: index schema 25, ord schema 26

@so7ow
Copy link
Author

so7ow commented May 29, 2024

I think that is just the nature of building from master. It should give you an error that the index.redb file needs to be upgraded. Something like this:

error: index at `/Users/raphael/Library/Application Support/ord/index.redb` appears to have been built with an older, incompatible version of ord, consider deleting and rebuilding the index: index schema 25, ord schema 26

Sure, I'm aware of that error, but that doesn't address the underlying issue I'm reporting: the fact that building from master produces an executable that reports a released version number, but isn't really that released version.

I'm not a dev, nor am I particularly proficient with GitHub, but as I understand it the bitcoin project has a method of dealing with this that seems fairly straightforward, if I'm understanding it correctly: they bump the version number to x.99 after publishing a release, so that if you subsequently build from master you get an executable of that x.99 version, rather than a mostly untested executable that reports the released version, but really isn't that version. It makes it crystal clear to anyone wondering, that the version running is not a released version.

Hopefully that wasn't too rambling. Cheers!

@raphjaph
Copy link
Collaborator

Thanks, that makes sense. I'll see what we can do about that

@raphjaph
Copy link
Collaborator

How about this?
#3793

@so7ow
Copy link
Author

so7ow commented May 30, 2024

How about this?

#3793

That method seems to take care of the general case, but it still might lead to similar confusion after a schema update on master: "if 0.18.5 can read this index file, why can't 0.18.5-abcxyz?" The convention has always been that indexes are compatible until the next major release. Not sure if there's a great way around that.

@raphjaph
Copy link
Collaborator

I think if you are building from master it should be expected that you understand this error message:

error: index at `/Users/raphael/Library/Application Support/ord/index.redb` appears to have been built with an older, incompatible version of ord, consider deleting and rebuilding the index: index schema 25, ord schema 26

Combined with the indication that this version is built from a commit hash, this should probably be sufficient to alleviate most confusion. There's probably a more elegant way but I don't see it right now. Open for suggestions.

@so7ow
Copy link
Author

so7ow commented May 31, 2024

I think if you are building from master it should be expected that you understand this error message:


error: index at `/Users/raphael/Library/Application Support/ord/index.redb` appears to have been built with an older, incompatible version of ord, consider deleting and rebuilding the index: index schema 25, ord schema 26

One might think that but spend some time in the Ordicord and you'll see that it's definitely not always the case. 🤣

Anyway, your proposal is an improvement. If anyone has any thoughts (or if I come up with anything myself) hopefully we can put it in this thread.

@cryptoni9n
Copy link
Collaborator

cryptoni9n commented May 31, 2024

@raphjaph this is probably a naive question, but is there a reason why the change that bumped the schema wasn't segregated into a new 0.18.x version? Wouldn't that have allowed those using 0.18.5 to continue on using it without running into this error, and then anyone who wanted the new change could just build from 0.18.6 or master and be informed that a new index would be necessary? Granted, even this might be confusing for some users as it has been expected that one index for 0.18.x would be good across any 0.18.x version - but it could be explained as an exception to that rule.

@so7ow
Copy link
Author

so7ow commented May 31, 2024

Well, remember, if we're following convention the new schema would trigger a version bump to 0.19.0 in this case, not 0.18.6.

But yeah, maybe it's as simple as bumping the major version as soon as the schema change is made, rather than waiting for a release?

@cryptoni9n
Copy link
Collaborator

Well, remember, if we're following convention the new schema would trigger a version bump to 0.19.0 in this case, not 0.18.6.

But yeah, maybe it's as simple as bumping the major version as soon as the schema change is made, rather than waiting for a release?

right - that's why I mentioned that even going to 0.18.6 would cause confusion since it's still a departure from precedent, but maybe it's a consolation if for some reason they're not ready to go to 0.19.0 just yet? In any case, the question boils down to - why didn't the version get bumped for this schema change?

@cryptoni9n
Copy link
Collaborator

@casey @raphjaph @so7ow hey guys, sorry to bump this closed issue, but are we experiencing the same issue again? Did the change for #3893 stop prior 0.19.1 indexes from working with new builds? Should we now be on 0.19.1-dev?

@so7ow
Copy link
Author

so7ow commented Aug 28, 2024

(And again, I'd like to say: the index incompatibility is only the most obvious issue. Under no circumstance (imo) should building from master create a binary that will report a version number that is no longer accurate.)

@casey
Copy link
Collaborator

casey commented Aug 28, 2024

(And again, I'd like to say: the index incompatibility is only the most obvious issue. Under no circumstance (imo) should building from master create a binary that will report a version number that is no longer accurate.)

Sorry, but this is just false. It is common practice to not to increment the version number until a new release is made. The master branch is implicitly unstable. If you want a stable, released version, you should use a tag.

Opened #3916 to add -dev in Cargo.toml, to hopefully make this less confusing.

@so7ow
Copy link
Author

so7ow commented Aug 28, 2024

Sorry, but this is just false

Haha I don't know how an opinion can be false, but ok. Obviously master is unstable and I'll pick a tag to get a stable version. I just think it's a bad call to leave the version number such that an unstable build reports a stable version number. On the Ordicord, we've frequently seen the problems it causes.

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

Successfully merging a pull request may close this issue.

4 participants