jlouis / graphql-erlang

GraphQL implementation in Erlang.
Other
313 stars 52 forks source link

Failing Hex package compilation #209

Open arnodirlam opened 4 years ago

arnodirlam commented 4 years ago

Hi there,

when getting the Hex package, compilation fails with the following log:

===> Fetching geas ({pkg,<<"geas">>,<<"2.4.6">>})
===> Version cached at /Users/arno/.cache/rebar3/hex/default/packages/geas-2.4.6.tar is up to date, reusing it
===> Plugin {geas_rebar3,{git,"https://github.com/crownedgrouse/geas_rebar3.git",
                              {branch,"master"}}} not available. It will not be used.
===> Fetching rebar3_hex ({pkg,<<"rebar3_hex">>,<<"6.8.0">>})
===> Version cached at /Users/arno/.cache/rebar3/hex/default/packages/rebar3_hex-6.8.0.tar is up to date, reusing it
===> Fetching hex_core ({pkg,<<"hex_core">>,<<"0.5.0">>})
===> Version cached at /Users/arno/.cache/rebar3/hex/default/packages/hex_core-0.5.0.tar is up to date, reusing it
===> Fetching verl ({pkg,<<"verl">>,<<"1.0.1">>})
===> Version cached at /Users/arno/.cache/rebar3/hex/default/packages/verl-1.0.1.tar is up to date, reusing it
===> Compiling verl
===> Compiling hex_core
===> Compiling rebar3_hex
===> Compiling _build/default/plugins/rebar3_hex/src/rebar3_hex_revert.erl failed
_build/default/plugins/rebar3_hex/ebin/rebar3_hex_revert.beam:none: failed to delete temporary file /Users/arno/dev/weaver/deps/graphql_erl/_build/default/plugins/rebar3_hex/ebin/rebar3_hex_revert.bea#: no such file or directory
_build/default/plugins/rebar3_hex/ebin/rebar3_hex_revert.beam:none: failed to rename /Users/arno/dev/weaver/deps/graphql_erl/_build/default/plugins/rebar3_hex/ebin/rebar3_hex_revert.bea# to /Users/arno/dev/weaver/deps/graphql_erl/_build/default/plugins/rebar3_hex/ebin/rebar3_hex_revert.beam: no such file or directory

===> Plugin rebar3_hex not available. It will not be used.
===> Fetching geas ({pkg,<<"geas">>,<<"2.4.6">>})
===> Version cached at /Users/arno/.cache/rebar3/hex/default/packages/geas-2.4.6.tar is up to date, reusing it
===> Compiling geas
===> Compiling geas_rebar3
===> Compiling geas
===> Compiling geas_rebar3
===> Compiling graphql
===> Compiling src/graphql_err.erl failed
src/graphql_err.erl:3: can't find include lib "graphql/include/graphql.hrl"

src/graphql_err.erl:117: variable 'ID' is unbound
src/graphql_err.erl:117: record directive undefined

** (Mix) Could not compile dependency :graphql_erl, "/Users/arno/.asdf/installs/elixir/1.9.4-otp-22/.mix/rebar3 bare compile --paths="/Users/arno/dev/weaver/_build/test/lib/*/ebin"" command failed. You can recompile this dependency with "mix deps.compile graphql_erl", update it with "mix deps.update graphql_erl" or clean it with "mix deps.clean graphql_erl"

To debug, I set the git commit of the Hex release (3ea1b3a) in mix.lock in combination with each of the following entries in mix.exs:

{:graphql, github: "shopgun/graphql-erlang", branch: "develop"}
{:graphql_erl, github: "shopgun/graphql-erlang", branch: "develop"}
{:graphql_erl, "~> 0.15.0"}

As only the 1. works, I assumed the changed package name on hex is an issue. So I went ahead and replaced all occurrences of graphql/include/ with graphql_erl/include/ and could compile the hex-installed package that way 🎉 Then I replaced these with ../include/ and found that this works as well. So I guess that would be a solution for both hex and non-hex installed compilations?

Would you accept a PR with this change? If yes, which branch should I base it off?

Thanks for the great work and all the best, Arno

jlouis commented 4 years ago

Try patching it to master. It is the one which is the farthest and is probably also a bit divergent from what Hex has.

So it would be the best to target it there.

arnodirlam commented 4 years ago

Great, thanks for the quick response! I've just opened the PR.

arnodirlam commented 4 years ago

CI passes.

I've also added a hint to the Hex package in the README, since that info is a bit hard to find. Let me know if you'd prefer to have it changed or removed again :)

arnodirlam commented 4 years ago

I actually found an even better solution, I think, at least for Elixir: Adding graphql_erl to the list of dependencies in mix.exs like so:

def deps do
  [
    {:graphql, "~> 0.16.0", hex: :graphql_erl}
  ]
end

My only experience is with Elixir though, for which this works fine. Would it still be helpful for erlang or other BEAM languages to use relative paths for header files? 🤔

arnodirlam commented 4 years ago

As there doesn't seem to be a need to make the include paths relative, I've closed that PR. It would still be helpful for the community to know how to properly add the dependency in Elixir, so please consider my new PR (only adding my previous finding to the README) 🙏

If you don't mind, a new release to hex would be greatly appreciated as well.

Lastly, setting the master branch as default could also help people new to the project see what's going on.

Thanks so much for the great work! ❤️

seriyps commented 3 years ago

Having the same issue trying to add it to the Erlang project. Seems indeed it's because of the name in hex and in the app.src doesn't match. Can be fixed like this:

{deps, [{graphql, "0.16.1", {pkg, graphql_erl}}]}.