Restore missing build fixes and imports from commit 2d24aeb - #2
Conversation
Co-authored-by: mo74m3ed <71875716+mo74m3ed@users.noreply.github.com>
| import ContainerizationExtras | ||
| import Foundation | ||
| import Logging | ||
| import SystemPackage |
There was a problem hiding this comment.
Imports rely on transitive dependency
The new SystemPackage imports in both plugin targets rely on transitive dependencies instead of direct target declarations. Builds that enable SwiftPM explicit target dependency checking reject these imports, so declare the product directly for container-network-vmnet and container-runtime-linux.
Prompt To Fix With AI
This is a comment left during a code review.
Path: Sources/Plugins/NetworkVmnet/NetworkVmnetHelper+Start.swift
Line: 29
Comment:
**Imports rely on transitive dependency**
The new `SystemPackage` imports in both plugin targets rely on transitive dependencies instead of direct target declarations. Builds that enable SwiftPM explicit target dependency checking reject these imports, so declare the product directly for `container-network-vmnet` and `container-runtime-linux`.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.There was a problem hiding this comment.
🟢 Approval recommended
The changes are small, align with the stated build-fix intent, and should restore clean compilation under warnings-as-errors.
Pull request overview
Restores build-related fixes that were previously reverted, aiming to keep the Swift packages compiling cleanly under warnings-as-errors by reintroducing missing imports and silencing an unused Task result warning.
Changes:
- Re-adds
import SystemPackageto two plugin helper start commands. - Changes
Task { ... }to_ = Task { ... }inBuilder.build(_:)to suppress the unused-task-result warning.
File summaries
| File | Description |
|---|---|
| Sources/Plugins/RuntimeLinux/RuntimeLinuxHelper+Start.swift | Restores SystemPackage import needed for build-related types used by the helper. |
| Sources/Plugins/NetworkVmnet/NetworkVmnetHelper+Start.swift | Restores SystemPackage import needed for build-related types used by the helper. |
| Sources/ContainerBuild/Builder.swift | Silences unused Task result warning when starting the SIGWINCH listener task. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if let terminal = config.terminal { | ||
| Task { | ||
| _ = Task { |
|
@copilot Fix the code for all comments in this review thread. When a review comment includes a suggested change, apply the suggestion exactly. Do not make changes beyond what is described in the linked review thread. |
Important
All commits must be signed and verified. Pull requests containing unsigned or unverified commits cannot be built or merged. See the GitHub documentation for instructions.
For all but trivial fixes, make sure to first create a GitHub issue that concisely describes the bug or desired enhancement as justification for the change. Large PRs with no justifying issue will be closed.
Type of Change
Motivation and Context
Several critical build fixes and dependency imports introduced in commit
2d24aeb491fad6ceea7f2736a252eae73b427ab7were inadvertently reverted during a subsequent branch merge (25aee91). This PR restores those changes to keep compilation clean and prevent warnings-as-errors failures.Suppressed Discardable Task Warning
In
Sources/ContainerBuild/Builder.swift:Task {with_ = Task {to silence unused task result warnings under-warnings-as-errors.Restored Missing SystemPackage Imports
Sources/Plugins/NetworkVmnet/NetworkVmnetHelper+Start.swift: Restoredimport SystemPackage.Sources/Plugins/RuntimeLinux/RuntimeLinuxHelper+Start.swift: Restoredimport SystemPackage.Testing
Greptile Summary
This PR restores warning-clean Swift build behavior and imports needed by two plugin startup implementations.
SystemPackageimports for the NetworkVmnet and RuntimeLinux helpers.SystemPackagerather than direct package dependencies.Confidence Score: 4/5
The PR appears safe to merge under the current build configuration, though both new imports should be backed by direct target dependencies for strict SwiftPM builds.
The restored imports resolve through existing transitive dependencies today, but explicit target dependency checking rejects them because neither importing target declares SystemPackage directly.
Files Needing Attention: Sources/Plugins/NetworkVmnet/NetworkVmnetHelper+Start.swift, Sources/Plugins/RuntimeLinux/RuntimeLinuxHelper+Start.swift, Package.swift
Important Files Changed
Prompt To Fix All With AI
Reviews (1): Last reviewed commit: "Restore missing changes from commit 2d24..." | Re-trigger Greptile