Investigating build failure and framework issue - #222
Conversation
…2026.420.3) Replace ProjectReference to submodule with PackageReference to ppy.osu.Framework/Android/iOS v2026.420.3 from GitHub Packages. - Add winnerspiros GitHub Packages NuGet source to NuGet.Config - Add GitHub Packages authentication to all CI/release/deploy workflows - Remove git submodule, .gitmodules, local-packages/ - Remove submodule projects from osu.sln and all .slnf filters - Remove SuppressSubmoduleWarnings.targets and Veldrid.SPIRV override - Clean up submodule-specific items in Android/iOS props - Add packages:read permission to workflows Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/bcb0923f-4d2a-490f-926c-cc48ab950edd Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
Agent-Logs-Url: https://github.com/winnerspiros/osu/sessions/600b3a67-ce7f-4cae-92fa-cd404afe9987 Co-authored-by: winnerspiros <1675249+winnerspiros@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR migrates the repo from an osu-framework git submodule setup to consuming a forked framework via NuGet (GitHub Packages), and updates CI/release/deploy pipelines accordingly to restore from that feed.
Changes:
- Replaced
osu-frameworksubmodule project references withPackageReferencetoppy.osu.Framework*v2026.420.3. - Updated NuGet configuration to use the
winnerspirosGitHub Packages feed; removed local package overrides and submodule-related build plumbing. - Updated GitHub Actions workflows to remove submodule checkout and add GitHub Packages authentication + permissions.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
osu.sln |
Removes submodule projects/solution folders and related configuration entries. |
osu.Desktop.slnf |
Removes submodule projects from the desktop solution filter. |
osu.Android.slnf |
Removes submodule projects from the Android solution filter. |
osu.iOS.slnf |
Removes submodule projects from the iOS solution filter. |
osu.Game/osu.Game.csproj |
Switches from ProjectReference to ppy.osu.Framework NuGet dependency. |
osu.Android.props |
Switches to ppy.osu.Framework.Android NuGet dependency and drops submodule-native-lib wiring. |
osu.iOS.props |
Switches to ppy.osu.Framework.iOS NuGet dependency and removes manual props import for submodule builds. |
NuGet.Config |
Replaces local-packages source with winnerspiros GitHub Packages feed. |
Directory.Build.props |
Removes Veldrid.SPIRV override and associated NU1605 suppression. |
.gitmodules |
Removes osu-framework submodule definition. |
local-packages/.gitignore |
Removes local package override ignore rules. |
build/SuppressSubmoduleWarnings.targets |
Removes submodule-specific warning suppression target. |
.github/workflows/ci.yml |
Removes submodule checkout and adds GitHub Packages auth + packages: read. |
.github/workflows/release.yml |
Removes submodule checkout, adds GitHub Packages auth, and adjusts Android native-lib verification messaging. |
.github/workflows/deploy.yml |
Adds GitHub Packages auth + packages: read to support restore during packing. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| (the reference crosses assembly boundaries via a ProjectReference chain), causing | ||
| TypeLoadException: 'Could not resolve type Veldrid.SPIRV.CrossCompileTarget' at | ||
| startup when the renderer tries to compile shaders. --> | ||
| <TrimmerRootAssembly Include="ppy.Veldrid.SPIRV" RootMode="all" /> | ||
| </ItemGroup> |
There was a problem hiding this comment.
This comment mentions the trimming issue occurring via a “ProjectReference chain”, but the framework is now consumed via PackageReference below. Updating the wording to reflect the current dependency layout (or removing the ProjectReference-specific detail) will keep this explanation accurate for future maintainers.
| <!-- winnerspiros/osu-framework fork NuGet packages (published to GitHub Packages) --> | ||
| <add key="winnerspiros-github" value="https://nuget.pkg.github.com/winnerspiros/index.json" /> |
There was a problem hiding this comment.
Now that the framework submodule is removed and replaced with a GitHub Packages feed, README.md still contains submodule-based setup instructions (e.g., cloning with --recurse-submodules / “Framework as submodule”). Please update the docs to match the new NuGet-based setup to avoid confusing contributors.
| <!-- winnerspiros/veldrid-spirv fork NuGet package (included in local-packages/). | ||
| Source: https://github.com/winnerspiros/veldrid-spirv/releases --> | ||
| <add key="local-packages" value="local-packages" /> | ||
| <!-- winnerspiros/osu-framework fork NuGet packages (published to GitHub Packages) --> |
There was a problem hiding this comment.
NuGet.Config adds the winnerspiros GitHub Packages feed but doesn’t document how contributors should authenticate locally (restore will fail without credentials for this source). Consider adding a short note here (or in README/CONTRIBUTING) with the recommended dotnet nuget add/update source command and required token scopes so local builds are reproducible.
| <!-- winnerspiros/osu-framework fork NuGet packages (published to GitHub Packages) --> | |
| <!-- | |
| winnerspiros/osu-framework fork NuGet packages (published to GitHub Packages). | |
| Local restore requires authenticating this source first, for example: | |
| dotnet nuget update source winnerspiros-github --username YOUR_GITHUB_USERNAME --password YOUR_GITHUB_TOKEN --store-password-in-clear-text | |
| If the source has not been added yet, use: | |
| dotnet nuget add source https://nuget.pkg.github.com/winnerspiros/index.json --name winnerspiros-github --username YOUR_GITHUB_USERNAME --password YOUR_GITHUB_TOKEN --store-password-in-clear-text | |
| The GitHub token should include at least the `read:packages` scope; add `repo` as well if the package/repository is private. | |
| --> |
| - name: Authenticate to GitHub Packages | ||
| run: dotnet nuget update source winnerspiros-github --username ${{ github.actor }} --password ${{ secrets.GITHUB_TOKEN }} --store-password-in-clear-text | ||
|
|
There was a problem hiding this comment.
The workflow authenticates to GitHub Packages using dotnet nuget update source ... --store-password-in-clear-text, which writes the token into a NuGet.Config file in plain text. To reduce the risk of accidental token exposure (e.g., if the workspace/config is ever uploaded), consider writing credentials to a temporary config under $RUNNER_TEMP and passing --configfile to restore/build, then deleting it after.
| run: dotnet workload install android | ||
|
|
||
| - name: Authenticate to GitHub Packages | ||
| run: dotnet nuget update source winnerspiros-github --username ${{ github.actor }} --password ${{ secrets.GITHUB_TOKEN }} --store-password-in-clear-text |
There was a problem hiding this comment.
This step stores the GitHub Packages token in clear text via dotnet nuget update source ... --store-password-in-clear-text. Consider instead generating a temporary NuGet.Config (or updating a temp config via --configfile) for the duration of the job and deleting it afterwards to minimize the chance of credentials being persisted or leaked via artifacts/logging.
| run: dotnet nuget update source winnerspiros-github --username ${{ github.actor }} --password ${{ secrets.GITHUB_TOKEN }} --store-password-in-clear-text | |
| run: | | |
| NUGET_CONFIG_FILE="$RUNNER_TEMP/NuGet.Config" | |
| cat > "$NUGET_CONFIG_FILE" <<'EOF' | |
| <?xml version="1.0" encoding="utf-8"?> | |
| <configuration> | |
| </configuration> | |
| EOF | |
| dotnet nuget add source "https://nuget.pkg.github.com/winnerspiros/index.json" \ | |
| --name winnerspiros-github \ | |
| --username "${{ github.actor }}" \ | |
| --password "${{ secrets.GITHUB_TOKEN }}" \ | |
| --store-password-in-clear-text \ | |
| --configfile "$NUGET_CONFIG_FILE" | |
| echo "NUGET_CONFIG_FILE=$NUGET_CONFIG_FILE" >> "$GITHUB_ENV" |
| - name: Authenticate to GitHub Packages | ||
| run: dotnet nuget update source winnerspiros-github --username ${{ github.actor }} --password ${{ secrets.GITHUB_TOKEN }} --store-password-in-clear-text | ||
|
|
There was a problem hiding this comment.
This job authenticates to GitHub Packages by writing the token into NuGet.Config in clear text (--store-password-in-clear-text). Consider using a temporary config file (via --configfile) scoped to the job and removing it after restore/pack to avoid any possibility of credentials being captured in uploaded artifacts or reused unexpectedly.
| - name: Verify native libraries in APK | ||
| run: | | ||
| APK="${{ steps.find_apk.outputs.apk_path }}" | ||
| SUBMOD="submodules/osu-framework/osu.Framework.Android/arm64-v8a" | ||
| echo "Checking APK for required native libraries..." | ||
|
|
There was a problem hiding this comment.
The native library verification here only checks that the required .so files are present. Previously there was also an ABI/compatibility validation to ensure libbass.so was the Android build (not a desktop/Linux variant), which helps catch hard-to-debug startup crashes. Consider reintroducing an equivalent compatibility check (e.g., extracting and inspecting libbass.so with readelf).
packages: readpermissiondotnet nuget update source winnerspiros-github --password $GITHUB_TOKENstep to every build jobsubmodules: recursivefrom all workflow checkouts