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

BUG: Fix setup.py build install egg_info, which did not previously build #10732

Merged
merged 1 commit into from
Mar 13, 2018

Conversation

eric-wieser
Copy link
Member

@eric-wieser eric-wieser commented Mar 12, 2018

Fixes #10646

It seems the original intent was to disable builds when only the egg_info target is requested, not whenever it is one of the targets.

It's kinda gross that we're adding a second layer of (fragile) argument parsing to setuptools here, but that something to address another time.

Not a regression, but it would be nice to slip it into the next release.

…build

Fixes numpy#10646

The original intent was to disable builds when _only_ the egg_info target is requested, not whenever it is one of the targets.
@eric-wieser eric-wieser added component: build 09 - Backport-Candidate PRs tagged should be backported labels Mar 12, 2018
@eric-wieser eric-wieser added this to the 1.14.2 release milestone Mar 12, 2018
@rgommers
Copy link
Member

Still don't think such combined commands are correct usage, but this change also can't hurt. So fine with me to merge.

@charris charris added 00 - Bug and removed 09 - Backport-Candidate PRs tagged should be backported labels Mar 13, 2018
@charris charris removed this from the 1.14.2 release milestone Mar 13, 2018
@charris charris merged commit 07fbd14 into numpy:master Mar 13, 2018
@charris
Copy link
Member

charris commented Mar 13, 2018

Thanks Eric.

@eric-wieser
Copy link
Member Author

@rgommers: I thought that too, but it's documented behavior:

$ python setup.py -h
<snip>
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

@eric-wieser eric-wieser deleted the chained-setuptools-command branch March 13, 2018 17:46
@rgommers
Copy link
Member

@rgommers: I thought that too, but it's documented behavior:

I know, but so are all the commands we explicitly disallow. My point was not what's an "official feature" but what makes sense to use/support/allow. The only common build chaining in the wild is build install, anything else is "use at your own risk" I think.

@eric-wieser
Copy link
Member Author

eric-wieser commented Mar 16, 2018

Unfortunately a proprietary build system I was using invokes setup.py with that incantation, and the authors seemed to think that they were justified in doing so in case state is needed between commands.

I don't buy that argument, but numpy's behavior here looks to have been unintended, so a fix seemed sensible regardless

@rgommers
Copy link
Member

Proprietary systems, always fun :)

No the behavior was not intended to be broken, such things are pretty much dont-cares given that it's untestable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants