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 the petsc external #19890

Merged
merged 1 commit into from
Jul 31, 2023

Conversation

jwnimmer-tri
Copy link
Collaborator

@jwnimmer-tri jwnimmer-tri commented Jul 31, 2023

We anticipate no longer using this soon, so it's time to start the clock for its removal.

+@xuchenhan-tri for both reviews, please.

Towards #17231 and #19882.


This change is Reviewable

We anticipate no longer using this soon, so it's time to start the
clock for its removal.
@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 Jul 31, 2023
Copy link
Contributor

@xuchenhan-tri xuchenhan-tri left a comment

Choose a reason for hiding this comment

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

To check my understanding: this has no observable effect when linking against PETSc internally (other than switching the spelling to petsc_nowarn) and we can remove the dependency entirely when we don't even need PETSc for internal use. Is that right?

Reviewed 4 of 4 files at r1, all commit messages.
Reviewable status: LGTM missing from assignee xuchenhan-tri(platform)

@jwnimmer-tri
Copy link
Collaborator Author

This has no observable effect when linking against PETSc internally (other than switching the spelling to petsc_nowarn) ...

Yes. There is no functional effect. The build system will run a few extra commands due to the "nowarn" shim, but nothing important.

... we can remove the dependency entirely when we don't even need PETSc for internal use.

We can delete tools/workspace/petsc after both conditions are satisfied:
(1) FEM no longer uses it, and
(2) the deprecation date has passed.

As of (1) we can drop the deps line in the fem BUILD file, but we need to keep the tools/workspace around until the deprecation date has passed. (Workspace rules are Stable API, unless marked internal.)

Copy link
Contributor

@xuchenhan-tri xuchenhan-tri left a comment

Choose a reason for hiding this comment

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

Understood. Thanks for the explanation.

:lgtm: x2

Reviewable status: :shipit: complete! all discussions resolved, LGTM from assignee xuchenhan-tri(platform)

@xuchenhan-tri xuchenhan-tri merged commit eb31832 into RobotLocomotion:master Jul 31, 2023
@jwnimmer-tri jwnimmer-tri deleted the deprecate-petsc branch July 31, 2023 18:31
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