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

Compiler warnings with Elixir 1.16 #534

Closed
kaifeldhoff opened this issue Apr 5, 2024 · 12 comments · Fixed by christhekeele/erlex#1 or #537
Closed

Compiler warnings with Elixir 1.16 #534

kaifeldhoff opened this issue Apr 5, 2024 · 12 comments · Fixed by christhekeele/erlex#1 or #537
Labels

Comments

@kaifeldhoff
Copy link

Environment

Erlang/OTP 26 [erts-14.2.3] [source] [64-bit] [smp:12:12] [ds:12:12:10] [async-threads:1] [jit:ns]
Elixir 1.16.2 (compiled with Erlang/OTP 26)

"dialyxir": {:hex, :dialyxir, "1.4.3", ....}

Current behavior

Compilation-warnings are thrown caused Erlex:

warning: in order to compile .yrl files, you must add "compilers: [:yecc] ++ Mix.compilers()" to the "def project" section of erlex's mix.exs
  (mix 1.16.2) lib/mix/tasks/compile.yecc.ex:70: Mix.Tasks.Compile.Yecc.preload/1
  (mix 1.16.2) lib/mix/compilers/erlang.ex:66: Mix.Compilers.Erlang.compile/6
  (mix 1.16.2) lib/mix/task.ex:478: anonymous fn/3 in Mix.Task.run_task/5
  (mix 1.16.2) lib/mix/tasks/compile.all.ex:124: Mix.Tasks.Compile.All.run_compiler/2
  (mix 1.16.2) lib/mix/tasks/compile.all.ex:104: Mix.Tasks.Compile.All.compile/4
  (mix 1.16.2) lib/mix/tasks/compile.all.ex:93: Mix.Tasks.Compile.All.with_logger_app/2

warning: in order to compile .xrl files, you must add "compilers: [:leex] ++ Mix.compilers()" to the "def project" section of erlex's mix.exs
  (mix 1.16.2) lib/mix/tasks/compile.leex.ex:69: Mix.Tasks.Compile.Leex.preload/1
  (mix 1.16.2) lib/mix/compilers/erlang.ex:66: Mix.Compilers.Erlang.compile/6
  (mix 1.16.2) lib/mix/task.ex:478: anonymous fn/3 in Mix.Task.run_task/5
  (mix 1.16.2) lib/mix/tasks/compile.all.ex:124: Mix.Tasks.Compile.All.run_compiler/2
  (mix 1.16.2) lib/mix/tasks/compile.all.ex:104: Mix.Tasks.Compile.All.compile/4
  (mix 1.16.2) lib/mix/tasks/compile.all.ex:93: Mix.Tasks.Compile.All.with_logger_app/2

My project has no Dialyzer-issues, so there is no evidence, that these warnings affect the functionality of Dialyzer/dialyxir.

Expected behavior

No severe compilation warnings.

I can only assume that these warnings are an issue for dialyxir. They started to occur after switching Elixir version from 1.15.7 to 1.16.2.

Unfortunately @asummers (maintainer of Erlex) has been inactive for years. A PR for this issue already existed since January. There is a rumor that he might even have passed away in 2022, hopefully this is simply not true.

How can we tackle this?

Many thanks!

@christhekeele
Copy link

I've reached out to both the maintainer and support@hex.pm to see if we can transfer ownership of the package to a maintained fork, in the event that @asummers is unable to or uninterested in continuing to keep working on erlex.

@jeremyjh
Copy link
Owner

@christhekeele Any word back on this? Another option would be for me to bring it back into the dialyxir repo and namespace. It started in this repo, but we wanted it to be available for other use cases as well is why erlex exists as a separate project, but if Andrew is no longer maintaining it then I think we have to here.

@jeremyjh jeremyjh added the bug label Apr 11, 2024
@jeremyjh
Copy link
Owner

Yes, Andrew is gone. Wow, it is very shocking and saddening to me.

https://www.gffh.com/andrew-david-summers/

@christhekeele
Copy link

@jeremyjh no word, I'm afraid.

@kaifeldhoff
Copy link
Author

@jeremyjh, so it is true. We've found his name on other pages but were never 100% sure. That is truly a sad story.

The discussion about all this started in https://elixirforum.com/t/erlex-dialyxir-dependenty-does-not-support-elixir-1-16-any-suggestions/62714?u=kai_feldhoff. (Seems like this is only visible for forum-members, so you should have access)
Here it's also mentioned that elixir-lsp is using erlex. They already forked and fixed the elixir 1.16 issue. And there are additional forks. Looks like the work of @asummers found more use-cases. So from my point of view it might not be the best option to put erlex back into dialyxir.

As we didn't know the history of dialyxir and that erlex has been a part of it, we just began to think about keeping @asummers-project alive. And that lead to the need for a new maintainer. @christhekeele was the first to take the initiative, with nobody else so far. As he mentioned he already reached out to hex.pm to start the 30 days counter that is needed to switch ownership for the project. A good idea from my point of view. I've thought about creating a looking-for-maintainer forum-thread to get things rolling further. Just in case to give the numberless volunteers a chance ;) .

But as I now know the deep relationship between dialyxir and erlex please tell me your thoughts about all this. Should we stay on this road or do you have other ideas or opinions?

@jeremyjh
Copy link
Owner

dialyxir is the only dependency listed in hex.pm. elixir-lsp vendors it but they have to since they are vendoring dialyxir, I don't think they use it directly. Issues in erlex mostly get reported here, so I'm of two minds about it. If @christhekeele wants to maintain erlex and will have control of the hex.pm package within the next couple of weeks I'm totally fine just letting that play out.

@kaifeldhoff
Copy link
Author

dialyxir is the only dependency listed in hex.pm. elixir-lsp vendors it but they have to since they are vendoring dialyxir, I don't think they use it directly. Issues in erlex mostly get reported here

That is an important point. Now I understand that integrating erlex into dialyxir is also a good option. But what happens with the hex-project of erlex? Could you please join the forum-thread? With the help of the community we hopefully get to the best decision.

@christhekeele
Copy link

If @christhekeele wants to maintain erlex and will have control of the hex.pm package within the next couple of weeks I'm totally fine just letting that play out.

👍


dialyxir is the only dependency listed in hex.pm. Issues in erlex mostly get reported here.

Now I understand that integrating erlex into dialyxir is also a good option. But what happens with the hex-project of erlex?

As you point out, the future of the package is separate from dialyxir's decision to fold the code into itself. I'd still update the package with pending/new community contributions, and eventually consider a warning-based deprecation path to sunsetting the package if non-dialyxir usecases and users do not emerge.

@christhekeele
Copy link

christhekeele commented Apr 14, 2024

No update from hex.pm, but I've spent some time today preparing a fork here.

  • The only changes to lib and src are applied from merged open PRs in the original.
  • The rest are tooling changes to move CI over to GH Actions, and fully test against all supported OS/OTP/Elixir versions of the project so development can proceed confidently that nobody's experience will break.

@christhekeele
Copy link

christhekeele commented May 30, 2024

Update @jeremyjh, I have been added as an owner to the erlex package. I will cut a 0.27.0-handoff pre-release with community patches applied a little later this week for testing and we can move forward warning-free from there!

@christhekeele
Copy link

christhekeele commented Jun 2, 2024

I've released 0.27.0-handoff on hex.pm. I'd appreciate it if folk watching this issue give the pre-release a go and report any issues they discover here (or better still, a lack of issues), and I'll drop the full release after a few confirmations! You should just be able to add this to your projects:

# mix.exs

defmodule My.MixProject do
  #...
  defp deps() do
    [
      #...
      {:erlex, "== 0.2.7-handoff",
       only: [:dev, :test], runtime: false, allow_pre: true, override: true},
      #...
    ]
  end
  #...
end

Thanks all for helping, especially @alappe, @marcelotto, and @bradhanks for submitting patches, @jeremyjh and @axelson for helping me to figure out how to proceed, and @ericmj for handling things on the hex.pm side of things.

@christhekeele
Copy link

Just released 0.27.0, this should hopefully be fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants