Skip to content

refactor(common): versioned namespace includes major, minor, patch #7464

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.

Sign up for GitHub

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

Merged
devjgm merged 3 commits into from
Oct 14, 2021
Merged

refactor(common): versioned namespace includes major, minor, patch #7464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
280a50e
refactor(common): versioned namespace includes major, minor, patch
devjgm Oct 14, 2021
ddd9d18
update abi dumps
devjgm Oct 14, 2021
ecf59c5
updated release instructions to update the ABI dumps after bumping th…
devjgm Oct 14, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,27 @@
details.
</details>

<details>
<summary>2022-11-01: remove `v1` and `gcpcxxV1` backward compatibility aliases</summary>
<br>

* On 2022-11-01 (or shortly after) we will remove the inline namespace aliases
of `v1` and `gcpcxxV1` that are declared in `google/cloud/version.h`. These
aliases exist because we changed the format of our inline namespace name to
include the major, minor, and patch numbers for each release, and we didn't
want to break users. Our Doxygen documentation was mistakenly recommending
that users include the inline namespace names in their code, but this was
also fixed to now recommend against doing so. Users should generally omit the
name of our versioned inline namespace name because that will tightly couple
their code to a specific release, and will make upgrading to newer versions
more difficult. Instead, users should simply omit the inline namespace name,
for example, instead of ~`google::cloud::v1::Status`~ use
`google::cloud::Status`. Please update your code to avoid naming the `v1` and
`gcpcxxV1` inline namespace names before 2022-11-01. For more info, see
https://github.com/googleapis/google-cloud-cpp/issues/7463 and
https://github.com/googleapis/google-cloud-cpp/issues/5976.
</details>

## v1.33.0 - TBD

## v1.32.1 - 2021-10
Expand Down
Binary file modified ci/abi-dumps/google_cloud_cpp_bigquery.expected.abi.dump.gz
View file
Open in desktop
Binary file not shown.
Binary file modified ci/abi-dumps/google_cloud_cpp_bigtable.expected.abi.dump.gz
View file
Open in desktop
Binary file not shown.
Binary file modified ci/abi-dumps/google_cloud_cpp_common.expected.abi.dump.gz
View file
Open in desktop
Binary file not shown.
Binary file modified ci/abi-dumps/google_cloud_cpp_grpc_utils.expected.abi.dump.gz
View file
Open in desktop
Binary file not shown.
Binary file modified ci/abi-dumps/google_cloud_cpp_iam.expected.abi.dump.gz
View file
Open in desktop
Binary file not shown.
Binary file modified ci/abi-dumps/google_cloud_cpp_pubsub.expected.abi.dump.gz
View file
Open in desktop
Binary file not shown.
Binary file modified ci/abi-dumps/google_cloud_cpp_spanner.expected.abi.dump.gz
View file
Open in desktop
Binary file not shown.
Binary file modified ci/abi-dumps/google_cloud_cpp_storage.expected.abi.dump.gz
View file
Open in desktop
Binary file not shown.
3 changes: 3 additions & 0 deletions ci/verify_deprecated_targets/verify_deprecated_targets.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,5 +17,8 @@

int main() {
std::cout << "Hello: " << google::cloud::version_string() << "\n";
// Verifies use of the backward-compatibility aliases: v1 and gcpcxxV1.
std::cout << "Hello: " << google::cloud::v1::version_string() << "\n";
std::cout << "Hello: " << google::cloud::gcpcxxV1::version_string() << "\n";
return 0;
}
11 changes: 7 additions & 4 deletions google/cloud/version.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,12 @@
"deprecated and will be be removed on 2022-04-01 or shortly " \
"after. See GitHub issue #5929 for more information.")

#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_VCONCAT(Ma, Mi, Pa) v##Ma##_##Mi##_##Pa
#define GOOGLE_CLOUD_CPP_VEVAL(Ma, Mi, Pa) GOOGLE_CLOUD_CPP_VCONCAT(Ma, Mi, Pa)
#define GOOGLE_CLOUD_CPP_NS \
GOOGLE_CLOUD_CPP_VEVAL(GOOGLE_CLOUD_CPP_VERSION_MAJOR, \
GOOGLE_CLOUD_CPP_VERSION_MINOR)
GOOGLE_CLOUD_CPP_VERSION_MINOR, \
GOOGLE_CLOUD_CPP_VERSION_PATCH)

/**
* Versioned inline namespace that users should generally avoid spelling.
Expand All @@ -49,7 +50,9 @@
#define GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_BEGIN \
inline namespace GOOGLE_CLOUD_CPP_NS {
#define GOOGLE_CLOUD_CPP_INLINE_NAMESPACE_END \
} // namespace GOOGLE_CLOUD_CPP_NS
} /* namespace GOOGLE_CLOUD_CPP_NS */ \
namespace v1 = GOOGLE_CLOUD_CPP_NS; \
namespace gcpcxxV1 = GOOGLE_CLOUD_CPP_NS;

// This preprocessor symbol is deprecated and should never be used anywhere. It
// exists solely for backward compatibility to avoid breaking anyone who may
Expand Down
23 changes: 5 additions & 18 deletions release/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,22 +20,6 @@ enumerated below.
Before beginning the release process, verify all CI builds are passing on
the `main` branch. This is displayed in the GitHub page for the project.

### Update the API baseline

Run the `check-api` build to update the API baseline. Once you cut the release
any new APIs are, well, released, and we should think carefully about removing
them.

```bash
ci/cloudbuild/build.sh -t check-api-pr --docker
```

The updated ABI dump files will be left in the `ci/abi-dumps` folder.

This may take a while, leave it running while you perform the next step. You
can, but are not required to, send a single PR to update the baseline and the
`CHANGELOG.md` file.

### Update CHANGELOG.md

To update the top-level [`CHANGELOG.md`] file, you run the script
Expand Down Expand Up @@ -118,8 +102,11 @@ send the PR for review against `main`. You need to:
- Update the version number in the top-level `CMakeLists.txt` file.
- Run the cmake configuration step, which will update the
`google/cloud/internal/version_info.h` file. Hint: Running
`ci/cloudbuild/build.sh --docker --trigger clang-tidy-pr` will do this, and
you can `CTRL-C` the build after CMake's configure step is finished.
`ci/cloudbuild/build.sh -t clang-tidy-pr` will do this, and you can `CTRL-C`
the build after CMake's configure step is finished.
- Update the ABI baseline to include the new version numbers in the inline
Copy link
Copy Markdown
Contributor

@coryan coryan Oct 14, 2021

Choose a reason for hiding this comment

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

Interesting. I had thought about this "enforcing" an ABI break on each release, but I had not thought about the impact on our baselines. I am not sure if this is good, bad, or indifferent.

Copy link
Copy Markdown
Contributor Author

@devjgm devjgm Oct 14, 2021

Choose a reason for hiding this comment

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

As far as our ABI baselines dumps are concerned, I think this change is roughly equivalent to before, because we were already updating the baseline with each release, but now we need to do it at a different point in the release process.

Copy link
Copy Markdown
Contributor

@coryan coryan Oct 14, 2021

Choose a reason for hiding this comment

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

Okay, I think we have a problem for patch releases:

  • We create release 1.100.0.
  • We update the version numbers.
  • We update the baseline, so all its symbols contain v1_101_0, e.g. assume google::cloud::v1_101_0::Bar exists.
  • Commit the baseline to main.
  • Some time goes by we add symbol google::cloud::v1_101_0::Foo, we don't necessarily update the baseline because adding symbols does not break the CI builds.
  • We create the v1.101.0 release.
  • The v1.101.x branch contains the baseline with v1_101_0 from whatever point it was last updated (which may not be the release point).
  • Let's say we want to create v1.101.1
  • We create a PR ... and send it to the CI builds.
  • The CI builds will detect some changes, but will not detect other changes:
    • Recall that the baseline was not updated when v1.101.0 was created, it is "stale"
    • Removing google::cloud::v1_101_0::Bar would be detected.
    • Removing google::cloud::v1_101_0::Foo would not be detected.

namespace by running `ci/cloudbuild/build.sh -t check-api-pr`, which will
leave the updated files in `ci/abi-dumps`.

**NOTE:** The Renovate bot will automatically update the Bazel deps in the
quickstart `WORKSPACE` files after it sees the new release published. Watch for
Expand Down