Improve precompiled DX #121
Conversation
mat-hek
force-pushed
the
precompiled-dx
branch
from
fa67897
to
e206574
Compare
mat-hek
force-pushed
the
precompiled-dx
branch
2 times, most recently
from
56c8e1d
to
18eda03
Compare
| require Logger | ||
|
|
||
| Logger.error(""" |
There was a problem hiding this comment.
| require Logger | |
| Logger.error(""" | |
| Bundlex.Output.warn(""" |
Are you sure Logger will work here? Loading a NIF may happen in the compile time
There was a problem hiding this comment.
I'm quite sure. I changed it from raise because previously we'd get both error and warning, both via Logger. I think Bundlex.Output.warn would work too, but I'm not sure about other Bundlex.Output functions, so I'd not use it in runtime
There was a problem hiding this comment.
Ok, so I checked and in the compilation the bundlex compilation is called first and there we do Application.ensure_all_started https://github.com/membraneframework/bundlex/blob/master/lib/mix/tasks/compile.bundlex.ex#L22 and because we have :logger in extra_applications in mix.exs, the logger should be started at that point. So, it seems that the logger is started before any C code is even compiled.
There was a problem hiding this comment.
If so, is there any reason to call IO.warn/1 in Bundlex.Output.warn/1 or Mix.shell().info/1 in Bundlex.Output.info/1?
There was a problem hiding this comment.
I don't know why we do this, probably we could use Logger as well ¯\_(ツ)_/¯
| Application.get_env(:bundlex, :disable_precompiled_os_deps, []) | ||
| |> Keyword.get(:apps, []) | ||
| |> Bunch.listify() | ||
| is_precompiled_disabled = |
There was a problem hiding this comment.
| is_precompiled_disabled = | |
| precompiled_disabled? = |
There was a problem hiding this comment.
The convention is to use the quotation mark in function names only AFAIK
|
|
||
| if native.app in disabled_apps do | ||
| if is_precompiled_disabled do | ||
| {:error, | ||
| """ | ||
| is disabled in the application configuration, check the config.exs file. |
There was a problem hiding this comment.
BTW this error message looks strange. What is disabled in the application configuration?
There was a problem hiding this comment.
It's concatenated to a prefix in line 90 :P
| ```elixir | ||
| config :bundlex, :disable_precompiled_os_deps, true | ||
| ``` | ||
|
|
||
| Note that this will affect the natives and libs defined in the `bundlex.exs` files of specified | ||
| applications only, not in their dependencies. | ||
| or for given applications (Mix projects), for example: | ||
|
|
||
| ```elixir | ||
| config :bundlex, :disable_precompiled_os_deps, | ||
| apps: [:my_application, :another_application] | ||
| ``` | ||
|
|
||
| Note that this will affect the natives and libs defined in the `bundlex.exs` files of specified | ||
| applications only, not in their dependencies. |
There was a problem hiding this comment.
Without it the indent breaks in the generated docs for some reason
| is_precompiled_disabled = | ||
| case Application.get_env(:bundlex, :disable_precompiled_os_deps, false) do | ||
| bool when is_boolean(bool) -> bool | ||
| [apps: apps] -> native.app in Bunch.listify(apps) |
There was a problem hiding this comment.
It is not mentioned in the Bundlex.Project moduledoc, that apps might be also just name of the app, eg. [apps: :my_app]
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
mat-hek
force-pushed
the
precompiled-dx
branch
from
dc01cc2
to
7af0f89
Compare
No description provided.