New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use kerl for managing Erlang versions #54

Merged
merged 12 commits into from Jan 10, 2018

Conversation

Projects
None yet
5 participants
@Stratus3D
Member

Stratus3D commented Dec 28, 2017

This PR is inspired by:

And aims to address issue #53.

Benefits over the existing asdf-erlang code:

  • Simpler code, we just use kerl and get all the benefits that it provides

Benefits over asdf-kerl:

  • Some code to symlink app-specific executables to the bin directory that asdf looks for, so all Erlang executables are on the path.
  • Installs kerl automatically inside the plugin

@Stratus3D Stratus3D force-pushed the use-kerl branch from 10a772f to 45d0ba6 Dec 28, 2017

@Stratus3D Stratus3D force-pushed the use-kerl branch 2 times, most recently from 84adb0f to 9cf0039 Dec 28, 2017

@Stratus3D Stratus3D force-pushed the use-kerl branch from 9cf0039 to cea5a50 Dec 28, 2017

@Stratus3D Stratus3D requested review from HashNuke, danhper and vic Dec 29, 2017

@HashNuke

This comment has been minimized.

Member

HashNuke commented Dec 29, 2017

Nice ~! I'll try it out tomorrow and ping back.

@Stratus3D

This comment has been minimized.

Member

Stratus3D commented Dec 29, 2017

@eproxus how does this look? Tried to combine the best of both plugins.

@danhper

This comment has been minimized.

Member

danhper commented Jan 3, 2018

Great, I am going to give it a try too. Thanks a lot!

@Stratus3D

This comment has been minimized.

Member

Stratus3D commented Jan 4, 2018

Noticed one rough edge with this implementation. The asdf-erlang's version of kerl interferes with user's installed version of kerl. For example the user can view asdf-erlang builds with their own installation of kerl in certain scenarios. This can be solved by setting KERL_BASE_DIR to a directory inside the plugin directory. KERL_CONFIG should probably be set for good measure to, so we don't default to using the user's kerl config (though maybe using it would be a good thing?).

I also need to go back through and remove some code. I was thinking kerl didn't install docs, but it looks like it does so I can remove my custom docs code from the plugin.

@Stratus3D

This comment has been minimized.

Member

Stratus3D commented Jan 4, 2018

I also need to update the readme to inform users of all this.

@Stratus3D

This comment has been minimized.

Member

Stratus3D commented Jan 8, 2018

I've addressed the issues I've identified. As best I can tell this is ready to merge.

@Stratus3D

This comment has been minimized.

Member

Stratus3D commented Jan 10, 2018

I've tested this on both my OSX laptop and my Ubuntu laptop. As best I can tell it's ready to go. Does anyone else want to test it out before I merge?

@danhper

This comment has been minimized.

Member

danhper commented Jan 10, 2018

It worked perfectly for me too. Thanks for working on this.
+1 for merging

@Stratus3D Stratus3D merged commit 5e34bfd into master Jan 10, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

shanesveller added a commit to shanesveller/asdf-erlang that referenced this pull request Jan 10, 2018

Use "git" kerl backend to support installation of patch releases
The `ref:OTP-20.1.2` syntax described in i.e. asdf-vm#42 and asdf-vm#48 no longer work after the transition to use `kerl` under the hood, when asdf-vm#54 got merged. This environment variable instructs `kerl` to parse Git tags rather than the original source (erlang.org?).

If the maintainers would prefer, I can instead submit a documentation update, as this configuration can also be placed in `~/.kerlrc`.

kerl/kerl#240
@eproxus

This comment has been minimized.

Contributor

eproxus commented Jan 11, 2018

Sorry for being late for the party, I'm now back from Christmas holidays.

@Stratus3D Very nice work, I really like the integration! I think setting KERL_CONFIG is the right way to go.

I guess the .kerlrc used by this plug-in doesn't exist? If so, it could be documented where it exists by default and that the user can add and customize it (or possibly symlink it to their existing one if desired).

ensure_kerl_installed() {
if [ ! -f "$(kerl_path)" ]; then
download_kerl
fi

This comment has been minimized.

@padde

padde Feb 21, 2018

@Stratus3D shouldn't this check somehow take the version into account to ensure the specified kerl version matches the installed version?

This comment has been minimized.

@Stratus3D

Stratus3D Feb 22, 2018

Member

Yes, you are correct. With this approach we won't be able to upgrade kerl. I need to fix this.

@Stratus3D Stratus3D deleted the use-kerl branch Jul 23, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment