googleapis / google-cloud-cpp

C++ Client Libraries for Google Cloud Services
https://cloud.google.com/
Apache License 2.0
545 stars 370 forks source link

Support clients for multiple versions of the same GCP service #10170

Closed dbolduc closed 1 year ago

dbolduc commented 1 year ago

See https://github.com/googleapis/google-cloud-cpp/blob/main/doc/adr/2022-11-11-multiple-versions-of-GCP-service-in-one-library.md

We need to:

See this branch for what some of the changes should look like. (Next to add them to the generator, and break down the work into logical PRs) https://github.com/googleapis/google-cloud-cpp/compare/main...dbolduc:google-cloud-cpp:speech-reorg-dev?expand=1

devbww commented 1 year ago

See #10278 (vmwareengine/v1) for another example of the proposed layout (without backwards-compatibility headers).

For what it's worth, I used this as a temporary hack for scaffolding generation:

--- a/generator/internal/scaffold_generator.cc
+++ b/generator/internal/scaffold_generator.cc
@@ -133,7 +133,10 @@ void GenerateScaffold(

   auto const vars = ScaffoldVars(googleapis_path, service, experimental);
   MakeDirectory(output_path + "/");
-  auto const destination = output_path + "/" + service.product_path() + "/";
+  auto destination = output_path + "/" + service.product_path() + "/";
+  if (service.product_path() == "google/cloud/vmwareengine/v1") {
+    destination = output_path + "/google/cloud/vmwareengine/";
+  }
   MakeDirectory(destination);
   MakeDirectory(destination + "doc/");
   MakeDirectory(destination + "quickstart/");
dbolduc commented 1 year ago

This is the script I used to generate #10857

#!/bin/bash
#
# Usage: ./relocate.sh apigateway kms osconfig
#
# If anything goes wrong, it will likely be in the "update builds" step.
#
# Verify the result with:
# ```
# ci/cloudbuild/build.sh -t cmake-install-pr
# bazel build //google/cloud/...
# ```

libraries=("$@")

function edit_config() {
  library=$1
  service_dirs=("\"\"") # Remember these. They are needed in Step 5.
  vals=($(grep "service_proto_path: .*${library}.*v[0-9]" generator/generator_config.textproto -no | sed "s;service_proto_path:.*${library}/;;"))
  # Assume that `product_path` immediately follows `service_proto_path`.
  line_offset=1
  for v in "${vals[@]}"; do
    line=${v%:*}
    subdir=${v#*:}
    service_dirs+=("\"${subdir}/\"")
    sed -i "$(($line+$line_offset))s;${library};${library}/${subdir};" generator/generator_config.textproto
    # We need to increment the index for each `forwarding_product_path` line we add.
    ((line_offset+=1))
    sed -i "$(($line+$line_offset))i \ \ forwarding_product_path: \"google/cloud/${library}\"" generator/generator_config.textproto
  done
  # Store in a file because dbolduc does not know how maps work.
  printf "%s\n" "${service_dirs[@]}" | sort | uniq > dirs_${library}.tmp
}

function update_bazel() {
  mapfile -t service_dirs < dirs_${library}.tmp
  {
    sed -n '1,18p' google/cloud/${library}/BUILD.bazel
    echo "service_dirs = ["
    for d in "${service_dirs[@]}"; do
      echo "    ${d}",
    done
    sed -n '22,39p' google/cloud/accessapproval/BUILD.bazel
    sed -n '39,$p' google/cloud/${library}/BUILD.bazel
    sed -n '62,$p' google/cloud/accessapproval/BUILD.bazel | sed "s/accessapproval/${library}/"
  } > dern.tmp && mv dern.tmp google/cloud/${library}/BUILD.bazel
}

function update_cmake() {
  mapfile -t service_dirs < dirs_${library}.tmp
  {
    sed '/set(DOXYGEN_PROJECT_NUMBER/q' google/cloud/${library}/CMakeLists.txt
    sed -n '21,37p' google/cloud/accessapproval/CMakeLists.txt | sed "s/accessapproval/${library}/" | sed "s;\"\" \"v1/\";${service_dirs[*]};"
    sed -n '/include(GoogleCloudCppDoxygen)/,$p' google/cloud/${library}/CMakeLists.txt | sed 's;"\*.h" "\*.cc" "internal/\*.h" "internal/\*.cc";${source_globs};' | sed 's;"mocks/\*.h";${mocks_globs};'
    # Use tpu because accessapproval breaks the comment onto two lines.
    sed -n '182,$p' google/cloud/tpu/CMakeLists.txt | sed "s;tpu;${library};g"
  } > dern.tmp && mv dern.tmp google/cloud/${library}/CMakeLists.txt
}

# Checkout new branch
git checkout -b relocate-libraries

# Edit generator configuration
for library in "${libraries[@]}"; do
  edit_config $library
done
git commit -m "refactor: versioned clients" generator/generator_config.textproto

# Regenerate libraries
ci/cloudbuild/build.sh -t generate-libraries-pr
git add google/
ci/cloudbuild/build.sh -t checkers-pr
git commit -m "generated changes" ci/ google/

# Remove stale code
for library in "${libraries[@]}"; do
  rm google/cloud/$library/*.cc
  rm -rf google/cloud/$library/internal
  rm -rf google/cloud/$library/samples
done
git add google/
ci/cloudbuild/build.sh -t checkers-pr
git commit -m "remove stale code; regenerate docs" google/

# Update build files
for library in "${libraries[@]}"; do
  update_bazel $library
  update_cmake $library
  rm dirs_${library}.tmp
done
ci/cloudbuild/build.sh -t checkers-pr
git commit -m "update build files" google/

# Update ABI dumps
ci/cloudbuild/build.sh -t check-api-pr
for library in "${libraries[@]}"; do
  git add ci/abi-dumps/google_cloud_cpp_$library.expected.abi.dump.gz
done
git commit -m "update ABI dumps"
dbolduc commented 1 year ago

For posterity, here is the final script I used:

#!/bin/bash
#
# Usage: ./relocate.sh apigateway kms osconfig
#
# If anything goes wrong, it will likely be in the "update builds" step.
#
# Verify the result with:
# ```
# ci/cloudbuild/build.sh -t cmake-install-pr
# bazel build //google/cloud/...
# ```

set -euo pipefail

libraries=("$@")

function edit_config() {
  library=$1
  gc_dir=$library
  if [[ $gc_dir == "dialogflow_cx" ]]; then gc_dir="dialogflow/cx"; fi
  if [[ $gc_dir == "gameservices" ]]; then gc_dir="gaming"; fi
  if [[ $gc_dir == "datamigration" ]]; then gc_dir="clouddms"; fi
  if [[ $gc_dir == "profiler" ]]; then gc_dir="cloudprofiler"; fi
  if [[ $gc_dir == "trace" ]]; then gc_dir="cloudtrace"; fi
  service_dirs=("\"\"") # Remember these. They are needed in Step 5.
  vals=($(grep "service_proto_path: .*/${gc_dir}/.*v[0-9]" generator/generator_config.textproto -no | sed "s;service_proto_path:.*${gc_dir}/;;"))
  inserts=0
  for v in "${vals[@]}"; do
    og_line=${v%:*}
    subdir=${v#*:}
    service_dirs+=("\"${subdir}/\"")
    # Find the index of the product_path. It is not always on the line after the
    # `service_proto_path`, if there are `additional_proto_files` defined.
    ((line=$og_line+$inserts+1))
    while [ $(sed -n "${line}p" generator/generator_config.textproto | grep product_path | wc -w) -eq 0 ]; do
      ((line+=1))
    done
    sed -i "${line}s;${library};${library}/${subdir};" generator/generator_config.textproto
    # We also need to increment the index for each `forwarding_product_path` line we add.
    ((inserts+=1))
    ((line+=1))
    sed -i "${line}i \ \ forwarding_product_path: \"google/cloud/${library}\"" generator/generator_config.textproto
  done
  # Store in a file because dbolduc does not know how maps work.
  printf "%s\n" "${service_dirs[@]}" | sort | uniq > dirs_${library}.tmp
}

function check_typos() {
  library=$1
  if [ $(grep "google/cloud/$library/" .typos.toml | wc -w) -gt 0 ]; then
    nvim .typos.toml
    git commit -m "manually edit typos" .typos.toml
  fi
}

function update_bazel() {
  mapfile -t service_dirs < dirs_${library}.tmp
  {
    sed -n '1,18p' google/cloud/${library}/BUILD.bazel
    echo "service_dirs = ["
    for d in "${service_dirs[@]}"; do
      echo "    ${d}",
    done
    sed -n '22,39p' google/cloud/accessapproval/BUILD.bazel
    sed -n '39,$p' google/cloud/${library}/BUILD.bazel
    sed -n '62,$p' google/cloud/accessapproval/BUILD.bazel | sed "s/accessapproval/${library}/"
  } > dern.tmp && mv dern.tmp google/cloud/${library}/BUILD.bazel
}

function update_cmake() {
  mapfile -t service_dirs < dirs_${library}.tmp
  {
    sed '/set(DOXYGEN_PROJECT_NUMBER/q' google/cloud/${library}/CMakeLists.txt
    sed -n '21,37p' google/cloud/accessapproval/CMakeLists.txt | sed "s/accessapproval/${library}/" | sed "s;\"\" \"v1/\";${service_dirs[*]};"
    sed -n '/include(GoogleCloudCppDoxygen)/,$p' google/cloud/${library}/CMakeLists.txt | sed 's;"\*.h" "\*.cc" "internal/\*.h" "internal/\*.cc";${source_globs};' | sed 's;"mocks/\*.h";${mocks_globs};'
    # Use tpu because accessapproval breaks the comment onto two lines.
    sed -n '182,$p' google/cloud/tpu/CMakeLists.txt | sed "s;tpu;${library};g"
  } > dern.tmp && mv dern.tmp google/cloud/${library}/CMakeLists.txt
}

# Checkout new branch
git checkout main
git pull --ff-only upstream main
git checkout -b relocate-${libraries[0]}

# Edit generator configuration
for library in "${libraries[@]}"; do
  edit_config $library
done
git commit -m "refactor(${libraries[0]}): versioned clients" generator/generator_config.textproto

for library in "${libraries[@]}"; do
  check_typos $library
done

# Regenerate libraries
ci/cloudbuild/build.sh -t generate-libraries-pr || true
git add google/
ci/cloudbuild/build.sh -t checkers-pr || true
git commit -m "generated changes" ci/ google/

# Remove stale code
for library in "${libraries[@]}"; do
  rm google/cloud/$library/*.cc
  rm -rf google/cloud/$library/internal
  # Libraries with handwritten samples need finer tuning
  rm -rf google/cloud/$library/samples
done
git add google/
ci/cloudbuild/build.sh -t checkers-pr || true
git commit -m "remove stale code; regenerate docs" google/

# Update build files
for library in "${libraries[@]}"; do
  update_bazel $library
  update_cmake $library
  # Cleanup temporary files
  rm dirs_${library}.tmp
done
ci/cloudbuild/build.sh -t checkers-pr || true
git commit -m "update build files" google/

# Update ABI dumps
mapfile -t libs < <(for library in ${libraries[@]}; do echo google_cloud_cpp_$library; done)
sed -i "131s/.*/libraries=(${libs[*]})/" ci/cloudbuild/builds/check-api.sh
ci/cloudbuild/build.sh -t check-api-pr || true
git add ci/abi-dumps
git restore ci/cloudbuild/builds/check-api.sh
git commit -m "update ABI dumps"

# Test and move on, lol.
for library in "${libraries[@]}"; do
  bazel build //google/cloud/$library/...
done
git push -u origin relocate-${libraries[0]}
dbolduc commented 1 year ago

Current State

All libraries have versioned clients except for the following exceptions.

Heavily handwritten:

Sorta versioned already:

@coryan suggests that we might want a single dialogflow library, which dialogflow_cx and dialogflow_es depend on for backwards compatibility. If we do this, we should track it in a separate issue.

Obnoxious googleapis directory structure

https://github.com/googleapis/google-cloud-cpp/blob/a5572b1818bbfcc41e12641e15454ebd08aeb753/generator/generator_config.textproto#L402-L404

Maybe we should just make it composer/v1