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.

Sign up for GitHub

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

Jump to bottom

add vsn that is the evaluated application vsn #2303

Merged
merged 3 commits into from
Jun 8, 2020
Merged

add vsn that is the evaluated application vsn #2303

merged 3 commits into from
Jun 8, 2020

Conversation

tsloughter
Copy link
Collaborator

I forgot to push this and don't remember if it was complete or not.

The issue was pointed out by @max-au with the latest relx. It would get the vsn like, {cmd, "..."} or git instead of what that evaluates to. The change is to have a separate key for the evaluated version and the original version.

@tsloughter tsloughter requested a review from ferd June 6, 2020 19:30
@max-au
Copy link
Contributor

max-au commented Jun 7, 2020

I tried this patch and it does not seem to fix the issue. Fails slightly differently though, now with meck:

[{filename,join1,
[undefined,[],
"-kcem/bil/1ppa/aw/tset/dliub_/revres/enad/emoh",
unix],
[{file,"filename.erl"},{line,472}]},
{filename,join,1,[{file,"filename.erl"},{line,433}]},
{rlx_assemble,copy_app,4,
[{file,"/home/dane/git/rebar3/_build/default/lib/relx/src/rlx_assemble.erl"},
{line,73}]},
{rlx_assemble,'-copy_app_directories_to_output/3-lc$^0/1-1-',4,
[{file,"/home/dane/git/rebar3/_build/default/lib/relx/src/rlx_assemble.erl"},
{line,58}]},

@tsloughter
Copy link
Collaborator Author

Thanks, yes, I reproduce that same issue. Looking into it.

tsloughter added 2 commits June 8, 2020 09:44
@tsloughter
set app_info:vsn from the app details during application discovery
80671df 
validate_app in rebar_otp_app does not need to set the vsn and
original_vsn because they are set from the app details during
discovery.

The original_vsn was set here before becuase when there is no
.app.src or .app.src.script the app file does not go through
preprocess/3 and preprocess/3 is where vsn is set based on evaluating
the vsn from .app.src. But if neither of these files exist it
means the discovery vsn was the correct version and there is
no need to update.
@tsloughter
update state's all_deps after they are compiled
5dd3a8a
@tsloughter
Copy link
Collaborator Author

Alright, this one should work. And I have a yaws test in rebar3_tests that I'll push shortly.

The main change in the latest commits was keeping the returned AppInfos from compilation.

There is also a place (rebar_otp_app:validate_app/2) where I removed setting of original_vsn and vsn because I don't think it is necessary. My reasoning is in the commit message, 80671df

@tsloughter tsloughter merged commit 67b0c7e into erlang:master Jun 8, 2020
@max-au
Copy link
Contributor

max-au commented Jun 8, 2020

confirming it did fix the issue. Thank you!

@yulgolem yulgolem mentioned this pull request Sep 2, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ferd ferd ferd approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tsloughter @max-au @ferd