classification
Title: bdist doesn’t pass --skip-build on to subcommands
Type: behavior Stage: resolved
Components: Distutils, Distutils2 Versions: Python 3.2, Python 3.3, Python 2.7, 3rd party
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: eric.araujo, kern, loewis, orsenthil, python-dev, radious, tarek, ygingras
Priority: normal Keywords: patch

Created on 2011-01-19 15:32 by eric.araujo, last changed 2011-09-19 13:33 by eric.araujo. This issue is now closed.

Files
File name Uploaded Description Edit
skip_build.patch radious, 2011-04-16 22:32
a099e7001aa8.diff eric.araujo, 2011-06-12 10:05 review
fix-bdist-skip-build.diff eric.araujo, 2011-08-29 16:08
Repositories containing patches
https://bitbucket.org/radious/cpython#skip_build_fix
Messages (11)
msg126530 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-01-19 15:32
Yannick has found that bdist_* commands don’t get the --skip-build option from bdist.  This should be solved easily thanks to set_undefined_options.

Adding people from #414775 (r25075) to nosy.
msg126578 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-01-20 01:55
I added the required calls to pass skip-build from bdist to bdist_* but that isn’t enough to make the test from https://bitbucket.org/ygingras/distutils2/changeset/71f5c39c9100 pass.  build gets somehow called, even though install, install_lib, bdist and bdist_dumb have skip_build set to 0.
msg133907 - (view) Author: Wojciech Wojtyniak (radious) Date: 2011-04-16 22:32
Could you please at least describe the test that you mentioned (link doesn't work). I added line that passes skip_build from bdist.py to its subcommands and it worked for me (at least for simple cases) - base.py is not called.

If bdist wouldn't call any install_* in other way then via bdist_*, then we could use set_undefined_options in the last ones, but I'm not sure if it's safe. (If it is, I can provide appropriate patch.) 

Using set_undefined_options in install_* is definitely not safe, because it fails if parent process has not set skip_build. Also skip_build has to be set to None in install_* (set_... needs that to work) which obviously breaks it. Actually bdist_* sets skip_build like in my patch.

Patch attached.

Anyway test_skip_build() is still broken and needs a fix.
msg133986 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-04-18 17:42
> Could you please at least describe the test that you mentioned (link
> doesn't work).
I still have a local copy \o/.  Yannick’s patch looks like this:

    def test_skip_build(self):
        pkg_pth, dist = self.create_dist()

        # With the skip_build option, command.build is not called.
        cmd = bdist(dist)
        cmd.skip_build = True
        cmd.ensure_finalized()
        cmd.run()
        self.assertFalse(dist.have_run['build'])

        # Without skip_build option, command.build is called.
        cmd = bdist(dist)
        cmd.skip_build = False
        cmd.ensure_finalized()
        cmd.run()
        self.assertTrue(dist.have_run['build'])

> I added line that passes skip_build from bdist.py to its subcommands
> and it worked for me (at least for simple cases)
What about non-simple cases?  They need to work too.

>- base.py is not called.
(Assuming you meant “build is not called”)

> Using set_undefined_options in install_* is definitely not safe,
> because it fails if parent process has not set skip_build.
Really?  Also, what is “parent process”?  Do you mean that it fails if the command used as source in set_undefined_options does not define the skip_build option in its user_options?

I’m not sure it’d be good to make the install_* skip_build option depend on (== set_undefined_options from) bdist’s skip_build.  I’ve written a bit of code to make testing interaction between commands easier (for example, to test that “setup.py build bdist --skip-build install” runs the second install command without skip_build set to True), I should put it in the main repo so that it can be used for this bug.  Getting good test coverage before we change code is absolute requisite for those dark areas.  Removing the easy keyword :)

> Anyway test_skip_build() is still broken and needs a fix.
Okay, so maybe fix that first.
msg138202 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-06-12 10:07
I’ll edit the patch to use set_undefined_options.
msg143153 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-08-29 16:08
The fix was actually very simple.  I have committed it to my 3.2 repo and will push later.
msg143212 - (view) Author: Roundup Robot (python-dev) Date: 2011-08-30 14:22
New changeset 326a7e44bb66 by Éric Araujo in branch '3.2':
Make bdist_* commands respect --skip-build passed to bdist (#10946)
http://hg.python.org/cpython/rev/326a7e44bb66

New changeset 2f69b4f3df2e by Éric Araujo in branch 'default':
Merge fix for #10946 from 3.2
http://hg.python.org/cpython/rev/2f69b4f3df2e

New changeset afb12c6b79ba by Éric Araujo in branch 'default':
Make bdist_* commands respect --skip-build passed to bdist (#10946).
http://hg.python.org/cpython/rev/afb12c6b79ba
msg143213 - (view) Author: Roundup Robot (python-dev) Date: 2011-08-30 14:26
New changeset 4ff92eb1a915 by Éric Araujo in branch '2.7':
Make bdist_* commands respect --skip-build passed to bdist (#10946)
http://hg.python.org/cpython/rev/4ff92eb1a915
msg143218 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-08-30 14:51
I simplified my patch and pushed it.  I had to discover again that I needed to inject customized command objects into dist.command_obj, like you found out a few months ago when we had a private email discussion :)

> Using set_undefined_options in install_* is definitely not safe,
> because it fails if parent [command] has not set skip_build.
We can be sure (by looking at the code) that bdist always sets skip_build to something (False by default), so it’s not unsafe.

> Also skip_build has to be set to None in install_* (set_... needs
> that to work) which obviously breaks it.
Yes, skip_build on the subcommands needed to change from None to False, but I don’t see how it is a breakage; it’s just an harmless change needed for this bug fix, so I did it. :)

Thanks to everyone involved in this bug.
msg143485 - (view) Author: Roundup Robot (python-dev) Date: 2011-09-04 06:56
New changeset a78629c62ee8 by Éric Araujo in branch '3.2':
Make bdist_* commands respect --skip-build passed to bdist (#10946)
http://hg.python.org/cpython/rev/a78629c62ee8
msg144269 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-09-19 13:33
Fixed in distutils2 in 95172b1c5498.
History
Date User Action Args
2011-09-19 13:33:17eric.araujosetmessages: + msg144269
2011-09-04 06:56:36python-devsetmessages: + msg143485
2011-08-30 14:51:26eric.araujosetstatus: open -> closed
resolution: fixed
messages: + msg143218

stage: commit review -> resolved
2011-08-30 14:26:27python-devsetmessages: + msg143213
2011-08-30 14:22:11python-devsetnosy: + python-dev
messages: + msg143212
2011-08-29 16:08:14eric.araujosetfiles: + fix-bdist-skip-build.diff

messages: + msg143153
stage: test needed -> commit review
2011-06-12 10:07:09eric.araujosetmessages: + msg138202
versions: + Python 3.3, - Python 3.1
2011-06-12 10:05:56eric.araujosetfiles: + a099e7001aa8.diff
2011-06-12 10:04:45eric.araujosethgrepos: + hgrepo27
2011-05-04 12:21:30orsenthilsetnosy: + orsenthil
2011-04-18 17:42:24eric.araujosetkeywords: - easy

messages: + msg133986
stage: needs patch -> test needed
2011-04-16 22:32:59radioussetfiles: + skip_build.patch

nosy: + radious
messages: + msg133907

keywords: + patch
2011-01-20 01:55:00eric.araujosetnosy: loewis, kern, tarek, ygingras, eric.araujo
messages: + msg126578
2011-01-19 15:33:13eric.araujosetnosy: + tarek
2011-01-19 15:32:47eric.araujocreate