axodotdev / cargo-dist

📦 shippable application packaging
https://axodotdev.github.io/cargo-dist/
Apache License 2.0
1.47k stars 66 forks source link

dist.toml's [dist] section is never read for packages #1308

Closed Gankra closed 1 month ago

Gankra commented 1 month ago

I am honestly completely shocked I missed this in testing.

The logic for reading the [dist] section out of a dist.toml or dist-workspace.toml is special cased to only exist when that file is the workspace file. If it's a package we just completely fail to find it, because that path didn't need to exist in the original impl of generic projects (where you could only have the one root dist.toml with no children).

Note that the [package] section gets read fine (name, version, repository, etc), which is what was the most complex to implement and where all my testing went. We have several tests for that in axoproject, but axoproject (currently) explicitly doesn't understand the [dist] section, so testing for this needs to exist in cargo-dist itself.

I kinda just took it for granted that the [dist] section was being read on packages, and all of our emphasis went into never needing to create a dist.toml for packages (making JS more native!) so the place where I would have tested it in axolotlsay-hybrid got deleted. Oops!

For most common cases of usage this isn't a fatal issue, but it definitely needs to be fixed asap for anyone who needs to configure js packages in shared workspaces.