gleam-lang / gleam

⭐️ A friendly language for building type-safe, scalable systems!
https://gleam.run
Apache License 2.0
17.77k stars 743 forks source link

Prefer `mix` to `rebar3` if `mix` is installed on the system #2338

Closed nsidnev closed 1 year ago

nsidnev commented 1 year ago

Hey!

I found this issue and the commit that fixes it. I understand the reason for such a solution, but it seems to me that it is not perfect.

I am currently looking into the option of implementing a wrapper on Gleam for a client for EdgeDB on Elixir and have run into a problem.

The client uses jose and crc dependencies, which have rebar3 and mix configs. And in both cases the Elixir wrapper over Erlang code is in the same package.

The current behavior of the Gleam compiler is such that it sees the option of compiling the package with rebar3 and ignores the Elixir code completely. And if the next Elixir package (EdgeDB) uses a wrapper (JOSE/CRC), then the compilation will fail because the required modules won't be found:

  Compiling crc
make: Entering directory '/home/nik/projects/edgedb/edgedb-gleam/build/dev/erlang/crc/c_src'
 C      crc_8.c
 C      checksum_xor.c
 C      crc_algorithm.c
 C      crc_model.c
 C      crc_nif.c
 C      xnif_slice.c
 C      crc_resource.c
 LD     crc_nif.so
make: Leaving directory '/home/nik/projects/edgedb/edgedb-gleam/build/dev/erlang/crc/c_src'
===> Analyzing applications...
===> Compiling crc
  Compiling jose
===> Analyzing applications...
===> Compiling jose
  Compiling edgedb
warning: CRC.ccitt_16_xmodem/1 is undefined (module CRC is not available or is yet to be defined)
  lib/edgedb/connection/config/cloud.ex:42: EdgeDB.Connection.Config.Cloud.parse_cloud_credentials/4

warning: JOSE.json_module/0 is undefined (module JOSE is not available or is yet to be defined)
  lib/edgedb/connection/config/cloud.ex:76: EdgeDB.Connection.Config.Cloud.read_dsn_zone_from_secret_key/1

Compilation failed due to warnings while using the --warnings-as-errors option
error: Shell command failure

There was a problem when running the shell command `mix`.

I think that Gleam, in addition to package tools, should check for the existence of mix executable before deciding which utility to use for the compilation.

I have a working branch ready for this change, but want to check if I'm thinking in the right direction before posting a new PR.

nsidnev commented 1 year ago

I was reviewing my branch and the code that is responsible for building dependencies and realized that instead of determining if there is a mix executable on the system, Gleam can determine in advance if the package depends on any pure Elixir package that requires a mix to be built and in that case delegate rebar3 packages building to mix.

I think that sounds better than my original suggestion.

lpil commented 1 year ago

I think we want to go with the original design as it'll give a better error message for the user, and also a non-Mix library could be making use of the Elixir modules.