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

ci: container improvements #6490

Merged
merged 8 commits into from
Dec 17, 2024
Merged

ci: container improvements #6490

merged 8 commits into from
Dec 17, 2024

Conversation

kwvg
Copy link
Collaborator

@kwvg kwvg commented Dec 15, 2024

Additional Information

  • This pull request pulls container-specific changes from dash#6387, dash#6400 and dash#6421

  • The HOST check before running setup_sdk.sh isn't a part of the script itself as the script is written to be independent of external variables set. The caller is expected to know the conditions needed to run setup_sdk.sh as the script is relatively agnostic to its environment.

  • The version attribute in the develop and guix container's docker-compose.yml has been dropped as the attribute has been deprecated in the compose spec (source).

  • Using LD_LIBRARY_PATH to point to LLVM's libraries are acceptable and will not interfere with executing binaries built using the distro's packaged compiler as it will eventually search default paths and find the libraries shipped with the distro (source).

  • Currently, running LLDB will result in a "personality set failed: Operation not permitted" error (source). This is caused by its attempt at disabling ASLR for debugging.

    To work around this error, the container will now operate under relaxed restrictions (seccomp=unconfined). As disabling ASLR is valuable when debugging and the container is meant for developers (i.e. it isn't used for CI), we have opted to relax restrictions instead of skipping ASLR disablement.

  • As of develop (a8e2316), packages built by the container are stored in /tmp, which is inadvisable as it is the same directory used to store functional test runs and it's not too difficult to delete /tmp's contents to save space in a long running develop container and then realize that both shellcheck and cppcheck are stored there and now you have to ditch the container you're working in and restart it.

    • To remedy this, packages are now built and stored in /opt in accordance with the FHS (source). /usr/local was a contender but it's pre-populated, meanwhile ls /opt would give you a very quick picture of what's built for the container.

    • /tmp will not be entirely empty because pypa/pip#10753 results in residual .pem files leaking into /tmp and pyenv stores its build log there and keeping it around has some debug value.

Breaking Changes

None expected.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests (note: N/A)
  • I have made corresponding changes to the documentation (note: N/A)
  • I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

@kwvg kwvg added this to the 22.1 milestone Dec 15, 2024
@kwvg kwvg marked this pull request as ready for review December 15, 2024 13:06
contrib/containers/guix/scripts/guix-start Outdated Show resolved Hide resolved
ci/dash/setup-sdk.sh Outdated Show resolved Hide resolved
Co-authored-by: UdjinM6 <UdjinM6@users.noreply.github.com>
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

Tested local macos build via docker-compose, Guix build on ubuntu via direct guix-start call and GH Guix CI. All looks good 👍

ACK 04ce1fe

Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK 04ce1fe

@PastaPastaPasta PastaPastaPasta merged commit 7530f3d into dashpay:develop Dec 17, 2024
24 of 25 checks passed
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.

3 participants