fix: license server toolset input + runAsHostUser HOME/USER (closes #739)#838
Open
frostebite wants to merge 1 commit into
Open
fix: license server toolset input + runAsHostUser HOME/USER (closes #739)#838frostebite wants to merge 1 commit into
frostebite wants to merge 1 commit into
Conversation
… runAsHostUser Addresses #739. Two opt-in, backwards-compatible changes for users on Linux Docker builds with a Unity floating-license server: 1. Floating license servers that host multiple toolsets had no way to tell the Licensing Client which toolset to request — Unity could fall through to an entitlement that lacks build-target support (e.g. Android), then silently produce a Linux Standalone artifact. The action now accepts an optional unityLicensingToolset input that is written into services-config.json. When unset, the rendered config is byte-for-byte identical to before. 2. With runAsHostUser: true, su was invoked without explicit HOME/USER, so the host user inherited root's environment (HOME=/root, USER unset). The Unity Licensing Client, which writes to ~/.config/unity3d, could not resolve a writable home directory, leading to intermittent license activation failures. Set HOME/USER/LOGNAME explicitly before sourcing build steps. Change lives entirely inside the existing runAsHostUser branch.
📝 WalkthroughWalkthroughThis PR adds a new ChangesUnity Licensing Toolset Feature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
3 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/model/platform-setup.ts (1)
36-40: ⚡ Quick winAdd contextual error handling around services config parsing.
Line 37 can throw a raw
JSON.parseerror with limited context. Wrapping this block to emit a targeted error (template path + invalid JSON/toolset context) will make failures much easier to diagnose in CI logs.Proposed patch
if (buildParameters.unityLicensingToolset) { - const parsed = JSON.parse(servicesConfig); - parsed.toolset = buildParameters.unityLicensingToolset; - servicesConfig = JSON.stringify(parsed, undefined, 2); + try { + const parsed = JSON.parse(servicesConfig); + parsed.toolset = buildParameters.unityLicensingToolset; + servicesConfig = JSON.stringify(parsed, undefined, 2); + } catch (error) { + core.error( + `Failed to render services config from template ${servicesConfigPathTemplate}: ${ + error instanceof Error ? error.message : String(error) + }`, + ); + throw error; + } }🤖 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 `@src/model/platform-setup.ts` around lines 36 - 40, The JSON.parse call that modifies servicesConfig can throw with no contextual info; wrap the block that reads buildParameters.unityLicensingToolset and calls JSON.parse in a try/catch inside the function or module where servicesConfig is handled (referencing buildParameters.unityLicensingToolset, servicesConfig, and parsed), and in the catch emit/throw a new error or processLogger.error that includes the template path/identifier and the offending unityLicensingToolset value (e.g., "Invalid servicesConfig JSON for template X with toolset Y: <original error message>") so CI logs show clear context for the failure.
🤖 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.
Nitpick comments:
In `@src/model/platform-setup.ts`:
- Around line 36-40: The JSON.parse call that modifies servicesConfig can throw
with no contextual info; wrap the block that reads
buildParameters.unityLicensingToolset and calls JSON.parse in a try/catch inside
the function or module where servicesConfig is handled (referencing
buildParameters.unityLicensingToolset, servicesConfig, and parsed), and in the
catch emit/throw a new error or processLogger.error that includes the template
path/identifier and the offending unityLicensingToolset value (e.g., "Invalid
servicesConfig JSON for template X with toolset Y: <original error message>") so
CI logs show clear context for the failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 39c3fb1f-9352-43e5-8034-ba0b603658a1
⛔ Files ignored due to path filters (4)
dist/index.jsis excluded by!**/dist/**dist/index.js.mapis excluded by!**/dist/**,!**/*.mapdist/licenses.txtis excluded by!**/dist/**dist/platforms/ubuntu/entrypoint.shis excluded by!**/dist/**
📒 Files selected for processing (5)
action.ymlsrc/model/build-parameters.test.tssrc/model/build-parameters.tssrc/model/input.tssrc/model/platform-setup.ts
This was referenced May 7, 2026
Merged
|
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #838 +/- ##
==========================================
- Coverage 45.34% 45.16% -0.19%
==========================================
Files 36 36
Lines 688 693 +5
Branches 199 201 +2
==========================================
+ Hits 312 313 +1
- Misses 337 341 +4
Partials 39 39
🚀 New features to boost your workflow:
|
frostebite
added a commit
to game-ci/orchestrator
that referenced
this pull request
May 7, 2026
Remove Unity-specific licensing fields from orchestrator's typed surfaces (BuildParameters, OrchestratorConfig, CLI input mapper, build/orchestrate yargs commands, cli-plugin adapter): - unitySerial - unityLicensingServer - skipActivation These were vestigial — BuildParameters.create() hardcoded them to empty strings, no orchestrator service read them for logic. They existed only to mirror unity-builder's BuildParameters shape, which is exactly the boundary violation: orchestrator's domain is dispatch + providers, not engine-specific licensing. The plugin contract (coreParams: Record<string, any>) is already opaque and engine-agnostic. The host (unity-builder today, @game-ci/cli in the future) passes its full BuildParameters object through; orchestrator reads only generic build context (targetPlatform, projectPath, etc.) and its plugin-owned config from env/inputs. Engine-specific keys ride in the dict untouched. No companion change needed in unity-builder: it continues to construct its own BuildParameters with whatever fields it wants and pass it as coreParams. The dict's index signature accepts everything. Documentation: - Tracking issue #25 lays out the full architecture, today/future state, and migration runway. - Code comments in plugin-lifecycle.ts, interfaces.ts, build-parameters.ts, build.ts, orchestrate.ts, input-mapper.ts, build-parameters-adapter.ts reference the issue and explain the boundary intent so the next contributor understands why these fields are not (and must not be) declared here. Out of scope (separate cleanups, noted in tracking issue): - cacheUnityInstallationOnMac / unityHubVersionOnMac in input-mapper (Mac runtime install caching, more entangled) - task-parameter-serializer.ts UNITY_SERIAL well-known-secret list (well-known-secrets generalization) - activate CLI command (Unity-specific legacy helper, leave as-is) Refs game-ci/unity-builder#739 and game-ci/unity-builder#838 (the user- facing fix that motivated this boundary cleanup).
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.
What this is
Two opt-in, backwards-compatible fixes for the issues users hit on Linux Docker builds with a Unity floating license server, surfaced in #739.
The original symptom in #739 was
-buildTarget Androidbeing silently ignored on Linux. Investigation in the issue thread revealed it's actually a licensing problem: when the Licensing Client can't resolve the right entitlement, Unity falls through to a license that lacks the requested platform support, and the editor produces a Linux Standalone artifact without raising an error. Two distinct root causes surfaced — addressed below.1.
unityLicensingToolsetinputFloating-license servers can host multiple toolsets. Without a
toolsetfield inservices-config.json, the Licensing Client relies on the server-side default toolset; if the server doesn't have one set (or the wrong one is set) it returns404 / Found 0 entitlement groups, and Unity falls through.This PR adds an optional
unityLicensingToolsetinput, threaded throughInput→BuildParameters→SetupShared, where it's injected into the renderedservices-config.jsononly when set. The Licensing Client then requests entitlements from the named toolset.2.
runAsHostUserlicense-client malfunctionWhen
runAsHostUser: true,entrypoint.shinvokedsu $USERNAME -c "..."without explicitHOME/USER. The host user inherited root's environment (HOME=/root,USERunset), so the Unity Licensing Client — which writes to~/.config/unity3d— could not resolve a writable home directory. This matches OP's observation in the issue thread that droppingrunAsHostUser: trueresolved their licensing failure even after they reverted the toolset workaround.Fix: set
HOME/USER/LOGNAMEexplicitly on the inner shell command, while preserving the rest of root's env viasu -p.Are both opt-in? Are both backwards compatible?
unityLicensingToolsetinputservices-config.jsonwhen unsetrunAsHostUserHOME/USER fixif RUN_AS_HOST_USER == "true"branchrunAsHostUser: true. Anyone whose pipeline silently relies on the brokenHOME=/rootwould see paths shift to/home/$USER. Almost certainly nobody does — but it is technically a behavior change for that opt-in branch.Users not using a license server and not using
runAsHostUsersee zero change.What I'm uncertain about
runAsHostUserfix is plausible but not confirmed. OP (sebas77) hedged on the cause ("almost definitively") and moved off game-ci before verifying. The/home/UNKNOWNlog line plus inspection ofentrypoint.shmakes this the strongest hypothesis — but it would be ideal if a reviewer with access to a license server validated before merge. Happy to split it into a follow-up PR if you'd prefer to land the toolset fix alone first.What I considered and dropped
services-config.json" input. Larger API surface, conflicts with existingunityLicensingServer, footgun in YAML. Single named input is narrower and safer; if more fields become useful, add them one at a time.com.unity.editor.headless not found) isn't always fatal — Unity probes entitlements during startup and may succeed on a later one — and log formats drift across Unity 5/2019/2022/6000/7000. False-firing would turn currently-green builds red. Worth its own investigation.Companion change
Mirrored across the orchestrator CLI surface: game-ci/orchestrator#TBD (linked once opened). The
entrypoint.shHOME/USER fix flows through automatically because orchestrator copies it fromunity-builder/dist/platforms/ubuntu/.Test plan
yarn typecheckcleanyarn lintclean (only pre-existing warnings)yarn test:ci— 347 passed, 2 skipped (no failures)yarn buildregeneratesdist/index.jscontaining the new fieldunityLicensingToolsetis honored (cannot do locally — need a reviewer with access)runAsHostUser: truethat licensing succeeds (cannot do locally — need a reviewer with access)Closes #739.
🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes
New Features
unityLicensingToolsetinput for configuring the named toolset identifier in Unity floating-license server scenarios.Documentation
StandaloneLinux64input description to explicitly note restoration of extensionless behavior from v4.