Refuse to publish packages that import dev dependencies #4567
Conversation
|
CI passes for me: https://github.com/gideongrinberg/gleam/actions/runs/14873076625 |
|
@inoas are there additional changes you think I should make? |
|
Sorry, I didn't mean to close that – are there any changes you are still looking for? |
|
Hello! Sorry for taking so long to review this. I've got a bit caught up in this current release. I'll get to this once v1.11 is out! |
|
Thanks, not a problem. I see that the merge commit (I accidentially synced my branch with main) is causing the CI failure. Should I revert the merge? |
|
You'll need to rebase instead, and remove the merge commit before this is merged. |
| for (imported_module, _) in &module.dependencies { | ||
| if let Some(import_info) = built.module_interfaces.get(imported_module) { | ||
| let package = &import_info.package; | ||
| if dev_dependencies.contains(&package) && module.origin == Origin::Src { |
There was a problem hiding this comment.
dev_dependencies.contains(&package) isn't enough to determine if it's a dev dep or not because it could be a transient dev dependency, which would not be in this list.
If we are going to use this approach we'll need to before this reject the package if it using transient deps.
|
|
||
| for module in built.root_package.modules.iter() { | ||
| for (imported_module, _) in &module.dependencies { |
There was a problem hiding this comment.
This is cool but I think we could improve it!
src/ code shouldn't ever really use dev dependencies, so we should make it emit a warning when this happens (as an error would be a breaking change).
In the checking for this during we code analysis can record on modules if they use dev deps or transient deps or not, and then in gleam publish and gleam export erlang-shipment we can return an error by checking this flag on the module.
This PR fixes #3143 by throwing an error when gleam publish is invoked and a dev dependency is imported in the code.
The check is implemented in
do_build_hex_tarball. Another approach would be to implement the check in the analysis phase, but, as far as I can tell, there's no way to detect if the compilation has been invoked bygleam publishor something else.I manually tested this and it works as expected, but I can add tests as well.