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

elixir_1_14: init at 1.14.0 #189355

Merged
merged 1 commit into from
Sep 5, 2022
Merged

elixir_1_14: init at 1.14.0 #189355

merged 1 commit into from
Sep 5, 2022

Conversation

shanesveller
Copy link
Contributor

@shanesveller shanesveller commented Sep 1, 2022

Followed the example of #148619 since everything included still seems pertinent. However, some included packages do not build successfully on 1.14.0 yet, so I skipped the default (elixir) version change for purposes of this PR. Please let me know if the desired flow is instead to wait for such projects to become 1.14-compatible on their own.

Description of changes

https://elixir-lang.org/blog/2022/09/01/elixir-v1-14-0-released/
https://hexdocs.pm/elixir/1.14.0/changelog.html

Things done

Extras:

Defaults:

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@DianaOlympos
Copy link
Contributor

Which packages break with 1.14?

@shanesveller
Copy link
Contributor Author

Which packages break with 1.14?

Sure, sorry for not including that info in the top post.

With this additional change:

diff --git a/pkgs/development/beam-modules/default.nix b/pkgs/development/beam-modules/default.nix
index 068e1da3313..66300e6bb2c 100644
--- a/pkgs/development/beam-modules/default.nix
+++ b/pkgs/development/beam-modules/default.nix
@@ -42,7 +42,7 @@ let
       elvis-erlang = callPackage ./elvis-erlang { };
 
       # BEAM-based languages.
-      elixir = elixir_1_13;
+      elixir = elixir_1_14;
 
       elixir_1_14 = lib'.callElixir ../interpreters/elixir/1.14.nix {
         inherit erlang;

I see compiler failures in prometheus_ex Hex package like so:

@nix { "action": "setPhase", "phase": "unpackPhase" }
unpacking sources
unpacking source archive /nix/store/0bcxv9k5qyfib5r8jh4shkc250laj5ar-source
source root is source
@nix { "action": "setPhase", "phase": "patchPhase" }
patching sources
@nix { "action": "setPhase", "phase": "configurePhase" }
configuring
@nix { "action": "setPhase", "phase": "buildPhase" }
building
warning: the VM is running with native name encoding of latin1 which may cause Elixir to malfunction as it expects utf8. Please ensure your locale is set to UTF-8 (which can be verified by running "locale" in your shell)
�[33mwarning: �[0muse Mix.Config is deprecated. Use the Config module instead
  config/config.exs:3

Compiling 19 files (.ex)

== Compilation error in file lib/prometheus/buckets.ex ==
** (UndefinedFunctionError) function Kernel.Utils.defdelegate/2 is undefined or private. Did you mean:

      * defdelegate_all/3
      * defdelegate_each/2

    (elixir 1.14.0) Kernel.Utils.defdelegate({:new, [line: 18], [{:arg, [line: 18], nil}]}, [])
    lib/prometheus/buckets.ex:18: (module)

And this fails the build for pleroma based on nixpkgs-review rev HEAD on x86_64-linux.

@bobby285271 bobby285271 added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Sep 2, 2022
@DianaOlympos
Copy link
Contributor

Thanks! Will have a look

That make me think we also need to fix the UTF8 problem, i keep having it.

@mveytsman
Copy link
Contributor

mveytsman commented Sep 4, 2022

FWIW plemora is depending on their own fork of prometheus_ex. The mainline repo compiles fine in Elixir 1.14.0.

I tried pinging someone who I think is a maintainer of the fork here: deadtrickster/prometheus.ex#48

@bhankas
Copy link
Contributor

bhankas commented Sep 4, 2022 via email

Comment on lines 3 to 4
# How to obtain `sha256`:
# nix-prefetch-url --unpack https://github.com/elixir-lang/elixir/archive/v${version}.tar.gz
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# How to obtain `sha256`:
# nix-prefetch-url --unpack https://github.com/elixir-lang/elixir/archive/v${version}.tar.gz

This is already in the contributing guide

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, this was added by me a while ago.
I do not often contribute and this way it was easier for me (lower the barrier).

So please remove.

Copy link
Contributor

Choose a reason for hiding this comment

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

PS: There are probably more of this comments across the code base from me - should I remove them all?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the review! Removed in the version committed as b73e0e9

Copy link
Member

Choose a reason for hiding this comment

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

PS: There are probably more of this comments across the code base from me - should I remove them all?

If the download link is not obvious then we can keep it but for github downloads at least we do not require them.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. Thanks Sandro. I'll remove theme step by step in the places where I left theme.

@cw789
Copy link
Contributor

cw789 commented Sep 5, 2022

That make me think we also need to fix the UTF8 problem, i keep having it.

Isn't setting LANG & LC_TYPE to C.UTF-8 enough anymore?
https://github.com/NixOS/nixpkgs/blob/master/pkgs/development/interpreters/elixir/generic-builder.nix#L35-L36

@bobby285271 bobby285271 removed the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Sep 5, 2022
@happysalada
Copy link
Contributor

Nice to see you around here @shanesveller (I enjoyed quite enjoyed your blog when I was doing elixir)! Thank you for the contribution!

I think this is fine to merge to make the update available to everyone.

Regarding the UTF8 problem, I've never found how to fix it with nix as well. We are doing exactly what you pointed @cw789 but it seems it's not enough.
However I don't think the compilation error is a UTF8 problem here, it looks like the dependency is using an old alias for a function (defdelegate) and it should be a matter of updating the dependency. Hopefully the maintainer answers and we update the main elixir version in nixpkgs.

(let me know of course if you disagree).

@happysalada happysalada merged commit 51d2625 into NixOS:master Sep 5, 2022
@DianaOlympos
Copy link
Contributor

Nah iirc there was a glibc patch a few years ago that broke it all and we need to patch it a bit. I have something somewhere. I never had time to dive in.

@shanesveller shanesveller deleted the shanesveller/package-elixir-1.14.0 branch September 8, 2022 13:03
@datafoo
Copy link
Contributor

datafoo commented Sep 12, 2022

Thank you for the work. I have a question/suggestion.

If this contribution had been split into 2 commits and 2 pull requests such as:

  • elixir_1_14: init at 1.14.0
  • elixir_1_9: remove following end of support

Would that have allowed to backport the first commit/PR to release-22.05?

@datafoo datafoo mentioned this pull request Sep 12, 2022
13 tasks
@happysalada
Copy link
Contributor

it would have indeed made the backport easier, we can think about that for next time! Thank you for your input on this.

@cw789 cw789 mentioned this pull request Sep 21, 2022
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants