[install] Generate version.h header

[tom-osika]

Towards #24343

Generates a version.h header that exposes the following macros:

1. DRAKE_VERSION_STR
2. DRAKE_VERSION_AT_LEAST

The file is populated from the variables set in stamp_version.cmake.

I added version_test.cc to verify some of the logic, and an additional line in find_package_drake_install_test.py to ensure that the header is present after an install.

I also verified end-to-end behavior by building drake locally (with the DRAKE_VERSION_OVERRIDE flag set), installing, and consuming in a small example project via find_package.

---

This change is 

[tom-osika]

+a:@mwoehlke-kitware for feature review, please.

@tom-osika made 1 comment.
Reviewable status: LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on mwoehlke-kitware).

[mwoehlke-kitware]

@mwoehlke-kitware partially reviewed 1 file and made 7 comments.
Reviewable status: 7 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on tom-osika).

---

tools/install/libdrake/version.h.in line 8 at r1 (raw file):

// version stamp that populates drake-config.cmake (see
// drake/tools/install/libdrake). For local development builds (i.e., builds
// that are not from a versioned release) the version string is "unknown" and

Suggestion:

release), the

---

tools/install/libdrake/version.h.in line 15 at r1 (raw file):

#define DRAKE_VERSION_STR "@DRAKE_VERSION@"

/** Drake's major version number (e.g. 1), or 0 if unknown. */

Nightlies also have <MAJOR>.<MINOR> == 0.0, so this isn't entirely accurate.

---

tools/install/libdrake/version.h.in line 18 at r1 (raw file):

#define DRAKE_VERSION_MAJOR @DRAKE_VERSION_MAJOR@

/** Drake's minor version number (e.g. 51), or 0 if unknown. */

In addition to the above, we may release 2.0 at some point, in which case 0 doesn't mean "unknown". This is especially the case for PATCH, which is usually 0.

---

tools/install/libdrake/version.h.in line 38 at r1 (raw file):

(major, minor, patch) release. Intended for use in preprocessor conditionals:

  #if DRAKE_VERSION_AT_LEAST(1, 51, 1, 20260311)

I'd sort of like this to support only release versions, by passing 0 for the date. (Also, so users that don't care about nightly-only support don't need to look up a date.)

It might also be nice to have e.g. a DRAKE_VERSION_IS_RELEASE.

---

tools/install/libdrake/version.h.in line 46 at r1 (raw file):

#define DRAKE_VERSION_AT_LEAST(major, minor, patch, yyyymmdd)           \
  ((DRAKE_VERSION_MAJOR == 0 && DRAKE_VERSION_MINOR == 0)               \
       ? (DRAKE_VERSION_PATCH >= (yyyymmdd))                            \

(Pursuant to prior comment...)

Suggestion:

DRAKE_VERSION_PATCH >= (yyyymmdd) && (yyyymmdd) > 0

---

tools/install/libdrake/version.h.in line 43 at r1 (raw file):

For local development builds where the version is unknown (all components 0),
this evaluates to false for any real release. */

I don't understand what "...for any real release" means, here. Maybe just drop that?

---

tools/install/libdrake/test/find_package_drake_install_test.py line 18 at r1 (raw file):

            // Confirm the installed drake/version.h is includable and that its
            // macros are usable in a constant expression.
            static_assert(DRAKE_VERSION_AT_LEAST(0, 0, 0, 0));

Note, might need to change this to ... || true; I think that still accomplishes the goal of lexical validation.

[tyler-yankee]

@tyler-yankee reviewed 5 files and all commit messages, and made 1 comment.
Reviewable status: 8 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on tom-osika).

---

-- commits line 2 at r2:
nit typo

Suggestion:

[install]

[tom-osika]

@tom-osika made 4 comments and resolved 1 discussion.
Reviewable status: 7 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on mwoehlke-kitware and tyler-yankee).

---

-- commits line 2 at r2:

Previously, tyler-yankee (Tyler Yankee) wrote…

nit typo

done 🤦

---

tools/install/libdrake/version.h.in line 15 at r1 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

Nightlies also have <MAJOR>.<MINOR> == 0.0, so this isn't entirely accurate.

Maybe the major, minor, patch, and tweak sections could share one comment above that block, something like:

Code snippet:

/** The major, minor, patch, and tweak version components. Default to 0 if unknown. */
#define DRAKE_VERSION_MAJOR @DRAKE_VERSION_MAJOR@
#define DRAKE_VERSION_MINOR @DRAKE_VERSION_MINOR@
.
.
.

---

tools/install/libdrake/version.h.in line 38 at r1 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

I'd sort of like this to support only release versions, by passing 0 for the date. (Also, so users that don't care about nightly-only support don't need to look up a date.)

It might also be nice to have e.g. a DRAKE_VERSION_IS_RELEASE.

Yeah this makes sense. I'll switch it over

---

tools/install/libdrake/version.h.in line 43 at r1 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

I don't understand what "...for any real release" means, here. Maybe just drop that?

Currently it just means for any release that isn't the pure unknown case. So any stable, nightly, etc. The only case where it would be true is if the user also passed in all 0s. I'm fine deleting it, but maybe rewording it could clarify? Something like:

Code snippet:

this evaluates to false for any stable, nightly, or snapshot release. */

[mwoehlke-kitware]

tools/install/libdrake/version.h.in line 15 at r1 (raw file):

Previously, tom-osika (Tom Osika) wrote…

Maybe the major, minor, patch, and tweak sections could share one comment above that block, something like:

Sure. But maybe something like "If the version is not known (e.g. developer builds), all of these will be 0." And maybe also say something about nightlies.

[tyler-yankee]

@tyler-yankee reviewed 1 file and all commit messages, and made 2 comments.
Reviewable status: 7 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on mwoehlke-kitware and tom-osika).

---

tools/install/libdrake/version.h.in line 18 at r1 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

In addition to the above, we may release 2.0 at some point, in which case 0 doesn't mean "unknown". This is especially the case for PATCH, which is usually 0.

(and given a couple months ago when we actually had to do 2 patch releases, would be relevant)

---

tools/install/libdrake/version.h.in line 38 at r1 (raw file):

It might also be nice to have e.g. a DRAKE_VERSION_IS_RELEASE

DRAKE_VERSION_IS_STABLE_RELEASE? or is that too verbose?

[jwnimmer-tri]

@jwnimmer-tri made 1 comment.
Reviewable status: 7 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on mwoehlke-kitware and tom-osika).

---

tools/install/libdrake/version.h.in line 38 at r1 (raw file):

It might also be nice to have e.g. a DRAKE_VERSION_IS_RELEASE.

The typical use case that we should encourage is the user always passing all four numbers. I'm OK allowing the YYYYMMDD to be zero if we want to let them be lazy and they get to keep the pieces when it breaks, but we should NOT encourage it with a special-purpose sugar macro that omits the final part. (They could always write their own sugar macro for that, if they choose.)

[mwoehlke-kitware]

@mwoehlke-kitware reviewed 1 file, made 2 comments, and resolved 1 discussion.
Reviewable status: 6 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on jwnimmer-tri, tom-osika, and tyler-yankee).

---

tools/install/libdrake/version.h.in line 38 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

It might also be nice to have e.g. a DRAKE_VERSION_IS_RELEASE.

The typical use case that we should encourage is the user always passing all four numbers. I'm OK allowing the YYYYMMDD to be zero if we want to let them be lazy and they get to keep the pieces when it breaks, but we should NOT encourage it with a special-purpose sugar macro that omits the final part. (They could always write their own sugar macro for that, if they choose.)

Either. @jwnimmer-tri, what I'm proposing takes no arguments. I agree it isn't worth trying to sugar a version of the version check that only takes three numbers. My idea was to have an easy way to 'opt out' of any non-release version, which isn't quite the same thing as a version check. In the moment, it seemed like something that might be useful, but if you hate it, I don't feel strongly.

---

tools/install/libdrake/version.h.in line 43 at r1 (raw file):

Previously, tom-osika (Tom Osika) wrote…

Currently it just means for any release that isn't the pure unknown case. So any stable, nightly, etc. The only case where it would be true is if the user also passed in all 0s. I'm fine deleting it, but maybe rewording it could clarify? Something like:

If anything, that makes even less sense. Maybe there's a fundamental problem with this sentence structure? I'm reading this as "if you are using a 'real release' of Drake, DRAKE_VERSION_AT_LEAST (always) evaluates to false", which is surely not what you meant.

That said, as the suggestion that this is false for nightlies if the yyyymmdd argument is 0 would make this false for any input except negative numbers, "this evaluates to false" without further qualification is closer to correct.

[jwnimmer-tri]

@jwnimmer-tri made 1 comment.
Reviewable status: 6 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on mwoehlke-kitware and tom-osika).

---

tools/install/libdrake/version.h.in line 38 at r1 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

Either. @jwnimmer-tri, what I'm proposing takes no arguments. I agree it isn't worth trying to sugar a version of the version check that only takes three numbers. My idea was to have an easy way to 'opt out' of any non-release version, which isn't quite the same thing as a version check. In the moment, it seemed like something that might be useful, but if you hate it, I don't feel strongly.

Ah, yes, I misunderstood the proposal.

I'm not super inclined to offer sugar to reject nightly releases. I hope that most downstream code allows them. If they really want to, I believe they could check DRAKE_VERSION_AT_LEAST(1, 0, 0, 99999999) which would allow all stable releases and reject all nightly releases.

[tom-osika]

@tom-osika made 1 comment.
Reviewable status: 6 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on mwoehlke-kitware).

---

tools/install/libdrake/version.h.in line 43 at r1 (raw file):

... "if you are using a 'real release' of Drake, DRAKE_VERSION_AT_LEAST (always) evaluates to false", which is surely not what you meant.

Not if you're using a real version of drake, if you pass in the version info corresponding to an actual release and are using drake built from source with bazel (where we have absolutely nothing for version info, all 0s). That's what the comment is trying to get across. I'll reword to "this evaluates to false", I just wanted to clarify

[tom-osika]

@tom-osika made 5 comments and resolved 2 discussions.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on mwoehlke-kitware and tyler-yankee).

---

tools/install/libdrake/version.h.in line 15 at r1 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

Sure. But maybe something like "If the version is not known (e.g. developer builds), all of these will be 0." And maybe also say something about nightlies.

I've updated the corresponding section. I'll leave this for you to resolve in case you prefer further edits.

---

tools/install/libdrake/version.h.in line 18 at r1 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

(and given a couple months ago when we actually had to do 2 patch releases, would be relevant)

Done - (See my other comment about this section. We can wordsmith further if necessary).

---

tools/install/libdrake/version.h.in line 38 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Ah, yes, I misunderstood the proposal.

I'm not super inclined to offer sugar to reject nightly releases. I hope that most downstream code allows them. If they really want to, I believe they could check DRAKE_VERSION_AT_LEAST(1, 0, 0, 99999999) which would allow all stable releases and reject all nightly releases.

Done - To summarize, it sounds like we are enforcing that a valid date is always be passed in, and avoiding adding DRAKE_VERSION_IS_RELEASE.

---

tools/install/libdrake/version.h.in line 46 at r1 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

(Pursuant to prior comment...)

Done - I've left this code unchanged but from the other discussion thread, we are expecting yyyymmdd to be nonzero from the user.

---

tools/install/libdrake/test/find_package_drake_install_test.py line 18 at r1 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

Note, might need to change this to ... || true; I think that still accomplishes the goal of lexical validation.

Done - I've updated this regardless of the date=0 discussion.

[mwoehlke-kitware]

@mwoehlke-kitware reviewed all commit messages, made 2 comments, and resolved 1 discussion.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on tyler-yankee).

---

tools/install/libdrake/version.h.in line 43 at r1 (raw file):

Previously, tom-osika (Tom Osika) wrote…

... "if you are using a 'real release' of Drake, DRAKE_VERSION_AT_LEAST (always) evaluates to false", which is surely not what you meant.

Not if you're using a real version of drake, if you pass in the version info corresponding to an actual release and are using drake built from source with bazel (where we have absolutely nothing for version info, all 0s). That's what the comment is trying to get across. I'll reword to "this evaluates to false", I just wanted to clarify

Right; my point is that the prior wording failed to indicate that distinction. You know what you meant, but it wasn't communicated well. 🙂

Current wording WFM; thanks.

---

tools/install/libdrake/version.h.in line 46 at r1 (raw file):

Previously, tom-osika (Tom Osika) wrote…

Done - I've left this code unchanged but from the other discussion thread, we are expecting yyyymmdd to be nonzero from the user.

That isn't consistent with what we discussed in the F2F meeting.

If a user writes DRAKE_VERSION_AT_LEAST(1, 57, 0, 0), that shouldn't be true for 0.0.20260721. Also, in its current form, that would be true for a development version, which is contrary to the preceding documentation.

I don't agree that we should just assume the user will never pass 0 for the yyyymmd. Nor can I think of any way to make it an error if they do. That means we are going to give a wrong answer for some cases. That being the case, I would rather give false negatives than false positives.

[tom-osika]

@tom-osika made 1 comment.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on mwoehlke-kitware and tyler-yankee).

---

tools/install/libdrake/version.h.in line 46 at r1 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

That isn't consistent with what we discussed in the F2F meeting.

If a user writes DRAKE_VERSION_AT_LEAST(1, 57, 0, 0), that shouldn't be true for 0.0.20260721. Also, in its current form, that would be true for a development version, which is contrary to the preceding documentation.

I don't agree that we should just assume the user will never pass 0 for the yyyymmd. Nor can I think of any way to make it an error if they do. That means we are going to give a wrong answer for some cases. That being the case, I would rather give false negatives than false positives.

Good points. I've added the safeguard and modified the documentation slightly

[tyler-yankee]

@tyler-yankee reviewed 3 files and all commit messages.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on mwoehlke-kitware).

[mwoehlke-kitware]

Everything but the test LGTM.

@mwoehlke-kitware reviewed 1 file and all commit messages, made 5 comments, and resolved 3 discussions.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee mwoehlke-kitware, needs platform reviewer assigned, needs at least two assigned reviewers, missing label for release notes (waiting on tom-osika).

---

tools/install/libdrake/version.h.in line 38 at r1 (raw file):

Previously, tom-osika (Tom Osika) wrote…

Done - To summarize, it sounds like we are enforcing that a valid date is always be passed in, and avoiding adding DRAKE_VERSION_IS_RELEASE.

Okay, I'm fine dropping it until/unless someone asks for it.

---

tools/install/libdrake/test/version_test.cc line 21 at r5 (raw file):

// without assuming which kind of build it is (stable release, nightly, or an
// unstamped "unknown" build).
GTEST_TEST(VersionTest, AtLeast) {

I suppose this is testing the ..._AT_LEAST macro, but it might be better to just name the test case 'basic' or some such? Or maybe '[Test]Macro'?

BTW, it might also not be horrible to have a test that the string is of the form '\d+[.]\d+[.]\d+[.]\d+'.

---

tools/install/libdrake/test/version_test.cc line 36 at r5 (raw file):

    // one.
    EXPECT_TRUE(DRAKE_VERSION_AT_LEAST(0, 0, 0, DRAKE_VERSION_PATCH));
    EXPECT_FALSE(DRAKE_VERSION_AT_LEAST(0, 0, 0, DRAKE_VERSION_PATCH + 1));

Suggestion: also test here that DRAKE_VERSION_AT_LEAST(9999, 0, 0, 0) is false.

---

tools/install/libdrake/test/version_test.cc line 39 at r5 (raw file):

  } else {
    // Unstamped ("unknown") build: never at least as new as any real release.
    EXPECT_FALSE(DRAKE_VERSION_AT_LEAST(0, 0, 0, 1));

The way the macro is set up, you don't actually need a 1 here.

Suggestion:

0

[mwoehlke-kitware]

tools/install/libdrake/test/version_test.cc line 21 at r5 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

I suppose this is testing the ..._AT_LEAST macro, but it might be better to just name the test case 'basic' or some such? Or maybe '[Test]Macro'?

BTW, it might also not be horrible to have a test that the string is of the form '\d+[.]\d+[.]\d+[.]\d+'.

Possible implementation:

{
    int parts = 1;
    for (char const* p = in; *p; ++p) {
        if (*p == '.') {
            if (p == in || p[-1] == '.' || !p[1]) {
                return false;
            }
            ++parts;
        } else if (!std::isdigit(*p)) {
            return false;
        }
    }
    return parts == 4;
}

[tom-osika]

@tom-osika made 1 comment and resolved 2 discussions.
Reviewable status: 1 unresolved discussion, LGTM missing from assignees jwnimmer-tri(platform),mwoehlke-kitware, missing label for release notes (waiting on mwoehlke-kitware and tyler-yankee).

---

tools/install/libdrake/test/version_test.cc line 21 at r5 (raw file):

...but it might be better to just name the test case 'basic' or some such? Or maybe '[Test]Macro'?

"Basic" seems good, I've renamed appropriately.

...have a test that the string is of the form '\d+[.]\d+[.]\d+[.]\d+'.

I've added a test using std::regex, but can switch to an implementation based on the code snippet you pasted here if we want to avoid including the extra <regex> header

[mwoehlke-kitware]

tools/install/libdrake/test/version_test.cc line 51 at r6 (raw file):

  // The four numeric components always form a "major.minor.patch.tweak" quad,
  // e.g. "1.51.1.0" or "0.0.0.0", whatever the build.
  const std::string version = std::to_string(DRAKE_VERSION_MAJOR) + "." +

This is going to be almost tautological. The intent was to test that DRAKE_VERSION_STR is sane. (Um... which needs an `|| equals "unknown").

[jwnimmer-tri]

@jwnimmer-tri reviewed 4 files and made 7 comments.
Reviewable status: 9 unresolved discussions, LGTM missing from assignees jwnimmer-tri(platform),mwoehlke-kitware, missing label for release notes (waiting on mwoehlke-kitware, tom-osika, and tyler-yankee).

---

a discussion (no related file):
One way or another, the macro(s) like DRAKE_VERSION_AT_LEAST that we expect users to use need to be documented as part of https://drake.mit.edu/doxygen_cxx/.

---

a discussion (no related file):
Working

Will need a final look from a fresh (not me) platform reviewer before merging.

---

tools/install/libdrake/BUILD.bazel line 246 at r5 (raw file):

    hdrs = [":version.h"],
    strip_include_prefix = "/tools/install/libdrake",
    visibility = ["//visibility:public"],

I'm not sure if the library should actually be public? At least, not with this label.

We don't really want @drake//tools/install/libdrake:anything to become part of our Stable API surface that we need to keep backwards-compatible.

However, I would buy an alias(name = "version", ...) somewhere else with public visibility, perhaps in common/BUILD.bazel or maybe in the root BUILD.bazel.

---

tools/install/libdrake/version.h.in line 13 at r5 (raw file):

/** Drake's full version string, e.g. "1.51.1", or "unknown" for an unstamped
build. */
#define DRAKE_VERSION_STR "@DRAKE_VERSION@"

Drake tends to eschew abbreviation. Also, Gemini says that "STRING" is more common than "STR" for this use case, citing several modern examples.

Suggestion:

DRAKE_VERSION_STRING

---

tools/install/libdrake/version.h.in line 15 at r5 (raw file):

#define DRAKE_VERSION_STR "@DRAKE_VERSION@"

/** The major, minor, patch, and tweak version components. For a stable

TLDR: Remove this group of macros.

The ticket was very particular in NOT requesting macros like these (DRAKE_VERSION_MAJOR, etc.). By establishing macros like these as part of our API, we are locking in a version format and semantics forever, never allowing us to change how we label our e.g. nightly or continuous releases.

The "AT_LEAST" macro provided an abstraction layer -- users can check whether their code should use the new/old spelling of API that changed in a specific version. If we change how our version numbers work, we can still do that so long as we're able to provide an updated implementation of the macro. By leaking the version components, we lose the opportunity to ever change it.

(The "AT_LEAST" macro can just refer to the @DRAKE_VERSION_MAJOR@ substitutions directly.)

---

tools/install/libdrake/version.h.in line 30 at r5 (raw file):

builds:

- For a stable release (e.g. "1.51.1"), it is true iff the version is greater

nit In american english, always use a comma after "e.g.".

Suggestion:

e.g., "1.51.1"

---

tools/install/libdrake/test/version_test.cc line 21 at r6 (raw file):

static_assert(sizeof(DRAKE_VERSION_STR) > 1);

// Exercises DRAKE_VERSION_AT_LEAST against whatever version this build reports,

I think a lot of this test is dead code. The only builds that set a version stamp are packaging builds, and they don't run Bazel tests (nor install tests). So everything here will always be zeros.

[mwoehlke-kitware]

tools/install/libdrake/test/version_test.cc line 21 at r5 (raw file):

Previously, tom-osika (Tom Osika) wrote…

...but it might be better to just name the test case 'basic' or some such? Or maybe '[Test]Macro'?

"Basic" seems good, I've renamed appropriately.

...have a test that the string is of the form '\d+[.]\d+[.]\d+[.]\d+'.

I've added a test using std::regex, but can switch to an implementation based on the code snippet you pasted here if we want to avoid including the extra <regex> header

Heh, I'm not used to having a std::regex to play with. That's fine; this isn't performance-sensitive code.

[tom-osika]

@tom-osika made 6 comments and resolved 2 discussions.
Reviewable status: 6 unresolved discussions, LGTM missing from assignees jwnimmer-tri(platform),mwoehlke-kitware, missing label for release notes (waiting on jwnimmer-tri, mwoehlke-kitware, and tyler-yankee).

---

tools/install/libdrake/BUILD.bazel line 246 at r5 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I'm not sure if the library should actually be public? At least, not with this label.

We don't really want @drake//tools/install/libdrake:anything to become part of our Stable API surface that we need to keep backwards-compatible.

However, I would buy an alias(name = "version", ...) somewhere else with public visibility, perhaps in common/BUILD.bazel or maybe in the root BUILD.bazel.

Maybe the root BUILD.bazel is slightly better? That's where the drake_shared_library alias is defined.

---

tools/install/libdrake/version.h.in line 13 at r5 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Drake tends to eschew abbreviation. Also, Gemini says that "STRING" is more common than "STR" for this use case, citing several modern examples.

Done.

---

tools/install/libdrake/version.h.in line 15 at r5 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

TLDR: Remove this group of macros.

The ticket was very particular in NOT requesting macros like these (DRAKE_VERSION_MAJOR, etc.). By establishing macros like these as part of our API, we are locking in a version format and semantics forever, never allowing us to change how we label our e.g. nightly or continuous releases.

The "AT_LEAST" macro provided an abstraction layer -- users can check whether their code should use the new/old spelling of API that changed in a specific version. If we change how our version numbers work, we can still do that so long as we're able to provide an updated implementation of the macro. By leaking the version components, we lose the opportunity to ever change it.

(The "AT_LEAST" macro can just refer to the @DRAKE_VERSION_MAJOR@ substitutions directly.)

Done - Thanks for pointing this out. They've been removed.

---

tools/install/libdrake/test/version_test.cc line 21 at r5 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

Heh, I'm not used to having a std::regex to play with. That's fine; this isn't performance-sensitive code.

As @jwnimmer-tri pointed out, the only DRAKE_VERSION_STRING these tests will see is "unknown", so the quad form check is moot.

---

tools/install/libdrake/test/version_test.cc line 21 at r6 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I think a lot of this test is dead code. The only builds that set a version stamp are packaging builds, and they don't run Bazel tests (nor install tests). So everything here will always be zeros.

Good point. I intentionally left this with the extra checks originally, just in case. But I've removed the dead code.

---

tools/install/libdrake/test/version_test.cc line 51 at r6 (raw file):

Previously, mwoehlke-kitware (Matthew Woehlke) wrote…

This is going to be almost tautological. The intent was to test that DRAKE_VERSION_STR is sane. (Um... which needs an `|| equals "unknown").

Done - see above comment. This has been updated to check against "unknown"

[jwnimmer-tri]

@jwnimmer-tri reviewed 2 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: 4 unresolved discussions, LGTM missing from assignees jwnimmer-tri(platform),mwoehlke-kitware, missing label for release notes (waiting on tom-osika).

---

tools/install/libdrake/BUILD.bazel line 246 at r5 (raw file):

Previously, tom-osika (Tom Osika) wrote…

Maybe the root BUILD.bazel is slightly better? That's where the drake_shared_library alias is defined.

I'm OK with the root.

[tom-osika]

@tom-osika made 3 comments and resolved 1 discussion.
Reviewable status: 3 unresolved discussions, LGTM missing from assignees jwnimmer-tri(platform),mwoehlke-kitware, missing label for release notes (waiting on jwnimmer-tri, mwoehlke-kitware, and tyler-yankee).

---

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

One way or another, the macro(s) like DRAKE_VERSION_AT_LEAST that we expect users to use need to be documented as part of https://drake.mit.edu/doxygen_cxx/.

I've put them in the Technical Notes section. This seemed to be the most reasonable spot, but happy to move it if there's a better location.

The version.h brief docs are also under Files.

---

tools/install/libdrake/BUILD.bazel line 246 at r5 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I'm OK with the root.

Done

---

tools/install/libdrake/test/version_test.cc line 21 at r6 (raw file):

Previously, tom-osika (Tom Osika) wrote…

Good point. I intentionally left this with the extra checks originally, just in case. But I've removed the dead code.

Done

[mwoehlke-kitware]

@mwoehlke-kitware made 1 comment.
Reviewable status: 3 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), missing label for release notes (waiting on jwnimmer-tri and tyler-yankee).

[tyler-yankee]

@tyler-yankee reviewed 5 files and all commit messages, and made 2 comments.
Reviewable status: 5 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), missing label for release notes (waiting on jwnimmer-tri and tom-osika).

---

tools/install/libdrake/version.h.in line 21 at r8 (raw file):

"1.51.1"; for a nightly or snapshot build like "0.0.20260721.143022+gitabc";
for an unstamped build it is "unknown".
@hideinitializer */

TIL hideinitializer!

---

tools/install/libdrake/version.h.in line 48 at r8 (raw file):

       ? (@DRAKE_VERSION_PATCH@ >= (yyyymmdd) && (yyyymmdd) > 0)                  \
       : (@DRAKE_VERSION_MAJOR@ > (major) ||                                      \
          (@DRAKE_VERSION_MAJOR@ == (major) && (@DRAKE_VERSION_MINOR@ > (minor) || (@DRAKE_VERSION_MINOR@ == (minor) && @DRAKE_VERSION_PATCH@ >= (patch))))))

nit can we improve wrapping here (and consistent spacing of \ above)? (I haven't followed too closely, but it looks like a prior revision had this?)

[tom-osika]

@tom-osika made 1 comment.
Reviewable status: 5 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), missing label for release notes (waiting on jwnimmer-tri and tyler-yankee).

---

tools/install/libdrake/version.h.in line 48 at r8 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

nit can we improve wrapping here (and consistent spacing of \ above)? (I haven't followed too closely, but it looks like a prior revision had this?)

The problem is that the linting runs on the generated file, not the version.h.in. So this needs to be setup in a way so that after replacements, it is formatted correctly (with consistent spacing of \, for example). The version major, minor, etc. will always be 0 during linting since its run from bazel.

Maybe its appropriate to ignore the generated file?

[tyler-yankee]

@tyler-yankee made 1 comment and resolved 1 discussion.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), missing label for release notes (waiting on jwnimmer-tri and tom-osika).

---

tools/install/libdrake/version.h.in line 48 at r8 (raw file):

Previously, tom-osika (Tom Osika) wrote…

The problem is that the linting runs on the generated file, not the version.h.in. So this needs to be setup in a way so that after replacements, it is formatted correctly (with consistent spacing of \, for example). The version major, minor, etc. will always be 0 during linting since its run from bazel.

Maybe its appropriate to ignore the generated file?

agh. I'll defer to others' thoughts

[jwnimmer-tri]

@jwnimmer-tri made 1 comment.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), missing label for release notes (waiting on tom-osika and tyler-yankee).

---

tools/install/libdrake/version.h.in line 48 at r8 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

agh. I'll defer to others' thoughts

Definitely no need to lint generated code.

[tyler-yankee]

@tyler-yankee made 1 comment.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), missing label for release notes (waiting on jwnimmer-tri and tom-osika).

---

tools/install/libdrake/version.h.in line 48 at r8 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Definitely no need to lint generated code.

I just thought it might be nice if the header we actually ship is well-formatted. But you need to choose between that and maintainers actually reading+editing the .in, and I agree that the latter is probably higher priority.

[jwnimmer-tri]

@jwnimmer-tri made 1 comment.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), missing label for release notes (waiting on tom-osika and tyler-yankee).

---

tools/install/libdrake/version.h.in line 48 at r8 (raw file):

Previously, tyler-yankee (Tyler Yankee) wrote…

I just thought it might be nice if the header we actually ship is well-formatted. But you need to choose between that and maintainers actually reading+editing the .in, and I agree that the latter is probably higher priority.

Part of the tension is due to the current design where the logic (version comparison macro) and substitutions values (stamped constants) appear in the same file. (There's really no reason they need to be in the same file.)

[tom-osika]

@tom-osika made 1 comment.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), missing label for release notes (waiting on jwnimmer-tri and tyler-yankee).

---

tools/install/libdrake/version.h.in line 48 at r8 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Part of the tension is due to the current design where the logic (version comparison macro) and substitutions values (stamped constants) appear in the same file. (There's really no reason they need to be in the same file.)

Good call, I'll separate them out. Then we can hand-write the version.h file

[tom-osika]

@tom-osika made 2 comments and resolved 1 discussion.
Reviewable status: 4 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), missing label for release notes (waiting on jwnimmer-tri, mwoehlke-kitware, and tyler-yankee).

---

tools/install/libdrake/version.h.in line 48 at r8 (raw file):

Previously, tom-osika (Tom Osika) wrote…

Good call, I'll separate them out. Then we can hand-write the version.h file

Done

---

tools/install/libdrake/version_values.h.in line 10 at r9 (raw file):

// the macros in drake/version.h.

#define DRAKE_VERSION_INTERNAL_STRING "@DRAKE_VERSION@"

These are only back to make the macro in version.h writable by hand. If there is a more preferred way to make them "internal" besides the naming and comment above, I can switch.

[mwoehlke-kitware]

tools/install/libdrake/version.h line 8 at r9 (raw file):

// build (e.g., a plain `bazel build`), the version string is "unknown".

#include "drake/version_values.h"

BTW, would version_internal.h be a better name?

[tyler-yankee]

@tyler-yankee partially reviewed 4 files and all commit messages, and made 1 comment.
Reviewable status: 5 unresolved discussions, LGTM missing from assignee jwnimmer-tri(platform), missing label for release notes (waiting on jwnimmer-tri and tom-osika).

---

tools/install/libdrake/version_values.h.in line 10 at r9 (raw file):

Previously, tom-osika (Tom Osika) wrote…

These are only back to make the macro in version.h writable by hand. If there is a more preferred way to make them "internal" besides the naming and comment above, I can switch.

My first instinct would be to spell _DRAKE_VERSION_STRING and the like (it seems we have a private helper _DRAKE_TEST_NO_THROW?), but I'll defer to @jwnimmer-tri for style.