Add support for new rebar3 output dir option #10018

starbelly · 2020-05-06T05:27:29Z

rebar3 as of 3.13.2 now supports a --output directory option which allows mix going forward to use said option and put build artifacts in _build which should result and less rebuilding of artifacts and/or better caching of artifacts. Details here: erlang/rebar3#2237

In order to make use of said feature we must put a variable in the system's environment that shall hold the desired output path. rebar3 will recognize said environment variable going forward starting at version 3.14 (yet to be released).

Updated Mix.Tasks.Deps.Compile to put REBAR_BARE_COMPILER_OUTPUT_DIR in system env with the dependency path as the value for use by rebar3.

lib/mix/lib/mix/rebar.ex

tsloughter · 2020-05-06T13:02:33Z

lib/mix/lib/mix/rebar.ex

+  @doc """
+  Returns the version of the available `rebar` command if any
+  """
+  @rebar_vsn_regex ~r/^## Rebar3 (?<vsn>[\w\.\d]+)$/m


Prefix isn't always ##. That is only for the ones that are installed with rebar3 local install.

The escript will have %%. And I just realized only the non-escript has a space there. escript is %%Rebar3 <vsn>.

The escript is %% Rebar3 version; the actual escript generator thing inserts the space itself:

→ head -n2 rebar3 #!/usr/bin/env escript %% Rebar3 3.14.0-rc1

Otherwise it is true that it sometimes is ## and sometimes is %%. Also if you use asdf, the shim spacing is different:

$ cat `which rebar3` #!/usr/bin/env bash # asdf-plugin: rebar 3.13.2 exec /usr/local/opt/asdf/bin/asdf exec "rebar3" "$@"

Also the thing to worry about is the semver-like pre-release flags:

%% Rebar3 3.14.0-rc1

So if you want to be sure, the regex likely looks like ^(##|%%).*[Rr]ebar3? (?<vsn>[\w\d.-]+)$

@ferd Does the asdf shim have one or octothorps? Not sure if you typed that out or if it was copy/paste.

I don't think the asdf plugin should affect this though. Doesn't mix always use it's own copy of rebar and not the system one?

Ah, sorry for the noise. I now see there's an option to use rebar from an env var. In that case could it make sense to always require new rebar when running the "local" one, but just call --version on the global one? How slow is that?

Checked with someone using asdf, mix should not end up referring to a shim, just escript.

I am still worried about the implications of this version check. It means folks can be using the same Rebar version but installed in different ways and they won't get the same behaviour. :/ Whatever check we do it is impossible to guarantee it will work on all the different ways rebar can be installed.

That is correct. But all of that is done as a nicety so mix doesn't have to call and cache the result of rebar3 version which successfully prints the full version since before the first alpha release, and which would be the true safest approach.

ferd · 2020-05-06T14:25:36Z

lib/mix/lib/mix/tasks/deps.compile.ex

+  end
+
+  defp rebar_compile_cmd_for(vsn, rebar_path, build_path, lib_path) do
+    case Version.compare("3.12.2", vsn) do


3.13.2 is the first one to support to be version annotated; 3.13.1 the first one to support the -o option. The moment you got an annotation you got something newer.

Yeah, I wondered if doing the Version comparison was overkill... sounds like it. Should probably still parse the Version to make sure it wasn't junk but that should happen in rebar_vsn/1. Thoughts?

You don't have to do a comparison, you just have to check if it is exists.

starbelly · 2020-05-11T00:16:12Z

So, I booted up a VM with windows 10 and I could not replicate the problem I'm seeing in CI in that, I used a similar path with escapes and so forth and was able to File.stream!/1 successfully.

That said, I could not run the tests myself as I could not get elixir to compile per a unicode properties compilation error. I can't paste the error, but I took a screen shot. However, I don't think the compilation error I got is relevant at all, but can share here if desired.

My thought at the moment is that this problem is limited to the CI environment for windows or possibly the test setup itself (i.e., it may be the file isn't actually there).

josevalim · 2020-05-11T07:40:32Z

Hi @starbelly, a quick heads up: we talked about this in the Build and Packaging WG and we agreed to go down the environment variable route. So first we will add the environment variable support to Rebar 3.14 and then pass it here in Mix.

Regarding the Windows problem, we had the same issue on another test in the past, and we just stopped running that particular test on Windows. Windows is a bit tricky when it comes to removing files: I believe it takes a while for Windows to notice the file is no longer being used, which leads to races.

tsloughter · 2020-05-11T13:32:48Z

May as well add the env variable to mix now. Sooner it is in the better for users.

starbelly · 2020-05-11T16:06:06Z

Right, so I'm synced up... Are we wanting to add said env var check in this PR?

josevalim · 2020-05-11T22:57:06Z

We will set the variable from Mix but rebar3 needs to be patched to read from it.

tsloughter · 2020-05-11T22:58:41Z

@josevalim right, but it doesn't need to be patched before mix starts setting the environment variable.

josevalim · 2020-05-11T23:07:57Z

As long as we agree on the name. :)

rebar3 as of 3.13.2 now supports a --output directory option which allows mix going forward to use said option and put build artifacts in _build which should result and less rebuilding of artifacts and/or better caching of artifacts. Details here: erlang/rebar3#2237 In order to make use of said feature we must put a variable in the system's environment that shall hold the desired output path. rebar3 will recognize said environment variable going forward starting at version 3.14 (yet to be released). - Updated Mix.Tasks.Deps.Compile to put REBAR_BARE_COMPILER_OUTPUT_DIR in system env with the dependency path as the value for use by rebar3.

starbelly · 2020-05-12T00:57:31Z

@josevalim @tsloughter @ferd The PR has been updated to just putting the env var in place. Updated description of issue and commit message.

josevalim · 2020-05-12T11:02:08Z

lib/mix/lib/mix/tasks/deps.compile.ex

@@ -189,7 +189,10 @@ defmodule Mix.Tasks.Deps.Compile do
    lib_path = Path.join(config[:env_path], "lib/*/ebin")

    env = [{"REBAR_CONFIG", config_path}, {"TERM", "dumb"}]


Instead of doing it with System.put_env, you can define the environemnt variable here. :)

🤣 Got it!

lib/mix/lib/mix/tasks/deps.compile.ex

josevalim · 2020-05-13T23:04:24Z

💚 💙 💜 💛 ❤️

This option will be enabled on Elixir v1.12 instead. This reverts commit fc9f769.

josevalim reviewed May 6, 2020

View reviewed changes

lib/mix/lib/mix/rebar.ex Outdated Show resolved Hide resolved

tsloughter reviewed May 6, 2020

View reviewed changes

ferd reviewed May 6, 2020

View reviewed changes

starbelly force-pushed the support-rebar-output-dir-opt branch from 80f7531 to d39a3d6 Compare May 12, 2020 00:54

starbelly force-pushed the support-rebar-output-dir-opt branch from d39a3d6 to ddc3e3f Compare May 12, 2020 00:56

starbelly marked this pull request as ready for review May 12, 2020 00:58

josevalim reviewed May 12, 2020

View reviewed changes

address feedback

9b078fa

josevalim reviewed May 13, 2020

View reviewed changes

lib/mix/lib/mix/tasks/deps.compile.ex Outdated Show resolved Hide resolved

Update lib/mix/lib/mix/tasks/deps.compile.ex

a3d748c

josevalim merged commit fc9f769 into elixir-lang:master May 13, 2020

starbelly mentioned this pull request Oct 21, 2020

Maybe handle symlinking to priv in mix erlef/build-and-packaging-wg#4

Closed

josevalim added a commit that referenced this pull request Dec 21, 2020

Revert "Add support for new rebar3 output env option (#10018)"

72ec902

This option will be enabled on Elixir v1.12 instead. This reverts commit fc9f769.

cw789 mentioned this pull request Mar 18, 2021

Building always has to recompile rebar3 dependencies webuhu/elixir_nix_example#4

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for new rebar3 output dir option #10018

Add support for new rebar3 output dir option #10018

starbelly commented May 6, 2020 •

edited

Loading

tsloughter May 6, 2020

ferd May 6, 2020 •

edited

Loading

starbelly May 6, 2020

starbelly May 6, 2020

michalmuskala May 6, 2020

michalmuskala May 6, 2020 •

edited

Loading

starbelly May 6, 2020

josevalim May 6, 2020

ferd May 6, 2020

ferd May 6, 2020 •

edited

Loading

starbelly May 6, 2020 •

edited

Loading

tsloughter May 6, 2020

starbelly commented May 11, 2020

josevalim commented May 11, 2020

tsloughter commented May 11, 2020

starbelly commented May 11, 2020

josevalim commented May 11, 2020

tsloughter commented May 11, 2020

josevalim commented May 11, 2020

starbelly commented May 12, 2020

josevalim May 12, 2020

starbelly May 12, 2020

josevalim commented May 13, 2020

		@@ -189,7 +189,10 @@ defmodule Mix.Tasks.Deps.Compile do
		lib_path = Path.join(config[:env_path], "lib/*/ebin")

		env = [{"REBAR_CONFIG", config_path}, {"TERM", "dumb"}]

Add support for new rebar3 output dir option #10018

Add support for new rebar3 output dir option #10018

Conversation

starbelly commented May 6, 2020 • edited Loading

Choose a reason for hiding this comment

ferd May 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michalmuskala May 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ferd May 6, 2020 • edited Loading

Choose a reason for hiding this comment

starbelly May 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

starbelly commented May 11, 2020

josevalim commented May 11, 2020

tsloughter commented May 11, 2020

starbelly commented May 11, 2020

josevalim commented May 11, 2020

tsloughter commented May 11, 2020

josevalim commented May 11, 2020

starbelly commented May 12, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josevalim commented May 13, 2020

starbelly commented May 6, 2020 •

edited

Loading

ferd May 6, 2020 •

edited

Loading

michalmuskala May 6, 2020 •

edited

Loading

ferd May 6, 2020 •

edited

Loading

starbelly May 6, 2020 •

edited

Loading