pypa / setuptools

Official project repository for the Setuptools build system
https://pypi.org/project/setuptools/
MIT License
2.5k stars 1.18k forks source link

Setuptools does not pass config_settings through backend #2491

Open di opened 3 years ago

di commented 3 years ago

The build project now has a --config-setting flag to pass options to the backend:

$ python -m build --help
usage: python -m build [-h] [--version] [--sdist] [--wheel] [--outdir dir] [--skip-dependencies]
                       [--no-isolation] [--config-setting CONFIG_SETTING]
                       [srcdir]

- - - >8 - - -

  --config-setting CONFIG_SETTING, -C CONFIG_SETTING
                        pass option to the backend

I wanted to use this to set wheel's --plat-name argument like so:

$ python -m build --wheel --config-setting plat-name=foo

I found that this worked as expected until the settings got to setuptool's _build_with_temp_dir function:

https://github.com/pypa/setuptools/blob/d368be2b5be8676ba8a3d55045b45b55090f6982/setuptools/build_meta.py#L190-L200

In this example, _build_with_temp_dir gets called with config_settings={'plat-name': 'foo'}, which then gets 'fixed' into config_settings={'plat-name': 'foo', '--global-option': []}, and then setuptools ignores everything in config_settings and only considers --global-option.

It almost seems like --global-option itself could be used for this, but unfortunately it doesn't work as it's value is passed to lots of things that don't accept the arguments I'm hoping to pass:

$ python -m build --wheel --config-setting="--global-option=--plat-name" --config-setting="--global-option=foo" -n
usage: _in_process.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
   or: _in_process.py --help [cmd1 cmd2 ...]
   or: _in_process.py --help-commands
   or: _in_process.py cmd --help

error: option --plat-name not recognized
...

I created a commit that "fixes" this for me as a proof of concept: https://github.com/pypa/setuptools/commit/fc95b3b83d6d5b561dc0a356995edf4c99785a6f

Possibly related issues:

jaraco commented 3 years ago

I could have used something like this too for Setuptools itself to disable post-release tags for #2500 whereas currently it's using this hack. I suspect Setuptools needs to devise a way to map "config settings" to tweak knobs at different scopes in the build process (global, per-command, ...).

abitrolly commented 3 years ago

@di why not put this into PR?

di commented 3 years ago

@abitrolly Because I'm not sure it's right. The means by which a frontend passes these arguments to the backend is not well defined.

abitrolly commented 3 years ago

@di what is missing here https://www.python.org/dev/peps/pep-0517/#config-settings ?

di commented 3 years ago

@abitrolly All these phrases from that section indicate that this is not well defined:

Build backends MAY assign any semantics they like to this dictionary.

Build frontends SHOULD provide some mechanism for users to specify arbitrary string-key/string-value pairs to be placed in this dictionary.

Build frontends MAY also provide arbitrary other mechanisms for users to place entries in this dictionary

abitrolly commented 3 years ago

@di I proposed some clarifications in https://github.com/python/peps/pull/1766 Not sure where it should go from there.

Normalizing parameters to be passed as "option": "value" without dashes seems to be a good compromise. Because interface to build system is defined as callback functions and not as CLI API, keeping dashes would mean that backend should somehow pass the names through argparse.

FFY00 commented 3 years ago

FYI I am also in favor of removing the dashes, I don't think they are needed in PEP 517.

layday commented 3 years ago

Incidentally, global options in setuptools seem to mean something different than they do in pip where they are placed in front of the setuptools command. Here, they are appended to the command. pip users might find the discrepancy surprising.

I'm not sure why arguments to sdist and bdist_wheel should not be funneled through config_settings verbatim - why is --global-option needed at all?

jaraco commented 3 years ago

I'm sure the reason why the PEP is non-specific about the format is because it would be difficult for a protocol like that to be specific without being over-constrained for a particular use-case.

I'm not sure why arguments to sdist and bdist_wheel should not be funneled through config_settings verbatim - why is --global-option needed at all?

I imagine that the sdist and bdist_wheel commands have settings that are unique to the command. I also expect there could be settings that are applied globally, independent of any command.

Regardless, what I'd like to see happen is for someone to examine the code and determine all the command-line parameters setuptools currently solicits that would be relevant to a build backend... and determine at what scopes those settings are relevant (globally, per command, other?). From there, we can create a scheme, something simple, maybe namespaced, to accept those parameters through config_settings.

layday commented 2 years ago

Issues raised against build concerning this behaviour:

abitrolly commented 2 years ago

@layday if you want to push this further, I guess the right way it to do what @brettcannon said in https://github.com/python/peps/pull/1766#issuecomment-762487258

Please discuss any proposed changes at https://discuss.python.org/c/packaging/14 first.

layday commented 2 years ago

I don't know how changing lists to comma-separated value strings in the PEP is going to help here.

abitrolly commented 2 years ago

@layday because PEP doesn't specify how to pass lists, nobody wants to write this.

dixonwhitmire commented 2 years ago

Hi @abitrolly so just to clarify for myself and possibly for others following this issue, is the next step here to discuss potential changes in https://discuss.python.org/c/packaging/14 so that the PEP is updated with well defined statements? Thank you!

abitrolly commented 2 years ago

@dixonwhitmire yes, that's correct.

PolyacovYury commented 2 years ago

Just encountered this. My use-case was to pass --quiet from python -m build call.

First there is an issue with build itself: passing -C--global-option=--quiet once causes an error, since config_settings['--global-option'] becomes a string, and setuptools expects a list. Maybe change _fix_config() to something like:

    def _fix_config(self, config_settings):
        config_settings = config_settings or {}
        config_settings.setdefault('--global-option', [])
        if not isinstance(config_settings['--global-option'], list):
            config_settings['--global-option'] = [config_settings['--global-option']]
        return config_settings

Second issue is that setuptools does NOT conform to https://www.python.org/dev/peps/pep-0517/#config-settings. --global-option should be inserted in front of the setup_command, and --build-option should go to the end of the argv. This way, invoking python -m build -C--global-option=--quiet will produce the expected result.

Although, even better would be to adjust build itself, so that instead of -C it has -G and -B (for --global-option and --build-option respectively), but that would be out of scope of this discussion.

abravalheri commented 2 years ago

Hi @PolyacovYury , I think the first part of the problem you described was solved in https://github.com/pypa/setuptools/pull/2876, wasn't it?

Regarding the second part:

--global-option should be inserted in front of the setup_command

It is not immediately clear to me that the spec mandates that --global-option should come before setup_command... Is there any specific part of the document or is this something you concluded is necessary for --quiet to work? (I wonder it this order, while working for --quiet might break other options that specific to the sdist or bdist_wheel subcommands).

layday commented 2 years ago

I commented on this upthread:

Incidentally, global options in setuptools seem to mean something different than they do in pip where they are placed in front of the setuptools command. Here, they are appended to the command. pip users might find the discrepancy surprising.


It is not immediately clear to me that the spec mandates that --global-option should come before setup_command [...]

This is all setuptools-specific stuff, the spec has no notion of global options nor does it expect that options will be passed to a CLI.

PolyacovYury commented 2 years ago

I think the first part of the problem you described was solved in https://github.com/pypa/setuptools/pull/2876, wasn't it?

Looks like it was, yes. Might need to update my venvs then, but when I tested this (the day before the comment was posted) this was either not published to PyPI or somehow managed to not get pulled by my pip. Me still having python 3.6 is to blame, perhaps?..


It is not immediately clear to me that the spec mandates that --global-option should come before setup_command... Is there any specific part of the document...?

Oops. Was kinda frustrated after a 2-day-long debug session, which resulted in me not checking the docs thoroughly enough.
@layday is right:

This is all setuptools-specific stuff.


the spec has no notion of global options nor does it expect that options will be passed to a CLI.

Despite that, having the config_options working consistently across tools...

global options in setuptools seem to mean something different than they do in pip where they are placed in front of the setuptools command. Here, they are appended to the command. pip users might find the discrepancy surprising.

...would be very helpful. Also, having --global-option affect the commands rather than the CLI as a whole makes my brain hurt while trying to come up with a name for an option which would affect the CLI. --dist-option? --setup-option?


is this something you concluded is necessary for --quiet to work?

Yes. As well as for any other command-line options defined in the distutils/dist.py:Distribuiton class's global_options list.


I wonder it this order, while working for --quiet, might break other options that specific to the sdist or bdist_wheel subcommands.

distutils/dist.py:Distribuiton's first call to FancyGetopt.getopt in its parse_command_line() pulls out arguments given before the first build command and applies them to the Distribution class itself (parser.getopt(args=self.script_args, object=self)), thus these arguments would never be passed to the actual build command. The args list returned by that call always starts with the first build command.

So, for example, if build_wheel had a --quiet option that would do something different from the global one, and I wanted the behaviours of both global and command options, I would need to invoke the CLI as setup.py --quiet bdist_wheel --quiet.
Running setup.py --quiet bdist_wheel results in bdist_wheel not even knowing that --quiet was ever there.

Incidentally, invoking setup.py bdist_wheel --quiet results in Distribution never getting the memo about quietness. Which is the reason of my arrival in this thread in the first place :)

As for current build commands' implementation's reaction to --quiet/--verbose... distutils/cmd.py:Command class's __init__ has you covered:

# verbose is largely ignored, but needs to be set for
# backwards compatibility (I think)?
self.verbose = dist.verbose

...and self.verbose is never referred to. Ever. Even distutils/ccompiler.py:new_compiler() ultimately does nothing with it.

abravalheri commented 2 years ago

This might be relevant for this thread: I believe that in v64.0.3+ setuptools is using config_settings["--global-option"] and config_settings["--build-option"] consistently with what used to happen in pip.

ssbarnea commented 2 years ago

@abravalheri Something is not right...

.package installdeps: setuptools >= 64.0.3, setuptools_scm[toml] >= 3.5.0, setuptools_scm_git_archive >= 1.0, wheel
/Users/ssbarnea/c/os/tendo/.tox/.package/lib/python3.11/site-packages/setuptools/build_meta.py:307: SetuptoolsDeprecationWarning: 
            The arguments ['--formats=gztar'] were given via `--global-option`.
            Please use `--build-option` instead,
            `--global-option` is reserved to flags like `--verbose` or `--quiet`.

  warnings.warn(msg, SetuptoolsDeprecationWarning)
abravalheri commented 2 years ago

Hi @ssbarnea , this behaviour is consistent with https://github.com/pypa/setuptools/issues/1928 isn't it?

alex commented 1 year ago

I am not sure if this is related, but when attempting to use --config-settings to pass py-limited-api, I'm getting the error:

  usage: _in_process.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
     or: _in_process.py --help [cmd1 cmd2 ...]
     or: _in_process.py --help-commands
     or: _in_process.py cmd --help

  error: option --py-limited-api not recognized
layday commented 1 year ago

It's probably 'cause pip is calling [setuptools.build_meta.]get_requires_for_build_wheel to ensure all build dependencies are satisfied prior to building the wheel, which in turn calls setup.py egg_info with your --config-settings, for which --py-limited-api=... is an invalid option. (If you pass --no-build-isolation to pip wheel, it won't call get_requires_for_build_wheel, but that does mean you'll have to install your build dependencies separately.) setuptools doesn't have a way to sift out build options by command.

hugovk commented 1 year ago

We're considering something like https://github.com/python-pillow/Pillow/pull/7171 to work around this with a custom backend. Does anyone have a nicer way to do it?

FFY00 commented 1 year ago

@hugovk IMO that is a very reasonable way to do it, and would also be my recommendation.

di commented 1 year ago

Interesting workaround!

sbidoul commented 1 year ago

This might be relevant for this thread: I believe that in v64.0.3+ setuptools is using config_settings["--global-option"] and config_settings["--build-option"] consistently with what used to happen in pip.

@abravalheri I'm preparing to remove --global-option and --build-option from pip and trying to pass options via config settings.

--global-option seems to work fine.

But I notice that the --build-option makes the setuptools backend fail. It looks like it tries to pass the build options to the egg_info command:

$ py -m pip -vv install --use-pep517  --config-setting="--global-option=--verbose" --config-setting="--global-option=--quiet" --config-setting="--build-option=--universal" .
Using pip 23.3.dev0 from /home/sbi-local/FOSS/pip/src/pip (python 3.8)
Non-user install because user site-packages disabled
Created temporary directory: /tmp/pip-build-tracker-t9g9dhw_
Initialized build tracking at /tmp/pip-build-tracker-t9g9dhw_
Created build tracker: /tmp/pip-build-tracker-t9g9dhw_
Entered build tracker: /tmp/pip-build-tracker-t9g9dhw_
Created temporary directory: /tmp/pip-install-lzd7kyz5
Created temporary directory: /tmp/pip-ephem-wheel-cache-qfdbhpi0
Processing /tmp/brol
  Added file:///tmp/brol to build tracker '/tmp/pip-build-tracker-t9g9dhw_'
  Created temporary directory: /tmp/pip-build-env-71nero8h
  Running command pip subprocess to install build dependencies
  Using pip 23.3.dev0 from /home/sbi-local/FOSS/pip/src/pip (python 3.8)
  Collecting setuptools>=40.8.0
    Obtaining dependency information for setuptools>=40.8.0 from https://files.pythonhosted.org/packages/bb/26/7945080113158354380a12ce26873dd6c1ebd88d47f5bc24e2c5bb38c16a/setuptools-68.2.2-py3-none-any.whl.metadata
    Using cached setuptools-68.2.2-py3-none-any.whl.metadata (6.3 kB)
  Collecting wheel
    Obtaining dependency information for wheel from https://files.pythonhosted.org/packages/b8/8b/31273bf66016be6ad22bb7345c37ff350276cfd46e389a0c2ac5da9d9073/wheel-0.41.2-py3-none-any.whl.metadata
    Using cached wheel-0.41.2-py3-none-any.whl.metadata (2.2 kB)
  Using cached setuptools-68.2.2-py3-none-any.whl (807 kB)
  Using cached wheel-0.41.2-py3-none-any.whl (64 kB)
  Installing collected packages: wheel, setuptools
    Creating /tmp/pip-build-env-71nero8h/overlay/bin
    changing mode of /tmp/pip-build-env-71nero8h/overlay/bin/wheel to 775
  Successfully installed setuptools-68.2.2 wheel-0.41.2
  Installing build dependencies ... done
  Running command Getting requirements to build wheel
  ************ ['setup.py', '--verbose', '--quiet', 'egg_info', '--universal']
  usage: setup.py [global_opts] cmd1 [cmd1_opts] [cmd2 [cmd2_opts] ...]
     or: setup.py --help [cmd1 cmd2 ...]
     or: setup.py --help-commands
     or: setup.py cmd --help

  error: option --universal not recognized
  error: subprocess-exited-with-error
sbidoul commented 1 year ago

I confirm the same error occurs with the build frontend.

sbidoul commented 1 year ago

I just realize this is discussed at length in https://github.com/pypa/setuptools/issues/3896 too.

layday commented 1 year ago

See also https://github.com/pypa/build/issues/264.

abravalheri commented 1 year ago

Hi @sbidoul, yes there is still the challenge of what to do with egg_info and sdist. Right now --build-option will be passed to all commands. If any of the commands do not accept certain flags/options, they will raise an exception.

@sbidoul, @layday, @henryiii do you believe that the best we can do right now is to only pass --build-option to bdist_wheel and ignoring it in any other command?

I don't have experience with these extra parameters, but if you think that this is the best way forward we can craft a PR that does that...

(I am considering that --global-option and --build-option are stop gap solutions and will never be 100% perfect, but that can be implemented immediately... indeed we need a better plan, as it is been discussed in #3896, but that may take more time to design.)

sbidoul commented 1 year ago

@sbidoul, @layday, @henryiii do you believe that the best we can do right now is to only pass --build-option to bdist_wheel and ignoring it in any other command?

I don't have experience with these parameters either, but what I can tell is that pip only passes its --build-option to the bdist_wheel command. So yes, I think what you propose is a good stop gap until a broader solution is designed.

layday commented 1 year ago

pip passes its config settings to all commands as far as I remember, meaning that the build option is passed to every build hook when specified this way. This is also the behaviour of build as of v1.

Restricting passing config settings to build_wheel is not a good solution. There is demand to pass config settings to all of these commands which is evidenced on the build tracker.

The easiest thing for setuptools to do in my mind is to introduce a set of “legacy” namespaced build options, one each for egg_info, sdist and wheel. Paper over the cracks while we hash out what is to become of setuptools’ config settings.

Sent with Proton Mail secure email.

------- Original Message ------- On Monday, October 2nd, 2023 at 16:14, Stéphane Bidoul @.***> wrote:

@.(https://github.com/sbidoul), @.(https://github.com/layday), @.***(https://github.com/henryiii) do you believe that the best we can do right now is to only pass --build-option to bdist_wheel and ignoring it in any other command?

I don't have experience with these parameters either, but what I can tell is that pip only passes it's --build-option to the bdist_wheel command. So yes, I think what you propose is a good stop gap until a broader solution is designed.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.Message ID: @.***>

sbidoul commented 1 year ago

In my understanding the --global-option and --build-option config settings have been designed to be direct PEP 517 equivalents to the pip --global-option and --build-option flags.

When pip invokes setup.py directly, it passes --global-option values to all commands. --build-option have only ever been passed by pip to setup.py bdist_wheel (and only when used in the pip wheel command).

So from that point of view it makes sense for the setuptools backend to only pass the --build-option config settings to bdist_wheel command, no? The --global-option settings would continue to be passed to all commands.

With that little change we'd have simple and clear transition path for users when pip drops the legacy setup.py invocations.

layday commented 1 year ago

I don't know why the PEP 517 options in setuptools were modeled on pip, but the main audience here isn't people transitioning from legacy pip installs to isolated pip installs - it's people transitioning from invoking setup.py directly, to build. A --build-option which only works with bdist_wheel is not an adequate replacement.

sbidoul commented 1 year ago

A --build-option which only works with bdist_wheel is not an adequate replacement.

I don't disagree a better mechanism for setuptools config settings is needed (and I personally have no idea what it should be - I'll leave that to setuptools experts), but that should not prevent a short term fix for the --build-option setting, which is clearly broken as it is today, since it crashes the backend.

abravalheri commented 1 year ago

I don't know why the PEP 517 options in setuptools were modeled on pip, but the main audience here isn't people transitioning from legacy pip installs to isolated pip installs - it's people transitioning from invoking setup.py directly, to build. A --build-option which only works with bdist_wheel is not an adequate replacement.

Right now, I don't think we are in a good position even to "paper over the cracks". The situation with egg_info/dist_info "idempotency" (among the multiple direct/indirect calls triggered by PEP 517/PEP 660 hooks in different sub-processes) is difficult to deal with.

Solving this and related problems is one of the major steps in creating a more definitive solution[^1]. So if the most immediate concern is to offer a solution for people migrating from direct running setup.py, I don't think is worth bothering with an intermediate stop-gap solution[^2][^3] and we just go with #3896.

On the other hand, if the most immediate concern is to offer a solution for compatibility with pip legacy flags, we can easily create a PR that ignores config_settings["--build-option"] for all the PEP 517 hooks except setuptools.build_meta:build_wheel.

[^1]: I did a summary of current state and challenges here: https://github.com/pypa/setuptools/issues/3896#issuecomment-1656714771.

[^2]: Again, designing a more definitive solution is difficult and can take time. In the discussion in https://github.com/pypa/setuptools/issues/3896#issuecomment-1656714771 and https://github.com/pypa/setuptools/issues/3896#issuecomment-1708438587, I highlighted on how I would go about it, but I invite any member of the community to discuss and contribute.

[^3]: The best thing people looking for an alternative to python setup.py arguments can do right now is to modify setup.cfg or use the DIST_EXTRA_CONFIG env var.

sbidoul commented 1 year ago

On the other hand, if the most immediate concern is to offer a solution for compatibility with pip legacy flags, we can easily create a PR that ignores config_settings["--build-option"] for all the PEP 517 hooks except setuptools.build_meta:build_wheel.

This may unlock a workaround to #3237. --config-setting=--build-option=--bdist-dir=/tmp/build

Edit: hm, or not, due to the .egg-info directory.

abravalheri commented 1 year ago

I was trying to see if we have further agreement on this, but I guess I can prepare a PR for that ... (update: #4079)

Edit: hm, or not, due to the .egg-info directory.

That is the thing... .egg-info gets called implicitly by other commands, so we need a persistent way of setting the parameters between the different independent calls to multiple processes.

Also some packages will simply not work if .egg-info is not on their project root, because they depend on importlib.metadata finding it there... (like setuptools itself - see https://github.com/pypa/setuptools/issues/3921#issuecomment-1543881963 and the following comment by Jason).

abravalheri commented 1 year ago

I opened a discussion in https://github.com/pypa/setuptools/discussions/4083 for a more long-term approach.

jameshilliard commented 8 months ago

that should not prevent a short term fix for the --build-option setting, which is clearly broken as it is today, since it crashes the backend.

It seems that #4079 made --build-option completely unusable for passing flags to commands like build_ext.

So if the most immediate concern is to offer a solution for people migrating from direct running setup.py, I don't think is worth bothering with an intermediate stop-gap solution23 and we just go with https://github.com/pypa/setuptools/issues/3896.

This is exactly our use case which was completely broken by #4079, I've opened a PR reverting it in #4218 as #4079 seems to clearly be a regression.

abravalheri commented 8 months ago

Hi @jameshilliard, I just pushed the bugfix you provided in v69.1.1, once the CI is done you can see if that works for you. Thank you very much for reporting the problem and working on the fix.

/fyi @sbidoul, the change in #4217 should not introduce problems for pip, but we never known... Please let me know if you find problems.