ocaml / setup-ocaml

GitHub Action for the OCaml programming language
https://ocaml.org/
MIT License
196 stars 38 forks source link

Option to add binaries to path #70

Open JasonGross opened 3 years ago

JasonGross commented 3 years ago

It would be nice if I could set some variable to true to have the opam environment added to path, so I don't need to opam exec everything. (It might be the case that running opam env >> $GITHUB_ENV is enough?)

JasonGross commented 3 years ago

Actually, I think you need opam env | sed "s/'//g; s/; export .*//g" >> $GITHUB_ENV

dra27 commented 3 years ago

It's better to use opam exec (because it's explicit), but I agree that this would be a useful feature

smorimoto commented 3 years ago

I agree with @dra27, opam exec is better. We can get rid of this by passing --enable-shell-hook to opam init, but still the behavior is often unstable.

dra27 commented 3 years ago

--enable-shell-hook won't work for the Windows shells (yet) - it only works for bash, etc.

avsm commented 3 years ago

I think @JasonGross posted the answer in his followup comment

opam env | sed "s/'//g; s/; export .*//g" >> $GITHUB_ENV

Modifying GITHUB_ENV is sufficient for future steps in the workflow to inherit the env, and it would simplify subsequent commands. It might be best to make this a non-default option in the workflow config though, in case it has unexpected consequences (such as an opam switch in a future step nullifying it).

smorimoto commented 3 years ago

Of course I can add something to the action that handles this well, but I also think it would be smarter to add a better fix to opam after this summer.

smorimoto commented 3 years ago

I just opened a PR to address this. Since v2 removed quite a lot of the differences between each platform, it was done in a fairly simple way.

rr0gi commented 2 years ago

having opam environment set up would allow to not duplicate build commands in yml, but invoke same wrapper as developers are using (e.g. make)

smorimoto commented 10 months ago

I feel that this is a more opam issue that needs to be fixed, but can it be fixed on the opam side? I actually use opam exec a lot locally as well as on CI, which is a bit odd.

smorimoto commented 3 months ago

~Since the exact version of opam is available on all platforms, opam exec will no longer be needed by default in the next release! (in other words, v3!) See #794~