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

CI: Add Conan testing, mass building and publishing #647

Closed
wants to merge 2 commits into from

Conversation

Croydon
Copy link
Contributor

@Croydon Croydon commented Jul 25, 2018

This

  • is adding a recipe for the Conan package manager
  • also adds a small test to the CI if Conan package creation is working
  • updated: also adds a upload script to the CI for the Conan recipe to a Conan repository

Please note that for future releases of benchmark the version string needs to be updated in conanfile.py (not anymore, see discussion)


Future work could include
Remaining to do list:

  • setting up a free repository on Bintray: https://bintray.com/dominichamon/benchmark
  • including the package in the official Conan repository conan-center
  • and extending the CI to upload new versions of the recipe and built packages to the Bintray repository automatically

To use the recipe after this pull request is merged it requires to clone this git repository first; when the recipe is included in conan-center every Conan client would be able to use it via one single install command.


Please let me know if you have any further questions regarding supporting Conan 😄

Fixes #635

//cc @raulbocanegra @p-groarke @danimtb @Mikayex @mpusz @iblis-ms

@coveralls
Copy link

coveralls commented Jul 25, 2018

Coverage Status

Coverage remained the same at 89.41% when pulling 866b121 on Croydon:conan into eec9a8e on google:master.

@AppVeyorBot
Copy link

Build benchmark 1340 completed (commit 480ea1fdc9 by @Croydon)

@Croydon
Copy link
Contributor Author

Croydon commented Jul 25, 2018

The failure of Travis seems to be unrelated since I have neither touched source code nor the previous existing Travis jobs.

@AppVeyorBot
Copy link

Build benchmark 1342 completed (commit 433dd01f47 by @Croydon)

conanfile.py Outdated Show resolved Hide resolved
conanfile.py Outdated Show resolved Hide resolved
conanfile.py Outdated Show resolved Hide resolved
conanfile.py Outdated Show resolved Hide resolved
@AppVeyorBot
Copy link

Build benchmark 1350 failed (commit e06e42ccde by @Croydon)

@AppVeyorBot
Copy link

Build benchmark 1351 completed (commit 725e138895 by @Croydon)

@Croydon
Copy link
Contributor Author

Croydon commented Jul 29, 2018

Tl;dr:

  • we can get rid of the version string
  • but then I even more strongly advocate for setting up a Conan repository on Bintray than I would have done anyway :)

Some thoughts..

A conan reference has the following elements:

<name> / <version> @ <user> / <channel>

So the variables for Benchmark are version & channel:

benchmark / <version> @ google / <channel>

The version is in most cases the common version number like 1.4.1

The channel is freely selectable, but common practice within the Conan ecosystem is to use either stable or testing.

The full package reference is decided at the Conan export step. So the reference, including the version and channel can't be changed at a later step (without a re-export(, or some form of alias)).
The channel is not set within the Conan recipe, but rather directly at export, together with the user, like this:

conan export . <user>/<channel>
conan export . google/testing

So common practice is: Name + version from the recipe; user + channel from the export step.

However, I fully understand that it is annoying to change the version via commit for each release and maybe even change it again directly after the release to something like "1.4.2-dev".

It is possible to remove the version string from the recipe, which I have done in my latest commits. This requires then to specify the full reference at the export step like:

conan export . benchmark/1.4.1@google/stable

By this way we got rid of the annoyance for the people doing the releases, but now got an very uncomfortable situation for all consumers of the recipe, which is a strong contradiction to one of the main goals of a package manager: Making it easier to use and handle the library. Consumers would now be required to clone the repository, then doing the export step with a reference which hopefully doesn't bring in some misunderstanding about what Benchmark version is currently in use; also two projects may use completely different references, even though both use the very same version of Benchmark.

Solution

The annoyance of manually maintaining the version string and the mess and complication for consumers can be best avoided by setting up a Conan repository and let Travis upload new recipe versions automatically.

I have already written the code for this. I have done it in a way that every tag gets a
benchmark/<version>@google/stable release and there is an evergreen benchmark/master@google/testing version.

The things you would now have to do would include:

  • use/create a free account on http://bintray.com
    • there is already a Google account, not sure if you would like to use this or setup a separate Benchmark one
  • create a new Conan repository ("Add New Repository" -> Type: Conan)
  • Go to https://travis-ci.org/google/benchmark/settings and create the environment variables CONAN_UPLOAD, CONAN_LOGIN_USERNAME and CONAN_PASSWORD
    • as I have documented directly in the .travis.yml file
  • the package will be created automatically by the first upload from Travis
  • merge this pull request, so that the first upload is happing
  • fill out the meta information for the package on Bintray (website, License etc.)
  • on the package overview page you can see a "Add to Conan Center" button; follow the steps
    • conan-center is the official, central Conan repository which is pre-added for every Conan client
    • since you are the maintainers themselves of the library you will get accepted for sure

Result

  • no manually version string maintenance
  • easiest way to install Benchmark for consumers
    • no git cloning
    • no manual exporting with self-defining of user/channel/version
    • just conan install benchmark/1.4.1@google/stable
  • only people who are working on Benchmark itself and do install/test it via Conan, will need to specify the full reference manually
  • bonus points: you are helping conan-center to grow :)

@Croydon Croydon force-pushed the conan branch 2 times, most recently from 70f8b4b to b05d776 Compare July 29, 2018 09:38
@AppVeyorBot
Copy link

Build benchmark 1352 completed (commit f8165d00d2 by @Croydon)

@LebedevRI
Copy link
Collaborator

Hmm wait, there is no 3rd party quality control of these packages?
And there is just one 'official' prebuilt version of the library (via travis) that is just assumed to be good enough for everybody?

@Croydon
Copy link
Contributor Author

Croydon commented Jul 29, 2018

Hmm wait, there is no 3rd party quality control of these packages?

There is a strict quality control for conan-center when you are packaging third-party libraries, it's much more relaxed when you are the maintainer of the library itself, for I guess obvious reasons.

And there is just one 'official' prebuilt version of the library (via travis) that is just assumed to be good enough for everybody?

The Travis script I have scripted right now is not uploading prebuilt packages, only the recipe. Conan is automatically building the package if there isn't a prebuilt package available for the current os/version/arch/configuration/options...

We can surely add this as well, but the CI is then going to be a lot bigger. I wanted to start small and expend afterwards based on feedback.

@AppVeyorBot
Copy link

Build benchmark 1354 completed (commit b1e036d229 by @Croydon)

@AppVeyorBot
Copy link

Build benchmark 1446 failed (commit 693d8b7d27 by @Croydon)

@AppVeyorBot
Copy link

Build benchmark 1458 failed (commit 97af35af59 by @Croydon)

@AppVeyorBot
Copy link

Build benchmark 1459 failed (commit 90f97b47e0 by @Croydon)

@AppVeyorBot
Copy link

Build benchmark 1463 failed (commit ec6546605f by @Croydon)

@AppVeyorBot
Copy link

Build benchmark 1464 failed (commit 9a8147b993 by @Croydon)

@dmah42
Copy link
Member

dmah42 commented Nov 13, 2018

Help me out: Is anything else necessary for this to be checked in if we use dominichamon/benchmark (and maybe update it later)?

I'm not completely content with having API keys in the travis config in github so an alternative to that would be useful.

@Croydon
Copy link
Contributor Author

Croydon commented Nov 13, 2018

Help me out: Is anything else necessary for this to be checked in if we use dominichamon/benchmark (and maybe update it later)?

No, it's good to go once the Bintray access variables are set.

I'm not completely content with having API keys in the travis config in github so an alternative to that would be useful.

Fully agreed, that it why I would recommend to set them via Travis and AppVeyor UI
For Travis: https://travis-ci.org/google/benchmark/settings
For AppVeyor: on the project page -> settings -> environment -> add environment variable

Please note that while Travis is encrypted via default, AppVeyor needs an explicit click on the lock symbol at the end of the line.

        #   CONAN_UPLOAD - a reference to the Conan repository e.g. https://api.bintray.com/conan/dominichamon/benchmark 
        #   CONAN_LOGIN_USERNAME - user for the upload e.g. dominichamon
        #   CONAN_PASSWORD - password for the CONAN_LOGIN_USERNAME; for Bintray this is the API key

@memsharded
Copy link

Hi!

Conan maintainer here.

I think this PR is very big, and I understand there are concerns about changing CI.

I would approach it step by step. The most important thing, which is not intrusive (won't change absolutely nothing in the build or in the CI) is the conanfile.py recipe and the "test_package" folder. I think that should go first, by its own.

CI is a different beast, so better wait until later. We are also working on some ideas that could help in greatly reducing or completely removing the required CI changes.

Once the conanfile.py is there, in the meantime (while we wait until the above ideas are released) we could try to setup a dedicated repo (in bincrafters or conan-community) for conan CI, without requiring to modify this repo CI scripts.

What do you think?

@dmah42
Copy link
Member

dmah42 commented Nov 26, 2018

Hi @memsharded.

Thanks so much for weighing in! If there is a way to split this i'd really appreciate it so i can understand the changes bit-by-bit.

@Croydon Croydon mentioned this pull request Nov 26, 2018
@Croydon Croydon changed the title Add Conan support (#635) CI: Add Conan testing, mass building and publishing (#635) Nov 26, 2018
@Croydon Croydon changed the title CI: Add Conan testing, mass building and publishing (#635) CI: Add Conan testing, mass building and publishing Nov 26, 2018
@Croydon
Copy link
Contributor Author

Croydon commented Nov 26, 2018

See: #728 - Minimal Conan support

After #728 is merged, I'm going to rebase this pull request and it will add CI for Conan, including building of Conan packages and publishing.

@vtkacenko
Copy link

@Croydon Is there anything one can help with to get this moving?

@dmah42
Copy link
Member

dmah42 commented Mar 18, 2019

#728 was merged. was there anything else you were waiting for?

@vtkacenko
Copy link

@dominichamon Not waiting for anything. Just wondering if there is anything I can help with to get the #647 merged; in order to have the package available in the conan-center.

@dmah42
Copy link
Member

dmah42 commented Apr 5, 2019

Revisiting this. I still haven't been able to get access to bintray for google and i'm not comfortable having this hosted on my name (as it's a Google project).

Given we don't have that, it still sounds like there's a benefit to having this landed should i ever succeed in getting that access, and at that point I'd need to set API keys in the various CI plugins?

@Croydon
Copy link
Contributor Author

Croydon commented Apr 5, 2019

@dominichamon Are you on IRC/Gitter/Matrix/Slack or something similar so that we could have a direct chat about this? :)

@dmah42
Copy link
Member

dmah42 commented Apr 8, 2019

@Croydon I'm on IRC (freenode, #googlebenchmark) most of the time.

@joachimHodara
Copy link

Thanks for your work on this issue @dominichamon and @Croydon. Any update regarding the access to bintray? Just like @vtkacenko, I would also benefit greatly from being able to get google benchmark from the conan-center. Let me know if you need help with anything.

@Croydon
Copy link
Contributor Author

Croydon commented Sep 1, 2019

This is blocked until @dominichamon has access to the Bintray Google account or decides to use a dedicated Bintray organisation/account for hosting a Benchmark Conan repository.

The Google Bintray organisation has 5 (public) members: @tjohns @ojw28 @andrewlewis @willnorris & https://bintray.com/anaddaf

Sorry, for the seemingly random pings, but if someone of you could get in contact with @dominichamon and have a talk about getting access to this Bintray organisation this would be awesome. This issue is stalled for a long time now.


By the way, Google Flatbuffers is hosted by aardappel's personal account https://bintray.com/aardappel/flatbuffers/flatbuffers%3Agoogle

Google Fruit, on the other hand, is hosted by the Google account https://bintray.com/google/fruit

@dmah42
Copy link
Member

dmah42 commented Sep 2, 2019 via email

@ojw28
Copy link

ojw28 commented Sep 2, 2019

Not sure if there's some nuance here, but I don't understand why this would be difficult to sort out. @dominichamon - Feel free to email me internally if needed.

@dmah42
Copy link
Member

dmah42 commented Sep 6, 2019 via email

@Croydon
Copy link
Contributor Author

Croydon commented Sep 8, 2019

Ironically, 3 days after my last comment here the Conan team released the conan-center-index: https://blog.conan.io/2019/09/04/A-new-binary-building-service-for-Conan-center-packages.html

Which makes the requirement for upstream projects to set up their own repositories and CI builds go away.

I'm part of the Early Access Program and will create an inclusion request somewhen in the next days here: https://github.com/conan-io/conan-center-index

Since this is a new situation, the Conan team and community still need to figure some stuff out, for instance if upstream projects should keep the conanfiles in their own repositories as well: conan-io/conan-center-index#22

However, the CI stuff which would be added via this pull request is not needed anymore and thereby you don't necessary need access to the Google Bintray org anymore @dominichamon

@Croydon Croydon closed this Sep 8, 2019
@dmah42
Copy link
Member

dmah42 commented Sep 30, 2019

fyi, https://bintray.com/google/benchmark is there finally. let me know if there's anything i need to do to support getting this included in conan.

@Croydon
Copy link
Contributor Author

Croydon commented Oct 4, 2019

@dominichamon Conan Center Index is really aiming to lowering the barriers for getting Conan support and ongoing maintenance. One of the perks is that it doesn't force upstream maintainers to setup their own Conan repository.

I'm really sorry about this bad timing and the work you put into getting access to the Google organisation. The Conan Center Index was just released a month ago and I didn't know any details beforehand. So I couldn't tell that it soon wouldn't be necessary anymore.

Anyhow, the goal is to get benchmark finally into the Conan Center and this should be done soon.

Pull request: conan-io/conan-center-index#174


Within the Conan community we didn't reach any conclusion yet, about what to do with in-source recipes. Benchmark has already merged a Conan recipe in its source, which has many advantages, but maintaining stuff at two places is not well manageable right now.

As soon as we have a conclusion, I will either create a pull request in this repository here and suggest the removal of the Conan recipe and test_package, or write a comment here and recommend to keep it 😃

@dmah42
Copy link
Member

dmah42 commented Oct 4, 2019

Oh I understand completely, and I'm really happy that you're going to keep moving this forward. Wherever we end up is fine by me, at least now we have options :)

@Croydon
Copy link
Contributor Author

Croydon commented Oct 10, 2019

Benchmark is now finally in Conan Center!

https://github.com/conan-io/conan-center-index/tree/master/recipes/benchmark
https://bintray.com/conan/conan-center/benchmark%3A_

It can be installed by either adding benchmark/1.5.0 to a conanfile.txt or a conanfile.py file.

Or via command line:

conan install benchmark/1.5.0@ --build missing

Thanks anyone! 🎉


I will report back once more when conan-io/conan-center-index#22 is resolved 😄

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

Successfully merging this pull request may close these issues.

[feature] Conan package.