Skip to content
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

Update package building scripts and images to use Conan #4435

Merged
merged 2 commits into from
Feb 28, 2023

Conversation

legleux
Copy link
Collaborator

@legleux legleux commented Feb 24, 2023

High Level Overview of Change

Lots of changes needed to update the docker containers we use to build DEB and RPM packages.

Context of Change

rippled's dependencies are handled by Conan now so the package building process needs to be updated.

bump version to rc3
test on Fedora 37
rm docker user mapping on container builds (doesn't work with conan)
rm dpkg dev pkg (was broken anyway)
rm dependency builds (dependencies are handled by Conan now)

Supersedes #4362

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)
  • Tests (You added tests for code that already exists, or your new feature included in this PR)
  • Documentation Updates
  • Release

bump version to rc3 
test on Fedora 37
rm docker user mapping  on container builds (doesn't work with conan)
rm dpkg dev pkg (was broken anyway)
rm dependency builds (handled by Conan now)
@legleux
Copy link
Collaborator Author

legleux commented Feb 24, 2023

@thejohnfreeman I'm still unsure about the removal from RippledRelease.cmake of

# Early return if the `containers` directory is missing,
# e.g. when we are building a Conan package.
if(NOT EXISTS containers)
  return()
endif()

Builds/containers/ubuntu-builder/ubuntu_setup.sh Outdated Show resolved Hide resolved
Builds/containers/shared/install_cmake.sh Outdated Show resolved Hide resolved
Builds/containers/shared/install_cmake.sh Show resolved Hide resolved
RUN if [ "${CI_USE}" = true ] ; then /tmp/install_cmake.sh 3.9.0 /opt/local/cmake-3.9; fi

RUN /tmp/build_deps.sh
ENV PLANTUML_JAR="/opt/plantuml/plantuml.jar"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is PlantUML gone? We need it to build the Doxygen documentation.

Builds/containers/packaging/rpm/rippled.spec Show resolved Hide resolved
Builds/containers/packaging/dpkg/debian/rules Outdated Show resolved Hide resolved
Builds/containers/packaging/dpkg/debian/rules Outdated Show resolved Hide resolved
Comment on lines +13 to +17
RUN /tmp/install_cmake.sh 3.16.3 /opt/local/cmake-3.16
RUN ln -s /opt/local/cmake-3.16 /opt/local/cmake
ENV PATH="/opt/local/cmake/bin:$PATH"
# also install min supported cmake for testing
RUN if [ "${CI_USE}" = true ] ; then /tmp/install_cmake.sh 3.9.0 /opt/local/cmake-3.9; fi
# TODO: Install latest CMake for testing
RUN if [ "${CI_USE}" = true ] ; then /tmp/install_cmake.sh 3.16.3 /opt/local/cmake-3.16; fi
Copy link
Collaborator

Choose a reason for hiding this comment

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

See my comments on the Ubuntu Dockerfile for my thoughts on moving all these RUN commands to the install script and greatly simplifying / trivializing the Dockerfile.

Builds/CMake/RippledRelease.cmake Show resolved Hide resolved
@intelliot
Copy link
Collaborator

I guess this might be a flaky test. I kicked it off again.

1> ripple.app.LedgerData
0> #299 failed: apply: Got temINVALID (The transaction is ill-formed.); Expected tesSUCCESS (The transaction was applied. Only final in a validated ledger.)
0> {
   "Account" : "rHb9CJAWyB4rj91VRWn96DkukG4bwdtyTh",
   "Amount" : "1000000010",
   "Destination" : "rJP5kd48NUvKuEMUUNdByiQCauJVYURBz8",
   "Fee" : "10",
   "Flags" : 2147483648,
   "Sequence" : 51,
   "SigningPubKey" : "0330E7FC9D56BB25D6893BA3F317AE5BCF33B3291BD63DB32654A313222F7FD020",
   "TransactionType" : "Payment",
   "TxnSignature" : "304502210092D9DA00AAE0404640A8697028B3E36BA45FCA7[500](https:/XRPLF/rippled/actions/runs/4266933705/jobs/7428013711#step:19:501)D475FAC8F6B97B01B1D80902206B51127825AD3AE0ED3F7CE16CC665533571BAF49B852B4B1E69DBD552BFA4E0"
}

0> #300 failed: unhandled exception: unexpected missing account root
0> ripple.app.LedgerReplayer ProofPath

Copy link
Collaborator

@sgramkumar sgramkumar left a comment

Choose a reason for hiding this comment

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

LGTM. Just a minor indentation missing

--slave /usr/bin/gcov-tool gcov-tool /usr/bin/gcov-dump-7 \
--slave /usr/bin/gcov-dump gcov-dump /usr/bin/gcov-tool-7
if [[ ${VERSION_ID} =~ ^20\. ]] ; then
apt-get install -y \
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: indentation missing

if [ "${CI_USE}" = true ] ; then
apt-get -y update
if [[ ${VERSION_ID} =~ ^20\. ]] ; then
for v in 12 14; do
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: indentation missing

Copy link
Contributor

@HowardHinnant HowardHinnant left a comment

Choose a reason for hiding this comment

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

I started with a empty ~/.conan/data, and it installed, generated, built and tested without errors for me on macOS, using the directions in BUILD.md.

@intelliot
Copy link
Collaborator

@legleux to confirm this is ready to merge, you can put the Passed label on it (or comment saying it's ready)

@legleux legleux added the Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. label Feb 28, 2023
@legleux
Copy link
Collaborator Author

legleux commented Feb 28, 2023

How about that, I can add labels?
Let's merge it.

@intelliot intelliot merged commit e13676f into XRPLF:develop Feb 28, 2023
ximinez added a commit to ximinez/rippled that referenced this pull request Mar 2, 2023
* upstream/develop:
  Update package building scripts and images to use Conan (XRPLF#4435)
  Disable duplicate detector: (XRPLF#4438)
ximinez added a commit to ximinez/rippled that referenced this pull request Mar 3, 2023
* upstream/develop:
  Set version to 1.10.0-rc4
  Rename 'NFT' to 'NFToken' in DisallowIncoming flags (XRPLF#4442)
  Update Docker.md (XRPLF#4432)
  Update package building scripts and images to use Conan (XRPLF#4435)
  Disable duplicate detector: (XRPLF#4438)
ximinez added a commit to ximinez/rippled that referenced this pull request Mar 3, 2023
* upstream/develop:
  Set version to 1.10.0-rc4
  Rename 'NFT' to 'NFToken' in DisallowIncoming flags (XRPLF#4442)
  Update Docker.md (XRPLF#4432)
  Update package building scripts and images to use Conan (XRPLF#4435)
  Disable duplicate detector: (XRPLF#4438)
ximinez added a commit to ximinez/rippled that referenced this pull request Mar 3, 2023
* upstream/develop:
  Set version to 1.10.0-rc4
  Rename 'NFT' to 'NFToken' in DisallowIncoming flags (XRPLF#4442)
  Update Docker.md (XRPLF#4432)
  Update package building scripts and images to use Conan (XRPLF#4435)
  Disable duplicate detector: (XRPLF#4438)
ximinez added a commit to ximinez/rippled that referenced this pull request Mar 3, 2023
* upstream/develop:
  Set version to 1.10.0-rc4
  Rename 'NFT' to 'NFToken' in DisallowIncoming flags (XRPLF#4442)
  Update Docker.md (XRPLF#4432)
  Update package building scripts and images to use Conan (XRPLF#4435)
  Disable duplicate detector: (XRPLF#4438)
ximinez added a commit to ximinez/rippled that referenced this pull request Mar 3, 2023
…ctionality

* upstream/develop:
  Set version to 1.10.0-rc4
  Rename 'NFT' to 'NFToken' in DisallowIncoming flags (XRPLF#4442)
  Update Docker.md (XRPLF#4432)
  Update package building scripts and images to use Conan (XRPLF#4435)
  Disable duplicate detector: (XRPLF#4438)
ximinez added a commit to ximinez/rippled that referenced this pull request Mar 3, 2023
…tpage

* upstream/develop:
  Set version to 1.10.0-rc4
  Rename 'NFT' to 'NFToken' in DisallowIncoming flags (XRPLF#4442)
  Update Docker.md (XRPLF#4432)
  Update package building scripts and images to use Conan (XRPLF#4435)
  Disable duplicate detector: (XRPLF#4438)
dangell7 pushed a commit to Transia-RnD/rippled that referenced this pull request Mar 5, 2023
* Set version to 1.10.0-rc3
* Test on Fedora 37
* Dependency builds are handled by Conan
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

6 participants