Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(bzlmod): cross-platform builds without experimental_index_url #2325

Merged
merged 42 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
4eb639e
feat: reuse the render_pkg_aliases for when filename is not known but…
aignas Oct 22, 2024
ab8b2f0
feat: support generating the extra config settings required
aignas Oct 22, 2024
645885c
feat: get_whl_flag_versions now generates extra args for the rules
aignas Oct 22, 2024
1d33f49
feat: make lock file generation the same irrespective of the host pla…
aignas Oct 22, 2024
9544386
test: add an extra test with multiple requirements files
aignas Oct 22, 2024
b81f86e
doc: CHANGELOG
aignas Oct 22, 2024
9b296d2
bring back code used in workspace
aignas Oct 22, 2024
77754c2
fixup the generation of the config settings
aignas Oct 22, 2024
d9a0d3b
Update python/private/pypi/extension.bzl
aignas Oct 22, 2024
fdfa1de
wip
aignas Oct 22, 2024
f7051ab
wip
aignas Oct 22, 2024
0c058a1
Update CHANGELOG.md
aignas Oct 22, 2024
e5b0190
test: check that parse_requirements handles multiple files with downl…
aignas Oct 23, 2024
0400154
ensure that we can support multi-platform setups with the legacy pip
aignas Oct 23, 2024
c75039d
fix: make the names of the repos more user friendly
aignas Oct 23, 2024
bf4f644
revert docs about cross-platform building
aignas Oct 23, 2024
4de5a45
Revert "revert docs about cross-platform building"
aignas Oct 23, 2024
0d468df
fix the docs to fully document what is possible today
aignas Oct 23, 2024
8d418f4
adjust the code so that the experimental_target_platforms is set in d…
aignas Oct 23, 2024
cfcc8c5
Merge branch 'main' into fix/lockfile-inconsistency
aignas Oct 23, 2024
cf76913
wip
aignas Oct 23, 2024
ecf5a5b
Merge branch 'main' into fix/lockfile-inconsistency
aignas Oct 23, 2024
c4fceb9
fixup test
aignas Oct 23, 2024
9482a13
fixup test
aignas Oct 23, 2024
a520e29
Merge branch 'main' into fix/lockfile-inconsistency
aignas Oct 26, 2024
8578b02
merge conflicts
aignas Oct 26, 2024
de98a02
fix integration tests
aignas Oct 26, 2024
5025f90
Merge branch 'main' into fix/lockfile-inconsistency
aignas Nov 1, 2024
e90d257
fixup! Merge branch 'main' into fix/lockfile-inconsistency
aignas Nov 1, 2024
8835d02
use a different scheme inspired by #2366
aignas Nov 1, 2024
e2c07ae
one more try
aignas Nov 1, 2024
74383f3
fix the tests
aignas Nov 1, 2024
4120c15
simplify
aignas Nov 2, 2024
b4dad40
Merge branch 'main' into fix/lockfile-inconsistency
aignas Nov 3, 2024
7582d5b
revert: start making the change in behaviour opt in
aignas Nov 3, 2024
e5d2ff4
add a flag to use all requirements to make the extension lock file en…
aignas Nov 4, 2024
5c45b4a
fixup! add a flag to use all requirements to make the extension lock …
aignas Nov 4, 2024
58e7a6c
fixup! fixup! add a flag to use all requirements to make the extensio…
aignas Nov 4, 2024
c0fe195
fixup tests
aignas Nov 4, 2024
1ffb52c
wip
aignas Nov 4, 2024
b7577ff
Merge branch 'main' into fix/lockfile-inconsistency
aignas Nov 5, 2024
a2b8b52
wip
aignas Nov 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,11 @@ A brief description of the categories of changes:

{#v0-0-0-fixed}
### Fixed
- Nothing yet
- The extension evaluation has been adjusted to always generate the same lock
file irrespective if `experimental_index_url` is set by any module or not.
Fixes [#2268](https://github.com/bazelbuild/rules_python/issues/2268). A known
issue is that it may break `bazel query` and in these usecases it is advisable
aignas marked this conversation as resolved.
Show resolved Hide resolved
to use `cquery` until we have `sdist` cross-building from source fully working.
Copy link
Collaborator

Choose a reason for hiding this comment

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

download_only=True is also an option, right? Assuming one is OK with not having sdists

Copy link
Collaborator Author

@aignas aignas Oct 22, 2024

Choose a reason for hiding this comment

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

It could be, but I haven't tested with regular requirements files, let me add a unit test to see how that would get wired through.

EDIT: With a few extra modifications, I think we can actually support that. This means that we can resolve #260 and have the stabilization for experimental_index_url done as a separate ticket. I've added extra docs on this, please check. :)


{#v0-0-0-added}
### Added
Expand Down
13 changes: 9 additions & 4 deletions MODULE.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,10 @@ register_toolchains("@pythons_hub//:all")
#####################
# Install twine for our own runfiles wheel publishing and allow bzlmod users to use it.

pip = use_extension("//python/private/pypi:pip.bzl", "pip_internal")
pip = use_extension("//python/extensions:pip.bzl", "pip")
pip.parse(
download_only = True,
experimental_index_url = "https://pypi.org/simple",
hub_name = "rules_python_publish_deps",
python_version = "3.11",
requirements_by_platform = {
Expand Down Expand Up @@ -89,17 +91,20 @@ dev_python.override(
)

dev_pip = use_extension(
"//python/private/pypi:pip.bzl",
"pip_internal",
"//python/extensions:pip.bzl",
"pip",
dev_dependency = True,
)
dev_pip.parse(
download_only = True, # this will not add the `sdist` values to the transitive closures at all.
download_only = True,
experimental_index_url = "https://pypi.org/simple",
hub_name = "dev_pip",
python_version = "3.11",
requirements_lock = "//docs:requirements.txt",
)
dev_pip.parse(
download_only = True,
experimental_index_url = "https://pypi.org/simple",
hub_name = "pypiserver",
python_version = "3.11",
requirements_lock = "//examples/wheel:requirements_server.txt",
Expand Down
5,999 changes: 2,532 additions & 3,467 deletions examples/bzlmod/MODULE.bazel.lock

Large diffs are not rendered by default.

13 changes: 12 additions & 1 deletion python/private/pypi/config_settings.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,13 @@ def config_settings(
)

def _dist_config_settings(*, suffix, plat_flag_values, **kwargs):
if kwargs.get("constraint_values"):
# Add python version + platform config settings
_dist_config_setting(
name = suffix.strip("_"),
**kwargs
)

flag_values = {_flags.dist: ""}

# First create an sdist, we will be building upon the flag values, which
Expand Down Expand Up @@ -277,7 +284,7 @@ def _plat_flag_values(os, cpu, osx_versions, glibc_versions, muslc_versions):

return ret

def _dist_config_setting(*, name, is_pip_whl, is_python, python_version, native = native, **kwargs):
def _dist_config_setting(*, name, is_python, python_version, is_pip_whl = None, native = native, **kwargs):
"""A macro to create a target that matches is_pip_whl_auto and one more value.

Args:
Expand Down Expand Up @@ -310,6 +317,10 @@ def _dist_config_setting(*, name, is_pip_whl, is_python, python_version, native
# `python_version` setting.
return

if not is_pip_whl:
native.config_setting(name = _name, **kwargs)
return

config_setting_name = _name + "_setting"
native.config_setting(name = config_setting_name, **kwargs)

Expand Down
131 changes: 37 additions & 94 deletions python/private/pypi/extension.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ load("//python/private:version_label.bzl", "version_label")
load(":attrs.bzl", "use_isolated")
load(":evaluate_markers.bzl", "evaluate_markers", EVALUATE_MARKERS_SRCS = "SRCS")
load(":hub_repository.bzl", "hub_repository")
load(":parse_requirements.bzl", "host_platform", "parse_requirements", "select_requirement")
load(":parse_requirements.bzl", "parse_requirements")
load(":parse_whl_name.bzl", "parse_whl_name")
load(":pip_repository_attrs.bzl", "ATTRS")
load(":render_pkg_aliases.bzl", "whl_alias")
Expand Down Expand Up @@ -212,7 +212,6 @@ def _create_whl_repos(
logger = logger,
)

repository_platform = host_platform(module_ctx)
for whl_name, requirements in requirements_by_platform.items():
# We are not using the "sanitized name" because the user
# would need to guess what name we modified the whl name
Expand Down Expand Up @@ -260,10 +259,10 @@ def _create_whl_repos(
if v != default
})

is_exposed = False
if get_index_urls:
# TODO @aignas 2024-05-26: move to a separate function
found_something = False
is_exposed = False
for requirement in requirements:
is_exposed = is_exposed or requirement.is_exposed
dists = requirement.whls
Expand Down Expand Up @@ -319,35 +318,30 @@ def _create_whl_repos(
exposed_packages[whl_name] = None
continue

requirement = select_requirement(
requirements,
platform = None if pip_attr.download_only else repository_platform,
)
if not requirement:
# Sometimes the package is not present for host platform if there
# are whls specified only in particular requirements files, in that
# case just continue, however, if the download_only flag is set up,
# then the user can also specify the target platform of the wheel
# packages they want to download, in that case there will be always
# a requirement here, so we will not be in this code branch.
continue
elif get_index_urls:
logger.warn(lambda: "falling back to pip for installing the right file for {}".format(requirement.requirement_line))

whl_library_args["requirement"] = requirement.requirement_line
if requirement.extra_pip_args:
whl_library_args["extra_pip_args"] = requirement.extra_pip_args
for i, requirement in enumerate(requirements):
rickeylev marked this conversation as resolved.
Show resolved Hide resolved
is_exposed = is_exposed or requirement.is_exposed
if get_index_urls:
logger.warn(lambda: "falling back to pip for installing the right file for {}".format(requirement.requirement_line))

whl_library_args["requirement"] = requirement.requirement_line
if requirement.extra_pip_args:
whl_library_args["extra_pip_args"] = requirement.extra_pip_args

repo_name = "{}_{}".format(pip_name, whl_name)
if len(requirements) > 1:
repo_name = "{}__{}".format(repo_name, i)

whl_libraries[repo_name] = dict(whl_library_args.items())
whl_map.setdefault(whl_name, []).append(
whl_alias(
repo = repo_name,
version = major_minor,
target_platforms = requirement.target_platforms if len(requirements) > 1 else None,
),
)

# We sort so that the lock-file remains the same no matter the order of how the
# args are manipulated in the code going before.
repo_name = "{}_{}".format(pip_name, whl_name)
whl_libraries[repo_name] = dict(whl_library_args.items())
whl_map.setdefault(whl_name, []).append(
whl_alias(
repo = repo_name,
version = major_minor,
),
)
if is_exposed:
exposed_packages[whl_name] = None

return struct(
is_reproducible = is_reproducible,
Expand Down Expand Up @@ -497,7 +491,8 @@ You cannot use both the additive_build_content and additive_build_content_file a
hub_group_map[hub_name] = pip_attr.experimental_requirement_cycles

return struct(
# We sort the output here so that the lock file is sorted
# We sort so that the lock-file remains the same no matter the order of how the
# args are manipulated in the code going before.
whl_mods = dict(sorted(whl_mods.items())),
hub_whl_map = {
hub_name: {
Expand All @@ -513,8 +508,14 @@ You cannot use both the additive_build_content and additive_build_content_file a
}
for hub_name, group_map in sorted(hub_group_map.items())
},
exposed_packages = {k: sorted(v) for k, v in sorted(exposed_packages.items())},
whl_libraries = dict(sorted(whl_libraries.items())),
exposed_packages = {
k: sorted(v)
for k, v in sorted(exposed_packages.items())
},
whl_libraries = {
k: dict(sorted(args.items()))
for k, args in sorted(whl_libraries.items())
},
is_reproducible = is_reproducible,
)

Expand Down Expand Up @@ -589,10 +590,8 @@ def _pip_impl(module_ctx):
# Build all of the wheel modifications if the tag class is called.
_whl_mods_impl(mods.whl_mods)

for name, args in sorted(mods.whl_libraries.items()):
# We sort so that the lock-file remains the same no matter the order of how the
# args are manipulated in the code going before.
whl_library(name = name, **dict(sorted(args.items())))
for name, args in mods.whl_libraries.items():
whl_library(name = name, **args)

for hub_name, whl_map in mods.hub_whl_map.items():
hub_repository(
Expand All @@ -617,13 +616,6 @@ def _pip_impl(module_ctx):
else:
return None

def _pip_non_reproducible(module_ctx):
_pip_impl(module_ctx)

# We default to calling the PyPI index and that will go into the
# MODULE.bazel.lock file, hence return nothing here.
return None

def _pip_parse_ext_attrs(**kwargs):
"""Get the attributes for the pip extension.

Expand Down Expand Up @@ -881,55 +873,6 @@ extension.
},
)

pypi_internal = module_extension(
doc = """\
This extension is used to make dependencies from pypi available.

For now this is intended to be used internally so that usage of the `pip`
extension in `rules_python` does not affect the evaluations of the extension
for the consumers.

pip.parse:
To use, call `pip.parse()` and specify `hub_name` and your requirements file.
Dependencies will be downloaded and made available in a repo named after the
`hub_name` argument.

Each `pip.parse()` call configures a particular Python version. Multiple calls
can be made to configure different Python versions, and will be grouped by
the `hub_name` argument. This allows the same logical name, e.g. `@pypi//numpy`
to automatically resolve to different, Python version-specific, libraries.

pip.whl_mods:
This tag class is used to help create JSON files to describe modifications to
the BUILD files for wheels.
""",
implementation = _pip_non_reproducible,
tag_classes = {
"override": _override_tag,
"parse": tag_class(
attrs = _pip_parse_ext_attrs(
experimental_index_url = "https://pypi.org/simple",
),
doc = """\
This tag class is used to create a pypi hub and all of the spokes that are part of that hub.
This tag class reuses most of the attributes found in {bzl:obj}`pip_parse`.
The exception is it does not use the arg 'repo_prefix'. We set the repository
prefix for the user and the alias arg is always True in bzlmod.
""",
),
"whl_mods": tag_class(
attrs = _whl_mod_attrs(),
doc = """\
This tag class is used to create JSON file that are used when calling wheel_builder.py. These
JSON files contain instructions on how to modify a wheel's project. Each of the attributes
create different modifications based on the type of attribute. Previously to bzlmod these
JSON files where referred to as annotations, and were renamed to whl_modifications in this
extension.
""",
),
},
)

def _whl_mods_repo_impl(rctx):
rctx.file("BUILD.bazel", "")
for whl_name, mods in rctx.attr.whl_mods.items():
Expand Down
6 changes: 5 additions & 1 deletion python/private/pypi/parse_requirements.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ def parse_requirements(
)

ret = {}
for whl_name, reqs in requirements_by_platform.items():
for whl_name, reqs in sorted(requirements_by_platform.items()):
requirement_target_platforms = {}
for r in reqs.values():
target_platforms = env_marker_target_platforms.get(r.requirement_line, r.target_platforms)
Expand Down Expand Up @@ -212,6 +212,8 @@ def parse_requirements(
def select_requirement(requirements, *, platform):
"""A simple function to get a requirement for a particular platform.

Only used in WORKSPACE.

Args:
requirements (list[struct]): The list of requirements as returned by
the `parse_requirements` function above.
Expand Down Expand Up @@ -243,6 +245,8 @@ def select_requirement(requirements, *, platform):
def host_platform(ctx):
"""Return a string representation of the repository OS.

Only used in WORKSPACE.

Args:
ctx (struct): The `module_ctx` or `repository_ctx` attribute.

Expand Down
3 changes: 1 addition & 2 deletions python/private/pypi/pip.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

"pip module extensions for use with bzlmod."

load("//python/private/pypi:extension.bzl", "pypi", "pypi_internal")
load("//python/private/pypi:extension.bzl", "pypi")

pip = pypi
pip_internal = pypi_internal
Loading