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] Deprecate public use of several externals #18690

Merged

Conversation

jwnimmer-tri
Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri commented Jan 29, 2023

Add a @suitesparse_internal repository to rebuild (the AMD part of) SuiteSparse from source, with hidden visibility.

Deprecate the public (host) @suitesparse external. Remove suitesparse from our OS dependencies.

Rename OSQP, QDLDL, SCS to use the _internal suffix and deprecate their non-internal spellings. (They are all C libraries that already use hidden visibility, so they don't need any C++ vendoring changes to their BUILD.)

Towards #17231.


This change is Reviewable

@jwnimmer-tri jwnimmer-tri added priority: low status: single reviewer ok https://drake.mit.edu/reviewable.html release notes: newly deprecated This pull request contains new deprecations labels Jan 29, 2023
@jwnimmer-tri jwnimmer-tri marked this pull request as ready for review January 30, 2023 14:54
@jwnimmer-tri
Copy link
Collaborator Author

@drake-jenkins-bot mac-x86-monterey-clang-bazel-experimental-release please

@jwnimmer-tri
Copy link
Collaborator Author

@drake-jenkins-bot linux-focal-unprovisioned-gcc-wheel-experimental-snopt-mosek-release please

@jwnimmer-tri
Copy link
Collaborator Author

+@sammy-tri for both reviews per schedule, please (Thursday is fine). I'm doing a bit of review-balancing to share the load and broaden the understanding of the build-system-internal work.

Copy link
Contributor

@sammy-tri sammy-tri left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 28 of 28 files at r1, all commit messages.
Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignee sammy-tri(platform) (waiting on @jwnimmer-tri)

Copy link
Collaborator Author

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 unresolved discussion (waiting on @jwnimmer-tri)


tools/workspace/suitesparse_internal/package.BUILD.bazel line 14 at r1 (raw file):

cc_library(
    name = "config",
    srcs = ["SuiteSparse_config/SuiteSparse_config.c"],

Working

I missed the hidden here.

Add a suitesparse_internal repository to rebuild (the AMD part of)
SuiteSparse from source with hidden visibility. Remove SuiteSparse
from the host and wheel dependencies.

Deprecate the public (host) suitesparse external.

Rename OSQP, QDLDL, SCS to use the "_internal" suffix and deprecate their
non-internal spellings. (They are all C libraries that already use hidden
visibility, so they don't need any C++ vendoring changes to their BUILD.)
@jwnimmer-tri jwnimmer-tri force-pushed the workspace-suitesparse branch from ee9ed60 to 2ebcb82 Compare January 31, 2023 18:08
Copy link
Collaborator Author

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignee sammy-tri(platform) (waiting on @sammy-tri)


tools/workspace/suitesparse_internal/package.BUILD.bazel line 14 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Working

I missed the hidden here.

Done.

Copy link
Contributor

@sammy-tri sammy-tri left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignee sammy-tri(platform) (waiting on @jwnimmer-tri)

@jwnimmer-tri jwnimmer-tri merged commit 3d7a94d into RobotLocomotion:master Jan 31, 2023
@jwnimmer-tri jwnimmer-tri deleted the workspace-suitesparse branch January 31, 2023 19:05
xuchenhan-tri pushed a commit to xuchenhan-tri/drake that referenced this pull request Feb 3, 2023
…n#18690)

Add a suitesparse_internal repository to rebuild (the AMD part of)
SuiteSparse from source with hidden visibility. Remove SuiteSparse
from the host and wheel dependencies.

Deprecate the public (host) suitesparse external.

Rename OSQP, QDLDL, SCS to use the "_internal" suffix and deprecate their
non-internal spellings. (They are all C libraries that already use hidden
visibility, so they don't need any C++ vendoring changes to their BUILD.)
marcoag pushed a commit to marcoag/drake that referenced this pull request Feb 6, 2023
…n#18690)

Add a suitesparse_internal repository to rebuild (the AMD part of)
SuiteSparse from source with hidden visibility. Remove SuiteSparse
from the host and wheel dependencies.

Deprecate the public (host) suitesparse external.

Rename OSQP, QDLDL, SCS to use the "_internal" suffix and deprecate their
non-internal spellings. (They are all C libraries that already use hidden
visibility, so they don't need any C++ vendoring changes to their BUILD.)
xuchenhan-tri pushed a commit to xuchenhan-tri/drake that referenced this pull request Feb 6, 2023
…n#18690)

Add a suitesparse_internal repository to rebuild (the AMD part of)
SuiteSparse from source with hidden visibility. Remove SuiteSparse
from the host and wheel dependencies.

Deprecate the public (host) suitesparse external.

Rename OSQP, QDLDL, SCS to use the "_internal" suffix and deprecate their
non-internal spellings. (They are all C libraries that already use hidden
visibility, so they don't need any C++ vendoring changes to their BUILD.)
marcoag pushed a commit to marcoag/drake that referenced this pull request Mar 8, 2023
…n#18690)

Add a suitesparse_internal repository to rebuild (the AMD part of)
SuiteSparse from source with hidden visibility. Remove SuiteSparse
from the host and wheel dependencies.

Deprecate the public (host) suitesparse external.

Rename OSQP, QDLDL, SCS to use the "_internal" suffix and deprecate their
non-internal spellings. (They are all C libraries that already use hidden
visibility, so they don't need any C++ vendoring changes to their BUILD.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: low release notes: newly deprecated This pull request contains new deprecations status: single reviewer ok https://drake.mit.edu/reviewable.html
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants