Skip to content

Support XDG Base Directory Specification #830

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

Closed
wants to merge 1 commit into from
Closed

Support XDG Base Directory Specification #830

wants to merge 1 commit into from

Conversation

ogarcia
Copy link

@ogarcia ogarcia commented Nov 11, 2021

Adds support to XDG Base Directory Specification maintaining backward compatibility. It looks for config file in ${HOME}/.pypirc and ${XDG_CONFIG_HOME}/pypi/pypirc. If both files exist then the first one is used. Related / Closes #754

Adds support to XDG Base Directory Specification maintaining backward
compatibility. It looks for config file in ${HOME}/.pypirc and
${XDG_CONFIG_HOME}/pypi/pypirc. If both files exist then the first one
is used. Closes #754
Copy link
Contributor

@bhrutledge bhrutledge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for putting this together. I'm still not convinced this is a good thing for Twine to do, so I've asked for review from other maintainers.

Before this can be merged, it will need some automated tests, but I would hold off on making any changes until other folks have reviewed the proposal.

XDG_CONFIG_HOME = os.environ.get(
"XDG_CONFIG_HOME", os.path.join(os.path.expanduser("~"), ".config")
)
XDG_CONFIG_FILE = os.path.join(XDG_CONFIG_HOME, "pypi", "pypirc")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should have noticed this in your suggestion on the PR: I don't think Twine should unilaterally decide to put its configuration in a pypi directory. First, I don't think it's accurate; despite the name, .pypirc can be used to define other package repositories besides PyPI. It could be twine, but I think it's also worth keeping in mind that other tools (like flit) can use .pypirc. So maybe it should be be pypa, but I'd want buy-in from other PyPA maintainers before claiming that name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pradyunsg I'm particularly interested in your thoughts from pip's perspective.

Maybe also @takluyver, with flit's perspective.

# ${XDG_CONFIG_HOME}/pypi/pypirc
DEFAULT_CONFIG_FILE = (
OLD_CONFIG_FILE if os.path.isfile(OLD_CONFIG_FILE) else XDG_CONFIG_FILE
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer using pathlib, which I think makes this more readable:

XDG_CONFIG_HOME = os.environ.get("XDG_CONFIG_HOME", Path.home() /  ".config")
XDG_CONFIG_FILE = XDG_CONFIG_HOME / "pypa" / "pypirc"

HOME_CONFIG_FILE = Path.home() / ".pypirc"

DEFAULT_CONFIG_FILE = (
    HOME_CONFIG_FILE if HOME_CONFIG_FILE.is_file() else XDG_CONFIG_FILE
)

@bhrutledge bhrutledge requested a review from takluyver November 13, 2021 11:48
@pradyunsg
Copy link
Member

Alternatively, use platformdirs?

@pradyunsg
Copy link
Member

Scratch that -- the location of the pypirc file is taken from how it is defined in distutils, which has ~/.pypirc hard-coded:

https://github.com/python/cpython/blob/v3.10.0/Lib/distutils/config.py#L40

I definitely think this need broader discussion -- likely on discuss.python.org -- about what exactly we want to do here. As @bhrutledge noted, this affects a lot more than just twine, and needs broader discussion.


I personally don't really think we should do this for another two years, until Python 3.12 removes distutils from the standard library -- we can then allow users of Python 3.12+ to have a platformdirs-respecting configuration file; and write down this expectation somewhere sensible. 🤷🏽

@sigmavirus24
Copy link
Member

Yeah, I'm in favor of not adding support for this until there's something that formally says (e.g., on the packaging guide) that this file might live in another directory. And as Brian points out pypi is a very broad folder name that even if we were to do this early, would likely conflict with other software and cause issues. Heck, I don't even think twine is the best name to use here because I don't think it can sufficiently disambiguate from other twine programs in other languages.

@takluyver
Copy link
Member

I know it's always frustrating when people say this, but... I think we should have a bigger rethink of how index server URLs & credentials are stored, not just the file location. 🙂

A few of the reasons:

  • The .pypirc format has some duplication - you have to list the section names separately. This catches people out at least sometimes.
  • The format was designed for one username & password for each index. Nowadays you may well want multiple credentials for the same site (project specific tokens), with a good way to identify secrets stored in keyring or something similar. And you could also have the same credentials for multiple indexes (e.g. devpi can host multiple separate indexes on one server).
  • I don't know of a good tool to programmatically modify INI format config - preserving formatting & comments, like tomlkit for TOML. This would allow programs to manage the list on the user's behalf, rather than requiring them to edit it manually.

@bhrutledge
Copy link
Contributor

bhrutledge commented Nov 13, 2021

I think we should have a bigger rethink of how index server URLs & credentials are stored, not just the file location.

I appreciate this, @takluyver. There are few issues/discussions about this already (and probably others I'm missing):

I'd like to gather all of this into a discussion on https://discuss.python.org/c/packaging/, but since my sense is that this is not urgent, it might be awhile. If someone else is motivated to kick that off, I'd happily contribute.

For now, I'm going to close this, with gratitude to @ogarcia for prompting the discussion. Further comments are welcome, but I think we should avoid going deeper here.

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.

Use XDG basedir spec to pypirc
5 participants