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

Add the abseil/20200205 #801

Merged
merged 8 commits into from
Mar 2, 2020
Merged

Conversation

Minimonium
Copy link
Contributor

Specify library name and version: abseil/20200205

Closes #710

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@conan-center-bot
Copy link
Collaborator

Some configurations of 'abseil/20200205' failed in build 1 (0275a7525b2bcec0ec0723c21cbd1d553fcfe039):

@Minimonium
Copy link
Contributor Author

@danimtb Abseil requires at least C++11. Are there any updates on how to handle this in CCI PRs?

@conan-center-bot
Copy link
Collaborator

Some configurations of 'abseil/20200205' failed in build 2 (ca110d788d696ad35aec551d2c0265576ffa8e7c):

@Minimonium
Copy link
Contributor Author

#error This file requires compiler and library support for the ISO C++ 2011 standard. This support must be enabled with the -std=c++11 or -std=gnu++11 compiler options.

🙂

@Minimonium Minimonium requested a review from danimtb February 8, 2020 20:36
@danimtb danimtb requested review from SSE4 and uilianries February 10, 2020 10:22
@SSE4
Copy link
Contributor

SSE4 commented Feb 10, 2020

we need to do something about projects requiring non-default cppstd.
this is 2020, and many projects are already started requiring modern standards (C++11, C++17, C++20).
I believe we should somehow allow the building of more configuration in CCI.
however, that may blow up a lot of unnecessary builds in many cases.
we have to think carefully about it. maybe the recipe should somehow indicate if it needs special standard(s)?
what do you think @uilianries @danimtb ?

@Minimonium
Copy link
Contributor Author

How does the C3I handle the package_id? Could it be possible to preparse it to extract the information relevant to which configurations we should do builds?

We don't really have a choice but to generate additional configurations in some cases. We can't assume that it's fine to use a non-default cppstd without actually marking it in the profile. Some libraries do break the ABI on the same library version but on different standard flags - Boost and Abseil do that if I recall correctly.

Other libraries could, for example, erase it in their package_id. The main goal is to have default packages compatible for all users as much as possible.

@danimtb
Copy link
Member

danimtb commented Feb 10, 2020

Related to #54 Please comment anything there related to this topic

cmake.install()
tools.rmdir(os.path.join(self.package_folder, "lib", "cmake"))

def package_info(self):
Copy link
Member

Choose a reason for hiding this comment

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

I don't like it for all versions. From my experience, Abseil changes the library constantly. I would say you should create one recipe per version. However, creating one recipe version is boring, and only the cpp_info.libs will change. So, we could accept the lib list in conandata.yml, but it requires a filter there is conan-center hook.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A good idea! The all folder is kinda a placeholder. Shouldn't be a problem to split recipes into different folders later on, isn't it?

Copy link
Member

@danimtb danimtb Feb 12, 2020

Choose a reason for hiding this comment

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

I am not a big fan of the idea of having this information in the conandata.yml file, as it will be harder to read and it is mainly used (in this repo) for things related to sources. But you can always make the libs conditional to the version of the recipe if you want to keep one recipe for all of them (easier to maintain). @Minimonium in case the recipe diverges too much we can always split it to one for each version

Co-Authored-By: Uilian Ries <uilianries@gmail.com>
@conan-center-bot
Copy link
Collaborator

Some configurations of 'abseil/20200205' failed in build 3 (b7d8026fa5c0b2e04f226589b6fc9040c3f990bb):

@conan-center-bot
Copy link
Collaborator

Some configurations of 'abseil/20200205' failed in build 4 (fd555cb66e4f8f1fa5a5bfa753f47d6bb11b6225):

@conan-center-bot
Copy link
Collaborator

Some configurations of 'abseil/20200205' failed in build 5 (517659815c106710273778960fc5e07fede07b35):

@conan-center-bot
Copy link
Collaborator

Some configurations of 'abseil/20200205' failed in build 6 (a6901311dbbeb16bbde542ae92769d65c5e435f1):

@conan-center-bot
Copy link
Collaborator

Some configurations of 'abseil/20200205' failed in build 7 (f717cd43a2abcbd64e7311a2daebda51dc5d736a):

@conan-center-bot
Copy link
Collaborator

All green in build 8 (98b864782bb70e28164e08f0c1218aff9a708e25)! 😊

@Minimonium
Copy link
Contributor Author

Nailed it.

@danimtb danimtb self-assigned this Mar 2, 2020
@danimtb danimtb merged commit fbcaedb into conan-io:master Mar 2, 2020
@Samuel-Tyler
Copy link

Thanks for getting this done!

@Minimonium Minimonium deleted the feature/abseil-20200205 branch June 16, 2020 16:59
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] abseil/latest
6 participants