Switch Conan 1 commands to Conan 2 and fix credentials#5655
Conversation
| echo "Uploading missing dependencies." | ||
| conan list --graph=build.json --graph-binaries=build --format=json > pkgs.json | ||
| conan upload --list=pkgs.json --confirm --check --remote xrplf |
There was a problem hiding this comment.
This shouldn't be a part of dependencies action, but in a separate workflow
There was a problem hiding this comment.
See earlier discussion. It's not going to be part of a separate workflow to avoid needless re-computation.
There was a problem hiding this comment.
Specifically this discussion:
And I agree with @bthomee that, given the number of platform combinations we need to build, it would be a huge PITA to have to do all the conan install on top of what we already do in the existing workflows. Even though I started the discussion from the opposite position.
There was a problem hiding this comment.
I don't say we have to do conan install in a different place. I'm saying conan upload should be a separate workflow.
And it would avoid needless re-computation if configured properly, yes.
There was a problem hiding this comment.
I will look at how to move conan upload to a separate workflow/action in the CI refactor PR - I think it will be done sometime next week. For this PR I consider it out of scope - the intention of this PR was to primarily fix the authentication issue, and secondarily to address the concern that re-uploading unchanged packages might change the package hash.
There was a problem hiding this comment.
I agree with @bthomee - let's try to keep the PR focused. Scope creep is an enemy of improvement.
There was a problem hiding this comment.
Well, I think the current change is not focused already.
When the PR title is "do this and do that" in the title, it usually means it should be 2 PRs - "do this" and "do that"
There was a problem hiding this comment.
So, I think it would be better to keep this PR as "Switch Conan 1 commands to Conan 2" (more-or-less revert these lines) and then in the next PR discuss how, where and when we should upload packages.
There was a problem hiding this comment.
Refactoring of uploading packages to a separate workflow can go into a separate PR. It's important to keep refactoring changes away from functional ones. This PR is functional and does exactly what its subject line says.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #5655 +/- ##
=========================================
- Coverage 78.8% 78.8% -0.0%
=========================================
Files 814 814
Lines 71283 71243 -40
Branches 8349 8341 -8
=========================================
- Hits 56175 56140 -35
+ Misses 15108 15103 -5 🚀 New features to boost your workflow:
|
Bronek
left a comment
There was a problem hiding this comment.
Only two small nits, this aside I think it's good 👍
High Level Overview of Change
This change updates some incorrect Conan commands for Conan 2. This change further uses the org-level variables and secrets rather than the repo-level ones.
Context of Change
Some of the Conan commands were still for 1.x. This change updates them to Conan 2.x. As some flags do not exist in Conan 2, such as
--settings build_type=[configuration], the commands have been adjusted accordingly.Type of Change
.gitignore, formatting, dropping support for older tooling)