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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to Elixir 1.18 #441

Merged
merged 6 commits into from
Jan 6, 2025
Merged

Update to Elixir 1.18 #441

merged 6 commits into from
Jan 6, 2025

Conversation

jiegillet
Copy link
Contributor

Nothing much to say, no warning.

@jiegillet
Copy link
Contributor Author

jiegillet commented Dec 21, 2024

@angelikatyborska I'm having trouble with this one. Thank god for smoke tests.

Basically, bin/build.sh which ultimately calls mix escript.build doesn't work as expected. It produces a 2.7K file versus 2.7M file on main. And when you run it, it gives

ERROR! Failed to start Elixir.
error: {error,{elixir,{"no such file or directory","elixir.app"}}}

I'm guessing because Elixir is not embeded in the escript as it should. The changelog only mentions one change about escript, this PR, but it should not be relevant.

I found a way to make it work locally by doing first mix compile, then ERL_LIBS=~/.asdf/installs/elixir/1.18.0-otp-27/lib:_build/dev/lib bin/elixir_analyzer slug . ., and I'm sure we could make it work on the docker file, but the escript should work.

The test runner works fine though, so I don't know what's up.

@jiegillet
Copy link
Contributor Author

jiegillet commented Dec 21, 2024

Update, I found the culprit: consolidate_protocols: false is the culprit (added here), and I think it's definitely related to the 1.18 change PR after all.
The problem is that it was there because # Turn off protocol consolidation to avoid warning in analyzed code so if I remove it, we get a warning in the smoke test:

warning: the String.Chars protocol has already been consolidated, an implementation for Clock has no effect. If you want to implement protocols after compilation or during tests, check the "Consolidation" section in the Protocol module documentation
  lib/clock.ex:51

So I guess we could either do the ERL_LIBS thing, or find another solution for the protocol warning, maybe simply filtering out those specific warnings (could they actually be real warnings from a valid solution?)

@angelikatyborska
Copy link
Member

angelikatyborska commented Dec 21, 2024

The change in behavior looks unexpected to me, so I reported it as a bug here: elixir-lang/elixir#14096

@jiegillet
Copy link
Contributor Author

Fixed and merged in two hours. Wow. Now we wait for 1.18.1

@jiegillet jiegillet marked this pull request as ready for review January 6, 2025 11:39
@jiegillet
Copy link
Contributor Author

jiegillet commented Jan 6, 2025

Great, seems to work fine with 1.18. I'll update the rest of the repos with the same docker image.

@angelikatyborska angelikatyborska merged commit bb56b95 into main Jan 6, 2025
6 checks passed
@angelikatyborska angelikatyborska deleted the jie-elixir-1.18 branch January 6, 2025 15:49
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.

2 participants