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

--config-setting example #205

Closed
abitrolly opened this issue Jan 15, 2021 · 14 comments
Closed

--config-setting example #205

abitrolly opened this issue Jan 15, 2021 · 14 comments
Labels
blocked bug Something isn't working

Comments

@abitrolly
Copy link
Contributor

It doesn't seem that -C option has any effect.

✗ python -m build --no-isolation -C --no-cache
usage: python -m build [-h] [--version] [--sdist] [--wheel] [--outdir dir]
                       [--skip-dependencies] [--no-isolation]
                       [--config-setting CONFIG_SETTING]
                       [srcdir]
python -m build: error: argument --config-setting/-C: expected one argument
✗ python -m build --no-isolation -C '--no-cache'
usage: python -m build [-h] [--version] [--sdist] [--wheel] [--outdir dir]
                       [--skip-dependencies] [--no-isolation]
                       [--config-setting CONFIG_SETTING]
                       [srcdir]
python -m build: error: argument --config-setting/-C: expected one argument
@abitrolly
Copy link
Contributor Author

setuptools doesn't fully support config-settings - pypa/setuptools#2491

That's why an example is needed.

Maybe document which backends are compatible, and which are not?
Maybe add an interface for backends to report settings that they support?

@uranusjr
Copy link
Member

uranusjr commented Jan 15, 2021

How should the example be structured? All the frontend can do is to --config-settings directly to the backend, and an example in build would be something along the line you can pass stuff with this flag, but what they are and how they work depends on the build backend, which not really achieve anything. You should find a backend that actually supports this flag first to make the example worthwhile.

Maybe document which backends are compatible, and which are not? Maybe add an interface for backends to report settings that they support?

These are issues toward the specification (PEP 517), not this repository. They should be raised on forums that discuss the specification (https://discuss.python.org these days), not here.

@layday
Copy link
Member

layday commented Jan 15, 2021

It doesn't seem that -C option has any effect.

The value is being interpreted as an argument to build because it begins with a hyphen. In such case you must either use an equals sign or you must not leave a space between -C and the configuration value:

python -m build --no-isolation -C--no-cache

The build will complete but --no-cache will have no effect. PEP 517 does not specify what should happen when a backend receives an unsupported configuration value and setuptools seems to simply discard it.

@abitrolly
Copy link
Contributor Author

abitrolly commented Jan 15, 2021

How should the example be structured?

There is a story of the user who tried to pass --no-cache flag and --help flag to setuptools and didn't get any response. Following the story gets the structure - from trying to figure out how to pass the option, to debugging if it was sent, and revealing that backend is not processing it. Then running the backend manually.

@abitrolly
Copy link
Contributor Author

The value is being interpreted as an argument to build because it begins with a hyphen.

Is it how the argument parsing is supposed to work if the setting is defined with non-optional value as -C CONFIG_SETTING?

@layday
Copy link
Member

layday commented Jan 15, 2021

No, it's a well-known issue with argparse.

@gaborbernat
Copy link
Contributor

gaborbernat commented Jan 15, 2021

No, it's a well-known issue with argparse.

Do you have an upstream ticket for this, if it's well-known? And considering this limitation I suppose this ticket may be addressed by documenting your comments from #205 (comment).

@layday
Copy link
Member

layday commented Jan 15, 2021

https://bugs.python.org/issue9334

@FFY00
Copy link
Member

FFY00 commented Jan 15, 2021

I agree this is an issue in argparse and should be addressed there. However, there are still two alternative forms, -C=--no-cache and -C--no-cache. I would recommend the -C--no-cache format. Since this is not a very big issue, I struggle to justify moving away from argparse. We should add an examples page to the documentation to mitigate this and avoid more people running into the issue and having to go search for a solution by themselves.

@FFY00 FFY00 added blocked bug Something isn't working labels Jan 15, 2021
@abitrolly
Copy link
Contributor Author

It doesn't seem that 10 years old bug will gain traction any time soon. Is switching to patched argparse possible?

@abitrolly
Copy link
Contributor Author

We should add an examples page to the documentation to mitigate this and avoid more people running into the issue and having to go search for a solution by themselves.

The output of --help should be changed.

@FFY00
Copy link
Member

FFY00 commented Jan 15, 2021

Well, since this issue is now in my interest, it is in my TODO, but I am not sure when I will be able to get to it, or how long it would take to get a patch reviewed, merged and released. Moving to a patched argparse would mean adding a new dependency, which I am very strongly opposed to as that would make the bootstrapping process more difficult, as it if were already not difficult enough 🙃. The most sensible solution to this would probably hack around this by manually joining -C --something to -C--something before passing the arguments to argparse, but I am not very happy with that. If this starts crippling down many users, I will consider.

I am sorry for the trouble this is causing, but there are alternative forms you can use, so this issue shouldn't be a blocker in any way. Instead of doing -C --no-cache, do -C--no-cache, which I think is a far better form anyway.

@gaborbernat
Copy link
Contributor

gaborbernat commented Jan 15, 2021

Until argparse gets fixed upstream I'm happy that we document this behavior in the help message. I'd approve such PR.

layday added a commit to layday/build that referenced this issue Jan 15, 2021
Added examples of argparse-compatible configuration settings
and harmonised option formatting.

Ref: pypa#205
layday added a commit to layday/build that referenced this issue Jan 15, 2021
Added examples of argparse-compatible configuration settings
and harmonised option formatting.

Ref: pypa#205
layday added a commit to layday/build that referenced this issue Jan 15, 2021
Added examples of argparse-compatible configuration settings
and harmonised option formatting.

Ref: pypa#205
FFY00 pushed a commit that referenced this issue Jan 15, 2021
Added examples of argparse-compatible configuration settings
and harmonised option formatting.

Ref: #205
@gaborbernat
Copy link
Contributor

Fixed by #207. Thanks!

@pypa pypa locked as resolved and limited conversation to collaborators Jan 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
blocked bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants