Activity

1. aignas commented on Oct 24, 2024

   

   aignas

   on Oct 24, 2024

   Collaborator

   More actions

   It looks like it is working as intended to be honest. If the sdist needs numpy to be successfully built, then it needs to find it somewhere. If numpy is not installed somewhere where it can leak into the build process of the wheel then, there is no way for us to take it from the requirements.txt, because the build of the sdist is separate from the rest of the requirements.txt file here.
2. mark-thm commented on Oct 24, 2024

   

   mark-thm

   on Oct 24, 2024

   ContributorAuthor

   More actions

   Could you help me understand a bit more about why this works in rules_python 0.34.0 but no longer works in later versions? I've been tracking the various wheel arg changes but tbh I skipped over understanding the more fundamental changes shipped in 0.35.0 with whl_library builds.

   Separately, I'm at a bit of a loss for how to move forward -- it seems like rules_python will be generally incompatible with any sdists that have dependencies outside of the blessed set of build deps that rules_python normally brings in, which leaves the options as something like:

   • figure out how to build this sdist outside of Bazel, publish it somewhere, and convince rules_python to fetch it from there
   • figure out how to get the main build of whatever dependencies are incompatible with rules_python to publish wheels for additional os/arch combinations
   • switch to rules_pycross

   Something here will probably work for me, for now, but IMO this is really raising the bar for adopting Bazel for Python.
3. mark-thm commented on Oct 24, 2024

   

   mark-thm

   on Oct 24, 2024

   ContributorAuthor

   More actions

   Alternatively, I'm able to produce a wheel successfully if instead of running pip wheel I run pip install (which will download, and, if necessary, build numpy), and a wheel is produced as a byproduct. That seems like it might produce a more intuitive result for cases like this.
4. rickeylev commented on Oct 25, 2024

   

   rickeylev

   on Oct 25, 2024

   Collaborator

   More actions

   If a C extension is being created, then the standard advice is that the project should be providing a wheel. Building C extension from source can be quite particular to projects.

   Otherwise, I think the other options are:

   • Provide a way to inject additional dependencies into an sdist build, or otherwise have more control over how an sdist is built -- i.e. something in pip.override()
   • Provide a way so that the generated code can plumb through the install-time dependencies, so that the later sdist building can work. I'm not sure if this can be made to Just Work. Best I can think of is if a bzlmod extension is given the sdist url, downloads the sdist, inspects it, and can then pass along the install-time packages to the later "regular" pypi codegen. (You can see how this might get messy quickly).
5. groodt commented on Oct 25, 2024

   

   groodt

   on Oct 25, 2024

   Contributor

   More actions

   Separately, I'm at a bit of a loss for how to move forward -- it seems like rules_python will be generally incompatible with any sdists that have dependencies outside of the blessed set of build deps that rules_python normally brings in

   I'm actually interested in how it was working at all in the first-place myself. It does indicate some sort of non-hermetic scenario in previous versions which have now been made to be hermetic...
6. groodt commented on Oct 25, 2024

   

   groodt

   on Oct 25, 2024

   Last edited by groodt

   Contributor

   More actions

   Alternatively, I'm able to produce a wheel successfully

   What you can do in this instance @mark-thm , depending on your specific environment and supply chain requirements. Is to upload the wheel to a github release and use direct_url references.

   So something like:
   bottleneck @ https://github.com/foo/bar/releases/download/v2.6.3/bottleneck-1.4.2-cp312-cp312-manylinux_2_17_aarch64.manylinux2014_aarch64.whl#sha256=d6f27c61f640ce0c4e4eb140eb7e53e57e48d87e7a336e0e31c777f5ca13e844

   (I just guessed the wheel tags in the example. They might be different based on your specific ABI etc)
7. groodt commented on Oct 25, 2024

   

   groodt

   on Oct 25, 2024

   Contributor

   More actions

   The python ecosystem is looking to partially solve some of this in the lockfile standard, but it gets quite gnarly. Different sdist might have different build-time dependencies for example. So you might need to be provided multiple independent build environments. And then some build environments take dependencies on the exec platform if they link out to OS libraries etc. So it becomes quite the rabbithole at the limit.

   The good news is that the vast majority of major python dependencies are already wheels: https://pythonwheels.com/

   And that tools like https://cibuildwheel.pypa.io/en/stable/ exist to help project authors package wheels for most popular platforms.

   sdist are inevitable though, but the ones that aren't pure-python are becoming more and more rare in my experience.

   👍React with 👍1Reacted by Richard Levasseur
8. mark-thm commented on Oct 25, 2024

   

   mark-thm

   on Oct 25, 2024

   ContributorAuthor

   More actions

   bottleneck already uses cibuildwheel, but hasn’t set up emulation to publish arm64 wheels because the CI system doesn’t run on arm64 (because GitHub doesn’t yet provide OSS Linux/arm64 runners). I’ve made a PR to add wheel builds but the maintainers (very reasonably) want to also run tests. That’s a bit trickier, I’m not sure there’s a great way to do this for low maintenance cost and for free.

   I think what feels tricky about rules_python’s current behavior here is a couple of things:

   1. The failure is cryptic, requiring a pretty nuanced understanding of what’s going on
   2. pip installing this dependency works fine

   Thus the state of the world is that nobody has done anything wrong (you’re saying expected behavior, dependency maintainer has correctly encoded build time dependencies, pip wheel is behaving as expected), and yet my build is failing.

   But I do think there’s a broken assumption here: pip wheel demands that you parse the pyproject.toml file and supply dependencies, and that’s not what’s happening here.

   pip install is a viable alternative that will fetch and build missing dependencies for the build, but that wont use the Bazel downloader (and it seems like rules_python is trying to take control of that interaction).

   Separately: yes, I can build all my deps into wheels and publish these things somewhere—my normal build orchestration tool is Bazel, so the task ahead is to make something like what rules_python does: parse a requirements files, download sdists, build those sdists using pip, and then also publish those files somewhere.

   A couple of ideas:

   • instead of parsing pyproject.toml files, support a non-hermetic option of using pip install in place of pip wheel, allowing it to download and build deps (this can be done somewhat sandboxed at the cost of possibly exponentially many builds of common deps)
   • build towards a build graph of deps and a runtime graph of deps, where the build graph wires up the actual build deps as parsed from sdists and the runtime graph is the declared wheel level deps (this seems like a massive project, though)

   👍React with 👍1Reacted by Greg Roodt
9. groodt commented on Oct 25, 2024

   

   groodt

   on Oct 25, 2024

   Last edited by groodt

   Contributor

   More actions

   Yes, that’s a good summary on the issue and possible solutions. We’ve had many similar discussions on issues and as maintainers.

   1. The failure is cryptic, requiring a pretty nuanced understanding of what’s going on

   It’s also a reflection of the current python packaging ecosystem realities. Even outside bazel the community struggles with these similar issues. Your upstream PR demonstrates that it isn’t even clear if upstream bottleneck is supported on arm64 by the maintainers. It’s nobody’s fault, rather the tragedy of the commons.

   The python packaging community has considered disabling sdist by default as a mitigation. Thats the origin of the message “this is likely not a problem with pip”. A similar framing could be taken “this is likely not a problem with bazel/rules_python” (But I’m still really curious how this issue only occurred upon update of the rules. I’m not sure we got to the bottom of what changed?)

   Emerging lockfile standards, wheel format improvements, index metadata, disabling sdists and installers like uv are all improving. rules_python is downstream of all these things and is also improving. These things just take time.

   2. pip installing this dependency works fine

   Are we sure that “pip wheel” outside bazel fails too? I don’t think it will. If I had an arm64 machine I could test it. My understanding of the PEP 517 build hooks is that they are invoked for both pip install and pip wheel.

   The problem here boils down essentially to: “running other people’s build makefiles”. So it’s a bit unrealistic to solve completely in the presence of non-hermetic, Turing complete systems. Stating that “pip install works” is the same as saying “works on my machine”.

   ---

   I think as you’ve mentioned, the workarounds in order of effort for you are probably:

   • Upstream bottleneck project starts publishing wheels for your platform
   • You build a bottleneck wheel out of band and store the wheel somewhere accessible to your lockfile producing system and your build system. Tools like https://github.com/python-wheel-build/fromager or https://cibuildwheel.pypa.io/en/stable/ might be useful
   • Switch to alternative rules like rules_pycross or rules_py that may have better support for sdist at the moment
   • Build your own bazel integration for sdist (and consider contributing them to rules_python)
10. mark-thm commented on Oct 25, 2024

    

    mark-thm

    on Oct 25, 2024

    Last edited by mark-thm

    ContributorAuthor

    More actions

    Are we sure that “pip wheel” outside bazel fails too?

    It fails for me, yes: replicating the pip wheel command with args outside of Bazel using a Linux arm64 machine with a fresh venv and no system level deps installed results in an identical error.

    Stating that “pip install works” is the same as saying “works on my machine”.

    I’m not an expert here, but I disagree with this. In particular, I’m trying to call attention to the differences in behavior of pip wheel and pip install. What I observe:

    • pip wheel assumes something external has satisfied build dependencies, and then builds a wheel
    • pip install will analyze declared build system dependency information and then do the work required to make those dependencies available (by downloading or downloading and building build system dependencies)
11. groodt commented on Oct 25, 2024

    

    groodt

    on Oct 25, 2024

    Last edited by groodt

    Contributor

    More actions

    That’s a misunderstanding I believe. I’ll try a repro on a non arm64 machine to demonstrate. Both subcommands invoke build backends hooks. eg. They both offer “—no-build-isolation” arguments.

    Indeed, before pip installs any sdist dependency, a wheel is always built first.
12. groodt commented on Oct 25, 2024

    

    groodt

    on Oct 25, 2024

    Last edited by groodt

    Contributor

    More actions

    Looking at the full wheel command executed in the trace, I think I know why pep517 build hooks aren’t pulling down the build dependencies declared in the bottleneck pyproject.toml.

    The “—no-build-isolation” option is provided to the wheel command. Removing that will probably fix this sdist, but might break others.
13. mark-thm commented on Oct 26, 2024

    

    mark-thm

    on Oct 26, 2024

    ContributorAuthor

    More actions

    Ok, can confirm that removal of --no-build-isolation allows this wheel to build with pip wheel ... in which case I think we're still in a bind because it seems without that arg you don't pick up the build-time deps that Bazel supplies?

    👍React with 👍1Reacted by Greg Roodt
14. groodt commented on Oct 27, 2024

    

    groodt

    on Oct 27, 2024

    Last edited by groodt

    Contributor

    More actions

    in which case I think we're still in a bind because it seems without that arg you don't pick up the build-time deps that Bazel supplies?

    Not necessarily. The majority of actively maintained projects should be using PEP-517 metadata to fully specify and configure their build dependencies (build-system.requires) and build backend (build-system.build-backend) e.g. flit, setuptools, hatchling, maturin etc. This lets the wheels build in isolated virtual environments. These are well-behaved builds. Not well-behaved in a bazel sense mind you, merely well-behaved in the sense that these sdist have a slightly better chance of a successful build on another machine because they declare some of their build dependencies.

    However, there are also a lot of builds that aren't well-behaved. That is why build frontends like uv and pip provide parameters like --no-build-isolation. So that these non-well-behaved builds can build in the current environment (and hopefully the environment has the necessary dependencies for a successful build).

    ---

    I've had a think and I think a good path forward to avoid disappointment, and to make expectations clear to users of the rules who have sdist in their dependency closures, might be to do the following:

    • Fail fast when encountering an sdist (and printing out a helpful error message that "sdist are opt-in and fragile. With links to docs on what to do."
    • Let users declare "Named build environments in bazel with a lockfile of dependencies". There can be 1 or more of these.
    • For an sdist to be enabled for best-effort build, it should have an entry that maps the name of the sdist to a named build environment
    • The build for the sdist then occurs with --no-build-isolation but inside the named build environment

    So in this bottleneck instance, it might look like this:

    Named build environment: bottleneck-build
    Build dependencies: [setuptools, versioneer, wheel, numpy]
    In MODULE.bazel something like:

    pip.override(
      hub_name = "pypi",
      package = "bottleneck",
      sdist_build_environment = "@bottleneck-build",
    )
    

    ---

    Now, that might all be some future implementation. For now, I don't see a strong reason for not removing --no-build-isolation. I don't remember why it's the default. WDYT @aignas ?
15. aignas commented on Oct 28, 2024

    

    aignas

    on Oct 28, 2024

    Collaborator

    More actions

    Yeah, removing --no-build-isolation for now would be better. Users can also opt-into that in other ways.

    Regarding the named build envs, it is a nice sketch and could solve majority of the issues that we are facing these days.
16. mark-thm commented on Oct 30, 2024

    

    mark-thm

    on Oct 30, 2024

    ContributorAuthor

    More actions

    Pushed #2360

    👍React with 👍1Reacted by Ignas Anikevicius
17. github-merge-queue

    added a commit that references this issue on Oct 31, 2024

    fix: Remove --no-build-isolation from pip install command (#2360)

    ...

    This issue will close once commit b6fc2a0 is merged into the 'main' branch.

    b6fc2a0
18. aignas

    closed this as completedin #2360on Oct 31, 2024
19. aignas

    self-assigned this

    on Mar 1, 2025