-
Notifications
You must be signed in to change notification settings - Fork 18
Use netbird as submodule #52
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
base: main
Are you sure you want to change the base?
Conversation
Now it only receives an optional tag version. If it exists in the submodule's tag list, it will check out the submodule at the provided tag and build the NetBirdSDK over it
Change gomobile-netbird usage to forked gomobile in $GOPATH temporarily (bind dependencies aren't being resolved)
📝 WalkthroughWalkthroughAdds a NetBird git submodule, replaces the Go/iOS build script with robust version/tag and platform handling, and updates GitHub Actions workflow steps and paths to use the submodule and the new build flow. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant GH as GitHub Actions
participant Repo as Repository (checkout)
participant Script as build-go-lib.sh
participant Submod as libs/netbird
participant GoMobile as gomobile / gomobile-netbird
participant Output as NetBirdSDK.xcframework
GH->>Repo: checkout repository + submodules
GH->>GoMobile: install gomobile tooling
GH->>Script: run build-go-lib.sh (platform, version)
Script->>Submod: validate/checkout tag or determine commit hash
Script->>GoMobile: init selected gomobile tool
Script->>GoMobile: gomobile bind with computed version
GoMobile->>Output: produce NetBirdSDK.xcframework
Script->>Repo: move NetBirdSDK.xcframework to repository root
GH->>GH: post-build verification steps
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR transitions the ios-client project to use the netbird repository as a Git submodule, replacing the previous approach of expecting the netbird repository to exist in a separate location. The build script has been completely rewritten to support platform-specific builds (iOS, tvOS, or both) and includes improved version management with automatic tag validation and normalization.
Changes:
- Added netbird as a Git submodule under
libs/netbird - Replaced
build-go-lib.shwithbuild-ios-lib.shthat includes platform selection and enhanced version handling - Added automatic version detection from git tags with fallback to dev/ci prefixes
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| libs/netbird | Adds netbird submodule at commit 5333e55 |
| build-ios-lib.sh | New build script with platform selection, version normalization, and tag validation |
| build-go-lib.sh | Removes old build script |
| .gitmodules | Configures netbird submodule reference |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@build-ios-lib.sh`:
- Around line 138-143: Remove the misleading CGO_DISABLED env var from the
gomobile bind invocation: delete the prefixed "CGO_ENABLED=0" before
"$gomobile_cmd bind" so the script simply runs gomobile bind with the existing
flags (target, bundleid, ldflags, -o, and the
"$netbirdPath/client/ios/NetBirdSDK" argument); leave all other options
(targets, bundleid, -ldflags, -o path) unchanged so gomobile can use its
required cgo settings for Apple targets.
🧹 Nitpick comments (2)
build-ios-lib.sh (2)
42-59: Consider using--quietflag for cleaner build output.The
git checkoutcommand will output status messages that may clutter the build log. Consider adding--quietfor a cleaner experience, especially in CI environments.♻️ Proposed change
if git -C "$repo_path" rev-parse "refs/tags/$tag" >/dev/null 2>&1; then - git -C "$repo_path" checkout "$tag" + git -C "$repo_path" checkout --quiet "$tag" return 0 fi # Try with 'v' prefix if not provided if [[ ! "$tag" =~ ^v ]] && git -C "$repo_path" rev-parse "refs/tags/v$tag" >/dev/null 2>&1; then - git -C "$repo_path" checkout "v$tag" + git -C "$repo_path" checkout --quiet "v$tag" return 0 fi
118-132: Consider verifying gomobile-netbird availability.The script assumes
gomobile-netbirdis available in PATH for tvOS and both platform builds. Consider adding a check to provide a clear error message if the tool is missing.♻️ Proposed check before build
esac +# Verify gomobile command exists +if ! command -v "$gomobile_cmd" &> /dev/null; then + echo "Error: '$gomobile_cmd' not found in PATH" >&2 + exit 1 +fi + # Initialize gomobile $gomobile_cmd init
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@build-go-lib.sh`:
- Around line 96-108: The workflows and README still call the old script
signature and must be updated to the new one: change invocations that pass a
path (e.g. "./build-go-lib.sh ../netbird") to pass a platform argument ("both"
or "ios" or "tvos") per the new validation around the platform variable and the
case statement that only accepts ios|tvos|both; update
.github/workflows/build.yml and .github/workflows/test.yml to call
"./build-go-lib.sh both" (or a specific platform as appropriate) and update
README.md (around the previous usage example at line ~91) to show the new usage
"./build-go-lib.sh [platform] [version]".
🧹 Nitpick comments (1)
build-go-lib.sh (1)
118-132: Verifygomobile-netbirdavailability.The script assumes
gomobile-netbirdis available in PATH for tvOS/both builds. Consider adding a check or documenting the installation requirement.💡 Optional: Add tool availability check
+# Verify required tools are available +if ! command -v "$gomobile_cmd" &> /dev/null; then + echo "Error: '$gomobile_cmd' is not installed or not in PATH" >&2 + exit 1 +fi + # Initialize gomobile $gomobile_cmd init
| # Parse arguments | ||
| platform="${1:-both}" | ||
| version_arg="${2:-}" | ||
|
|
||
| # Validate platform argument | ||
| case "$platform" in | ||
| ios|tvos|both) | ||
| ;; | ||
| *) | ||
| echo "Error: Invalid platform '$platform'. Must be 'ios', 'tvos', or 'both'." >&2 | ||
| exit 1 | ||
| ;; | ||
| esac |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# Find all invocations of build-go-lib.sh
rg 'build-go-lib\.sh' -n --max-count=20Repository: netbirdio/ios-client
Length of output: 546
🏁 Script executed:
# Check GitHub Actions workflows
fd -type f \( -name '*.yml' -o -name '*.yaml' \) .github/workflows/ 2>/dev/null | xargs cat 2>/dev/null || echo "No workflows found"Repository: netbirdio/ios-client
Length of output: 46
🏁 Script executed:
# Get the full build-go-lib.sh file to see the complete interface
fd 'build-go-lib\.sh' -type fRepository: netbirdio/ios-client
Length of output: 234
🏁 Script executed:
# View the README around line 91 to see the old usage
sed -n '85,95p' README.mdRepository: netbirdio/ios-client
Length of output: 296
🏁 Script executed:
# Search for GitHub Actions workflows more carefully
find .github -name "*.yml" -o -name "*.yaml" 2>/dev/null | head -20Repository: netbirdio/ios-client
Length of output: 118
🏁 Script executed:
# Also search for any references to build-go-lib in yaml/yml files
rg 'build-go-lib' -A 5 -B 5Repository: netbirdio/ios-client
Length of output: 1722
🏁 Script executed:
# Check the actual workflow files to see how they invoke the script
cat .github/workflows/build.yml
cat .github/workflows/test.ymlRepository: netbirdio/ios-client
Length of output: 9789
Update script invocations and documentation for new API signature.
The new script signature ./build-go-lib.sh [platform] [version] is a breaking change from the old ./build-go-lib.sh <path> interface. Both GitHub Actions workflows (.github/workflows/build.yml and .github/workflows/test.yml) and the README.md still use the old invocation pattern ./build-go-lib.sh ../netbird, which will fail validation in the platform check (lines 100-106) since ../netbird does not match the valid platforms (ios, tvos, both).
Update:
- Both workflows to call:
./build-go-lib.sh both(or specific platform) - README.md line 91 to show the new usage pattern
🤖 Prompt for AI Agents
In `@build-go-lib.sh` around lines 96 - 108, The workflows and README still call
the old script signature and must be updated to the new one: change invocations
that pass a path (e.g. "./build-go-lib.sh ../netbird") to pass a platform
argument ("both" or "ios" or "tvos") per the new validation around the platform
variable and the case statement that only accepts ios|tvos|both; update
.github/workflows/build.yml and .github/workflows/test.yml to call
"./build-go-lib.sh both" (or a specific platform as appropriate) and update
README.md (around the previous usage example at line ~91) to show the new usage
"./build-go-lib.sh [platform] [version]".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/build.yml (1)
22-32:⚠️ Potential issue | 🟠 MajorSubmodule pin is bypassed by a separate checkout.
This step pulls the default branch of NetBird, ignoring the submodule commit tracked by the repo and making builds non‑reproducible. Ifbuild-go-lib.shchecks out tags, the shallow clone will also miss tags. Prefer checking out submodules with full history (or explicitly setrefto the submodule commit) and remove the extra checkout.🛠️ Suggested fix (use submodules and fetch tags/history)
- name: Checkout ios-client uses: actions/checkout@v4 with: path: ios-client + submodules: recursive + fetch-depth: 0 - - - name: Checkout netbird - uses: actions/checkout@v4 - with: - repository: netbirdio/netbird - path: ios-client/libs/netbird
🧹 Nitpick comments (1)
.github/workflows/build.yml (1)
39-47: Avoid@latestfor gomobile-tvos-fork / gomobile-netbird in CI.
Using@latestmakes builds non‑reproducible and can break unexpectedly. Consider pinning to a known tag/commit (and update deliberately).🔧 Example pinning (replace with the version you validate)
- - name: Install gomobile-netbird - run: GOPROXY=direct go install github.com/netbirdio/gomobile-tvos-fork/cmd/gomobile-netbird@latest + - name: Install gomobile-netbird + run: GOPROXY=direct go install github.com/netbirdio/gomobile-tvos-fork/cmd/gomobile-netbird@vX.Y.Z - name: Add gomobile-tvos-fork to netbird dependencies working-directory: ios-client/libs/netbird - run: go get github.com/netbirdio/gomobile-tvos-fork@latest + run: go get github.com/netbirdio/gomobile-tvos-fork@vX.Y.Z
This PR configures github.com/netbirdio/netbird as a submodule for the ios-client project.
Some changes were made in the build script to normalize and validate provided tag values for the submodule, and to also parametrize platform builds.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.