Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[google-cloud-cpp] Add recipe for Google Cloud Services #5424

Merged
merged 36 commits into from
May 20, 2021

Conversation

jgsogo
Copy link
Contributor

@jgsogo jgsogo commented May 4, 2021

Specify library name and version: google-cloud-cpp/1.26.1

Pushing this recipe, so we can use C++ to create GCP microservices... for ConanCenter itself 🤔

This recipe requires:

Also, I will rebase/squash this PR once dependencies are merged.

close #5212

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@jgsogo jgsogo force-pushed the google-cloud-cpp branch from a83b044 to 59917da Compare May 17, 2021 14:54
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@conan-center-bot conan-center-bot requested a review from danimtb May 18, 2021 11:57
prince-chrismc
prince-chrismc previously approved these changes May 18, 2021
Copy link
Contributor

@prince-chrismc prince-chrismc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think my comment will break... but it's a minor detail

✔️

recipes/google-cloud-cpp/all/conanfile.py Outdated Show resolved Hide resolved
@conan-center-bot

This comment has been minimized.

@conan-center-bot
Copy link
Collaborator

All green in build 19 (38c006dfd00d22501be52bba8f8c178b281d0ffa):

  • google-cloud-cpp/1.26.1@:
    All packages built successfully! (All logs)

if self.options.shared:
del self.options.fPIC

if self.settings.os == 'Windows' and self.options.shared:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this configuration is not working, why adding set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON) in CMakeLists.txt?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... I tried to too, and probably it is possible, but without a local Windows to test and try I gave up. Also, it will probably build for older VS versions... Someone will take care in another PR if needed 😊

@conan-center-bot conan-center-bot merged commit ade1798 into conan-io:master May 20, 2021
@jgsogo jgsogo deleted the google-cloud-cpp branch May 20, 2021 14:37
Comment on lines +102 to +103
def package_info(self):
# Using componen google-cloud-common, so it reduces the risk of conflict (pkgconfig generator)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at installed pkgconfig files, pkg_config files are not properly emulated. This component google-cloud-common could have been called common with pkgconfig name google_cloud_cpp_common.

CMake targets seem ok, but generators can't emulate official config files, and google-cloud-cpp generates more than one config file.
Actually, there is no google-cloud-cpp-config.cmake file upstream.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I find your suggestion much more convenient. From the consumer perspective, I prefer to use google-cloud-cpp::common over google-cloud-cpp::google-cloud-common to refer to that component. I think it is worth a PR and probably a suggestion to follow for other PRs, wdyt @prince-chrismc ?

Copy link
Contributor

@prince-chrismc prince-chrismc May 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the CMake perspective google-cloud-cpp::common should be working since we added those names 🤔 both names are currently available AFAIK.

My original comment stems for all the times Space flagged this potential issue, I did not check the pkg_config log but we could have done the inverse as is suggested. So I have no objections here.

Instead of changing the component name we could simple just add the corrected name? Avoid changing it's usage in the recipe

self.cpp_info.components["google-cloud-common"].names["pkg_config"] = "google_cloud_cpp_common"

I assume they did not use "common" since it was too common xD

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My comment was about Conan components, not related to CMake targets. From a consumer package you will refer to that component as google-cloud-cpp::google-cloud-common (now). I'm voting to changing it back to google-cloud-cpp::common.

CMake targets are not changed. Pkgconfig would require the line you suggested.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[request] google-cloud-cpp/1.26.0
6 participants