-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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] Upgrade vtk_internal to latest commit #21799
[workspace] Upgrade vtk_internal to latest commit #21799
Conversation
Remove patches that have been upstreamed to VTK.
417da11
to
6b88d2b
Compare
I'll take this one over. There's more subtle work involved than just disabling the patches. We also need to trim some deps. |
For the record, here's the VTK segfault this works around:
Stack:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r1, 1 of 3 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: LGTM missing from assignee jwnimmer-tri(platform), commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @BetsyMcPhail)
+@xuchenhan-tri for both reviews per schedule, please. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FTR I'm not familiar with the what we need or don't need when building VTK. Please find an additional reviewer if you want more scrutiny on that.
Reviewed 4 of 4 files at r1, 1 of 3 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: commits need curation (https://drake.mit.edu/reviewable.html#curated-commits) (waiting on @BetsyMcPhail)
Thanks, I'm okay with just your review. The rule of thumb is that when CI is passing, then everything we need is being correctly built. |
The commit message subject line should be the PR title.
The commit message body should be the text from the above stanza.
Towards #21759.
This change is