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

Remove inadvertent splatting of the name attribute #3547

Merged
merged 6 commits into from Aug 24, 2022

Conversation

jeamland
Copy link
Contributor

The name attribute of a Distribution object is used by the command-line processing system and is not intended to hold the name of the distribution itself. Setting it to the name will cause the command-line processing system to have a bad time.

Fixes #3545

@abravalheri
Copy link
Contributor

abravalheri commented Aug 19, 2022

Hi @jeamland, thank you very much for providing this.

Would it be possible to add some tests to avoid future regression? (Usually what I would do is create another commit with the tests, then reorder them, in such a way that the first commit demonstrates the tests failing and the second one fixes them).

I think there is another more subtle aspect that is also worthy investigating (in addition to the changes you are proposing here). Setuptools should not try to discover the distribution name, because it is given... I can have a look on that myself after we merge this PR (but by any means feel free to investigate if you are interested).

abravalheri and others added 6 commits August 19, 2022 18:20
The `name` attribute of a `Distribution` object is used by the
command-line processing system and is not intended to hold the
name of the distribution itself. Setting it to the name will
cause the command-line processing system to have a bad time.
According to issue 3545 it seems that "name-discovery" happens, even
when the project already explicitly sets it.

This is related to parsing of dynamic versions (via `attr` directive),
which triggers the auto-discovery to obtain the value of `package_dir`.

The value of `package_dir` is used to find the path to the module
in `version = {"attr" = "module_name.attr_name"}`.
@abravalheri abravalheri force-pushed the avoid-splatting-name-attribute branch from a9abf2e to 9a4b45f Compare August 19, 2022 17:46
@abravalheri
Copy link
Contributor

Hi @jeamland, I took advantage of this PR already being open and added:

  • Tests to ensure we don't have problems setting dist.name
  • Tests/Fixes to ensure name auto-discovery does not happen when the user explicitly sets the name

I also reorganised the order of the commits, with the tests being introduced prior to the fixes. This way we can verify that the errors happen without the fixes.

I hope you don't mind the modifications. But please feel free to modify the code to your liking, or propose different changes.

@jeamland
Copy link
Contributor Author

All looks good to me. Thanks!

@abravalheri abravalheri merged commit af1ff35 into pypa:main Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] ConfigDiscovery.analyse_name incorrectly sets the name attribute on Distribution
2 participants