[FR] Expose {package} placeholder to the build stage

[webknjaz]

Description

I mentioned before that I set the PIP_CONSTRAINT env var when building wheels, to improve the reproducibility (#1666).

I got that integrated into yarl, and it works well when I use my https://github.com/re-actors/checkout-python-sdist action to check out the project from sdist instead of Git.

Later on, I learned that cibuildwheel can consume *.tar.gz files directly, so I figured why not try that out. There's no huge practical gain in my case, since the cibuildwheel action combined with my action already achieves this. And this doesn't reduce the number of steps in the job, just replaces one thing with another. But I wanted to see it in action.

Long story short, it didn't actually work. The first obstacle was figuring out how to pass an sdist into the action. I read the source and found out that there's an input called package-dir for passing sdists 🤯 (#1682). I replaced my action with a simple download, but I only had a wildcard for the tarball name — and the action quotes the input internally so it wouldn't be auto-expanded. So I had to add some supporting code to look up the actual sdist filename (which is fine — I wanted to do that at some point anyway).

I thought, that would be the end of it, but it crashed on the build step, with all the above setup! Apparently, since I was no longer checking out the project to the CWD, the relative path in the PIP_CONSTRAINT variable was pointing to a non-existent location 🤷‍♂️
Oh, well, I thought I'd find something in the docs. And I did find mentions of some placeholders. I tried out {project} (confusing where it's supposed to point to) and {package} but the internal pip install was still reporting a “file not found”, with those placeholders remaining non-rendered, as is.

Later, I found the notes at the very bottom of the options page, mentioning that not all settings interpolate values. And realized that maybe, it's just not implemented.

So here I am, filing this feature request to make it work. While doing so, I realized that while implementing this (with the placeholder pointing to a temporary directory where the sdist is unpacked) will likely fix it for me (unless, new issues arise at later stages, like having to switch the tests dir path to {package}, I suppose).
But then, is it really worth it? Is it the best UX? After all, the thing I had was already doing what I needed, following KISS / DRY and typical *NIX composability considerations. Maybe, cibuildwheel (the action, not the PyPI dist!) should really delegate this to checkout-python-sdist instead of complicating the setup. Or, maybe, it should just call the action internally, bypassing the corresponding inputs there. WDYT?

The PR is here, if you're curious: aio-libs/yarl#967. Though, I'll probably keep using my action that is a bit more generic, and I use it in other jobs (like tests) as well.

Build log

No response

CI config

No response

[joerick]

When cibuildwheel builds an sdist, it changes working directory to the expanded tarball before doing the build. So, I would expect a relative path in PIP_CONSTRAINT to (kinda) work - the issues in #1675 notwithstanding.

When building an sdist, {project} and {package} are the same, the only time they're different is when the user runs cibuildwheel on a subdirectory of a project.

Given that bashlex is already parsing our CIBW_ENVIRONMENT option, I'd be inclined not to add the curly-brace expansion there too, the bash syntax is complex enough as it is :) There is perhaps some argument to expand the env vars set by cibuildwheel to include CIBUILDWHEEL_PROJECT and CIBUILDWHEEL_PACKAGE, so they can be referenced by other variables.

Having said all that I would still expect a relative path to work. I can't see the commit that tried that in your PR, perhaps due to a force-push?

[webknjaz]

Having said all that I would still expect a relative path to work. I can't see the commit that tried that in your PR, perhaps due to a force-push?

Yeah, that was the very first iteration. Let me double-check, then.

[webknjaz]

So it's failing trying to provision an ephemeral PEP 517 build env: https://github.com/aio-libs/yarl/actions/runs/7013832623/job/19080617779?pr=967#step:5:273.

[joerick]

ah, got it. Then I wonder if build is changing the CWD...

[joerick]

Ah, nope, that's not it. Actually my statement above is incorrect.

cibuildwheel/cibuildwheel/__main__.py

Line 163 in 4cedf1c

with chdir(temp_dir):

cibuildwheel doesn't chdir to the expanded sdist, it chdirs one level up. Looks like a bug to me, I can't think why anyone would want that - the temp_dir is an implementation detail of cibuildwheel.

If you have time to put in a PR for that that would be appriciated.