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

Add setuptools.command.build #3256

Merged
merged 8 commits into from
Jun 13, 2022
Merged

Add setuptools.command.build #3256

merged 8 commits into from
Jun 13, 2022

Conversation

isuruf
Copy link
Contributor

@isuruf isuruf commented Apr 7, 2022

Summary of changes

In order to override distutils.command.build on downstream projects
it is good to have a setuptools specific command which allows
downstream projects to avoid importing distutils.

Closes

Pull Request Checklist

isuruf and others added 4 commits April 7, 2022 14:32
In order to override distutils.command.build on downstream projects
it is good to have a setuptools specific command which allows
downstream projects to avoid importing distutils.
@abravalheri
Copy link
Contributor

Thank you very much @isuruf, sorry for the confusion with the extra commits, I was trying to fix the error with flake8, I hope that is OK with you.

I am OK with this change, but I will let the other maintainers have a look because I don't know if there are other implications.

@isuruf
Copy link
Contributor Author

isuruf commented May 10, 2022

Ping on this

@abravalheri
Copy link
Contributor

abravalheri commented Jun 9, 2022

Thank you very much @isuruf and sorry for the delay.
Since no one has found any problem with this implementation, I believe we can go ahead...

Should we add any protection/migration warnings for the case an user was previously relying on distutils.command.build to setup subcommands?

For example, how about something like:

_ORIGINAL_SUBCOMMANDS = {"build_py", "build_clib", "build_ext", "build_scripts"}

class build(_build):
    sub_commands = _build.sub_commands[:]  # copy to avoid sharing the object with parent class

    def run(self):
       subcommands = {cmd[0] for cmd in _build.sub_commands}
       if subcommands - _ORIGINAL_SUBCOMMANDS:
           msg = """
           It seems that you are using `distutils.command.build.build` to add new subcommands.
           Using `distutils` directly is considered deprecated, please use `setuptools.command.build`.
           """
           warning.warns(msg, SetuptoolsDeprecationWarning)
           self.subcommands = _build.sub_commands
       super().run()
  ...

What do you think?

@isuruf
Copy link
Contributor Author

isuruf commented Jun 9, 2022

Makes sense to me. There's a small typo in the line before last. self.subcommands should be self.sub_commands. Can you push into this PR or should I?

@abravalheri
Copy link
Contributor

I can do that later today, but if you would like to go ahead and work on it, that would be great (since I would have to spend some time to create a simple test case).

@abravalheri
Copy link
Contributor

Hi @isuruf, I tried to fix the tests failing in the CI.
Please feel free to revert my changes and do something different.

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.

2 participants