Repository navigation
Add .gitlab-ci.yml file - #1145
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds a comprehensive GitLab CI configuration (.gitlab-ci.yml) that implements multi-stage, multi-platform build, signing/notarization, packaging, and release flows with many per-platform templates, artifact assembly, and conditional release gates (e.g., DO_RELEASE, SIGN_MAC). Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (push/merge)
participant GitLab as GitLab CI
participant Runner as Runners (Linux/Windows/macOS/Android/iOS/Web)
participant Notary as Notary / Signing Service
participant Store as Artifact Storage / Release
participant NuGet as NuGet Registry
Dev->>GitLab: push triggers pipeline
GitLab->>Runner: schedule platform build jobs (per-template, per-arch)
Runner->>Runner: compile/package artifacts (zips, aars, apks, xcframeworks, nupkg)
alt macOS signing enabled (SIGN_MAC & DO_RELEASE)
Runner->>Notary: submit for notarization / codesign
Notary-->>Runner: notarization result / stapled artifact
end
Runner->>Store: upload artifacts and metadata (sha256, version)
alt DO_RELEASE true
GitLab->>Store: create consolidated release tarball
GitLab->>NuGet: push .nupkg (if mono and API key present)
end
Store-->>Dev: release artifacts available
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
.gitlab-ci.yml (7)
350-377: Consider extracting version parsing logic to reduce duplication.The version string construction logic (lines 350-377) is duplicated almost identically in the
package-buildsjob (lines 1229-1254). Consider extracting this into a reusable script or storing the version strings as job artifacts that downstream jobs can consume.🔎 Suggested approach
Create a shared script or use GitLab CI's
dotenvartifact reports to pass version variables between jobs:create-tarball: # ... existing config ... script: - # ... version parsing ... - echo "VERSIONSTRING=${VERSIONSTRING}" >> version.env - echo "VERSIONTXT=${VERSIONTXT}" >> version.env artifacts: reports: dotenv: version.envThen downstream jobs can use
needs: [create-tarball]and access$VERSIONSTRINGdirectly.
401-423: Unnecessary architecture case statement.This job always uses
ARCH: "x86_64"(line 437), so the full architecture case statement is unnecessary. The job could be simplified by directly setting the x86_64 toolchain:🔎 Simplified before_script
before_script: - export SCONS="scons -j$(nproc) verbose=no warnings=no progress=no" - export BUILD_NAME="official" - - | - case "$ARCH" in - x86_32) - # ... other cases ... - x86_64) - export PATH="$GODOT_SDK_LINUX_X86_64/bin:$PATH" - export CC="x86_64-godot-linux-gnu-gcc" - export CXX="x86_64-godot-linux-gnu-g++" - ;; - # ... other cases ... - esac + - export PATH="$GODOT_SDK_LINUX_X86_64/bin:$PATH" + - export CC="x86_64-godot-linux-gnu-gcc" + - export CXX="x86_64-godot-linux-gnu-g++"
1221-1227: Missingneedsdeclaration for artifact dependencies.The
package-buildsjob relies on artifacts from numerous build stage jobs (tarball, editors, templates, signing jobs) but doesn't declare them vianeeds. While thepackagestage runs afterbuild, without explicitneeds, artifact availability isn't guaranteed if jobs fail or artifacts expire.🔎 Add explicit needs
package-builds: stage: package needs: - create-tarball - linux-editor-x64 - linux-editor-arm64 - linux-editor-x64-mono - linux-editor-mono-arm64 - windows-editor-x64 - windows-editor-x64-mono - sign-mac-editor - sign-mac-editor-mono - sign-mac-templates - Web-Release-Template - Android-Release-Template - Android-Release-Template-Mono - package-ios # ... plus all template build jobs
972-1170: Significant code duplication in macOS signing jobs.The
sign-mac-editorandsign-mac-editor-monojobs contain nearly identicalsign_macos()shell functions (~80 lines each). Consider consolidating into a single parameterized job using GitLab CI'sparallel:matrixor extracting the signing logic into a shared script.🔎 Consolidation approach
sign-mac-editor: stage: build needs: [Merge-Mac-Editor] rules: - if: $DO_RELEASE == "true" - if: $SIGN_MAC == "true" tags: [macos] parallel: matrix: - VARIANT: "standard" RELDIR: "out/macos/universal/tools" BINNAME: "redot.macos.editor.universal" IS_MONO: "0" - VARIANT: "mono" RELDIR: "out/macos/universal/tools-mono" BINNAME: "redot.macos.editor.universal.mono" IS_MONO: "1" script: - # shared sign_macos function called with matrix variablesOr extract
sign_macos()intomisc/ci/sign_macos.shand source it in both jobs.
1313-1321: Missingneedsdeclaration for NuGet job.The
pushNugetjob requires artifacts fromlinux-editor-x64-monobut doesn't declare this dependency vianeeds. Add explicit dependency to ensure artifacts are available.🔎 Proposed fix
pushNuget: stage: package image: $CI_REGISTRY/redot-engine/redot-production-containers/redot-linux:latest tags: [docker] + needs: [linux-editor-x64-mono] rules: - if: $DO_RELEASE == "true"
725-728: Consider narrowing artifact paths to reduce storage.The wildcard
out/macos/*captures all architecture subdirectories (x86_64, arm64, universal). After merging, only theuniversaldirectory is needed by downstream jobs. Consider:paths: - out/macos/universal/
1284-1285: Redundantcatin pipeline.
echo "${VERSIONTXT}" | cat >> filecan be simplified toecho "${VERSIONTXT}" >> file. Thecatadds no value here. Same applies to line 1292.🔎 Simplified version
- - echo "${VERSIONTXT}" | cat >> ./templates/version.txt + - echo "${VERSIONTXT}" >> ./templates/version.txt
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.gitlab-ci.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (15)
- GitHub Check: 🐧 Linux / Template w/ Mono, release (target=template_release)
- GitHub Check: 🏁 Windows / Editor (target=editor)
- GitHub Check: 🤖 Android / Template arm32 (target=template_release, arch=arm32)
- GitHub Check: 🐧 Linux / Editor w/ Mono (target=editor)
- GitHub Check: 🏁 Windows / Template w/ GCC (target=template_release, use_mingw=yes)
- GitHub Check: 🤖 Android / Template arm64 (target=template_release, arch=arm64)
- GitHub Check: 🐧 Linux / Editor with ThreadSanitizer (target=editor, dev_build=yes, use_tsan=yes, use_llvm=yes, linker=lld)
- GitHub Check: 🏁 Windows / Template (target=template_release)
- GitHub Check: 🐧 Linux / Template w/ Mono, debug (target=template_debug)
- GitHub Check: 🐧 Linux / Minimal template (target=template_release, everything disabled)
- GitHub Check: 🌐 Web / Template w/ threads (target=template_release, threads=yes)
- GitHub Check: 🍏 iOS / Template (target=template_release)
- GitHub Check: 🍎 macOS / Template (target=template_release)
- GitHub Check: 🌐 Web / Template w/o threads (target=template_release, threads=no)
- GitHub Check: 🍎 macOS / Editor (target=editor)
🔇 Additional comments (5)
.gitlab-ci.yml (5)
1-42: Base templates look well-structured.Minor observation: there are inconsistent double spaces in
mkdir -pcommands (lines 14, 28, 40). Not a functional issue, but could be cleaned up for consistency.The template hierarchy with
.template-build,.build-release-template, and.build-release-template-monoprovides good reusability.
325-341: LGTM!The mono build template correctly depends on
linux-editor-x64-monofor glue generation and properly copies the generated files before building.
759-766: Verify web template output file naming.The
mvcommands referencegodot.web.*filenames. Verify that/root/scripts/buildWeb.shoutputs files with thegodotprefix (notredot). If the script has been updated to useredotprefix, these commands will fail.
1017-1021: Security note: Keychain handling looks appropriate.The keychain operations use a temporary keychain (
temp.keychain) and properly reference secrets via CI variables. Ensure these variables are configured as protected and masked in GitLab CI/CD settings.
153-160: Remove dead ARM64 handling code.All Windows jobs use
ARCH: "x86_64". The ARM64 conditional branches (lines 154-158 and 182-186) will never execute and should be removed. Keep only theelsebranch that handles x86_64.
6644e35 to
a265ed4
Compare
a265ed4 to
06e1147
Compare
Transfer over the build pipeline file from the 4.4 branch, and remove jobs for 32 bit builds and windows arm64.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.