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

relx inappropriately strips version details from {vsn, Version} data #88

Closed
AeroNotix opened this issue Oct 28, 2013 · 17 comments · Fixed by #89
Closed

relx inappropriately strips version details from {vsn, Version} data #88

AeroNotix opened this issue Oct 28, 2013 · 17 comments · Fixed by #89

Comments

@AeroNotix
Copy link

We ran across a bug where a dependency had tagged a commit causing the version to be returned as {vsn, v0.7.0} (piqi for those interested).

Relx depends on erlware commons ec_semver to parse out the version of the release which also strips out the v.

As below:

2> ec_semver:parse(<<"v0.7.0">>).
{{0,7,0},{[],[]}}

Resist the temptation to take pride in complexity.

@ericbmerritt
Copy link
Member

@AeroNotix we do and for good reason. During the dependency solve phase of the release build we have to compare versions. We have to have some basis on which to compare versions. That basis is the semantic versioning spec. If we can parse a version can be parsed as a semantic version we do that. Since a preceding v is part of that spec we parse it out. That said, that shouldn't affect the users as once the dependency graph is solve that parsed version shouldn't be used any more.

That said, you haven't actually described what the problem is. Just what you think caused the problem. So if you could describe how it messed you up, that would be helpful.

@AeroNotix
Copy link
Author

The problem was that the version missing the v was being used in the .app files in the release. When I removed the v portion from the version - it was fine.

I understand the need to compare versions but are you 100% that you then use the original value which was in the .src file to create the later files?

@ericbmerritt
Copy link
Member

Relx never touches the contents of the OTP Application. It does copy/symlink (depending on options) it into the release, but it doesn't rewrite the .app file or anything like that. The one potential place for there to be problem is the directory where it copies the release. It targets a directory in <release-dir>/lib/<appname>-<appvsn> where app vsn is the one that it parsed. It probably shouldn't be doing that, and I will put a fix in this afternoon to make sure it uses the original version for that directory name.

If your *.app file is getting modified, something besides relx is doing it.

@tsloughter
Copy link
Member

Or are you saying what ends up in the .rel for an app is the wrong version?

@ericbmerritt
Copy link
Member

I suspect it does actually end up in the *.rel file for similar reasons.

@AeroNotix
Copy link
Author

Then I may have been confused about the final outcome of misusing the version number but it certainly is stripping it out. Surely this is trivial for you to test?

And yes, sorry, I was talking about the .rel.

@tsloughter
Copy link
Member

@AeroNotix do you have an example to show us?

@AeroNotix
Copy link
Author

Make a release with piqi in it. That should expose this.

@tsloughter
Copy link
Member

Would be quicker if you said what happened when you make a release with piqi, hehe. Is the .rel file wrong? Is the _rel/libs/piqi-<vsn> name wrong? Does it just not find the piqi dep at all and fails?

@AeroNotix
Copy link
Author

It's the _rel/libs/piqi-<vsn> which is wrong.

Sorry it was on another machine.

@tsloughter
Copy link
Member

Ah ok, we are on it.

@ericbmerritt
Copy link
Member

We will get this fixed. However, you should really push back on the authors
of piqi. Its a really horrendously bad idea to have that 'v' as part of the
actual version string (not just in the tag). While relx is pretty smart and
will continue to parse that as an actual version other tools may not be.
That version used in the the .app and the directory name is designed to be
machine readable/parsable. Having letters inserted at the beginning which
can't be interpreted as a version is almost as crazy as just using a
straight git ref as a version number.

On Mon, Oct 28, 2013 at 2:18 PM, Tristan Sloughter <notifications@github.com

wrote:

Ah ok, we are on it.


Reply to this email directly or view it on GitHubhttps://github.com//issues/88#issuecomment-27257821
.

@AeroNotix
Copy link
Author

@ericbmerritt alavrik/piqi-erlang#12 already done before I made this ticket.

As for the git reference number - surely if you have access to the .git repository you can ascertain an ordering for the shas?

@ericbmerritt
Copy link
Member

@AeroNotix you can absolutely. However, thats not super useful. Lets take the human case. When i look at version 1.1.0 and 1.2.0. I have a good idea about the relationship of those two versions. The same is true of 2.0.0 and 25.0.0. I don't have detailed information, but I can derive useful information at a glance because version numbers have a set of intrinsic characteristics related to ordering. Now if I look at bc231ff656 and d0c4edee9f. What can you tell me about the relationship between those two? Absolutely nothing. I happen to know that that bc231ff656 is a newer ref then d0c4edee09f but without looking at the repositories in question you are at a complete loss. Basically, using refs massively increases the cognitive load on the reader of the version numbers and renders version numbers pretty much useless for nearly all the purposes for which they where designed. Its worse in the tool case, I can (and actually have written) a tool to parse version and use the intrinsic order within version numbers for useful purposes. There is not a single thing I can do algorithmically with version numbers that does not require access to the repository in which they are stored. Access that the tool probably doesn't have. In the vast majority of cases the tools don't even know where the repository lives. So aside from laziness on the part of maintainer what possible argument could there be in favor of refs in place of actual version numbers?

@AeroNotix
Copy link
Author

@ericbmerritt

Not trying to cause any arguments here, I think my goal has been achieved with this ticket but:

That version used in the the .app and the directory name is designed to be
machine readable/parsable.

and your previous comment seem to be at odds with each other, no?

Regardless, I agree - judging by the way things are set up it doesn't seem wise to include a v in front of names. Though it is instinctual when writing version numbers - perhaps the author wasn't aware of the myriad of issue it could cause?

@ericbmerritt
Copy link
Member

@AeroNotix not trying to get in an argument either. I just feel really strongly on the subject.
On your second point. I think you are right that the original author tags his repo with v. Which is perfectly acceptable and actually nice to read. He just isn't using the right tools/options to ensure that only the version ends up in the .app file in his built project.

I think my two posts there back each other up. That is that those are designed to be machine readable, and that semver style versions are the right way to go for that purpose. While refs may be machine readable, they are not very machine usable without access to the original repo which isn't available most of the time. So perhaps I should have called it 'machine usable' instead of 'machine readable'.

@AeroNotix
Copy link
Author

I see, jolly good.

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.

3 participants