-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Conversation
✅ Build benchmark 1340 completed (commit 480ea1fdc9 by @Croydon) |
The failure of Travis seems to be unrelated since I have neither touched source code nor the previous existing Travis jobs. |
✅ Build benchmark 1342 completed (commit 433dd01f47 by @Croydon) |
❌ Build benchmark 1350 failed (commit e06e42ccde by @Croydon) |
✅ Build benchmark 1351 completed (commit 725e138895 by @Croydon) |
Tl;dr:
Some thoughts..A conan reference has the following elements:
So the variables for Benchmark are version & 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 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)).
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:
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. SolutionThe 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 The things you would now have to do would include:
Result
|
70f8b4b
to
b05d776
Compare
✅ Build benchmark 1352 completed (commit f8165d00d2 by @Croydon) |
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.
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. |
✅ Build benchmark 1354 completed (commit b1e036d229 by @Croydon) |
❌ Build benchmark 1446 failed (commit 693d8b7d27 by @Croydon) |
❌ Build benchmark 1458 failed (commit 97af35af59 by @Croydon) |
❌ Build benchmark 1459 failed (commit 90f97b47e0 by @Croydon) |
❌ Build benchmark 1463 failed (commit ec6546605f by @Croydon) |
❌ Build benchmark 1464 failed (commit 9a8147b993 by @Croydon) |
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. |
No, it's good to go once the Bintray access variables are set.
Fully agreed, that it why I would recommend to set them via Travis and AppVeyor UI Please note that while Travis is encrypted via default, AppVeyor needs an explicit click on the lock symbol at the end of the line.
|
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? |
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 Is there anything one can help with to get this moving? |
#728 was merged. was there anything else you were waiting for? |
@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 |
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? |
@dominichamon Are you on IRC/Gitter/Matrix/Slack or something similar so that we could have a direct chat about this? :) |
@Croydon I'm on IRC (freenode, #googlebenchmark) most of the time. |
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. |
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 |
I'm still trying :)
…On Sun, Sep 1, 2019 at 8:45 AM Croydon ***@***.***> wrote:
This is blocked until @dominichamon <https://github.com/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 <https://bintray.com/google> organisation has 5
(public) members: @tjohns <https://github.com/tjohns> @ojw28
<https://github.com/ojw28> @andrewlewis <https://github.com/andrewlewis>
@willnorris <https://github.com/willnorris> & https://bintray.com/anaddaf
Sorry, for the seemingly random pings, but if someone of you could get in
contact with @dominichamon <https://github.com/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
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#647?email_source=notifications&email_token=AAD4QMVGISTMDJU2Z6MB7M3QHNXI3A5CNFSM4FMCXZQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5T4VDA#issuecomment-526895756>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAD4QMTXMYSPVMAPCG5YLOLQHNXI3ANCNFSM4FMCXZQQ>
.
|
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. |
Emailed internally and followed up with other bintray owners.
I agree it shouldn't be difficult :)
…On Mon, Sep 2, 2019 at 11:33 AM Oliver Woodman ***@***.***> wrote:
Not sure if there's some nuance here, but I don't understand why this
would be difficult to sort out. @dominichamon
<https://github.com/dominichamon> - Feel free to email me internally if
needed.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#647?email_source=notifications&email_token=AAD4QMXNSTQVRDRPRR3USKLQHTTV7A5CNFSM4FMCXZQ2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD5VN3TI#issuecomment-527097293>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAD4QMXH2M2GGA64BMKOB6DQHTTV7ANCNFSM4FMCXZQQ>
.
|
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 |
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. |
@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 😃 |
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 :) |
Benchmark is now finally in Conan Center! https://github.com/conan-io/conan-center-index/tree/master/recipes/benchmark It can be installed by either adding Or via command line:
Thanks anyone! 🎉 I will report back once more when conan-io/conan-center-index#22 is resolved 😄 |
This
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 includeRemaining to do list:
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