Installation prefix + cmake dependencies

Hi,

This is really just a general comment on the way gapid and its components are built, installed, and used (i.e. by downstream components, such as the Java UI).

Initially, I just want to be able to spy on Vulkan or OpenGL applications running natively on Linux (local apps), which don't really involve or require either the Android SDK/NDK, or even the Java JDK. Frankly, at first, I was just curious about the interception technology.

Being a Go lang noob, and not having so much success understanding or debugging what the do (cmd/do/*.go) scripts were "doing" (they seemed to just get in the way), I decided to just launch cmake directly, and have it discover the location of its dependencies.

It implies bypassing do config and the .gapid-config file it generates, and leaving it to cmake to find dependencies:

go (and GOPATH) has already been configured (so why need a variable such as CMAKE_Go_COMPILER?). Check version is 1.8+?
find_package(PythonInterp 3.5 REQUIRED)
find_package(Java 1.8 REQUIRED) (which probably checks for JAVA_HOME)
Ninja should just be put on the PATH
Check that ANDROID_NDK_ROOT and ANDROID_HOME are present (but we don't fail if they are not, and even if they are, we don't default ANDROID_ACTIVE_ABI_LIST to all potential targets "armeabi-v7a" "arm64-v8a" "x86_64" unless requested)
Assume -DHOST_ONLY=1 by default?
Is another copy of llvm required under third_party (it is likely to available, and cmake should just validate that the proper version is installed)?
To the extent possible, strings in .gapid-config get moved into the CMakeCache.txt

This implies discarding TARGET_INSTALL_PATH (for "bin", "lib", "etc"). For example :

install(TARGETS gapir DESTINATION bin)
install(TARGETS gapii LIBRARY DESTINATION lib)
For gapis and gapit: go_install(DESTINATION bin)
install(FILES ${PROPERTIES_FILE} DESTINATION bin)
install(FILES ${GO_BIN}/strings/en-us.stb DESTINATION "bin/strings")
install(TARGETS VkLayer_VirtualSwapchain RUNTIME DESTINATION lib)
install(FILES $<TARGET_FILE_DIR:VkLayerGraphicsSpy>/GraphicsSpyLayer.json DESTINATION lib)

We don't assume that the installation prefix is $HOME/gapid, or $USERPROFILE/gapid (even if this convention agrees with development habit), and we don't override CMAKE_INSTALL_PREFIX (from within cmake)

Once installed (i.e. after sudo ninja install into /usr/local), finding binaries and libraries is not a problem, but I did run into problems afterwards for the other files (GraphicsSpyLayer.json, strings/en-us.stb):

build.properties is used in core/app/layout/layout.go, to deduce the installation layout
gapic/src/main/com/google/gapid/server/GapiPaths.java should just look on the path to find an installed copy of gapis to launch (it finds the strings/en-us.stb file, and propagates it using the --strings flag, but the program itself should know how to find its strings)

Basically, I'm suggesting to have the installation prefix burned into the binaries, allowing the data and configuration files to be found at runtime (using SYSCONFDIR, which defaults to $PREFIX/etc), and not just a sub-directory of bin.

In the end, the workflow becomes:

$ cmake -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DHOST_ONLY=1 ..
$ ninja
$ sudo ninja install

I have experimented with some of these proposed changes locally, but they are not ready/complete, and I just wanted to gauge the level interest. Please comment.

Thanks.

Issues that I ran into during my explorations:

428 (I'm not a big SWT/JFace expert, or I would have done it myself),
636 (which I independently resolved).
https://github.com/golang/go/issues/20935: trying to debug go programs, and not having the confidence that the debugger is not "hiding things", I tried to disable optimizations (adding -gcflags "-v -N -l" cmake/modules/GoCompile.cmake). Unfortunately, attempting to use Go 1.9 was to be a lost cause
I have noted the complexity of first generating the protobuf files (recursive call to cmake).

Hi @DanOlivier,

Frankly, at first, I was just curious about the interception technology.

I'm happy to answer any questions you might have. From a high level perspective:

The interception approaches differ by platform.

For OpenGL ES:

Android uses our driver patching library (libinterceptor) as there's currently no platform supported way to intercept functions.
Windows, Mac theoretically support tracing via dll substitution and DYLD_FRAMEWORK_PATH, respectively, but we've made no effort in the last year to support it, and is almost certainly broken. Given the tiny number of OpenGL ES non-embedded applications out there, it's not really a priority for us right now. We'll happily accept PRs to support these if there's enough interest.
We accepted a PR to add a non-Android ARM linux target for the interceptor, which uses LD_PRELOAD for the interception mechanism.

For Vulkan, we use layers for interception on all Vulkan supported platforms.

Being a Go lang noob, and not having so much success understanding or debugging what the do (cmd/do/*.go) scripts were "doing" (they seemed to just get in the way), I decided to just launch cmake directly, and have it discover the location of its dependencies.

do is simply a wrapper around cmake. It serves a few goals:

It does some basic sanity checking on your env and dependencies. Over half of the issues we had reported were to do with basic mis-configuration of the environment. As issues have been reported, we try to add more sanity checks to do. This has greatly reduced the number of issues reported (but we still have more work to do).
It lets you work from the source directory without having to hop over to the build directory each time you want to compile.
It sets up symlinks to the current build configuration allowing you to use static env paths, but dynamically have those point to the current build (release / debug).
It prunes undesirable directories from PATH. Building successfully on windows is a fine art, made especially difficult by the way CMake will change behavior based on what it finds on PATH. do tries to ensure that CMake is passed an env that only contains things we need.
It builds the Java client. This should have probably been done in CMake, but the developer found it quicker to get going in Go than CMake.

It implies bypassing do config and the .gapid-config file it generates, and leaving it to cmake to find dependencies:

go (and GOPATH) has already been configured (so why need a variable such as CMAKE_Go_COMPILER?). Check version is 1.8+?

Having do require a working go is one of those chicken and egg problems, but in theory the go versions could be different. GAPID requires 1.8, do is probably far less picky.

The main reason for the existence of CMAKE_Go_COMPILER is that we want to explicitly specify all dependencies and toolchains.

Yes, we've already resolved a go compiler in order to run do, but we also don't want to force you to use do. It's a helper. If you don't use do, then you can't assume there's an already configured go available.

GOPATH is a bit special for GAPID. Due to some issues we encountered building executables from go vendored packages, we instead prepend the third_party directory to GOPATH.

find_package(PythonInterp 3.5 REQUIRED) find_package(Java 1.8 REQUIRED) (which probably checks for JAVA_HOME) Ninja should just be put on the PATH Check that ANDROID_NDK_ROOT and ANDROID_HOME are present (but we don't fail if they are not, and even if they are, we don't default ANDROID_ACTIVE_ABI_LIST to all potential targets "armeabi-v7a" "arm64-v8a" "x86_64" unless requested)

This implicit behavior is something we've been trying hard to avoid - not because we want to make things harder for developers, but because it's near impossible to help someone figure out why their builds fail and ours don't. Asking someone to list their full environment variables, installed packages and versions is impractical. We'd rather have something like do ask the developer up-front for all the dependencies we use, check they're all correct, and then let them build.

Assume -DHOST_ONLY=1 by default?

Maybe. Currently the developers are primarily focused on Android development, but we are working on adding proper support for tracing host Vulkan apps. I don't care so much about defaults for non-do usage.

Is another copy of llvm required under third_party (it is likely to available, and cmake should just validate that the proper version is installed)?

Again, you're trading checkout time / size for more possibility of random build issues.

To the extent possible, strings in .gapid-config get moved into the CMakeCache.txt

As far as I'm aware, all the variables passed down from do to cmake are all declared with CACHE. Are there some that aren't cached?

This implies discarding TARGET_INSTALL_PATH (for "bin", "lib", "etc"). For example :

I have no problems with TARGET_INSTALL_PATH becoming a CACHEd variable. Why do you want to split them up by directory?

We don't assume that the installation prefix is $HOME/gapid, or $USERPROFILE/gapid (even if this convention agrees with development habit)

We don't assume, it's just a default that's used if you haven't pre-declared it with GAPID_HOME or INSTALL_PREFIX.

build.properties is used in core/app/layout/layout.go, to deduce the installation layout gapic/src/main/com/google/gapid/server/GapiPaths.java should just look on the path to find an installed copy of gapis to launch (it finds the strings/en-us.stb file, and propagates it using the --strings flag, but the program itself should know how to find its strings)

Why should gapis be on the path?

Basically, I'm suggesting to have the installation prefix burned into the binaries, allowing the data and configuration files to be found at runtime (using SYSCONFDIR, which defaults to $PREFIX/etc), and not just a sub-directory of bin.

I'm not 100% sure I follow you here. Are you saying we enforce where GAPID gets installed, or just the relative file layout? If it's the former, this is not desirable. We want to allow users to install / extract GAPID where they like and have it work.

In the end, the workflow becomes:

$ cmake -G Ninja -DCMAKE_BUILD_TYPE=RelWithDebInfo -DHOST_ONLY=1 .. $ ninja $ sudo ninja install

So, I'm not completely opposed to having the defaults for variables omitted become automatically found, but it's going to take a lot of convincing before I encourage anyone to use this approach over explicit dependencies.

What isn't clear to me is why you want to invoke cmake by hand. Is there something about do that you cannot achieve?

It's worth mentioning that the longer term plan is to replace CMake with Bazel. Bazel has numerous benefits over CMake, the most important to me is that it fetches all the dependencies it needs automatically, removing the possibility of wrong packages altogether. This work has been blocked on Bazel features we're waiting on. The current ETA is ~ 8 weeks.

428 (I'm not a big SWT/JFace expert, or I would have done it myself)

I've asked @pmuetschard to put aside some time this week to look at implementing this.

636 (which I independently resolved).

Glad you found a way to resolve it. Can you remember what your solution was?

golang/go#20935: trying to debug go programs, and not having the confidence that the debugger is not "hiding things", I tried to disable optimizations (adding -gcflags "-v -N -l" cmake/modules/GoCompile.cmake).

I've tried repeatedly to set up a working debugging environment for Go. Each time I've failed. I've heard that Delve is working for a few people now (not specifically GAPID though), so I may give that another shot.

Unfortunately, attempting to use Go 1.9 was to be a lost cause

As 1.9 is still in beta I haven't spent any time looking into updating the toolchain. I will as soon as it's released though.

I have noted the complexity of first generating the protobuf files (recursive call to cmake).

There's also the sub-cmake builds for the Android ABIs, which are vile. Another reason we want to switch to Bazel.

Thank you for taking so much time to give us this feedback! Ben

google / gapid