Have rebar3.cmd work on a Windows CMD shell

[paulo-ferraz-oliveira]

Closes #147.

@lpil

[lpil]

Wonderful, thank you!

[paulo-ferraz-oliveira]

I will still want to add a Rust -specific case, as we briefly discussed before, to see if this fits the bill.

Edit: I'm doing that now.

[paulo-ferraz-oliveira]

Added a Rust-specific test case, but rebar3's executable cannot be found: https://github.com/erlef/setup-beam/actions/runs/3308594785/jobs/5461095772#step:10:20... even though it can from shell cmd. 😕

[starbelly]

Very nice 😎

[paulo-ferraz-oliveira]

It would be even nicer if I could understand why Rust can't find the executable, but the shell it runs on can. I'll try to find some time to continue this today, at night.

[paulo-ferraz-oliveira]

Even though this seems to be working, it's not. Look at cargo test... it's not being run.

[paulo-ferraz-oliveira]

Yeah, so the binary is in the PATH, as per Rust output, but Rust is probably not using that same variable's content to search for stuff.

[paulo-ferraz-oliveira]

@lpil, from Rust, for example,

process::Command::new("cmd").args(&["/C", "rebar3"])...

can, as of this pull request, at least, find rebar3. (my understanding is that process::Command does not run inside a shell or consult PATH, so workarounds are required)

Another possible way to do it is

let where_is_rebar3 = env::var("INSTALL_DIR_FOR_REBAR3").unwrap() + "/bin/";
process::Command::new(where_is_rebar3 + "rebar3.cmd").arg("version")...

for example. Let me know if this is enough (you can also test with this branch and find out if you need to adjust).

[lpil]

Hi @paulo-ferraz-oliveira . std::process::Command searches the path for the program specified and I've not had this problem with other programs, so I suspect there is an issue on the side of this action.

Adding an environment variable would not be suitable because that environment variable would only be set on GitHub actions when using this action, and we need to use the same code for all environments.

[paulo-ferraz-oliveira]

But, in that case, would

process::Command::new("cmd").args(&["/C", "rebar3"])...

be suitable?

Can you give me an example (either on GitHub or by pasting code here) of what you're actually using? That way I can hack it until it passes 😆 (not your code, but mine).

[lpil]

It would not, we don't spawn a shell to run subprocesses for any other program.

Here's where we use it: https://github.com/gleam-lang/gleam/blob/75f3e02af3a35f236d6dd9fd0f673953e7cf5959/compiler-core/src/build/project_compiler.rs#L319-L325

This runs std::process::Command::new("rebar3.cmd").args(...) but there's a few layers of indrection because the compiler doesn't directly perform any IO

[starbelly]

Waiting for env var issues to be sorted out.

[paulo-ferraz-oliveira]

Force pushed to rebase onto main.

[paulo-ferraz-oliveira]

Hey, @lpil.

I've been trying to update this pull request with different degrees of sustained frustration. Let me know if this is enough (or maybe you can test the action as-is, from this branch - just use paulo-ferraz-oliveira/setup-beam@feature/rebar3.cmd instead of erlef/setup-beam@...), or otherwise suggest a change to the test so that we can potentially move forward.

I'm not Rust savvy so don't actually understand what's preventing this from working as expected.

The result is apparently Ok, but there's the potential for the current implementation to not serve your needs.

Thank you.

[lpil]

Hi @paulo-ferraz-oliveira ! That looks perfect, thank you! 💜

[paulo-ferraz-oliveira]

Hey, @starbelly, I believe we're ready for review, finally 😄

Have fun!

[starbelly]

🥳

[paulo-ferraz-oliveira]

Merging... @lpil, let me know if you'd require a release for this, otherwise it can wait a while longer for the other open pull request to get merged in too...