Skip to content

Conversation

aignas
Copy link
Collaborator

@aignas aignas commented Aug 17, 2025

After merging #3058, I wanted to try creating a universal requirements
file for our sphinx setup and have an integration test in that way. It
seems that alabaster has different versions for different python
versions and we were not handling this correctly. In #3058 I have
attempted adding a unit test for this but it escaped me that the only
time where this bug manifests itself is when the parse_requirements is
used multiple times, once per each python_version.

With this I think it is safe to say that the #2797 is fixed, because we
found a bug, where we were not skipping requirements.

As an added counter measure, I have added an extra check elsewhere to
catch a regression.

Fixes #2797

After merging bazel-contrib#3058, I wanted to try creating a `universal` requirements
file for our `sphinx` setup and have an integration test in that way. It
seems that `alabaster` has different versions for different python
versions and we were not handling this correctly. In bazel-contrib#3058 I have
attempted adding a unit test for this but it escaped me that the only
time where this bug manifests itself is when the `parse_requirements` is
used multiple times, once per each `python_version`.

With this I think it is safe to say that the bazel-contrib#2797 is fixed, because we
found a bug, where we were not skipping requirements.

As an added counter measure, I have added an extra check elsewhere to
catch a regression.

Fixes bazel-contrib#2797
Copy link
Collaborator

@rickeylev rickeylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of scope for this PR, I think, but the whl_from_dir was so handy for testing I can't help but bring it up:

How could we test this using a checked-in requirements.txt and avoid hitting the network to talk to pypi?

Maybe a special bazel:// url that the pip integration translates into file:// urls (so that pip itself can understand them)?

And we do like:

# requirements.txt
foo @ bazel://whatever

# module.bazel
whl_from_dir(name = "whatever")

pip.parse(
  requirements = "requirements.txt"
  custom_packages = {
    "whatever": "@whatever//whatever.whl"
  }
)

Or maybe a pip.custom_index() that we pass all the whl_from_dir's to, and...can we pass a file:// url to pip for its index?

@aignas
Copy link
Collaborator Author

aignas commented Aug 17, 2025

I am thinking it would be better to do custom index URL. Example:

index.from_dir(
    name = "pypi_cache",
    path = "<path-to-dir>",  # similar to local_repository
)

use_repo(index, "pypi_cache")

dev_pip.parse(
    experimental_index_url = "@pypi_cache//index", # a label, but because we are doing this as a dev dep and only supporting dev deps, we can then do `ctx.path("@pypi_cache//index/pkg")`
)

Should we support non-dev dependencies for this on-disk PyPI cache?

@aignas aignas added this pull request to the merge queue Aug 17, 2025
Merged via the queue into bazel-contrib:main with commit 5e75007 Aug 17, 2025
3 checks passed
@aignas aignas deleted the exp.docs.use.universal branch August 17, 2025 06:51
@rickeylev
Copy link
Collaborator

Non dev deps: hm. Does isolate solve it instead? That seems cleaner and easier.

Custom index url vs line-level: index level does seem a bit cleaner, I think I like that better

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

Successfully merging this pull request may close these issues.

Support universal requirements for multiple python versions
2 participants