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

Ensure stdlib,kernel as application dependencies #567

Conversation

lrascao
Copy link
Collaborator

@lrascao lrascao commented Feb 3, 2017

Make this dependency explicit as it was causing
apps with empty application lists to not be included
in the generated release.

Fixes #434, #566

@tsloughter
Copy link
Member

I don't think we can do this. There are cases people do not want kernel and stdlib.

@bitwalker
Copy link
Contributor

I was under the impression both are required for releases?

@tsloughter
Copy link
Member

It would be a nice default to automatically include them, but I don't know that it is worth breaking existing usage to do so.

Though I am not 100% positive this is the case and that it is still used by those who needed this.

@fhunleth you were one of the people who didn't want kernel/stdlib always for embedded systems, right? Is that true and are you still using relx? I saw you were using Elixir now so maybe using distillery anyway?

@bitwalker
Copy link
Contributor

I believe Nerves is currently using Distillery, but I'll defer to @fhunleth on that. If so, this default is already being used.

@tsloughter
Copy link
Member

Hmm, interesting. The other embedded person I can remember is is @peerst so maybe it was him that needed this?

@fhunleth
Copy link

@tsloughter We did recently switch to Distillery. There are projects that are locked to older versions of Nerves that probably won't upgrade to the Nerves version that uses Distillery. In the unlikely event that they upgrade relx, I think this change is still fine for them. I only know of one Nerves project that doesn't use stdlib and kernel, and it was a proof of concept.

@peerst
Copy link

peerst commented Mar 14, 2017

@tsloughter: it was definitely not me. We target smaller systems than Nerves but still want stdlib and kernel. If we want to run on very small SoC (relatively speaking) we might want to throw out some modules from stdlib and kernel but we would still include them, just our own version of them.

For that we would then probably use reltool under rebar3 (have a hacked version of a plugin for that) since this would give us the possibility of dropping single modules from a release. Unless relx would get this feature too though ...

@lrascao
Copy link
Collaborator Author

lrascao commented Mar 14, 2017

@peerst it does! #541

@tsloughter
Copy link
Member

Thanks @peerst it sounds like it was Frank but is also unneeded.

Note that relx does have exclude_modules option.

@tsloughter
Copy link
Member

So sounds like this is ok, I'll merge this then.

@tsloughter
Copy link
Member

Though curious, @lrascao, should we also make sure that not only if it is [] but if it is any list of apps that kernel and stdlib are there?

@peerst
Copy link

peerst commented Mar 14, 2017

For me its definitely ok (I was the OP for #434) and its also better compatibility with reltool which does this implicit kernel/stdlib thing

@peerst
Copy link

peerst commented Mar 14, 2017

@lrascao: cool that relx can exclude_modules! Very helpful to squeeze into small systems

@lrascao
Copy link
Collaborator Author

lrascao commented Mar 14, 2017

@tsloughter that does sound better, i'll make the change

Make this dependency explicit as it was causing
apps with empty application lists to not be included
in the generated release.
@lrascao lrascao force-pushed the feature/empty_application_apps_silently_skipped branch from 283d45e to 3a5137a Compare March 20, 2017 23:08
@tsloughter tsloughter merged commit c1e3796 into erlware:master Mar 20, 2017
@lrascao lrascao deleted the feature/empty_application_apps_silently_skipped branch March 20, 2017 23:55
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 this pull request may close these issues.

5 participants