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

Fix SetuptoolsDeprecationWarning about using --global-option #2487

Merged
merged 1 commit into from Sep 8, 2022

Conversation

adamchainz
Copy link
Contributor

Fixes #2478.

Contribution checklist:

(also see CONTRIBUTING.rst for details)

  • wrote descriptive pull request text
  • added/updated test(s)
  • updated/extended the documentation
  • added relevant issue keyword
    in message body
  • added news fragment in changelog folder
    • fragment name: <issue number>.<type>.rst for example (588.bugfix.rst)
    • <type> is must be one of bugfix, feature, deprecation, breaking, doc, misc
    • if PR has no issue: consider creating one first or change it to the PR number after creating the PR
    • "sign" fragment with -- by :user:`<your username>`.
    • please, use full sentences with correct case and punctuation, for example:
      Fixed an issue with non-ascii contents in doctest text files -- by :user:`superuser`.
    • also see examples
  • added yourself to CONTRIBUTORS (preserving alphabetical order)

@asottile
Copy link
Contributor

asottile commented Sep 7, 2022

what versions of setuptools does the new argument require ?

@gaborbernat gaborbernat merged commit 628964d into tox-dev:master Sep 8, 2022
@adamchainz
Copy link
Contributor Author

adamchainz commented Sep 8, 2022

I tested down to setuptools 50 and got no error. But, the first mention of --build-option in the setuptools source was introduced alongside the deprecation in setuptools 63: pypa/setuptools#3380 . It seems older versions just ignore all unrecognized arguments.

I found that --build-option was specified PEP 517: https://peps.python.org/pep-0517/#config-settings , but only recently implemented.

If we want to ensure that gztar is built on older setuptools, we could do a version check?

@adamchainz adamchainz deleted the fix_global_option branch September 8, 2022 08:57
@asottile
Copy link
Contributor

asottile commented Sep 8, 2022

@gaborbernat I don't think this could go out as is given the above

@gaborbernat
Copy link
Member

gaborbernat commented Sep 8, 2022

I guess the question here if older setuptools just ignored this flag or actually used it 🤔 Wonder if need this flag at all or we can just drop it 🤔

@asottile
Copy link
Contributor

asottile commented Sep 8, 2022

I believe on windows the flag is important -- otherwise it'll default to zip ?

@gaborbernat
Copy link
Member

Would that cause a problem for pip?

@asottile
Copy link
Contributor

asottile commented Sep 8, 2022

it shouldn't be a problem for pip -- but I thought tox tries to read the tgz -- maybe I'm misremembering how this works -- not at a computer to check

@gaborbernat
Copy link
Member

tox does not read the file 🤔

@asottile
Copy link
Contributor

asottile commented Sep 8, 2022

this is what I was thinking of but looks like it already supports zip:

_REGEX_FILE_NAME_WITH_VERSION = re.compile(r"[\w_+.-]+-(.*)\.(zip|tar\.gz)")

we can probably drop the option entirely then

@adamchainz
Copy link
Contributor Author

That seems reasonable to me too, done in #2497.

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

Successfully merging this pull request may close these issues.

Warning about "--global-option": ["--formats=gztar"] emitted when using setuptools 64.0.1
3 participants