-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Mix: warn when project app name matches a dependency #15013
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
base: main
Are you sure you want to change the base?
Mix: warn when project app name matches a dependency #15013
Conversation
|
Hi @pckrishnadas88, thank you for the PR! Although I would likely move the check elsewhere, perhaps on Mix.Dep.Loader or somewhere else where we already traverse the dependencies... |
This ensures Mix emits a warning if a project's app name conflicts with one of its dependencies.
|
Hi @josevalim, Thanks for the feedback! I’ve moved the check to Mix.Dep.Loader and added a test that verifies the warning prints when the project app name matches a dependency. Verified locally with a temporary project. I’m still learning Elixir, so any guidance or feedback is greatly appreciated. PTAL! —Krishnadas |
lib/mix/lib/mix/dep/loader.ex
Outdated
| mix_children(Mix.Project.config(), locked?, []) ++ Mix.Dep.Umbrella.unloaded() | ||
| deps = mix_children(Mix.Project.config(), locked?, []) ++ Mix.Dep.Umbrella.unloaded() | ||
| # warn if project app matches a dep | ||
| warn_on_duplicate_app_name(Mix.Project.config()[:app], deps) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move it inside the with_scm_and_app, so we don't have to traverse deps twice!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the feedback!
I’ve moved the duplicate app name warning into with_scm_and_app. Please let me know if anything else is needed.
|
|
||
| {scm, opts} = | ||
| if is_nil(scm) and Mix.Hex.ensure_installed?(locked?) do | ||
| if is_nil(scm) and locked? and Mix.Hex.ensure_installed?(locked?) do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change? 🤔
Summary
This PR adds a warning in Mix.Project when a project’s application name is the same as one of its dependencies.
Details
Currently, if a project declares an app name that is identical to one of its dependencies, Elixir doesn’t warn the user. This can lead to unexpected behaviors, such as hanging compilers when trying to start tasks from the dependency.
This fix implements
warn_on_duplicate_app_name/1inmix/project.exthat prints a warning usingMix.shell().error/1. The associated testMix.ProjectTestverifies the behavior.Example
If a user creates a project with:
and adds
{:aoc, "~> 0.16.0"}todeps, running:will print:
Related Issue
Fixes: [elixir-lang/elixir#14972](#14972)