Remove some MuseScore-specific files from buildscripts folder#50
Open
cbjeukendrup wants to merge 4 commits into
Open
Remove some MuseScore-specific files from buildscripts folder#50cbjeukendrup wants to merge 4 commits into
cbjeukendrup wants to merge 4 commits into
Conversation
📝 WalkthroughWalkthroughThis PR removes support for 32-bit ARMv7L architecture and consolidates CI infrastructure. Architecture configuration is updated across build scripts by replacing armv7l-specific conditionals with unconditional blocks, updating architecture documentation to remove armv7l references, and changing build-tool installation from architecture-based checks to tool-presence checks. AppImage build logic removes armv7l-to-armhf mapping and associated 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
buildscripts/ci/linux/setup.sh (1)
51-56: 🧹 Nitpick | 🔵 Trivial | 💤 Low valueConsider removing the dead code block or adding a clearer comment.
The
if falseconstruct creates unreachable code. If this is intentionally preserved as a template for future Docker container usage, consider adding a comment explaining when/how to enable it (e.g.,# Uncomment for Docker: if [ -f /.dockerenv ]; then). Otherwise, this block could be removed entirely.♻️ Suggested alternatives
Option 1 - Remove the dead code:
-if false; then # Use this when running in a Docker container - SUDO="" - export DEBIAN_FRONTEND="noninteractive" TZ="Europe/London" -else - SUDO="sudo" -fi +SUDO="sudo"Option 2 - Use a more explicit Docker detection:
-if false; then # Use this when running in a Docker container +if [ -f /.dockerenv ]; then # Running inside a Docker container SUDO="" export DEBIAN_FRONTEND="noninteractive" TZ="Europe/London" else SUDO="sudo" fi🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@buildscripts/ci/linux/setup.sh` around lines 51 - 56, The current unreachable "if false" block that sets SUDO, DEBIAN_FRONTEND and TZ should be removed or converted into an explicit Docker-detection branch; locate the block that references SUDO, export DEBIAN_FRONTEND="noninteractive" and TZ="Europe/London" and either delete it if not needed, or replace the `if false` with a real condition (e.g., check for /.dockerenv or an explicit env flag) and add a short comment explaining when to enable that branch (e.g., "Enable for containerized runs"). Ensure the variable SUDO is still set in the fallback branch and keep the intent of setting DEBIAN_FRONTEND/TZ when running in container mode, with a clear comment explaining how to toggle it.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@buildscripts/ci/linux/setup.sh`:
- Around line 51-56: The current unreachable "if false" block that sets SUDO,
DEBIAN_FRONTEND and TZ should be removed or converted into an explicit
Docker-detection branch; locate the block that references SUDO, export
DEBIAN_FRONTEND="noninteractive" and TZ="Europe/London" and either delete it if
not needed, or replace the `if false` with a real condition (e.g., check for
/.dockerenv or an explicit env flag) and add a short comment explaining when to
enable that branch (e.g., "Enable for containerized runs"). Ensure the variable
SUDO is still set in the fallback branch and keep the intent of setting
DEBIAN_FRONTEND/TZ when running in container mode, with a clear comment
explaining how to toggle it.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: ce3865cf-533b-4668-834d-808004d2f563
⛔ Files ignored due to path filters (1)
buildscripts/ci/crashdumps/linux/armv7l/dump_syms.7zis excluded by!**/*.7z
📒 Files selected for processing (37)
buildscripts/ci/backend/build.shbuildscripts/ci/backend/build_docker.shbuildscripts/ci/backend/check_update_config_on_s3.shbuildscripts/ci/backend/convertor.inbuildscripts/ci/backend/docker/Dockerfilebuildscripts/ci/backend/docker/install_mu_template.shbuildscripts/ci/backend/docker/mu2/Dockerfilebuildscripts/ci/backend/docker/mu2/build_docker.shbuildscripts/ci/backend/docker/mu2/install_mu_template.shbuildscripts/ci/backend/docker/mu2/publish_to_registry.shbuildscripts/ci/backend/docker/mu2/setup.shbuildscripts/ci/backend/docker/mu3/Dockerfilebuildscripts/ci/backend/docker/mu3/build_docker.shbuildscripts/ci/backend/docker/mu3/install_mu_template.shbuildscripts/ci/backend/docker/mu3/publish_to_registry.shbuildscripts/ci/backend/docker/mu3/setup.shbuildscripts/ci/backend/docker/setup.shbuildscripts/ci/backend/package.shbuildscripts/ci/backend/publish_to_registry.shbuildscripts/ci/backend/publish_to_s3.shbuildscripts/ci/backend/setup.shbuildscripts/ci/crashdumps/ci_files.cmakebuildscripts/ci/linux/arm32/install_qt.shbuildscripts/ci/linux/arm32/run_lupdate.shbuildscripts/ci/linux/build.shbuildscripts/ci/linux/setup.shbuildscripts/ci/linux/tools/make_appimage.shbuildscripts/ci/release/correct_release_info.pybuildscripts/ci/tools/osuosl/index.htmlbuildscripts/ci/tools/osuosl/osuosl_nighlies_rsa.encbuildscripts/ci/tools/osuosl/publish.shbuildscripts/ci/vtests/build_and_pack.shbuildscripts/ci/vtests/generate_pngs.shbuildscripts/ci/vtests/run.inbuildscripts/cmake/FindFFmpeg.cmakebuildscripts/cmake/SetupSndFile.cmakeframework/diagnostics/CMakeLists.txt
💤 Files with no reviewable changes (31)
- buildscripts/ci/linux/arm32/run_lupdate.sh
- buildscripts/ci/vtests/generate_pngs.sh
- buildscripts/ci/backend/docker/mu3/install_mu_template.sh
- buildscripts/ci/tools/osuosl/index.html
- buildscripts/ci/vtests/run.in
- buildscripts/ci/backend/convertor.in
- buildscripts/ci/backend/build_docker.sh
- buildscripts/ci/linux/arm32/install_qt.sh
- buildscripts/ci/backend/docker/mu2/build_docker.sh
- buildscripts/ci/backend/docker/install_mu_template.sh
- buildscripts/ci/crashdumps/ci_files.cmake
- buildscripts/ci/backend/docker/mu3/setup.sh
- buildscripts/ci/backend/docker/mu3/build_docker.sh
- buildscripts/ci/backend/check_update_config_on_s3.sh
- buildscripts/ci/backend/publish_to_registry.sh
- buildscripts/ci/backend/docker/mu3/publish_to_registry.sh
- buildscripts/ci/backend/docker/mu2/setup.sh
- buildscripts/ci/backend/docker/mu3/Dockerfile
- buildscripts/cmake/SetupSndFile.cmake
- buildscripts/ci/vtests/build_and_pack.sh
- buildscripts/ci/backend/docker/setup.sh
- buildscripts/ci/backend/docker/mu2/install_mu_template.sh
- buildscripts/ci/backend/package.sh
- buildscripts/ci/backend/build.sh
- buildscripts/ci/backend/docker/Dockerfile
- buildscripts/ci/backend/docker/mu2/Dockerfile
- buildscripts/ci/backend/docker/mu2/publish_to_registry.sh
- buildscripts/ci/tools/osuosl/publish.sh
- buildscripts/ci/backend/setup.sh
- buildscripts/ci/backend/publish_to_s3.sh
- buildscripts/cmake/FindFFmpeg.cmake
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There’s a lot more that should be either removed or generalised, but let’s decide on a policy for that.