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

gz_math_internal needs update to latest #17907

Closed
Tracked by #17807
mwoehlke-kitware opened this issue Sep 13, 2022 · 4 comments · Fixed by #17913
Closed
Tracked by #17807

gz_math_internal needs update to latest #17907

mwoehlke-kitware opened this issue Sep 13, 2022 · 4 comments · Fixed by #17913
Assignees
Labels
component: build system Bazel, CMake, dependencies, memory checkers, linters priority: low

Comments

@mwoehlke-kitware
Copy link
Contributor

gz_math_internal needs to be updated to the latest release. It appears that a number of headers (and possible other files) have moved, making Bazel unhappy, and the changes are sufficiently non-trivial as to require sorting out on an individual file level (i.e. some things have changed and some apparently haven't, and someone needs to go through the files one at a time to identify which is which).

@jwnimmer-tri jwnimmer-tri added the component: build system Bazel, CMake, dependencies, memory checkers, linters label Sep 13, 2022
@jwnimmer-tri
Copy link
Collaborator

If you have time to work on this yourself, that's fine by me. If not, feel free to assign the ticket to me and I'll investigate next week.

Or possibly this is related to #17873 and @scpeters wants to own it?

@mwoehlke-kitware
Copy link
Contributor Author

I don't think it's related to libsdformat.

I took a closer look, and it appears all of _MOST_HDRS just needs s/ignition/gz/. However, things are still unhappy because:

The following files matched a glob of upstream sources, but were not covered by the package.BUILD.bazel file:
  [
    "include/ignition/math.hh",
    "include/ignition/math/Export.hh",
    "include/ignition/math/config.hh"
  ]

The first appears to be new. However, it is unclear why the last are present in _HDRS but are being excluded from check_lists_consistency, which I think is the source of the above complaint?

Thus, I am punting to someone that knows more about what's going on there...

@scpeters
Copy link
Contributor

scpeters commented Sep 13, 2022

gz_math_internal needs to be updated to the latest release.

The latest release is 6.13.0, but 7.0.0 has a prerelease and will have a stable release within the next few weeks. There are several changes to header files related to the name change from ignition -> gz for reasons outlined in the following post:

In gz-math 7.0.0, the ignition/* header files and ignition:: C++ namespace are deprecated in favor of gz/* and gz:: respectively (see Migration guide). In 6.13.0, we backported the gz/* header files and gz:: namespace to simplify the migration from 6 -> 7.

Or possibly this is related to #17873 and @scpeters wants to own it?

I don't think it's related to libsdformat.

It's kinda related because upgrading to libsdformat13 requires upgrading to gz-math7 (and gz-utils2). I have already done much of the bazel work required for this as part of my work on #17873 (see 915bfcf). If this is urgent, I could split out the portion of that work relevant to gz-math 6.13.0 and submit a pull request, but if it's not urgent, we could wait a few weeks for gz-math 7.0.0 to be released, at which point we could leapfrog over 6.13.0 directly to gz-math 7.0.0 along with libsdformat13 and gz-utils2.

@jwnimmer-tri
Copy link
Collaborator

If this is urgent, ...

It's not urgent. Drake upgrades our externals on a monthly cadence so that we don't get too far behind upstream and so that we can obtain timely feedback about any regressions. Pushing that back by a few weeks or a month (especially when there is a Draft PR already under CI) is no burden.

So, we can treat this as part of #17873 and so I'll reassign it accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: build system Bazel, CMake, dependencies, memory checkers, linters priority: low
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants