Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move away from OTP 18 and adopt rebar3_lint 0.3.0 #101

Merged
merged 5 commits into from
Jan 7, 2021
Merged

Move away from OTP 18 and adopt rebar3_lint 0.3.0 #101

merged 5 commits into from
Jan 7, 2021

Conversation

paulo-ferraz-oliveira
Copy link
Contributor

@paulo-ferraz-oliveira paulo-ferraz-oliveira commented Jan 7, 2021

I move away from OTP 18, but get back 19.1, 19.2, 20.1 and 21. 馃槃
In any case, if you're willing to accept this, it might solve what issues you faced.
Trying to support OTP 18.3 (in testing) is incompatible with rebar3 and the newest rebar3_lint, which I also update and fix here (I didn't set off to mix both these issues, but won't want to fix them separately).

Edit: maybe closes #99 and closes #100.

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@@ -47,28 +43,22 @@ jobs:
image: 'erlang:${{ matrix.otp_vsn }}'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we can try to use erlang:${{ matrix.otp_vsn }}-alpine? But I think Erlang only supports alpine starting from OTP-20

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me check that for you. Edit: I'm curious at the advantage of it, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, 19.x not available as alpine. Everything else available as alpine.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I vote down this suggestion. I'm also curious to hear your pov @seriyps , but going ubuntu to debian already adds unnecessary friction that has little to do with Erlang (see minimum git version for github actions), and going debian to alpine could potentially add even more - if not now, later.

My pov is that obviously there might be reasons to deploy systems running on specific distros. Similarly, if you wanted to test software in different distros, you'd go for specific distros. But in this case, we're testing some Erlang sofware that we think has no distro-specific behaviour, so ideally I would test with the easiest distro to work with - Ubuntu.

@paulo-ferraz-oliveira you mentioned the setup-erlang action https://github.com/marketplace/actions/setup-erlang . A cursory read makes me assume that they download a "compiled" version (fast install times). Is that right? Did you try it out?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've already tried it out, but IIRC I stumbled upon a more difficult version management (not all versions were available, for example). Want me to test it out here? I'm assuming you're looking for faster builds.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reasoning is more:

  • there has to be one canonical way for erlang repos to test in CI, specifically github actions.
  • that canonical way should probably be based on Ubuntu, for reasons of keeping the bar low

Speed should rarely be a goal, but it can be a show stopper - like when I tried to use kerl, and it would build OTP for 10+ minutes.

Let's merge this PR as is, because it is an improvement over what we have. Thank you for that.

IFF you want, file another PR with setup-erlang. From the looks of it, it could be that canonical solution that I'm referring to, and if you have already implemented it for inaka's repos, all the better.

rebar.config Show resolved Hide resolved
Copy link
Member

@andreineculau andreineculau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I went down the path of installing git@2.18 myself in fb3fb97 but I ended up with a longer build times https://github.com/for-GET/jesse/actions/runs/465881686

my deps were
apt-get install -y make libssl-dev libghc-zlib-dev libcurl4-gnutls-dev libexpat1-dev gettext unzip
based on https://linuxize.com/post/how-to-install-git-on-debian-9/#installing-git-from-a-source

I left some review items, but all in all yeah. Let's ditch OTP 18, and build git from source. Ideally the erlang docker images would up the git version 馃

.github/workflows/ci.yml Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@paulo-ferraz-oliveira
Copy link
Contributor Author

Ideally the erlang docker images would up the git version 馃

One day I'm sure they will, but ideally so would they use the most recent rebar3 version (or at least one that wouldn't break the CI).

@paulo-ferraz-oliveira
Copy link
Contributor Author

If the latest commit's CI tests pass, I guess this is good for merge (?)

@andreineculau andreineculau merged commit b288673 into for-GET:master Jan 7, 2021
@andreineculau
Copy link
Member

THANK YOU!

@paulo-ferraz-oliveira paulo-ferraz-oliveira deleted the remove/support_for_otp_18 branch January 9, 2021 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CI: use kerl/linuxbrew to install specific erlang versions breaks with rebar3_lint@0.3.0
3 participants