Skip to content

Conversation

@Bronek
Copy link
Collaborator

@Bronek Bronek commented Aug 11, 2025

High Level Overview of Change

This should fix workflow errors for PRs from forks

Context of Change

As stated in https://github.com/XRPLF/rippled/blob/develop/BUILD.md , rippled requires certain workarounds to be applied in external dependencies e.g. to ensure static linking of libstdc++ . This used to be applied by means of conan export --version . . . , but recently we have switched workflows to use a Conan remote https://conan.ripplex.io where we apply the relevant patches. We decided to put this remote in configuration variable but forgot that configuration variables are not available to PRs built from forks. As a result, PRs such as #5637 will use dependencies without workarounds, causing CI errors.

This PR replaces configuration variable with hardcoded https://conan.ripplex.io , making it possible for PRs from forks to use the Conan remote with workarounds.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactor (non-breaking change that only restructures code)
  • Performance (increase or change in throughput and/or latency)
  • Tests (you added tests for code that already exists, or your new feature included in this PR)
  • Documentation update
  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
  • Release

@Bronek Bronek requested a review from bthomee August 11, 2025 11:06
@Bronek Bronek added the Trivial Simple change with minimal effect, or already tested. Only needs one approval. label Aug 11, 2025
@bthomee
Copy link
Collaborator

bthomee commented Aug 11, 2025

You can just hardcode both the URL (https://conan.ripplex.io) and the name (xrplf).

I had added it like this because env vars cannot be used as a "with" input to call reusable workflows, which I'm using in my CI refactor PR that's still under development. However I suspected this would be an issue in forks, which is now confirmed.

Therefore, hardcoding seems to be the only option, and I'll figure out a workaround to pass them to reusable workflows later.

@Bronek
Copy link
Collaborator Author

Bronek commented Aug 11, 2025

hardcoding seems to be the only option, and I'll figure out a workaround to pass them to reusable workflows later

Perhaps hardcode an env. variable, like I did ?

@Bronek Bronek added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label Aug 11, 2025
@bthomee bthomee enabled auto-merge (squash) August 11, 2025 11:47
@codecov
Copy link

codecov bot commented Aug 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 78.8%. Comparing base (bdfc376) to head (3740283).
⚠️ Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #5662   +/-   ##
=======================================
  Coverage     78.8%   78.8%           
=======================================
  Files          814     814           
  Lines        71294   71294           
  Branches      8368    8369    +1     
=======================================
+ Hits         56165   56167    +2     
+ Misses       15129   15127    -2     

see 6 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bthomee bthomee merged commit abf12db into develop Aug 11, 2025
29 checks passed
@bthomee bthomee deleted the Bronek/workflow_fixes branch August 11, 2025 13:02
ximinez added a commit that referenced this pull request Aug 18, 2025
…actoring-1

* XRPLF/develop:
  fix: Modify jobs to use '>>' instead of 'tee' for GITHUB_OUTPUT (#5699)
  refactor: Revamp CI workflows (#5661)
  refactor: Decouple net from xrpld and move rpc-related classes to the rpc folder (#5477)
  Set version to 2.6.0-rc2
  docs: Updates list of maintainers and reviewers (#5687)
  fix: Change log to debug level for AMM offer retrieval and IOU payment check (#5686)
  fix: Add -Wno-deprecated-declarations for Clang only (#5680)
  Update .git-blame-ignore-revs for #5657 (#5675)
  Fix BUILD.md instruction (#5676)
  Set version to 2.6.0-rc1
  fix: Improve logging of the reason to refuse a peer connection (#5664)
  fix: Make test suite names match the directory name (#5597)
  chore: Run prettier on all files (#5657)
  chore: Set CONAN_REMOTE_URL also for forks (#5662)
  chore: Cleanup bin/ directory (#5660)
  perf: Optimize hash performance by avoiding allocating hash state object (#5469)
ximinez added a commit that referenced this pull request Aug 18, 2025
…to ximinez/lending-XLS-66

* XRPLF/ximinez/lending-refactoring-4:
  fix: Modify jobs to use '>>' instead of 'tee' for GITHUB_OUTPUT (#5699)
  refactor: Revamp CI workflows (#5661)
  refactor: Decouple net from xrpld and move rpc-related classes to the rpc folder (#5477)
  Set version to 2.6.0-rc2
  docs: Updates list of maintainers and reviewers (#5687)
  fix: Change log to debug level for AMM offer retrieval and IOU payment check (#5686)
  fix: Add -Wno-deprecated-declarations for Clang only (#5680)
  Update .git-blame-ignore-revs for #5657 (#5675)
  Fix BUILD.md instruction (#5676)
  Set version to 2.6.0-rc1
  fix: Improve logging of the reason to refuse a peer connection (#5664)
  fix: Make test suite names match the directory name (#5597)
  chore: Run prettier on all files (#5657)
  chore: Set CONAN_REMOTE_URL also for forks (#5662)
  chore: Cleanup bin/ directory (#5660)
  perf: Optimize hash performance by avoiding allocating hash state object (#5469)
This was referenced Aug 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. Trivial Simple change with minimal effect, or already tested. Only needs one approval.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants