Added a helper bash script to make building RedotSharp easier#1263
Conversation
WalkthroughAdds a LinuxBSD Mono build orchestration script that sets up a local NuGet source, builds the Redot editor with Mono support via scons, generates Mono glue sources, and packages C# assemblies into the local NuGet repository. ChangesLinuxBSD Mono build automation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mono_build_scripts_linux.sh (1)
21-21:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winTrailing blank line removed by pre-commit hook.
The pipeline failure indicates that a trailing blank line was automatically removed by the
file-formatpre-commit hook. Please commit the auto-fixed version.🤖 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 `@mono_build_scripts_linux.sh` at line 21, The file mono_build_scripts_linux.sh had a trailing blank line removed by the file-format pre-commit hook; update the repository by accepting the auto-fixed file and committing that change (e.g., run the pre-commit locally or re-stage mono_build_scripts_linux.sh and create a commit that includes the hook's removal of the trailing blank line), then push the commit so the pipeline no longer fails.
🧹 Nitpick comments (1)
mono_build_scripts_linux.sh (1)
1-19: ⚡ Quick winConsider adding error handling.
The script currently continues executing even if commands fail. Adding
set -eat the beginning would make the script exit immediately if any command fails, preventing confusing cascading errors (e.g., trying to run a binary that failed to build).♻️ Proposed addition
#!/bin/bash +set -e + SOURCE_NAME="MyLocalNugetSource"🤖 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 `@mono_build_scripts_linux.sh` around lines 1 - 19, The script lacks fail-fast behavior so later commands run even if earlier steps fail; add a shell safety setting at the top (e.g., enable "set -e" and optionally "set -o pipefail") so the script exits immediately on any command failure; update the top of the script (before use of SOURCE_NAME/SOURCE_PATH and before running commands like dotnet nuget add source, scons compiledb=yes ..., bin/redot.linuxbsd.editor.dev.x86_64.mono, and ./modules/mono/build_scripts/build_assemblies.py) to enable this behavior.
🤖 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.
Inline comments:
In `@mono_build_scripts_linux.sh`:
- Line 1: The script's shebang is incorrect ("`#/bin/bash`"); update it to the
correct shebang ("#!/bin/bash") at the top of the file so the OS can locate the
interpreter; modify the first line to start with "#!" followed by "/bin/bash" to
fix execution of the script.
- Line 19: The command invoking build_assemblies.py passes a literal
"~/MyLocalNugetSource/" (tilde inside quotes) so the shell won't expand the home
directory; update the invocation that calls
./modules/mono/build_scripts/build_assemblies.py --push-nupkgs-local to use an
expanded path (either $HOME/MyLocalNugetSource/ or the existing SOURCE_PATH
variable) without surrounding quotes that prevent tilde expansion, e.g. replace
the quoted "~/MyLocalNugetSource/" with $HOME/MyLocalNugetSource/ or
${SOURCE_PATH} so the push-nupkgs-local argument receives a valid absolute path.
- Line 16: The hardcoded binary name "bin/redot.linuxbsd.editor.dev.x86_64.mono"
will fail on non-x86_64 hosts; update the glue-generation invocation to locate
the built editor binary dynamically (e.g., glob for
"bin/redot.linuxbsd.editor.dev.*.mono" or construct the name using the host arch
via uname -m) and then call that discovered binary with --headless
--generate-mono-glue modules/mono/glue so the script works on ARM64 and other
architectures.
---
Outside diff comments:
In `@mono_build_scripts_linux.sh`:
- Line 21: The file mono_build_scripts_linux.sh had a trailing blank line
removed by the file-format pre-commit hook; update the repository by accepting
the auto-fixed file and committing that change (e.g., run the pre-commit locally
or re-stage mono_build_scripts_linux.sh and create a commit that includes the
hook's removal of the trailing blank line), then push the commit so the pipeline
no longer fails.
---
Nitpick comments:
In `@mono_build_scripts_linux.sh`:
- Around line 1-19: The script lacks fail-fast behavior so later commands run
even if earlier steps fail; add a shell safety setting at the top (e.g., enable
"set -e" and optionally "set -o pipefail") so the script exits immediately on
any command failure; update the top of the script (before use of
SOURCE_NAME/SOURCE_PATH and before running commands like dotnet nuget add
source, scons compiledb=yes ..., bin/redot.linuxbsd.editor.dev.x86_64.mono, and
./modules/mono/build_scripts/build_assemblies.py) to enable this behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 22e49829-a0c0-41b2-9bc7-3e04b64e6cd2
📒 Files selected for processing (1)
mono_build_scripts_linux.sh
5344deb to
16575b7
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (1)
mono_build_scripts_linux.sh (1)
19-19: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winUse the SOURCE_PATH variable for consistency.
The script defines
SOURCE_PATHat line 4 but hardcodes"$HOME/MyLocalNugetSource/"here instead. This creates a maintenance burden—if the path changes, both locations must be updated.♻️ Proposed fix to use the defined variable
-./modules/mono/build_scripts/build_assemblies.py --godot-output-dir=./bin --godot-platform=linuxbsd --push-nupkgs-local "$HOME/MyLocalNugetSource/" +./modules/mono/build_scripts/build_assemblies.py --godot-output-dir=./bin --godot-platform=linuxbsd --push-nupkgs-local "$SOURCE_PATH/"🤖 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 `@mono_build_scripts_linux.sh` at line 19, The invocation of build_assemblies.py hardcodes "$HOME/MyLocalNugetSource/" instead of using the defined SOURCE_PATH variable; update the command that calls ./modules/mono/build_scripts/build_assemblies.py to replace the hardcoded path with the SOURCE_PATH variable so the script consistently uses SOURCE_PATH throughout (locate the command line with --push-nupkgs-local and substitute the literal path with SOURCE_PATH).
🤖 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.
Inline comments:
In `@mono_build_scripts_linux.sh`:
- Line 16: The shell command starting with
"./bin/redot.linuxbsd.editor.dev.*.mono" incorrectly repeats the arguments
"--headless --generate-mono-glue modules/mono/glue" twice; edit that invocation
(the command token containing redot.linuxbsd.editor.dev.*.mono) to remove the
duplicated second set so each argument appears only once, leaving a single
"--headless --generate-mono-glue modules/mono/glue" sequence.
- Line 16: Validate the glob ./bin/redot.linuxbsd.editor.dev.*.mono before
running it: expand it into an array, check that the match count is exactly one,
and only then execute that single path with the --headless --generate-mono-glue
modules/mono/glue arguments; if zero or >1 matches, print a clear error and exit
non‑zero. Locate the line that runs the globbed command and replace it with
logic that performs the expansion, count check, and then executes the resolved
binary (preserving the original flags) to avoid "command not found" or
wrong-argument behavior.
- Around line 6-10: Enable error detection for the dotnet commands by setting
bash to fail on pipeline errors (e.g., set -o pipefail) and explicitly check the
exit status of the initial "dotnet nuget list source" invocation before piping
to grep; if that command fails, log an error including the command and exit
non‑zero. Likewise, check the exit status of "dotnet nuget add source
$SOURCE_PATH --name $SOURCE_NAME" and handle failures by printing a descriptive
error referencing SOURCE_PATH and SOURCE_NAME and exiting; ensure the script
does not continue silently when the dotnet binary is missing or returns an
error.
- Line 13: The scons invocation "scons compiledb=yes dev_build=yes
platform=linuxbsd target=editor module_mono_enabled=yes" is not checked for
failure; update mono_build_scripts_linux.sh to verify the build succeeded before
continuing by either adding "set -e" at the top or explicitly testing scons'
exit status (e.g., run the scons command and if it returns non‑zero, print an
error and exit non‑zero) so the script does not attempt to execute a missing
binary after a failed build.
---
Duplicate comments:
In `@mono_build_scripts_linux.sh`:
- Line 19: The invocation of build_assemblies.py hardcodes
"$HOME/MyLocalNugetSource/" instead of using the defined SOURCE_PATH variable;
update the command that calls ./modules/mono/build_scripts/build_assemblies.py
to replace the hardcoded path with the SOURCE_PATH variable so the script
consistently uses SOURCE_PATH throughout (locate the command line with
--push-nupkgs-local and substitute the literal path with SOURCE_PATH).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 6d34879f-3250-4785-b4d3-6accad676e41
📒 Files selected for processing (1)
mono_build_scripts_linux.sh
16575b7 to
5c98356
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (2)
mono_build_scripts_linux.sh (2)
24-24:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate glob resolution before executing editor binary (Line 24).
Using a glob directly as the command is unsafe: 0 matches executes a literal pattern, and multiple matches cause ambiguous execution.
💡 Safer invocation diff
# Generate glue sources -./bin/redot.linuxbsd.editor.dev.*.mono --headless --generate-mono-glue modules/mono/glue +shopt -s nullglob +editor_bins=(./bin/redot.linuxbsd.editor.dev.*.mono) +shopt -u nullglob + +if [ "${`#editor_bins`[@]}" -ne 1 ]; then + echo "Error: expected exactly one editor mono binary, found ${`#editor_bins`[@]}." + exit 1 +fi + +"${editor_bins[0]}" --headless --generate-mono-glue modules/mono/glue#!/bin/bash # Verify direct glob-as-command usage still exists. rg -n '\./bin/redot\.linuxbsd\.editor\.dev\.\*\.mono\s+--headless' mono_build_scripts_linux.sh🤖 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 `@mono_build_scripts_linux.sh` at line 24, The current direct glob usage './bin/redot.linuxbsd.editor.dev.*.mono --headless --generate-mono-glue modules/mono/glue' is unsafe; change it to explicitly expand and validate the glob before executing: perform a shell glob expansion into an array variable (e.g., matches=(./bin/redot.linuxbsd.editor.dev.*.mono)), check the count (${`#matches`[@]}) and error/exit if zero or >1, and then run the single resolved binary (matches[0]) with the same flags (--headless --generate-mono-glue modules/mono/glue). Ensure the script fails fast with a clear error message when the glob does not resolve to exactly one file.
11-15:⚠️ Potential issue | 🟠 Major | ⚡ Quick winHandle
dotnet nuget list sourcepipeline failures explicitly (Line 11).If
dotnet nuget list sourcefails, the pipeline can still fall into the “source missing” path. Capture the command result first, then parse output only on success.💡 Minimal hardening diff
+set -o pipefail + -if ! dotnet nuget list source | grep -q "$SOURCE_NAME"; then +if ! NUGET_SOURCES="$(dotnet nuget list source)"; then + echo "Error: failed to list NuGet sources." + exit 1 +fi + +if ! grep -q "$SOURCE_NAME" <<< "$NUGET_SOURCES"; then echo "Local nuget store not found, making one in $HOME..." mkdir -p "$SOURCE_PATH" dotnet nuget add source "$SOURCE_PATH" --name "$SOURCE_NAME" fi#!/bin/bash # Verify this script still has a piped dotnet+grep check and no pipefail setting. rg -n 'set -o pipefail|dotnet nuget list source\s*\|\s*grep -q' mono_build_scripts_linux.sh🤖 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 `@mono_build_scripts_linux.sh` around lines 11 - 15, The pipeline check using "dotnet nuget list source | grep -q \"$SOURCE_NAME\"" can mask failures from dotnet; change it to run dotnet nuget list source into a variable or capture its exit code (e.g., run the command first, check its exit status), only run grep on the output if dotnet succeeded, and treat dotnet failures as an error path (or enable "set -o pipefail" at script start) so the script does not incorrectly assume the source is missing; update the check around the dotnet nuget list source invocation and the conditional that references SOURCE_NAME/SOURCE_PATH accordingly.
🤖 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.
Duplicate comments:
In `@mono_build_scripts_linux.sh`:
- Line 24: The current direct glob usage './bin/redot.linuxbsd.editor.dev.*.mono
--headless --generate-mono-glue modules/mono/glue' is unsafe; change it to
explicitly expand and validate the glob before executing: perform a shell glob
expansion into an array variable (e.g.,
matches=(./bin/redot.linuxbsd.editor.dev.*.mono)), check the count
(${`#matches`[@]}) and error/exit if zero or >1, and then run the single resolved
binary (matches[0]) with the same flags (--headless --generate-mono-glue
modules/mono/glue). Ensure the script fails fast with a clear error message when
the glob does not resolve to exactly one file.
- Around line 11-15: The pipeline check using "dotnet nuget list source | grep
-q \"$SOURCE_NAME\"" can mask failures from dotnet; change it to run dotnet
nuget list source into a variable or capture its exit code (e.g., run the
command first, check its exit status), only run grep on the output if dotnet
succeeded, and treat dotnet failures as an error path (or enable "set -o
pipefail" at script start) so the script does not incorrectly assume the source
is missing; update the check around the dotnet nuget list source invocation and
the conditional that references SOURCE_NAME/SOURCE_PATH accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 62edfb8d-825d-4eb9-a835-2df5b951d025
📒 Files selected for processing (1)
mono_build_scripts_linux.sh
A lot of contributors are having issues building and debugging mono builds.
I am adding a bash script to help with this.
Note, this only supports Linux, but Windows and Mac users should be able to look at the script and infer what commands need to be ran for their platform.
If anyone wants to make dedicated windows or mac versions, I will be more than happy to approve and merge those too.
Summary by CodeRabbit