Setup python extracted from build function

[Czaki]

For windows and macos extract python setup to separate function and rename windows.shell to windows.call to unify build function between systems.

Main advantage of this PR is separate build wheel logic from python setup logic. Build function is shorter and easier to analyse.

This also allow to simplify user porting on CI which are not possible to test from github like gitlab (#158)

[joerick]

I don't like the multiple return here, the env makes a lot of sense, but the dependency_constraint_flags seems too trivial. Could that go elsewhere? I think a method DependencyConstraints.pip_flags(python_version) would neaten this up.

[YannickJadoul]

Isn't it even better to resolve dependency_constraints.get_for_python_version(python_configuration.version) before setup_python, and pass it as an argument?

Now, dependency_constraints is passed, and dependency_constraint_flags is returned, but that's just a remnant of cutting and pasting the code from before. There's no semantic reason why setup_python would "produce" dependency_constraint_flags.

[Czaki]

I move dependecy_constrains_flags outside python_setup function.

[joerick]

call() on macOS doesn't use a shell by default, here Windows always does. I think that's why the names are different. If we're changing it, I'd recommend using a shell only where necessary - when running user commands.

[YannickJadoul]

It might actually be nice to keep the distinction, then?

[Czaki]

it is not possible because of this bug https://bugs.python.org/issue8557 (with shell=False environment variable PATH passed with env=env is ignored).

Change name allow to use same build function for both systems.

[joerick]

Ah, that is a problem. Probably that means we should stick with shell on Windows.

[Czaki]

Ok. I revert this change.

[YannickJadoul]

To be fair, personally, I don't really see the advantage of moving that one piece of code to its own function. We're not calling it multiple times, anyway, so it seems to only clutter up stuff more, as demonstrated by the multiple return values, no?

[YannickJadoul]

This also allow to simplify user porting on CI which are not possible to test from github like gitlab (#158)

I don't really understand how this is going to be easier, either?

[Czaki]

This changes are connected with my propositions from #281.

To be fair, personally, I don't really see the advantage of moving that one piece of code to its own function. We're not calling it multiple times, anyway, so it seems to only clutter up stuff more, as demonstrated by the multiple return values, no?

For me first reason is that function build is to long. So I suggest to extract from it thinks not connected with build.

This also allow to simplify user porting on CI which are not possible to test from github like gitlab (#158)

I don't really understand how this is going to be easier, either?

def custom_main():
    from cibuildwheel import windows
    windows.setup_python = custom_setup

    from cibuildwheel.__main__ import main
    main()

For example someone have installed python version and would like to setup python create virtualenv instead of install new python versions on machine.

[YannickJadoul]

This changes are connected with my propositions from #208

I know, but I wasn't really convinced before, either. Anyway, just voicing my opinion; it's up to @joerick to decide

For me first reason is that function build is to long. So I suggest to extract from it thinks not connected with build.

I don't see how it's too long, because for me needing to go up and down might be more annoying than just seeing the series of commands. But that's just taste, I guess; I'm not saying this is horrible, either :-)

def custom_main():
    from cibuildwheel import windows
    windows.setup_python = custom_setup

    from cibuildwheel.__main__ import main
    main()

For example someone have installed python version and would like to setup python create virtualenv instead of install new python versions on machine.

Pffff, we don't have a public API, so let's not count on this! If we want to allow these kinds of things, lots of other things need to happen on the level of architecture and design. For now, if anyone wants a custom cibuildwheel, it'll have to be a fork anyway, I suspect.

[joerick]

For me, the neatness of having a setup_python() that returns an env with the correct python available is pretty neat. (Also, seeing it like that, you could imagine chucking an lru_cache on there and potentially only having to run that function a few times per build, which could speed up our tests a bit. We'd need to eliminate any global state, though.)

[Czaki]

For me, the neatness of having a setup_python() that returns an env with the correct python available is pretty neat. (Also, seeing it like that, you could imagine chucking an lru_cache on there and potentially only having to run that function a few times per build, which could speed up our tests a bit. We'd need to eliminate any global state, though.)

lru_cache will not work if we ftill call next tests with subprocess.check_call in cibuildwheel_run from test/shared/uitils.

But I see other option for optimalization. Wrote code which will check environment and avoid obsolete pip install call?

[YannickJadoul]

But I see other option for optimalization. Wrote code which will check environment and avoid obsolete pip install call?

pip install takes only a few seconds, but the whole setup is already fragile, sometimes. I don't think that's going to be worth it?

[Czaki]

But I see other option for optimalization. Wrote code which will check environment and avoid obsolete pip install call?

pip install takes only a few seconds, but the whole setup is already fragile, sometimes. I don't think that's going to be worth it?

Extract to separate function should simplify of profiling. We could test how many times it consume in next steps.

[joerick]

lru_cache will not work if we ftill call next tests with subprocess.check_call in cibuildwheel_run from test/shared/uitils.

Good point - it's a subprocess, so the cache would be thrown away each time.

Plus, there is tons of global state - the python installs themselves, and the whole filesystem, which could be modified by CIBW_ENVIRONMENT. Erm, so yeah, not my finest idea 😳!

pip install takes only a few seconds, but the whole setup is already fragile, sometimes. I don't think that's going to be worth it?

It could be pip install, it could be pip wheel, I honestly don't know! It's a bit of a mystery to me why our tests are so slow. Line profiling would be helpful.