Usage in Windows / MSVC / Unreal Engine Contexts

[MrMoose]

Hello everyone,

I hope this is a suitable place to spark a discussion about ease of integration of this library in non-linux environments, specifically when it gets a bit "weirder", such as within the Unreal engine. Please take this as constructive criticism if possible. It is meant as such.

Let me illustrate my journey here a little. I come from a 20 year background in C++ and have quite a bit of experience building various Google libs such as protobuf, gRPC and others on Linux and Windows. Over the last years I have seen and tackled many of the challenges that come with it. This time around, I'm working on a new (Open Source) plugin for the Unreal Engine which is supposed to make use of google-cloud-cpp. Which basically requires me to produce a MSVC build of the library plus all dependencies for linkage within a DLL. It took a long time but finally I have a build and can integrate into my Unreal plugin.

Which brings me to my first topic. Those dependencies and building a Windows version of this library really are a lot. It took many days and without my prior experience with gRPC I doubt I could have done it. If you excuse me comparing this to the AWS SDK for C++, which I have done the same thing with, it was a matter of an hour or so. Clone the repo, run CMake, build, go. Easy. I argue that the build dependencies involved here are either too great or should be hidden from a user's build system enough for people to create a Windows build without having to resort to building all the dependencies as well. If only to be competitive with the competition's SDK here.

So, moving on. To be able to compile I needed a workaround for two compile errors that appeared as soon as I include anything from this library. I needed to disable compile errors C4582 C4583 as seen here.
A similar use case to your usage in gc::StatusOr<>. Apparently MSVC won't have it and Unreal declared this an error rather than a warning, which is why I can disable them using:

#pragma warning(push)
#pragma warning(disable:4583)
#pragma warning(disable:4582)
#include "google/cloud/storage/client.h"
#pragma warning(pop)

With things finally building, I have started to implement a simple write operation to Storage. Very much like in your examples. It compiles and seems to be able to create the client object but unfortunately the actual write() operation fails. No matter what I try, I'm always getting an exception of type absl::lts_2020_09_23::bad_optional_access when I dereference the StatusOr. I am suspecting this is directly related with above disabled compiler warnings.

Which brings me to my next topic: Let's call it "integration friendlyness". C++ is an incredibly diverse cosmos with a huge number of flavors, language versions and so on. Unreal being by far not the freakiest I've come across.

Starting with the question why the exception is raised in the first place. In the CMake settings this lib exposes a setting to enable or disable exceptions. I have them enabled as you state you would std::abort() otherwise anyway and catch them in Unreal code. But is the choice between using exceptions and std::abort() really a choice? It feels more like "use exceptions or crash" to me. Specifically within an environment where I can' influence std::abort. I would consider the choice between exceptions and some kind of response code or outcome a better choice.

Another question is the usage of types like StatusOr. I know, I'm an old school dude but in my view, it is good for a client library to be easy to integrate. Which means, it's better to do things in a 'usual' or common way, even though they might not be as fancy as they could be. In order for as many potential users as possible to look at an example and say, OK, I see.
Can't a factory create that client as a shared ptr which might be null? Otherwise, you have exceptions on anyway, you could just throw when the client can't be created.
Don't get me wrong, I appreciate new ways of doing things but they ought to be hidden in the impl of a client library, not forcibly exposed to the end users. They might be in an environment where they can easily deal with that, or not. It may even turn out to be a showstopper, as I am starting to fear it is for me.
In my view, the simpler and more common language features an API interface uses, the better. The higher the possibility that integration into any environment is possible.

So, I hope you didn't take this as a rant. I really just try to contribute some feedback about this library. Perhaps someone may find this useful.

Cheers,

Moose

[coryan]

Hello everyone,

Hello! Thanks for joining the discussion.

Let me illustrate my journey here a little. I come from a 20 year background in C++ and have quite a bit of experience building various Google libs such as protobuf, gRPC and others on Linux and Windows.

Nice.

Over the last years I have seen and tackled many of the challenges that come with it. This time around, I'm working on a new (Open Source) plugin for the Unreal Engine which is supposed to make use of google-cloud-cpp. Which basically requires me to produce a MSVC build of the library plus all dependencies for linkage within a DLL. It took a long time but finally I have a build and can integrate into my Unreal plugin.

Which portions of google-cloud-cpp are you interested in using? If (and based on the discussion below this sounds plausible) you only want GCS, then you could drop the dependencies on gRPC and protobuf. The magic incantation is:

cmake -DGOOGLE_CLOUD_CPP_ENABLE=storage -S . -B ...

Which brings me to my first topic. Those dependencies and building a Windows version of this library really are a lot. It took many days and without my prior experience with gRPC I doubt I could have done it. If you excuse me comparing this to the AWS SDK for C++, which I have done the same thing with, it was a matter of an hour or so. Clone the repo, run CMake, build, go. Easy. I argue that the build dependencies involved here are either too great or should be hidden from a user's build system enough for people to create a Windows build without having to resort to building all the dependencies as well. If only to be competitive with the competition's SDK here.

We know this is a problem, and it is something we are constantly working on. The next release might help, building all the dependencies becomes:

cd $HOME
git clone https://github.com/Microsoft/vcpkg
git clone https://github.com/googleapis/google-cloud-cpp
cd google-cloud-cpp
cmake -DCMAKE_TOOLCHAIN_FILE=$HOME/vcpkg/scripts/buildsystems/vcpkg.cmake -S . -B ...

That should download and compile all the dependencies for you. It will not solve the DLL problems, for the reasons in #5849.

So, moving on. To be able to compile I needed a workaround for two compile errors that appeared as soon as I include anything from this library. I needed to disable compile errors C4582 C4583 as seen here.
A similar use case to your usage in gc::StatusOr<>. Apparently MSVC won't have it and Unreal declared this an error rather than a warning, which is why I can disable them using:

I am slightly confused, we compile with MSVC (2017 and 2019) after each PR. We also compile with /WX /W4... Can you let us know what version of MSVC are you compiling with? Maybe we need to add that version to our build matrix, or maybe those warnings are disabled for some reason.

#pragma warning(push)
#pragma warning(disable:4583)
#pragma warning(disable:4582)
#include "google/cloud/storage/client.h"
#pragma warning(pop)

Ack. I cannot find much documentation about those warnings beyond "introduced in MSVC 2015 RTM":

https://docs.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warnings-by-compiler-version?view=msvc-160#warnings-introduced-in-visual-studio-2015-rtm-compiler-version-1900230260

With things finally building, I have started to implement a simple write operation to Storage. Very much like in your examples. It compiles and seems to be able to create the client object but unfortunately the actual write() operation fails.

Okay... Can you try a simpler operation first, like GetObjectMetadata or GetBucketMetadata?

No matter what I try, I'm always getting an exception of type absl::lts_2020_09_23::bad_optional_access when I dereference the StatusOr. I am suspecting this is directly related with above disabled compiler warnings.

That is possible. It could also be a runtime problem. How are you dereferencing the StatusOr<>? Just operator*, or .value()? Do you check the status in the StatusOr<>? Can you print it?

Were you able to compile and test any of the examples with your setup?

Which brings me to my next topic: Let's call it "integration friendlyness". C++ is an incredibly diverse cosmos with a huge number of flavors, language versions and so on. Unreal being by far not the freakiest I've come across.

We should share a beverage some day and complain about C++ freaky environments.

Starting with the question why the exception is raised in the first place. In the CMake settings this lib exposes a setting to enable or disable exceptions. I have them enabled as you state you would std::abort() otherwise anyway and catch them in Unreal code. But is the choice between using exceptions and std::abort() really a choice? It feels more like "use exceptions or crash" to me. Specifically within an environment where I can' influence std::abort. I would consider the choice between exceptions and some kind of response code or outcome a better choice.

Well, that is what we offer, a StatusOr<> is an outcome-type class. There are a few places where you may ask us to raise an exception or error (e.g. StatusOr<>::value(), the iostreams when setting some exception flags) and if you do that and you disabled exceptions at compile time, then we call std::abort(). Not like we have much choice.

Can't a factory create that client as a shared ptr which might be null? Otherwise, you have exceptions on anyway, you could just throw when the client can't be created.

Frankly, I would prefer to just throw, but as you noted earlier some folks disable exceptions. Returning nullptr would indicate that something failed, but not what, returning StatusOr<std::shared_ptr<T>> is err .. not the most elegant thing ... but at least you get to know what failed.

I must confess that I do not know at this point if using exceptions is a problem for you or not. It seems that you can use them. If I am correct please consider using StatusOr<T>::value(), as in:

StatusOr<Foo> G();

void F() {
  Foo v = G().value();
}

So, I hope you didn't take this as a rant. I really just try to contribute some feedback about this library. Perhaps someone may find this useful.

I took it as "Okay, we still have work to do in (a) making this easier to compile / integrate, (b) easier to use in larger frameworks like Unreal, and (c) document how StatusOr<> is supposed to be used".

[coryan]

Having users download certificates was not really a viable option to me. It should be working with the 'regular' certificate bundles being installed on most machines.

gRPC may be a challenge:

grpc/grpc#16571

[MrMoose]

gRPC may be a challenge:

grpc/grpc#16571

Eew! I guess I will encounter that very soon then as I am currently working on Pubsub. Haven't tried it out yet. Well, seems like there's no way around then. Thanks for the hint. I guess when I have to use those root certs anyway, I might as well give a curl/openssl build another go and see if that fixes it in the same way.

[MrMoose]

A little update on that one in case it is of interest.

Meanwhile I have got a basic impl of pubsub running but unexpectedly without running into grpc/grpc#16571 . I don't know the reason for this. I have no manually downloaded root.pem cert bundle anywhere and yet it still seems to work OK.

[coryan]

Good to hear that. My guess (emphasis on guess) is that grpc installed its own roots.pem:

https://github.com/grpc/grpc/tree/master/etc

using this rule:

https://github.com/grpc/grpc/blob/8489a6050b64a9c135aaac1c4ccb59b9d5dfe076/CMakeLists.txt#L15517-L15518

I understand such files should be periodically updated, but maybe this will work for you.

[MrMoose]

Well, I just had a look at this and you are right, this file is installed. Yet my fears that the path to it is somehow hardcoded in gRPC were unfounded. If I remove it after the build it still works.

It would appear as if either the file's contents are somehow linked into the binary gRPC libs or the file it not needed at runtime after all.