Consider changing our inline namespace name with each release

[devjgm]

We use an inline namespace in our code to help with library versioning. For example:

google-cloud-cpp/google/cloud/spanner/numeric.h

Lines 30 to 33 in b5b37ac

	namespace google {
	namespace cloud {
	namespace spanner {
	inline namespace SPANNER_CLIENT_NS {

which today resolves to v1 for all of our libraries. We rarely change this inline namespace name.

The purpose of using an an inline namespace is to allow for versioning at the API and ABI levels. However, for this to actually work, we should change the inline namespace anytime we make an API or ABI change. An easy way to do this is to simply change the inline namespace name with every release.

Proposal

We should change the GOOGLE_CLOUD_CPP_NS define

google-cloud-cpp/google/cloud/version.h

Lines 22 to 26 in b5b37ac

	#define GOOGLE_CLOUD_CPP_VCONCAT(Ma, Mi) v##Ma
	#define GOOGLE_CLOUD_CPP_VEVAL(Ma, Mi) GOOGLE_CLOUD_CPP_VCONCAT(Ma, Mi)
	#define GOOGLE_CLOUD_CPP_NS \
	GOOGLE_CLOUD_CPP_VEVAL(GOOGLE_CLOUD_CPP_VERSION_MAJOR, \
	GOOGLE_CLOUD_CPP_VERSION_MINOR)

to include the whole released version.

For example, the currently released version is v1.25.0, and so the inline namespace name should probably expand to v1_25_0

Other considerations

1. We should update our Support docs to specify how we expect users to use inline namespaces. In general, users should not name the inline namespace. This shortens their code and gives them a path to upgrading to newer versions without breaking, as long as they recompile. Users should only spell the inline namespace name when they tightly depend on a specific version. This should be rare. By default, users should not spell the inline namespace name. I believe this is all standard C++ advice that would likely apply to any C++ code that uses inline namespaces.
2. We should announce this change in the CHANGELOG.md.
3. What should be the inline namespace name for the unreleased code at HEAD? We could use the next library version, but that's not really correct, because the eventual release with that version will be different. We could use the string head, which is what Abseil does. Or we could use something like v1_26_0_preview.

[coryan]

This looks like a dup of #5621, but maybe with better motivation and details. I am copying the text here for future reference:

The main rational for the inline namespaces e.g. google::cloud::pubsub::v1 is to support linking multiple versions of the client library in a single binary, for example, because an application depends on libFoo which requires an specific version of the Pub/Sub library and the application also needs the Spanner library, but a slightly different version. Both will want to link google_cloud_cpp_common, and they might need different versions.

More often this is only a problem when using binary artifacts. This can be a problem even when the application compiles everything from source. It might be easier to avoid in that case, but not always possible. Consider the case where the user does not control libFoo or is unable to fix libFoo, and libFoo depends on specific APIs that have been removed or changed in newer versions of the Cloud C++ libraries.

Because we do not support semver, we should include the minor version in the inline namespace, otherwise all releases have the same version. Arguably we should include the patch version too, but in practice that is less of a problem.

We should also consider allowing the application to override the version during the build.... Hmmm, maybe they can already, by passing -DGOOGLE_CLOUD_CPP_VERSION_MAJOR=2 in the CMake configuration step.

[devjgm]

Let me summarize the changes that I think we need to make for this:

GOAL: Updated our inline namespace name with each released version, e.g., google::cloud::spanner::v1_31_1.

I believe the steps to get there are:

• Mention how our inline namespaces work in our https://github.com/googleapis/google-cloud-cpp/blob/main/README.md#support doc. Specifically mention that inline namespaces should generally be omitted unless your code has a hard binary requirement on a specific library version. This change should be called out in CHANGELOG.md, though it's not actually a breaking change.
• All generated code should switch to using GOOGLE_CLOUD_CPP_NS rather than GOOGLE_CLOUD_CPP_GENERATED_NS. AFAICT, there's no need to have these inline namespaces be different, and making them the same should simplify things.
• Delete the GOOGLE_CLOUD_CPP_GENERATED* preprocessor symbols from google/cloud/version.h. This is a breaking change and needs to be called out in CHANGELOG.md. Optionally, if we want to avoid this breaking change, we could keep these symbols. We can discussion how we want to proceed here. UPDATE: We will keep the GOOGLE_CLOUD_CPP_GENERATED_NS preprocessor symbol so that we don't break customers, but we will add a comment saying that it's deprecated and its use should be avoided.
• Change GOOGLE_CLOUD_CPP_NS to resolve to a string like v1_31_1 including the major, minor, and patch numbers.
• Mention all this in CHANGELOG.md as an FYI, because I don't believe this is a breaking change, other than the one step above where the breaking change is explicitly called out.

Note: There's still a question of what the version should be for non-release versions. Abseil uses head, which we could do too, but that would require some changes to our release process. I think we could proceed with this plan and have pre-released code use and install with the next release's version number, which doesn't seem wrong and is effectively the status quo. We could decide at a later time to change this string to something else and possibly match Abseil's behavior if we want.

[coryan]

We have a few *.dox files that reference v1 explicitly (for bad Doxygen reasons I think). We would probably want those files to change automatically in each release?

Also, consider just using google::cloud::spanner::v1_31. I do not think we ever break API (or ABI) on patch releases.

[devjgm]

We have a few *.dox files that reference v1 explicitly (for bad Doxygen reasons I think). We would probably want those files to change automatically in each release?

Thanks for pointing this out. I'll make sure to check this and see how to update it.

Also, consider just using google::cloud::spanner::v1_31. I do not think we ever break API (or ABI) on patch releases.

That's probably true, but is there any advantage to omitting the patch number? Alternatively is there any disadvantage to including it? I don't feel strongly either way, other than having the version and namespace numbers actually match w/ all three components seems somewhat nice.