build: fix Docker and Dev Container Flutter 3.29 build error#2542
build: fix Docker and Dev Container Flutter 3.29 build error#2542
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request revamps the development environment configuration for Flutter and Android. A new Dockerfile is introduced while several outdated scripts and Dockerfiles are removed. The devcontainer configuration is updated to reference the new Dockerfile, modify post-attach and post-create commands, add bind mounts and port forwarding, and remove host constraints. Additionally, the Android SDK Dockerfile and build scripts have been updated to use newer version numbers, and the Komodo wallet Android Dockerfile now downloads the Flutter SDK directly. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Docker as Docker Engine
participant Container as Dev Container
Dev->>Docker: Initiate build using new Dockerfile
Docker->>Container: Execute sequential RUN commands for system setup
Container->>Container: Install dependencies (Android SDK, Flutter SDK, libraries) & configure user/environment
Dev->>Container: Run postCreateCommand to set permissions
Dev->>Container: Run postAttachCommand ("flutter pub get")
Container-->>Dev: Development environment ready
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Visit the preview URL for this PR (updated for commit fbdc78f): https://walletrc--pull-2542-merge-f7fdg6ze.web.app (expires Thu, 27 Feb 2025 16:00:50 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: f66a4ff03faa546f12f0ae5a841bd9eff2714dcc |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (5)
.docker/komodo-wallet-android.dockerfile (1)
13-18: Flutter SDK Installation Update – Consider Integrity Verification
The new RUN block downloads and extracts the Flutter SDK directly. While this simplifies the process by removing the cloning step, it may be beneficial to add a checksum verification after download for enhanced security and reliability..docker/build.sh (1)
31-33: Quote Command Substitution for User ID
The-u $(id -u):$(id -g)option in thedocker runcommand should be quoted to prevent potential word splitting. For example:- -u $(id -u):$(id -g) \ + -u "$(id -u):$(id -g)" \🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 31-31: Quote this to prevent word splitting.
(SC2046)
[warning] 31-31: Quote this to prevent word splitting.
(SC2046)
.devcontainer/Dockerfile (3)
1-9: Environment Variable Setup and PATH Concatenation
The Dockerfile sets key environment variables for Flutter and the Android SDK. The successive modifications to the PATH (adding$HOME/flutter/bin,/android-ndk/bin, and later the Android SDK tools) are cumulative but could be consolidated into a single ENV statement for improved readability.
73-99: Avoid Using Sudo in RUN Instructions
The RUN block that sets up the Android command-line tools usessudo(e.g.,sudo chown -R $USER:$USER /opt). According to best practices and Hadolint recommendations, consider replacingsudowith a tool likegosuto drop privileges more predictably. For instance:- && sudo chown -R $USER:$USER /opt \ + && gosu $USER chown -R $USER:$USER /opt \This change can help avoid potential issues related to sudo’s behavior in containerized environments.
🧰 Tools
🪛 Hadolint (2.12.0)
[error] 73-73: Do not use sudo as it leads to unpredictable behavior. Use a tool like gosu to enforce root
(DL3004)
108-117: Flutter SDK Download – Consider Checksum Verification
Similar to the approach in the Android image, the Flutter SDK download (viacurland extraction) might benefit from an additional checksum verification step to ensure the integrity and authenticity of the downloaded archive before extraction.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
.devcontainer/Dockerfile(1 hunks).devcontainer/dev-setup.sh(0 hunks).devcontainer/devcontainer.json(1 hunks).devcontainer/komodo-wallet-android-dev.dockerfile(0 hunks).docker/android-sdk.dockerfile(1 hunks).docker/build.sh(1 hunks).docker/kdf-android.dockerfile(0 hunks).docker/komodo-wallet-android.dockerfile(1 hunks).dockerignore(1 hunks)
💤 Files with no reviewable changes (3)
- .devcontainer/dev-setup.sh
- .docker/kdf-android.dockerfile
- .devcontainer/komodo-wallet-android-dev.dockerfile
✅ Files skipped from review due to trivial changes (1)
- .dockerignore
🧰 Additional context used
🪛 Shellcheck (0.10.0)
.docker/build.sh
[warning] 31-31: Quote this to prevent word splitting.
(SC2046)
[warning] 31-31: Quote this to prevent word splitting.
(SC2046)
🪛 Hadolint (2.12.0)
.devcontainer/Dockerfile
[error] 73-73: Do not use sudo as it leads to unpredictable behavior. Use a tool like gosu to enforce root
(DL3004)
🔇 Additional comments (11)
.docker/komodo-wallet-android.dockerfile (1)
1-6: Base Image and Environment Variables Update
The update to useFROM komodo/android-sdk:35 AS finaland the removal of theFLUTTER_HOMEvariable (with PATH now utilizing$HOME) is clear and consistent with the updated Docker setup..docker/build.sh (2)
19-23: Platform Detection on Darwin
The conditional usingif [ "$(uname)" = "Darwin" ]; thenis straightforward and correctly sets thePLATFORM_FLAGfor macOS environments.
25-25: Updated Docker Build Command for Android SDK
The Docker build command now targetskomodo/android-sdk:35, which aligns with the other configuration updates. This looks correct..devcontainer/devcontainer.json (5)
4-4: Updated Dockerfile Reference
Changing"dockerFile": "komodo-wallet-android-dev.dockerfile"to"Dockerfile"is clear and aligns with the updated Docker setup.
6-8: Mount Configuration Added
The new bind mount configuration ensures that the local workspace is correctly mapped to the container’s workspace. This change should help maintain file consistency during development.
10-11: Post-Create and Post-Attach Commands Update
Replacing the previous setup script with a directpostAttachCommand(flutter pub get) and adding apostCreateCommandto adjust ownership and permissions streamline the container setup.
12-20: Run Arguments and Port Forwarding Configuration
The added run arguments (with--privilegedand USB device mounting) and theforwardPortssettings (8081 and 5037) are well configured for the environment needs.
23-25: Extension Configuration Update
The customization to include theDart-Code.flutterextension (while keeping essential Dart extensions) is correct and supports Flutter development..docker/android-sdk.dockerfile (1)
28-33: Android SDK and NDK Version Bump
The updates to setANDROID_PLATFORM_VERSION=35,ANDROID_BUILD_TOOLS_VERSION=35.0.1, andANDROID_NDK_VERSION=27.2.12479018are consistent with the other version updates across the configuration files. Ensure these versions are thoroughly tested with your build pipeline..devcontainer/Dockerfile (2)
31-37: User Creation and Permission Configuration
The section that installssudo, creates thekomodouser, and adjusts ownership usingchownis standard. However, since you’re running subsequent commands as the non-root user, consider whether these sudo commands are strictly necessary.
38-68: Dependency Installation Block
The installation commands for additional packages and dependencies are comprehensive. The cleanup steps help keep the image lean. No issues were detected here.
takenagain
changed the title
takenagain
requested review from
AndrewDelaney,
CharlVS,
ShantanuSharma9873 and
smk762
2 participants
build_config.jsonto disable KDF downloads).git. This is to mitigate the 3.29 bug when installing flutter viagit clone, and because the official documentation now recommends using the SDK archive or IDE extensions to download flutter.NOTE: the Dev Container setup takes ownership of the project directory, so you might need to run
sudo chown -R $USER:$USER ./if you are switching between local files and dev containers.Summary by CodeRabbit