FIX: Add custom library build handler to godotcpp.py for chunked object processing to prevent CI/CD from crashing#35
Closed
Arctis-Fireblight wants to merge 5 commits into
Conversation
…ject processing to prevent CI/CD from crashing GitHub Actions runners have a limit of 1024 arguments, which would cause the CI jobs to fail if more 1024 `.o` files got generated during the build process.
📝 WalkthroughWalkthroughAdds a chunked static-library archiver integrated into the build; expands GDExtension C API typedefs/structs and converts many comments to Doxygen; updates Windows CI runners to windows-2022; and adds/enables tests asserting internal class behavior. ChangesGDExtension API and docs updates
Custom static library archiving
CI Windows runner updates
Test updates
🎯 4 (Complex) | ⏱️ ~45 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tools/godotcpp.py`:
- Around line 533-566: Replace the manual POSIX-style command construction using
ar/arflags with SCons' platform-specific archive template: instead of building
cmd = "%s %s %s %s" % (ar, arflags, target_lib, " ".join(chunk)), call env.subst
on env['ARCOM'] (or "$ARCOM") while passing TARGET=target_lib and SOURCES="
".join(chunk) (or SOURCES=chunk as appropriate) to produce the correct platform
command for each chunk, then env.Execute the substituted command/Action; keep
the existing verbose print and the chunking loop around objects/target_lib
unchanged and remove the hardcoded use of ar/arflags/ranlib when invoking the
archive command.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/project/main.gd (1)
293-295: ⚡ Quick winAdd an explicit non-null assertion before calling internal class methods.
This makes failures clearer if internal instance retrieval regresses.
Suggested patch
var internal_class = example.test_get_internal_class() + assert_not_equal(internal_class, null) assert_equal(internal_class.get_the_answer(), 42) assert_equal(internal_class.get_class(), "ExampleInternal")🤖 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 `@test/project/main.gd` around lines 293 - 295, The test calls methods on example.test_get_internal_class() without asserting the returned instance is non-null; add an explicit non-null assertion for the returned value (e.g., assert_not_null or assert_true(internal_class != null)) right after calling example.test_get_internal_class() so failures clearly indicate a missing internal instance before invoking internal_class.get_the_answer() and internal_class.get_class().
🤖 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 `@tools/godotcpp.py`:
- Around line 545-554: The chunked archiving loop that builds cmd via
env.subst("$ARCOM", target=target, source=chunk) and calls env.Execute
repeatedly will overwrite MSVC archives because lib.exe /OUT:target replaces the
file each time; update the logic to detect MSVC (e.g. env["is_msvc"] or
equivalent platform flag) and for that case avoid chunking by passing the full
objects list to a single env.subst("$ARCOM", target=target, source=objects) /
single env.Execute call (or use an MSVC-specific incremental strategy), while
keeping the existing chunked path for non-MSVC toolchains; ensure references to
target_lib, objects, env.subst("$ARCOM", ...), and env.Execute are used so you
modify the exact archiving code path.
---
Nitpick comments:
In `@test/project/main.gd`:
- Around line 293-295: The test calls methods on
example.test_get_internal_class() without asserting the returned instance is
non-null; add an explicit non-null assertion for the returned value (e.g.,
assert_not_null or assert_true(internal_class != null)) right after calling
example.test_get_internal_class() so failures clearly indicate a missing
internal instance before invoking internal_class.get_the_answer() and
internal_class.get_class().
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2a618a48-3756-4faf-aeb8-a6ea51444872
📒 Files selected for processing (6)
.github/workflows/ci-cmake.yml.github/workflows/ci-scons.ymlgdextension/extension_api.jsongdextension/gdextension_interface.htest/project/main.gdtools/godotcpp.py
Comment on lines
+545
to
+554
| # Process in chunks of 100 | ||
| for i in range(0, len(objects), 100): | ||
| chunk = objects[i : i + 100] | ||
| # Use Execute with a substituted $ARCOM to produce the correct platform command for each chunk. | ||
| cmd = env.subst("$ARCOM", target=target, source=chunk) | ||
| if not env["verbose"]: | ||
| print("Archiving %s (chunk %d-%d) ..." % (os.path.basename(str(target_lib)), i, i + len(chunk))) | ||
| res = env.Execute(cmd) | ||
| if res != 0: | ||
| return res |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check which Windows toolchain is configured and whether ARCOM supports incremental archiving
# Check Windows platform tool configuration
echo "=== Windows platform tool configuration ==="
cat tools/windows.py | grep -A 10 -B 5 "ARCOM\|AR\s*=\|lib\.exe" || echo "No explicit ARCOM/AR override found"
# Check CI workflow for toolchain hints
echo ""
echo "=== CI Windows workflow toolchain hints ==="
rg -i "msvc|mingw|clang|visual.studio|compiler" .github/workflows/ --type yaml -C 2 | head -50
# Check if there's any existing handling for MSVC vs MinGW archiving
echo ""
echo "=== Any MSVC-specific archive handling ==="
rg -i "lib\.exe|/OUT:|msvc.*ar" tools/ --type pyRepository: Redot-Engine/redot-cpp
Length of output: 3562
🏁 Script executed:
# Read tools/windows.py to find actual ARCOM configuration
head -200 tools/windows.py | tail -150
# Also check if there's more content about ARCOM
rg -n "ARCOM|AR\s*=" tools/windows.py -A 3 -B 3Repository: Redot-Engine/redot-cpp
Length of output: 5468
🏁 Script executed:
# Read the full context of the modified godotcpp.py to understand integration
cat -n tools/godotcpp.py | sed -n '530,605p'Repository: Redot-Engine/redot-cpp
Length of output: 3221
🏁 Script executed:
# Check if the new function is actually being called
rg -n "_godot_cpp_build_library" tools/godotcpp.pyRepository: Redot-Engine/redot-cpp
Length of output: 191
🏁 Script executed:
# Check how many object files are typically compiled
rg -c "add_sources|env.Object" tools/godotcpp.py
# Look for any existing bug reports or comments about MSVC archiving issues
rg -i "msvc.*arch|lib\.exe|chunk.*msvc" . --type py -C 2
# Check if there are any recent changes or comments about this issue
git log --oneline --all --grep="chunk\|archive\|msvc\|windows" -i | head -20Repository: Redot-Engine/redot-cpp
Length of output: 1492
🏁 Script executed:
# Check the actual line count of sources being added
sed -n '578,585p' tools/godotcpp.py
# Search for any conditional handling of MSVC vs MinGW for archiving
rg -n "is_msvc\|use_mingw" tools/godotcpp.pyRepository: Redot-Engine/redot-cpp
Length of output: 355
🏁 Script executed:
# Count actual source files to see if chunking would be triggered (>100 objects)
find src -name "*.cpp" -type f | wc -l
# Check if there are any error reports or test failures with MSVC builds
git log --all --oneline | grep -i "msvc.*fail\|windows.*fail\|archive" | head -10
# Check if the new _godot_cpp_build_library function has been tested or if there are any known issues
rg -i "godot_cpp_build_library\|chunking\|argument.*limit" . --type mdRepository: Redot-Engine/redot-cpp
Length of output: 69
🏁 Script executed:
# Look for any SCons documentation or examples of how ARCOM should be used with mslib
rg -n "ARCOM\|mslib\|lib.exe" tools/ --type py -B 2 -A 2
# Check if there's any MSVC-specific conditional logic that should be handling this differently
ast-grep --pattern 'if $env["is_msvc"] { $$$ }'Repository: Redot-Engine/redot-cpp
Length of output: 48
🏁 Script executed:
# Check how many objects would be created and if chunking would actually trigger
# Count cpp files to estimate objects
echo "Source files:"
find src -name "*.cpp" -type f | wc -l
echo ""
# Check if any bindings are generated that would add more objects
rg -n "bindings" tools/godotcpp.py | grep -i "extend\|\.cpp"
# Look for any previous uses of chunking or special MSVC handling in SCons
rg -n "chunk\|ARCOM.*chunk" . --type pyRepository: Redot-Engine/redot-cpp
Length of output: 157
🏁 Script executed:
# Most importantly: check the actual MSVC lib.exe behavior with /OUT: flag
# Look for any SCons documentation comments or tests about this
rg -i "lib\.exe\|/OUT" . --type py -B 2 -A 2
# Check if there's any indication that the library is built incrementally or should be
rg -n "incremental\|append.*archive" . --type pyRepository: Redot-Engine/redot-cpp
Length of output: 48
MSVC archive overwrite issue will corrupt libraries during chunked archiving.
With >100 object files, the chunking loop will execute ARCOM multiple times. On MSVC, lib.exe /OUT:target creates a fresh archive on each invocation rather than appending. This means:
- Chunk 1: Creates archive with objects 0-99
- Chunk 2: Overwrites archive with objects 100-199 (losing chunk 1)
- Chunk N: Final archive contains only the last chunk's objects
MinGW ar r appends correctly, so this only affects MSVC Windows builds, which are tested in the CI matrix.
To fix this for MSVC, either:
- Accumulate all objects and pass them in a single ARCOM call (avoiding chunking overhead)
- Use platform-specific logic (
if env["is_msvc"]) to handle MSVC differently - Use a different archiving strategy for MSVC that supports incremental builds
🤖 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 `@tools/godotcpp.py` around lines 545 - 554, The chunked archiving loop that
builds cmd via env.subst("$ARCOM", target=target, source=chunk) and calls
env.Execute repeatedly will overwrite MSVC archives because lib.exe /OUT:target
replaces the file each time; update the logic to detect MSVC (e.g.
env["is_msvc"] or equivalent platform flag) and for that case avoid chunking by
passing the full objects list to a single env.subst("$ARCOM", target=target,
source=objects) / single env.Execute call (or use an MSVC-specific incremental
strategy), while keeping the existing chunked path for non-MSVC toolchains;
ensure references to target_lib, objects, env.subst("$ARCOM", ...), and
env.Execute are used so you modify the exact archiving code path.
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.
GitHub Actions runners have a limit of 1024 arguments, which would cause the CI jobs to fail if more 1024
.ofiles got generated during the build process.Summary by CodeRabbit
New Features
Chores
Tests