[cmake] WITH_USER_foo properly sandboxes shared libraries

[jwnimmer-tri]

Closes #18025. (Improves upon #16652 to be hermetically correct.)

Towards #16987.

For testing, I used the README from https://github.com/RobotLocomotion/drake-external-examples/tree/main/drake_cmake_external instructions after pointing it at my local Drake with this patch:

--- a/drake_cmake_external/CMakeLists.txt
+++ b/drake_cmake_external/CMakeLists.txt
@@ -148,13 +148,7 @@ set(DRAKE_INSTALL_PREREQS_USER_ENVIRONMENT
 
 ExternalProject_Add(drake
   DEPENDS eigen fmt spdlog
-  URL https://github.com/RobotLocomotion/drake/archive/master.tar.gz
-  # Or from a commit (download and use "shashum -a 256 'xxx.tar.gz'" on it to
-  # get the URL_HASH.
-  # URL https://github.com/RobotLocomotion/drake/archive/65c4366ea2b63278a286b1e22b8d464d50fbe365.tar.gz
-  # URL_HASH SHA256=899d98485522a7cd5251e50a7a6b8a64e40aff2a3af4951a3f0857fd938cafca
-  TLS_VERIFY ON
-  PATCH_COMMAND ${DRAKE_INSTALL_PREREQS_USER_ENVIRONMENT}
+  SOURCE_DIR /home/jwnimmer/jwnimmer-tri/drake
   CMAKE_ARGS
     -DCMAKE_BUILD_TYPE:STRING=${CMAKE_BUILD_TYPE}
     -DCMAKE_C_COMPILER:FILEPATH=${CMAKE_C_COMPILER}

Doing that, I observed that (1) this PR doesn't break it, and (2) if I additionally unrevert 9a0f31a (#17746) in Drake then the build still passes (modulo the list of modules evolving in the meantime, requiring a small tweak to the manifest first).

---

This change is 

[jwnimmer-tri]

+@svenevs for feature review, please.

[svenevs]

Reviewable status: 1 unresolved discussion, LGTM missing from assignee svenevs, needs platform reviewer assigned, needs at least two assigned reviewers (waiting on @svenevs)

a discussion (no related file):
Working, I'm having trouble replicating based on your instructions:

ERROR: /.../drake-external-examples/drake_cmake_external/build/drake/_bazel_stephen.mcdowell/ee02002baa74f48a22b323e7ae7e7201/external/ibex/BUILD.bazel:200:11: Compiling external/ibex/filibsrc-3.0.2.2/interval/stdfun/filib_consts.cpp failed: (Exit 1): cc failed: error executing command /usr/bin/cc -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -g0 -O2 '-D_FORTIFY_SOURCE=1' -DNDEBUG -ffunction-sections ... (remaining 30 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
cc: error: unrecognized command line option '-std=c++20'; did you mean '-std=c++2a'?

which seems very unrelated here. The path /home/jwnimmer/jwnimmer-tri/drake is supposed to be a clean checkout of this PR right?

---

[jwnimmer-tri]

Yes, just a simple checkout of this PR. Maybe check what drake/gen/environment.bazelrc looks like (in case it's for Jammy, and you're on Focal?).

(Sorry I can't reply in reviewable; GitHub is spuriously rate-throttling me again.)

[svenevs]

GitHub is spuriously rate-throttling me again

🤖 🙂

This with a small nit about insulating against not finding the property we need. +@SeanCurtis-TRI for platform review please.

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee SeanCurtis-TRI(platform) (waiting on @jwnimmer-tri and @SeanCurtis-TRI)

a discussion (no related file):

Previously, svenevs (Stephen McDowell) wrote…

Working, I'm having trouble replicating based on your instructions:

ERROR: /.../drake-external-examples/drake_cmake_external/build/drake/_bazel_stephen.mcdowell/ee02002baa74f48a22b323e7ae7e7201/external/ibex/BUILD.bazel:200:11: Compiling external/ibex/filibsrc-3.0.2.2/interval/stdfun/filib_consts.cpp failed: (Exit 1): cc failed: error executing command /usr/bin/cc -U_FORTIFY_SOURCE -fstack-protector -Wall -Wunused-but-set-parameter -Wno-free-nonheap-object -fno-omit-frame-pointer -g0 -O2 '-D_FORTIFY_SOURCE=1' -DNDEBUG -ffunction-sections ... (remaining 30 arguments skipped)

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
cc: error: unrecognized command line option '-std=c++20'; did you mean '-std=c++2a'?

which seems very unrelated here. The path /home/jwnimmer/jwnimmer-tri/drake is supposed to be a clean checkout of this PR right?

Done, did it on a different machine and things worked as expected.

---

---

CMakeLists.txt line 399 at r1 (raw file):

      file(MAKE_DIRECTORY "${workspace}/${NAME}/lib")
      # Link the full library name (i.e., libfmt.so.6.1.2 in the case of shared).
      get_target_property(location ${TARGET} LOCATION_${config})

Minor, given the adventures we had in finding this, it may be good to add

if(NOT location)
  message(FATAL_ERROR "Unable to find library LOCATION_${config} property for target ${TARGET}")
endif()

See also NOTFOUND constant for if() docs.

This is not strictly speaking necessary since ultimately it'll get linker failures if it didn't find it -- those error messages will be confusing for users though. You can also use SEND_ERROR if you'd like it to error, but finish the configuration step (FATAL_ERROR kills it immediately).

[jwnimmer-tri]

Reviewable status: 1 unresolved discussion, LGTM missing from assignee SeanCurtis-TRI(platform) (waiting on @jwnimmer-tri and @SeanCurtis-TRI)

---

CMakeLists.txt line 399 at r1 (raw file):

Previously, svenevs (Stephen McDowell) wrote…

Minor, given the adventures we had in finding this, it may be good to add

if(NOT location)
  message(FATAL_ERROR "Unable to find library LOCATION_${config} property for target ${TARGET}")
endif()

See also NOTFOUND constant for if() docs.

This is not strictly speaking necessary since ultimately it'll get linker failures if it didn't find it -- those error messages will be confusing for users though. You can also use SEND_ERROR if you'd like it to error, but finish the configuration step (FATAL_ERROR kills it immediately).

Both fmt and spdlog can be configured to build in header-only mode, in which case I didn't want to fail-bomb the user.

But typing that out loud now, I realize that it might not quite work even as I have this written today. I'll test.

Working

[SeanCurtis-TRI]

Pass done. What can I say? No misspelled English. Boy am I glad we've got a good feature reviewer.

Reviewed 3 of 3 files at r1, all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee SeanCurtis-TRI(platform) (waiting on @jwnimmer-tri)

---

CMakeLists.txt line 399 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Both fmt and spdlog can be configured to build in header-only mode, in which case I didn't want to fail-bomb the user.

But typing that out loud now, I realize that it might not quite work even as I have this written today. I'll test.

Working

I'm delaying my tag to see how this plays out.

[jwnimmer-tri]

CMakeLists.txt line 399 at r1 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

I'm delaying my tag to see how this plays out.

Aha. While both fmt and spdlog do support a header-only mode, doing that requires that we use a different target name (e.g., fmt::fmt-header-only instead of fmt::fmt), so as-written the user cannot use a header-only fmt or spdlog anyway. So I'll move the error-checking up to here.

Still working.

[jwnimmer-tri]

Reviewable status: LGTM missing from assignee SeanCurtis-TRI(platform) (waiting on @SeanCurtis-TRI and @svenevs)

---

CMakeLists.txt line 399 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Aha. While both fmt and spdlog do support a header-only mode, doing that requires that we use a different target name (e.g., fmt::fmt-header-only instead of fmt::fmt), so as-written the user cannot use a header-only fmt or spdlog anyway. So I'll move the error-checking up to here.

Still working.

Done.

It took me embarrassingly long to test this. I was trying to manually play with the error handling by passing the typo LOCATION_xxx to get_target_property() above in order to obtain NOTFOUND.

But that typo happily returns a library path.

I guess the suffix handling is magic, and LOCATION_xxx just gives me LOCATION. I ended up using xxx_LOCATION to inject the fault.

[SeanCurtis-TRI]

Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status: 1 unresolved discussion, LGTM missing from assignee SeanCurtis-TRI(platform) (waiting on @jwnimmer-tri)

---

CMakeLists.txt line 399 at r1 (raw file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

Done.

It took me embarrassingly long to test this. I was trying to manually play with the error handling by passing the typo LOCATION_xxx to get_target_property() above in order to obtain NOTFOUND.

But that typo happily returns a library path.

I guess the suffix handling is magic, and LOCATION_xxx just gives me LOCATION. I ended up using xxx_LOCATION to inject the fault.

Bwa, ha, ha!

---

cmake/external/workspace/fmt/BUILD.bazel.in line 17 at r1 (raw file):

    srcs = glob(
        ["lib/**"],
        allow_empty = True,  # Allow for header-only builds.

BTW So, am I to infer that this old comment was in error?

[SeanCurtis-TRI]

Reviewable status: 1 unresolved discussion (waiting on @jwnimmer-tri)

[jwnimmer-tri]

Reviewable status: complete! all discussions resolved, LGTM from assignees SeanCurtis-TRI(platform),svenevs (waiting on @jwnimmer-tri)

---

cmake/external/workspace/fmt/BUILD.bazel.in line 17 at r1 (raw file):

Previously, SeanCurtis-TRI (Sean Curtis) wrote…

BTW So, am I to infer that this old comment was in error?

Right-o. I had never actually tested it, just did it on spec to avoid breaking users who were trying off-nominal things. Now that I've realized it would have never worked anyway, best to fail-fast here.

[SeanCurtis-TRI]

I'm good for merging -- @jwnimmer-tri, I don't know if you want to give @svenevs one more look or just hit the button. I defer to you.

Reviewable status: complete! all discussions resolved, LGTM from assignees SeanCurtis-TRI(platform),svenevs (waiting on @jwnimmer-tri)