Skip to content

Commit d3a7f36

Browse files
coryancoryan
authored
fix(googleapis): install directories for protos (#5783)
Fix the destination directory for `.proto` files. Improve the `clang-tidy` build to better detect such problems in the future.
1 parent 8e10f12 commit d3a7f36

File tree

2 files changed

+91
-51
lines changed

2 files changed

+91
-51
lines changed

‎ci/kokoro/docker/build-in-docker-cmake.sh‎

Lines changed: 88 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -378,30 +378,39 @@ if [[ "${TEST_INSTALL:-}" == "yes" ]]; then
378378
echo
379379
io::log_yellow "testing install script for runtime components"
380380
cmake --install "${BINARY_DIR}" --component google_cloud_cpp_runtime
381-
EXPECTED_RUNTIME_DIRS=(
382-
"/var/tmp/staging/"
383-
"/var/tmp/staging/${libdir}"
384-
)
385-
readarray -t EXPECTED_RUNTIME_DIRS < <(printf "%s\n" "${EXPECTED_RUNTIME_DIRS[@]}" | sort)
386-
readonly EXPECTED_RUNTIME_DIRS
387-
if comm -23 \
388-
<(find /var/tmp/staging/ -type d | sort) \
389-
<(/usr/bin/printf "%s\n" "${EXPECTED_RUNTIME_DIRS[@]}") | grep -q /var/tmp; then
390-
io::log_red "Installed directories do not match expectation:"
391-
diff -u \
392-
<(find /var/tmp/staging/ -type d | sort) \
393-
<(printf "%s\n" "${EXPECTED_RUNTIME_DIRS[@]}")
394-
exit 1
381+
# Static builds do not create any runtime components, so the previous step
382+
# may create no directories.
383+
if grep -q '^BUILD_SHARED_LIBS' "${BINARY_DIR}/CMakeCache.txt"; then
384+
echo
385+
io::log_yellow "verify the expected runtime directories are created"
386+
EXPECTED_RUNTIME_DIRS=(
387+
"/var/tmp/staging/"
388+
"/var/tmp/staging/${libdir}"
389+
)
390+
readarray -t EXPECTED_RUNTIME_DIRS < <(printf "%s\n" "${EXPECTED_RUNTIME_DIRS[@]}" | sort)
391+
readonly EXPECTED_RUNTIME_DIRS
392+
IFS=$'\n' readarray -t ACTUAL_RUNTIME_DIRS < <(find /var/tmp/staging/ -type d | sort)
393+
readonly ACTUAL_RUNTIME_DIRS
394+
if comm -23 \
395+
<(/usr/bin/printf '%s\n' "${EXPECTED_RUNTIME_DIRS[@]}") \
396+
<(/usr/bin/printf '%s\n' "${ACTUAL_RUNTIME_DIRS[@]}") | grep -q /var/tmp; then
397+
io::log_red "Installed runtime directories do not match expectation:"
398+
diff -u \
399+
<(/usr/bin/printf '%s\n' "${EXPECTED_RUNTIME_DIRS[@]}") \
400+
<(/usr/bin/printf "%s\n" "${ACTUAL_RUNTIME_DIRS[@]}")
401+
exit 1
402+
fi
395403
fi
396404

397405
echo
398406
io::log_yellow "testing install script for development components"
399407
cmake --install "${BINARY_DIR}" --component google_cloud_cpp_development
408+
409+
echo
410+
io::log_yellow "verify the expected directories for CMake and pkgconfig are created"
400411
EXPECTED_LIB_DIRS=(
401412
"/var/tmp/staging/${libdir}/"
402413
"/var/tmp/staging/${libdir}/cmake"
403-
"/var/tmp/staging/${libdir}/cmake/bigtable_client"
404-
"/var/tmp/staging/${libdir}/cmake/firestore_client"
405414
"/var/tmp/staging/${libdir}/cmake/google_cloud_cpp_bigtable"
406415
"/var/tmp/staging/${libdir}/cmake/google_cloud_cpp_common"
407416
"/var/tmp/staging/${libdir}/cmake/google_cloud_cpp_firestore"
@@ -412,46 +421,79 @@ if [[ "${TEST_INSTALL:-}" == "yes" ]]; then
412421
"/var/tmp/staging/${libdir}/cmake/google_cloud_cpp_pubsub"
413422
"/var/tmp/staging/${libdir}/cmake/google_cloud_cpp_spanner"
414423
"/var/tmp/staging/${libdir}/cmake/google_cloud_cpp_storage"
424+
"/var/tmp/staging/${libdir}/pkgconfig"
425+
# TODO(#5726) - these can be removed after 2022-02-15
426+
"/var/tmp/staging/${libdir}/cmake/bigtable_client"
427+
"/var/tmp/staging/${libdir}/cmake/firestore_client"
415428
"/var/tmp/staging/${libdir}/cmake/googleapis"
416429
"/var/tmp/staging/${libdir}/cmake/pubsub_client"
417430
"/var/tmp/staging/${libdir}/cmake/spanner_client"
418431
"/var/tmp/staging/${libdir}/cmake/storage_client"
419-
"/var/tmp/staging/${libdir}/pkgconfig"
432+
# TODO(#5726) - END
420433
)
421434
readarray -t EXPECTED_LIB_DIRS < <(printf "%s\n" "${EXPECTED_LIB_DIRS[@]}" | sort)
422435
readonly EXPECTED_LIB_DIRS
436+
IFS=$'\n' readarray -t ACTUAL_LIB_DIRS < <(find "/var/tmp/staging/${libdir}/" -type d | sort)
423437
if comm -23 \
424-
<(find "/var/tmp/staging/${libdir}/" -type d | sort) \
425-
<(/usr/bin/printf "%s\n" "${EXPECTED_LIB_DIRS[@]}") | grep -q /var/tmp/staging; then
426-
io::log_red "Installed directories do not match expectation:"
438+
<(/usr/bin/printf '%s\n' "${EXPECTED_LIB_DIRS[@]}") \
439+
<(/usr/bin/printf '%s\n' "${ACTUAL_LIB_DIRS[@]}") | grep -q /var/tmp/staging; then
440+
io::log_red "Installed library directories do not match expectation:"
427441
diff -u \
428-
<(find "/var/tmp/staging/${libdir}/" -type d | sort) \
429-
<(printf "%s\n" "${EXPECTED_LIB_DIRS[@]}")
442+
<(/usr/bin/printf '%s\n' "${EXPECTED_LIB_DIRS[@]}") \
443+
<(/usr/bin/printf '%s\n' "${ACTUAL_LIB_DIRS[@]}")
430444
exit 1
431445
fi
432446

433-
# Also verify that the install directory does not get unexpected files or
434-
# directories installed.
435447
echo
436-
io::log_yellow "Verify installed headers created only expected directories."
437-
EXPECTED_INCLUDE_DIRS=(
438-
"/var/tmp/staging/include/google/cloud"
439-
"/var/tmp/staging/include/google/cloud/bigquery"
440-
"/var/tmp/staging/include/google/cloud/bigquery/connection"
448+
io::log_yellow "Verify installed protos create the expected directories."
449+
EXPECTED_PROTO_DIRS=(
450+
"/var/tmp/staging/include/google/api"
451+
"/var/tmp/staging/include/google/bigtable/admin/v2"
452+
"/var/tmp/staging/include/google/bigtable/v2"
441453
"/var/tmp/staging/include/google/cloud/bigquery/connection/v1beta1"
442-
"/var/tmp/staging/include/google/cloud/bigquery/datatransfer"
443454
"/var/tmp/staging/include/google/cloud/bigquery/datatransfer/v1"
444-
"/var/tmp/staging/include/google/cloud/bigquery/internal"
445-
"/var/tmp/staging/include/google/cloud/bigquery/logging"
446455
"/var/tmp/staging/include/google/cloud/bigquery/logging/v1"
447-
"/var/tmp/staging/include/google/cloud/bigquery/storage"
448456
"/var/tmp/staging/include/google/cloud/bigquery/storage/v1beta1"
449457
"/var/tmp/staging/include/google/cloud/bigquery/v2"
450-
"/var/tmp/staging/include/google/cloud/bigtable"
451-
"/var/tmp/staging/include/google/cloud/bigtable/internal"
452-
"/var/tmp/staging/include/google/cloud/dialogflow"
453458
"/var/tmp/staging/include/google/cloud/dialogflow/v2"
454459
"/var/tmp/staging/include/google/cloud/dialogflow/v2beta1"
460+
"/var/tmp/staging/include/google/cloud/speech/v1"
461+
"/var/tmp/staging/include/google/cloud/texttospeech/v1"
462+
"/var/tmp/staging/include/google/devtools/cloudtrace/v2"
463+
"/var/tmp/staging/include/google/iam/credentials/v1"
464+
"/var/tmp/staging/include/google/iam/v1"
465+
"/var/tmp/staging/include/google/logging/type"
466+
"/var/tmp/staging/include/google/logging/v2"
467+
"/var/tmp/staging/include/google/longrunning"
468+
"/var/tmp/staging/include/google/monitoring/v3"
469+
"/var/tmp/staging/include/google/pubsub/v1"
470+
"/var/tmp/staging/include/google/rpc"
471+
"/var/tmp/staging/include/google/spanner/admin/database/v1"
472+
"/var/tmp/staging/include/google/spanner/admin/instance/v1"
473+
"/var/tmp/staging/include/google/spanner/v1"
474+
"/var/tmp/staging/include/google/storage/v1"
475+
"/var/tmp/staging/include/google/type"
476+
)
477+
readarray -t < <(printf "%s\n" "${EXPECTED_PROTO_DIRS[@]}" | sort)
478+
readonly EXPECTED_PROTO_DIRS
479+
IFS=$'\n' readarray -t ACTUAL_PROTO_DIRS < <(find /var/tmp/staging/ -name '*.proto' -printf '%h\n' | sort -u)
480+
readonly ACTUAL_PROTO_DIRS
481+
if comm -23 \
482+
<(/usr/bin/printf '%s\n' "${EXPECTED_PROTO_DIRS[@]}") \
483+
<(/usr/bin/printf '%s\n' "${ACTUAL_PROTO_DIRS[@]}") | grep -q /var/tmp; then
484+
diff -u \
485+
<(/usr/bin/printf '%s\n' "${EXPECTED_PROTO_DIRS[@]}") \
486+
<(/usr/bin/printf '%s\n' "${ACTUAL_PROTO_DIRS[@]}")
487+
exit 1
488+
fi
489+
490+
echo
491+
io::log_yellow "Verify installed headers create the expected directories."
492+
EXPECTED_INCLUDE_DIRS=(
493+
"/var/tmp/staging/include/google/cloud"
494+
"/var/tmp/staging/include/google/cloud/internal"
495+
"/var/tmp/staging/include/google/cloud/bigtable"
496+
"/var/tmp/staging/include/google/cloud/bigtable/internal"
455497
"/var/tmp/staging/include/google/cloud/firestore"
456498
"/var/tmp/staging/include/google/cloud/grpc_utils"
457499
"/var/tmp/staging/include/google/cloud/iam"
@@ -467,28 +509,25 @@ if [[ "${TEST_INSTALL:-}" == "yes" ]]; then
467509
"/var/tmp/staging/include/google/cloud/spanner"
468510
"/var/tmp/staging/include/google/cloud/spanner/internal"
469511
"/var/tmp/staging/include/google/cloud/spanner/mocks"
470-
"/var/tmp/staging/include/google/cloud/speech"
471-
"/var/tmp/staging/include/google/cloud/speech/v1"
472512
"/var/tmp/staging/include/google/cloud/storage"
473513
"/var/tmp/staging/include/google/cloud/storage/internal"
474514
"/var/tmp/staging/include/google/cloud/storage/oauth2"
475-
"/var/tmp/staging/include/google/cloud/storage/testing"
476-
"/var/tmp/staging/include/google/cloud/testing_util"
477-
"/var/tmp/staging/include/google/cloud/texttospeech"
478-
"/var/tmp/staging/include/google/cloud/texttospeech/v1")
479-
readarray -t < <(printf "%s\n" "${EXPECTED_INCLUDE_DIRS[@]}" | sort)
515+
"/var/tmp/staging/include/google/cloud/storage/testing")
516+
readarray -t EXPECTED_INCLUDE_DIRS < <(printf "%s\n" "${EXPECTED_PROTO_DIRS[@]}" "${EXPECTED_INCLUDE_DIRS[@]}" | sort -u)
480517
readonly EXPECTED_INCLUDE_DIRS
518+
IFS=$'\n' readarray -t ACTUAL_INCLUDE_DIRS < <(find /var/tmp/staging/include -name '*.h' -printf '%h\n' | sort -u)
519+
readonly ACTUAL_INCLUDE_DIRS
481520
if comm -23 \
482-
<(find /var/tmp/staging/include/google/cloud -type d | sort) \
483-
<(/usr/bin/printf "%s\n" "${EXPECTED_INCLUDE_DIRS[@]}") | grep -q /var/tmp; then
484-
io::log_red "Installed directories do not match expectation:"
521+
<(/usr/bin/printf '%s\n' "${EXPECTED_INCLUDE_DIRS[@]}") \
522+
<(/usr/bin/printf '%s\n' "${ACTUAL_INCLUDE_DIRS[@]}") | grep -q /var/tmp; then
523+
io::log_red "Installed include directories do not match expectation:"
485524
diff -u \
486-
<(find /var/tmp/staging/include/google/cloud -type d | sort) \
487-
<(printf "%s\n" "${EXPECTED_INCLUDE_DIRS[@]}")
525+
<(/usr/bin/printf '%s\n' "${EXPECTED_INCLUDE_DIRS[@]}") \
526+
<(/usr/bin/printf '%s\n' "${ACTUAL_INCLUDE_DIRS[@]}")
488527
exit 1
489528
fi
490529

491-
io::log_yellow "Verify no extraneous files were installed."
530+
io::log_yellow "Verify only files with expected extensions are installed."
492531
export PKG_CONFIG_PATH="/var/tmp/staging/${libdir}/pkgconfig:${PKG_CONFIG_PATH:-}"
493532

494533
# Get the version of one of the libraries. These should all be the same, so

‎cmake/CompileProtos.cmake‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,14 +233,15 @@ function (google_cloud_cpp_install_proto_library_headers target)
233233
endfunction ()
234234

235235
# Install protos for a C++ proto library.
236-
function (google_cloud_cpp_install_proto_library_protos target)
236+
function (google_cloud_cpp_install_proto_library_protos target source_dir)
237237
get_target_property(target_protos ${target} PROTO_SOURCES)
238238
foreach (header ${target_protos})
239239
# Skip anything that is not a header file.
240240
if (NOT "${header}" MATCHES "\\.proto$")
241241
continue()
242242
endif ()
243-
string(REPLACE "${CMAKE_CURRENT_BINARY_DIR}/" "" relative "${header}")
243+
string(REPLACE "${source_dir}/" "" relative "${header}")
244+
string(REPLACE "${CMAKE_CURRENT_BINARY_DIR}/" "" relative "${relative}")
244245
get_filename_component(dir "${relative}" DIRECTORY)
245246
# This is modeled after the Protobuf library, it installs the basic
246247
# protos (think google/protobuf/any.proto) in the include directory for

0 commit comments

Comments
 (0)