Skip to content

Conversation

@legleux
Copy link
Collaborator

@legleux legleux commented Nov 6, 2025

High Level Overview of Change

#5975 Changed the target of the xrpld symlink to xrpld and now points to itself.

We renamed the rippled target to xrpld but set the output binary to still be rippled so this symlink should not have been updated.

To be clear, this PR recreates the previous configuration:

bin
├── rippled
└── xrpld -> rippled

or we can just rip the band-aid off now and have rippled be a symlink to xrpld.

Type of Change

  • Bug fix (non-breaking change which fixes an issue)

@codecov
Copy link

codecov bot commented Nov 6, 2025

Codecov Report

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

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #6012   +/-   ##
=======================================
  Coverage     78.3%   78.3%           
=======================================
  Files          816     816           
  Lines        68887   68887           
  Branches      8304    8303    -1     
=======================================
+ Hits         53943   53945    +2     
+ Misses       14944   14942    -2     

see 2 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.

@bthomee
Copy link
Collaborator

bthomee commented Nov 7, 2025

Thanks - I'll push a commit to the renaming script as well soon.

@bthomee
Copy link
Collaborator

bthomee commented Nov 7, 2025

I updated the rename/cmake.sh script to achieve this programmatically.

Note that after running cmake --install build/Release --prefix custom_install I noticed that the order needed to be swapped to actually get the xrpld symlink to correctly point to rippled, as otherwise a rippled symlink would get created pointing to a non-existent xrpld.

We will rip off the bandaid soon, see #5983 (which still needs more work), but I do not plan to merge that until after 3.0.0 to avoid us having to update the deployment scripts in a rush.

@bthomee bthomee requested review from a1q123456 and bthomee November 7, 2025 07:31
@bthomee
Copy link
Collaborator

bthomee commented Nov 7, 2025

@legleux I also looked at the statement you said had no effect, even though on my machine it was complaining. I tried again and now it's compiling fine, so I also removed it from the script.

Copy link
Collaborator

@a1q123456 a1q123456 left a comment

Choose a reason for hiding this comment

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

lgtm

@bthomee bthomee enabled auto-merge (squash) November 7, 2025 10:43
@bthomee bthomee disabled auto-merge November 7, 2025 10:43
@bthomee bthomee enabled auto-merge (squash) November 7, 2025 10:44
@bthomee bthomee merged commit c39f9c5 into develop Nov 7, 2025
38 checks passed
@bthomee bthomee deleted the legleux/fix-symlink branch November 7, 2025 10:51
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.

4 participants