-
Notifications
You must be signed in to change notification settings - Fork 260
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: support wider range of verbosity settings on other build backends #2339
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Henry Schreiner <[email protected]>
Should we be trying to get something consistent between frontends ? i.e. 0 maps to |
If we want to keep this option, I like this idea. Previously I was in favour of a light-touch approach to adding arguments to so that would be
I think it would be nice to warn on unsupported settings i.e. (not supported, above) or outside The other approach, of course, is to avoid this mess, deprecate the |
That is very similar to what I had, but with one difference: 0 is "native", which doesn't add any flags, keeping to the native behavior of the tool, and allows a user to use the
I prefer the default not adding flags (especially since we have a mechanism for users to add flags themselves). I expect most users won't change the default (which now produces build output because it uses Since this is such an important feature, I like having the flag for it (probably not enough to have added it if we didn't already have it, but certainly happy to keep it). I have a |
Co-authored-by: Matthieu Darbois <[email protected]>
Just fyi: |
Yeah, seeing it in a table, I agree with you @henryiii. It's better to have a default that matches the build-frontend's default.
Also a good point. |
docs/options.md
Outdated
* `-1`: Hide as much build output as possible; passes `-q` to the build frontend. Not supported by `build`/`build[uv]`. | ||
* `0`: The default. On pip, this hides the build output if the build succeeds, other build frontends produce output from the build backend. | ||
* `1`: Produces build backend output. On `pip`, this passes `-v`. Other frontends do this by default. | ||
* `2`: Produces extra output from resolving packages too. On `pip`, this passes `-vv`, other build frontends use `-v`. | ||
* `3`: Even more resolving output from pip with `-vvv`, other build frontends continue to just pass `-v`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd consider replacing this with the table from your comment - I think the table does a better job of conveying the meaning, especially with uv soon to be added to the mix.
* `-1`: Hide as much build output as possible; passes `-q` to the build frontend. Not supported by `build`/`build[uv]`. | |
* `0`: The default. On pip, this hides the build output if the build succeeds, other build frontends produce output from the build backend. | |
* `1`: Produces build backend output. On `pip`, this passes `-v`. Other frontends do this by default. | |
* `2`: Produces extra output from resolving packages too. On `pip`, this passes `-vv`, other build frontends use `-v`. | |
* `3`: Even more resolving output from pip with `-vvv`, other build frontends continue to just pass `-v`. | |
| | Description | build | pip | uv | | |
|-------------|----------------------------------------|-------|-------|------| | |
| -2 | quiet mode | N/A | `-q` | N/A | | |
| -1 | reduce output, where supported | N/A | ` ` | `-q` | | |
| 0 (default) | default for build tool | ` ` | ` ` | ` ` | | |
| 1 | print backend output | ` ` | `-v` | ` ` | | |
| 2 | print log messages e.g. resolving info | `-v` | `-vv` | `-v` | | |
| 3 | print even more debug info | `-vv` | `-vvv` | `-vv` | | |
elif not 0 <= level < 2: | ||
elif level > 1: | ||
return ["-v"] | ||
elif level < 0: | ||
msg = f"build_verbosity {level} is not supported for {frontend} frontend. Ignoring." | ||
log.warning(msg) | ||
return [] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think there are a few changes required here to match the table in your comment. e.g. -1 on pip doesn't do anything. It might be worth changing this to a map, with only a few supported settings per frontend, warning otherwise; up to you.
Okay, new version. Added the table, and modified it a tiny bit after observing |
Signed-off-by: Henry Schreiner <[email protected]>
cd633eb
to
e2a3160
Compare
Also, |
See #2338.
This makes the verbosity setting a bit more useful on other backends. This tries to map the current
pip
meanings to other backends. The key changes are:<0
will supportuv
(feat: option to build directly with uv #2322),build
continues to produce a warning. This is a feature that's been requested before for build, btw.1
continues to do nothing on non-pip
backends-v
to non-pip backends, since both build and uv support adding extra resolving info (similar to pip's-vv
) if passing-v
.