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

[workspace] Improve qhull hidden visibility #17229

Merged
merged 1 commit into from
Jun 2, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion geometry/optimization/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ drake_cc_library(
"//solvers:mosek_solver",
"//solvers:scs_solver",
"//solvers:solve",
"@qhull",
"@qhull_internal//:qhull",
],
)

Expand Down
4 changes: 2 additions & 2 deletions geometry/optimization/vpolytope.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
#include <memory>
#include <numeric>

#include <drake_vendor/libqhullcpp/Qhull.h>
#include <drake_vendor/libqhullcpp/QhullVertexSet.h>
#include <fmt/format.h>
#include <libqhullcpp/Qhull.h>
#include <libqhullcpp/QhullVertexSet.h>

#include "drake/common/is_approx_equal_abstol.h"
#include "drake/geometry/read_obj.h"
Expand Down
4 changes: 3 additions & 1 deletion tools/workspace/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ drake_py_binary(
name = "vendor_cxx",
srcs = ["vendor_cxx.py"],
visibility = [
# These should all be of the form "@foo_internal//:__pkg__".
"@qhull_internal//:__pkg__",
"@yaml_cpp_internal//:__pkg__",
],
deps = [":module_py"],
Expand Down Expand Up @@ -86,7 +88,7 @@ _DRAKE_EXTERNAL_PACKAGE_INSTALLS = ["@%s//:install" % p for p in [
"org_apache_xmlgraphics_commons",
"petsc",
"pybind11",
"qhull",
"qhull_internal",
"sdformat",
"spdlog",
"tinyobjloader",
Expand Down
11 changes: 9 additions & 2 deletions tools/workspace/default.bzl
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# -*- python -*-

load("@drake//tools/workspace:deprecation.bzl", "add_deprecation")
load("@drake//tools/workspace:mirrors.bzl", "DEFAULT_MIRRORS")
load("@drake//tools/workspace:os.bzl", "os_repository")
load("@drake//tools/workspace/abseil_cpp_internal:repository.bzl", "abseil_cpp_internal_repository") # noqa
Expand Down Expand Up @@ -74,7 +75,7 @@ load("@drake//tools/workspace/pycodestyle:repository.bzl", "pycodestyle_reposito
load("@drake//tools/workspace/pygame_py:repository.bzl", "pygame_py_repository") # noqa
load("@drake//tools/workspace/python:repository.bzl", "python_repository")
load("@drake//tools/workspace/qdldl:repository.bzl", "qdldl_repository")
load("@drake//tools/workspace/qhull:repository.bzl", "qhull_repository")
load("@drake//tools/workspace/qhull_internal:repository.bzl", "qhull_internal_repository") # noqa
load("@drake//tools/workspace/ros_xacro:repository.bzl", "ros_xacro_repository") # noqa
load("@drake//tools/workspace/rules_pkg:repository.bzl", "rules_pkg_repository") # noqa
load("@drake//tools/workspace/rules_python:repository.bzl", "rules_python_repository") # noqa
Expand Down Expand Up @@ -255,7 +256,13 @@ def add_default_repositories(excludes = [], mirrors = DEFAULT_MIRRORS):
if "qdldl" not in excludes:
qdldl_repository(name = "qdldl", mirrors = mirrors)
if "qhull" not in excludes:
qhull_repository(name = "qhull", mirrors = mirrors)
add_deprecation(
name = "qhull",
date = "2022-10-01",
cc_aliases = {"qhull": "@qhull_internal//:qhull"},
)
if "qhull_internal" not in excludes:
qhull_internal_repository(name = "qhull_internal", mirrors = mirrors)
if "ros_xacro" not in excludes:
ros_xacro_repository(name = "ros_xacro", mirrors = mirrors)
if "rules_pkg" not in excludes:
Expand Down
46 changes: 46 additions & 0 deletions tools/workspace/deprecation.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# -*- python -*-

def _impl(repo_ctx):
name = repo_ctx.attr.name
date = repo_ctx.attr.date
cc_aliases = repo_ctx.attr.cc_aliases

build = "package(default_visibility = [\"//visibility:public\"])\n"
deprecation = "".join([
"DRAKE DEPRECATED: The @{} external is deprecated".format(name),
" and will be removed from Drake on or after {}.".format(date),
])
for label, actual in cc_aliases.items():
build += "cc_library({})\n".format(", ".join([
"name = " + repr(label),
"deps = [" + repr(actual) + "]",
"deprecation = " + repr(deprecation),
]))

repo_ctx.file("BUILD.bazel", build)

add_deprecation = repository_rule(
doc = """Adds a repository rule with deprecated aliases to other targets.
This is particularly useful when renaming an external repository.

Example:
add_deprecation(
name = "qhull",
date = "2038-01-19",
cc_aliases = {"qhull": "@qhull_internal//:qhull"},
)
""",
attrs = {
"date": attr.string(
doc = "Scheduled removal date of the deprecated target(s).",
mandatory = True,
),
"cc_aliases": attr.string_dict(
doc = """
Optional mapping for cc_library deprecations. The keys are
deprecated target names, the values are the non-deprecated labels.
""",
),
},
implementation = _impl,
)
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ load(
"@drake//tools/install:install.bzl",
"install",
)
load(
"@drake//tools/workspace:vendor_cxx.bzl",
"cc_library_vendored",
)

licenses(["notice"]) # Qhull

Expand Down Expand Up @@ -53,15 +57,35 @@ _SRCS_CPP = [
]

cc_library(
name = "qhull",
hdrs = _HDRS_C + _HDRS_CPP,
name = "qhull_r",
hdrs = _HDRS_C,
copts = [
"-fvisibility=hidden",
],
includes = ["src"],
srcs = _SRCS_C + _SRCS_CPP,
srcs = _SRCS_C,
linkstatic = 1,
)

cc_library_vendored(
name = "qhull",
hdrs = _HDRS_CPP,
hdrs_vendored = [
x.replace("src/libqhullcpp/", "drake_src/drake_vendor/libqhullcpp/")
for x in _HDRS_CPP
],
includes = ["drake_src"],
edit_include = {
"libqhullcpp/": "drake_vendor/libqhullcpp/",
},
srcs = _SRCS_CPP,
srcs_vendored = [
x.replace("src/", "drake_src/drake_vendor/")
for x in _SRCS_CPP
],
linkstatic = 1,
visibility = ["//visibility:public"],
deps = [":qhull_r"],
)

# Install the license file.
Expand Down
27 changes: 27 additions & 0 deletions tools/workspace/qhull_internal/patches/vendor_cxx.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
For private access within the module, don't use out-of-namespace friendship with
an `extern "C"` function. This doesn't work with drake's namespace vendoring.

Instead, just make the fields public.

--- src/libqhullcpp/QhullQh.h.orig
+++ src/libqhullcpp/QhullQh.h
@@ -58,7 +58,7 @@
#//!\name Constants

#//!\name Fields
-private:
+public:
int qhull_status; //!< qh_ERRnone if valid
std::string qhull_message; //!< Returned messages from libqhull_r
std::ostream * error_stream; //!< overrides errorMessage, use appendQhullMessage()
@@ -66,8 +66,9 @@
double factor_epsilon; //!< Factor to increase ANGLEround and DISTround for hyperplane equality
bool use_output_stream; //!< True if using output_stream

+private:
//! modified by qh_fprintf in QhullUser.cpp
- friend void ::qh_fprintf(qhT *qh, FILE *fp, int msgcode, const char *fmt, ... );
+ //friend void ::qh_fprintf(qhT *qh, FILE *fp, int msgcode, const char *fmt, ... );

static const double default_factor_epsilon; //!< Default factor_epsilon is 1.0, never updated

Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,18 @@

load("@drake//tools/workspace:github.bzl", "github_archive")

def qhull_repository(
def qhull_internal_repository(
name,
mirrors = None):
github_archive(
name = name,
repository = "qhull/qhull",
commit = "2020.2",
sha256 = "59356b229b768e6e2b09a701448bfa222c37b797a84f87f864f97462d8dbc7c5", # noqa
build_file = "@drake//tools/workspace/qhull:package.BUILD.bazel",
build_file = "@drake//tools/workspace/qhull_internal:package.BUILD.bazel", # noqa
patches = [
"@drake//tools/workspace/qhull:patches/disable_dead_code.patch",
"@drake//tools/workspace/qhull_internal:patches/disable_dead_code.patch", # noqa
"@drake//tools/workspace/qhull_internal:patches/vendor_cxx.patch",
],
mirrors = mirrors,
)