Skip to content

Commit

Permalink
[workspace] Clarify os_result.target (formerly distribution) (RobotLo…
Browse files Browse the repository at this point in the history
…comotion#17291)

* [workspace] Clarify os_result.target (formerly distribution)

The os_result.distribution was documented to be fine-grained, with wheel
builds using a different value, but in 65d0bde we mistakenly reverted it
to "ubuntu" even for wheel builds.

Here we clarify the documentation, rename it to "target" for even more
clarity, and update using-code to match.

We also start to make room for the is_macos_wheel option, even though
it is neither set nor used yet.

Co-authored-by: Matthew Woehlke <[email protected]>
  • Loading branch information
jwnimmer-tri and mwoehlke-kitware authored May 31, 2022
1 parent 9d45600 commit cd4bb4c
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 57 deletions.
11 changes: 6 additions & 5 deletions tools/skylark/drake_cc_per_os.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ load(
)
load(
"@drake_detected_os//:os.bzl",
"DISTRIBUTION",
"MACOS_RELEASE",
"UBUNTU_RELEASE",
)

def drake_cc_googletest_ubuntu_only(
Expand All @@ -29,7 +30,7 @@ def drake_cc_googletest_ubuntu_only(
Because this test is not cross-platform, the visibility defaults to
private.
"""
if DISTRIBUTION in ["ubuntu", "manylinux"]:
if UBUNTU_RELEASE != None:
drake_cc_googletest(
name = name,
visibility = visibility,
Expand All @@ -47,7 +48,7 @@ def drake_cc_library_ubuntu_only(
Because this library is not cross-platform, the visibility defaults to
private and the headers are excluded from the installation.
"""
if DISTRIBUTION in ["ubuntu", "manylinux"]:
if UBUNTU_RELEASE != None:
drake_cc_library(
name = name,
hdrs = hdrs,
Expand All @@ -63,7 +64,7 @@ def drake_cc_package_library_per_os(
"""Declares a drake_cc_package_library, where the deps of the library are
conditioned on whether we are building on macOS or Ubuntu.
"""
if DISTRIBUTION == "macos":
if MACOS_RELEASE != None:
drake_cc_package_library(deps = macos_deps, **kwargs)
elif DISTRIBUTION in ["ubuntu", "manylinux"]:
if UBUNTU_RELEASE != None:
drake_cc_package_library(deps = ubuntu_deps, **kwargs)
6 changes: 3 additions & 3 deletions tools/skylark/drake_py_per_os.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ load(
)
load(
"@drake_detected_os//:os.bzl",
"DISTRIBUTION",
"UBUNTU_RELEASE",
)

def drake_py_binary_ubuntu_only(
Expand All @@ -24,7 +24,7 @@ def drake_py_binary_ubuntu_only(
The visibility defaults to private because this binary is not
cross-platform.
"""
if DISTRIBUTION == "ubuntu":
if UBUNTU_RELEASE != None:
drake_py_binary(
name = name,
visibility = visibility,
Expand All @@ -41,7 +41,7 @@ def drake_py_unittest_ubuntu_only(
The visibility defaults to private because this binary is not
cross-platform.
"""
if DISTRIBUTION == "ubuntu":
if UBUNTU_RELEASE != None:
drake_py_unittest(
name = name,
visibility = visibility,
Expand Down
1 change: 0 additions & 1 deletion tools/wheel/image/packages-focal
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ zip
# Build dependencies (general).
libgl1-mesa-dev
libglib2.0-dev
libjchart2d-java
libnlopt-dev
libnlopt-cxx-dev
libxt-dev
Expand Down
2 changes: 1 addition & 1 deletion tools/workspace/com_jidesoft_jide_oss/repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def com_jidesoft_jide_oss_repository(
drake_java_import(
name = name,
licenses = ["restricted"], # GPL-2.0 WITH Classpath-exception-2.0
local_distributions = ["ubuntu"],
local_os_targets = ["ubuntu"],
local_jar = "/usr/share/java/jide-oss.jar",
maven_jar = "com/jidesoft/jide-oss/2.9.7/jide-oss-2.9.7.jar", # noqa
maven_jar_sha256 = "a2edc2749cf482f6b2b1331f35f0383a1a11c19b1cf6d9a8cf7c69ce4cc8e04b", # noqa
Expand Down
2 changes: 1 addition & 1 deletion tools/workspace/commons_io/repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def commons_io_repository(
drake_java_import(
name = name,
licenses = ["notice"], # Apache-2.0
local_distributions = ["ubuntu"],
local_os_targets = ["ubuntu"],
local_jar = "/usr/share/java/commons-io.jar",
maven_jar = "commons-io/commons-io/1.3.1/commons-io-1.3.1.jar", # noqa
maven_jar_sha256 = "3307319ddc221f1b23e8a1445aef10d2d2308e0ec46977b3f17cbb15c0ef335b", # noqa
Expand Down
10 changes: 5 additions & 5 deletions tools/workspace/java.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def _impl(repo_ctx):
build_content = """\
package(default_visibility = ["//visibility:public"])
"""
if os_result.distribution in repo_ctx.attr.local_distributions:
if os_result.target in repo_ctx.attr.local_os_targets:
is_local = True
filename = basename(repo_ctx.attr.local_jar)
repo_ctx.symlink(
Expand Down Expand Up @@ -66,7 +66,7 @@ install(
_internal_drake_java_import = repository_rule(
attrs = {
"licenses": attr.string_list(mandatory = True),
"local_distributions": attr.string_list(mandatory = True),
"local_os_targets": attr.string_list(mandatory = True),
"local_jar": attr.string(mandatory = True),
},
implementation = _impl,
Expand All @@ -76,13 +76,13 @@ def drake_java_import(
name,
*,
licenses,
local_distributions,
local_os_targets,
local_jar,
maven_jar,
maven_jar_sha256,
mirrors):
"""A repository rule to bring in a Java dependency, either from the host's
OS distribution, or else Maven. The list of local_distributions indicates
OS distribution, or else Maven. The list of local_os_targets indicates
which distributions provide this jar; for those, the local_jar is the full
path to the jar. Otherwise, the maven_jar will be used.
"""
Expand All @@ -98,6 +98,6 @@ def drake_java_import(
_internal_drake_java_import(
name = name,
licenses = licenses,
local_distributions = local_distributions,
local_os_targets = local_os_targets,
local_jar = local_jar,
)
2 changes: 1 addition & 1 deletion tools/workspace/net_sf_jchart2d/repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def net_sf_jchart2d_repository(
drake_java_import(
name = name,
licenses = ["restricted"], # LGPL-3.0+
local_distributions = ["ubuntu"],
local_os_targets = ["ubuntu"],
local_jar = "/usr/share/java/jchart2d.jar",
maven_jar = "net/sf/jchart2d/jchart2d/3.3.2/jchart2d-3.3.2.jar", # noqa
maven_jar_sha256 = "41af674b1bb00d8b89a0649ddaa569df5750911b4e33f89b211ae82e411d16cc", # noqa
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ def org_apache_xmlgraphics_commons_repository(
drake_java_import(
name = name,
licenses = ["notice"], # Apache-2.0
local_distributions = ["ubuntu"],
local_os_targets = ["ubuntu"],
local_jar = "/usr/share/java/xmlgraphics-commons.jar",
maven_jar = "org/apache/xmlgraphics/xmlgraphics-commons/1.3.1/xmlgraphics-commons-1.3.1.jar", # noqa
maven_jar_sha256 = "7ce0c924c84e2710c162ae1c98f5047d64f528268792aba642d4bae5e1de7181", # noqa
Expand Down
85 changes: 52 additions & 33 deletions tools/workspace/os.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -52,24 +52,31 @@ def exec_using_which(repository_ctx, command):

def _make_result(
error = None,
ubuntu_release = None,
macos_release = None,
is_manylinux = False,
ubuntu_release = None,
is_wheel = False,
homebrew_prefix = None):
"""Return a fully-populated struct result for determine_os, below."""
if ubuntu_release != None:
distribution = "ubuntu"
elif macos_release != None:
distribution = "macos"
is_macos = (macos_release != None) and not is_wheel
is_macos_wheel = (macos_release != None) and is_wheel
is_ubuntu = (ubuntu_release != None) and not is_wheel
is_manylinux = (ubuntu_release != None) and is_wheel
if is_macos:
target = "macos"
elif is_macos_wheel:
target = "macos_wheel"
elif is_ubuntu:
target = "ubuntu"
elif is_manylinux:
distribution = "manylinux"
target = "manylinux"
else:
distribution = None
target = None
return struct(
error = error,
distribution = distribution,
is_macos = (macos_release != None),
is_ubuntu = (ubuntu_release != None and not is_manylinux),
target = target,
is_macos = is_macos,
is_macos_wheel = is_macos_wheel,
is_ubuntu = is_ubuntu,
is_manylinux = is_manylinux,
ubuntu_release = ubuntu_release,
macos_release = macos_release,
Expand Down Expand Up @@ -112,7 +119,7 @@ def _determine_linux(repository_ctx):
if ubuntu_release in ["20.04"]:
return _make_result(
ubuntu_release = ubuntu_release,
is_manylinux = is_manylinux,
is_wheel = is_manylinux,
)

# Nothing matched.
Expand Down Expand Up @@ -168,32 +175,44 @@ def _determine_macos(repository_ctx):

def determine_os(repository_ctx):
"""
A repository_rule helper function that determines which of the supported OS
versions we are targeting.
A repository_rule helper function that determines which of the supported
build environments (OS versions or wheel environments) we should target.
We support four options, which are mutually exclusive and collectively
exhaustive: "macos" or "macos_wheel" or "ubuntu" or "manylinux".
Note that even if the operating system hosting the build is Ubuntu, the
target OS might be "manylinux", which means that we only use the most basic
host packages from Ubuntu (libc, libstdc++, etc.). In that case, the
value of is_ubuntu will be False.
The "manylinux" target indicates this build will be packaged into a Python
wheel that conforms to a "manylinux" standard such as manylinux_2_31; see
https://github.com/pypa/manylinux. Currently we compile this in an Ubuntu
container using only the most basic host packages from Ubuntu (libc,
libstdc++, etc.). In this case, the value of is_ubuntu will be False, but
ubuntu_release will still be provided.
TODO(jwnimmer-tri) The is_macos_wheel option isn't detected / active yet.
In case of an error, the "error" attribute of the struct will be set, and
all of the other fields will be None or False.
Argument:
repository_ctx: The context passed to the repository_rule calling this.
Result:
a struct, with attributes:
- error: str iff any error occurred, else None
- distribution: str either "ubuntu" or "macos" or "manylinux" iff no
error occurred, else None
- is_macos: True iff on a supported macOS release, else False
- macos_release: str like "11" or "12" iff on a supported macOS,
else None
- homebrew_prefix: str "/usr/local" or "/opt/homebrew" iff is_macos,
else None.
- is_ubuntu: True iff on a supported Ubuntu version, else False
- ubuntu_release: str like "20.04" iff on a supported ubuntu, else None
- is_manylinux: True iff this build will be packaged into a Python
wheel that confirms to a "manylinux" standard such as
manylinux_2_27; see https://github.com/pypa/manylinux.
- target: str "macos" or "macos_wheel" or "ubuntu" or "manylinux"
- is_macos: True iff targeting a macOS non-wheel build
- is_macos_wheel: True iff targeting a macOS wheel build
- is_ubuntu: True iff targeting an Ubuntu non-wheel build
- is_manylinux: True iff targeting a Linux wheel build
- ubuntu_release: str like "20.04" (set any time the build platform is
Ubuntu, even for builds targeting "manylinux")
- macos_release: str like "11" or "12" (set any time the build platform
is macOS, even for builds targeting "macos_wheel")
- homebrew_prefix: str "/usr/local" or "/opt/homebrew" (set any time
the build platform is macOS, even for builds targeting
"macos_wheel")
"""

os_name = repository_ctx.os.name
Expand Down Expand Up @@ -288,12 +307,12 @@ def _os_impl(repo_ctx):
fail(os_result.error)

constants = """
DISTRIBUTION = {distribution}
TARGET = {target}
UBUNTU_RELEASE = {ubuntu_release}
MACOS_RELEASE = {macos_release}
HOMEBREW_PREFIX = {homebrew_prefix}
""".format(
distribution = repr(os_result.distribution),
target = repr(os_result.target),
ubuntu_release = repr(os_result.ubuntu_release),
macos_release = repr(os_result.macos_release),
homebrew_prefix = repr(os_result.homebrew_prefix),
Expand All @@ -305,6 +324,6 @@ os_repository = repository_rule(
)

"""
Provides the fields `DISTRIBUTION`, `UBUNTU_RELEASE` and `MACOS_RELEASE` from
Provides the fields `TARGET`, `UBUNTU_RELEASE` and `MACOS_RELEASE` from
`determine_os`.
"""
9 changes: 3 additions & 6 deletions tools/workspace/python/repository.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,9 @@ def repository_python_info(repository_ctx):
os_result = determine_os(repository_ctx)
if os_result.error != None:
fail(os_result.error)
if os_result.is_macos:
os_key = os_result.distribution
elif os_result.is_ubuntu:
os_key = os_result.distribution + ":" + os_result.ubuntu_release
else:
os_key = "manylinux"
os_key = os_result.target
if os_result.is_ubuntu:
os_key += ":" + os_result.ubuntu_release
versions_supported = _VERSION_SUPPORT_MATRIX[os_key]

if os_result.is_macos:
Expand Down

0 comments on commit cd4bb4c

Please sign in to comment.