Skip to content

feat: Generate Debian packages with CPack#2282

Merged
legleux merged 10 commits intoXRPLF:developfrom
legleux:package-with-cpack
Aug 21, 2025
Merged

feat: Generate Debian packages with CPack#2282
legleux merged 10 commits intoXRPLF:developfrom
legleux:package-with-cpack

Conversation

@legleux
Copy link
Collaborator

@legleux legleux commented Jul 2, 2025

Builds the Linux packages with CPack.
Generate them by running Conan with --options:host "&:package=True" --options:host "&:static=True" then after the build you can run cpack . in the build directory.

@mathbunnyru Where do you think this should be built? QA needs a package per-commit.

@godexsoft What to do with the config.json and service file. I can just remove them or strip the comment out but it still won't work out the box with the default rippled.cfg. Relates to #2191.

@codecov
Copy link

codecov bot commented Jul 2, 2025

Codecov Report

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

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #2282      +/-   ##
===========================================
+ Coverage    79.93%   79.95%   +0.01%     
===========================================
  Files          381      381              
  Lines        15762    15762              
  Branches      8254     8254              
===========================================
+ Hits         12600    12602       +2     
+ Misses        1892     1891       -1     
+ Partials      1270     1269       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@legleux legleux changed the title feature: Generate Debian packages with CPack feat: Generate Debian packages with CPack Jul 2, 2025
@godexsoft
Copy link
Collaborator

@legleux imo we can drop the service file but let's keep the config. We can later adjust it to work with a default rippled (local) config if it does not already 👍

Copy link
Collaborator

@mathbunnyru mathbunnyru left a comment

Choose a reason for hiding this comment

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

Is there any way to check this works in CI?
It would be nice to have a workflow

@mathbunnyru
Copy link
Collaborator

Where do you think this should be built? QA needs a package per-commit.

I don't mind having it here, if it's also tested here, that's why let's add a workflow

@legleux
Copy link
Collaborator Author

legleux commented Jul 9, 2025

Is there any way to check this works in CI?

How do you test changes to CI nowadays?

Where do you think this should be built? QA needs a package per-commit.

I don't mind having it here, if it's also tested here, that's why let's add a workflow

I was thinking it should probably be part of build_and_test.yml since they want it for every commit and it needs the CMAKE_BINARY_DIR.

@mathbunnyru
Copy link
Collaborator

Is there any way to check this works in CI?

How do you test changes to CI nowadays?

Could you clarify your question, please?

Where do you think this should be built? QA needs a package per-commit.

I don't mind having it here, if it's also tested here, that's why let's add a workflow

I was thinking it should probably be part of build_and_test.yml since they want it for every commit and it needs the CMAKE_BINARY_DIR.

I think you can add it to build_impl.yml under a flag, and probably add the same flag to build_and_test.yml.
It won't be saved anywhere though.

@legleux legleux force-pushed the package-with-cpack branch from 3fbe2c3 to b4ba113 Compare July 15, 2025 23:08
@legleux legleux force-pushed the package-with-cpack branch from e8153b0 to ab65f82 Compare July 24, 2025 02:29
@godexsoft
Copy link
Collaborator

@legleux imo we can drop the service file but let's keep the config. We can later adjust it to work with a default rippled (local) config if it does not already 👍

#2377 should set the config up to work with default rippled configuration

@godexsoft
Copy link
Collaborator

@legleux what remains for this to be merged?

@legleux
Copy link
Collaborator Author

legleux commented Jul 29, 2025

@godexsoft When I had added the step to build the packages similar to the code_coverage step, I couldn't get to see the final outcome because the build kept failing (I'm pretty sure due to OOM on the runner).

@legleux legleux force-pushed the package-with-cpack branch 3 times, most recently from 1df0838 to 40eefe9 Compare August 14, 2025 19:30
Copy link
Collaborator

@mathbunnyru mathbunnyru left a comment

Choose a reason for hiding this comment

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

Left some comments and question

I've also taken a look why the CI fails - you need to adjust build_impl.yml so it doesn't upload tests when it's package (and you might need to adjust clio_server upload as well)

@legleux legleux force-pushed the package-with-cpack branch 2 times, most recently from 14c562c to bb3ca71 Compare August 18, 2025 21:08
Copy link
Collaborator

@mathbunnyru mathbunnyru left a comment

Choose a reason for hiding this comment

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

I left some comments, mostly things I don't understand

@legleux legleux force-pushed the package-with-cpack branch from 09b6683 to 263dc25 Compare August 20, 2025 17:00
@legleux legleux merged commit 2d48de3 into XRPLF:develop Aug 21, 2025
141 of 142 checks passed

include(GNUInstallDirs)

install(TARGETS clio_server DESTINATION ${CMAKE_INSTALL_BINDIR})
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to escape this as well?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, fixed in #2469
Also, fixed some rename issues we had

@legleux just for the future, I think it is be better to rename variables separately as a refactor, to not miss places where rename is needed

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