Fix: Linux CI failing due to too many arguments in ar command#37
Fix: Linux CI failing due to too many arguments in ar command#37Arctis-Fireblight wants to merge 3 commits into
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesLinux Build Spawn Wrapper
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 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 Tip You can disable poems in the walkthrough.Disable the |
Arctis-Fireblight
force-pushed
the
fix-linux-lines-too-long
branch
from
9d0747a to
71e44a1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/linux.py`:
- Around line 14-16: The file tools/linux.py was changed but not formatted,
causing CI to fail; run the project's formatter (e.g., ruff format) on
tools/linux.py (or the repo) to apply the blank-line and style fixes, verify
changes around the configure() function and the other noted sections (around
lines ~56-60 and ~97-99), and commit the formatted file so the formatting job
passes the CI.
- Line 57: The global env.Replace(ARFLAGS=["q"]) should be removed and the
ARFLAGS=["q"] setting applied only inside the batched ar handling path (the code
block that implements the batched/concatenated ar invocation), so that other
archive operations are unaffected; locate the batched path in tools/linux.py
(the block that builds the batched ar command) and set ARFLAGS or pass the "q"
flag locally there before invoking ar, leaving the global env.ARFLAGS unchanged.
- Around line 17-24: The code in mySubProcess currently reparses a command
string with shlex.split and passes that to subprocess.Popen, which can mangle
original argv boundaries; change mySubProcess to accept and pass an argv list
(or, if cmdline can be a list, detect and use it directly) and remove
shlex.split so you call subprocess.Popen(argv, env=env, ...) with the original
sequence; apply the same change to the other occurrence that mirrors lines 43-52
in this file (replace any re-parsing of a cmdline string with using the original
argv list or explicit shell=True if you truly need shell parsing).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| return rv | ||
|
|
||
| env["SPAWN"] = mySpawn | ||
| env.Replace(ARFLAGS=["q"]) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "linux.py" --path "*/tools/*"Repository: Redot-Engine/redot-cpp
Length of output: 300
🏁 Script executed:
cat -n tools/linux.py | head -80Repository: Redot-Engine/redot-cpp
Length of output: 2998
🏁 Script executed:
rg -i "batch.*ar|ar.*batch" tools/linux.py -B 2 -A 2Repository: Redot-Engine/redot-cpp
Length of output: 308
🏁 Script executed:
wc -l tools/linux.pyRepository: Redot-Engine/redot-cpp
Length of output: 85
🏁 Script executed:
cat -n tools/linux.pyRepository: Redot-Engine/redot-cpp
Length of output: 3805
🏁 Script executed:
rg "use_llvm" tools/linux.py -B 3 -A 3Repository: Redot-Engine/redot-cpp
Length of output: 447
🏁 Script executed:
rg "ARFLAGS|ar\.command|\bAR\b" tools/linux.py -B 2 -A 2Repository: Redot-Engine/redot-cpp
Length of output: 127
🏁 Script executed:
git log --oneline tools/linux.py | head -20Repository: Redot-Engine/redot-cpp
Length of output: 136
🏁 Script executed:
git diff HEAD~5 tools/linux.pyRepository: Redot-Engine/redot-cpp
Length of output: 255
🏁 Script executed:
fd -t f "\.py$" tools/ | xargs rg -l "batch.*ar|ar.*batch" 2>/dev/nullRepository: Redot-Engine/redot-cpp
Length of output: 81
🏁 Script executed:
rg -i "batch.*ar|ar.*batch|archived.*batch" --type pyRepository: Redot-Engine/redot-cpp
Length of output: 123
🏁 Script executed:
rg "ar\.command|ARFLAGS" --type py -B 3 -A 3Repository: Redot-Engine/redot-cpp
Length of output: 906
🏁 Script executed:
find . -name "*.gn" -o -name "*.gni" | head -20Repository: Redot-Engine/redot-cpp
Length of output: 48
🏁 Script executed:
rg "ARFLAGS|ar_command|ar\.command" --type gn --type gni -B 2 -A 2 2>/dev/null | head -50Repository: Redot-Engine/redot-cpp
Length of output: 48
🏁 Script executed:
rg -i "batch" --type gn | head -20Repository: Redot-Engine/redot-cpp
Length of output: 215
🏁 Script executed:
cat tools/linux.pyRepository: Redot-Engine/redot-cpp
Length of output: 3105
🌐 Web query:
GNU ar command q flag behavior replace update
💡 Result:
In GNU ar, the q (quick append) flag is designed to add files to the end of an archive without checking for existing members with the same name [1][2][3]. Because it skips this check, it is faster than the r (replace) operation [1][4]. Key behaviors of the q flag include: 1. No Replacement: It does not replace existing members; it simply appends new files to the end of the archive [1][2]. 2. Placement: Modifiers such as a, b, or i, which are used to specify the position of new members, do not affect the q operation; new members are always placed at the end [1][3]. 3. Symbol Table: Historically, q did not update the archive's symbol table index to maintain speed [1][2]. However, because many systems expect the index to be current, GNU ar will rebuild the symbol table even when using q [2]. 4. Compatibility Note: GNU ar treats the command qs as a synonym for r, meaning it will replace existing files and append new ones [2][4][3]. 5. Incompatibility with u: The u (update) modifier, which only inserts files newer than those already in the archive, is not allowed with q because checking timestamps would negate the speed advantage of the quick append operation [1][4][3].
Citations:
- 1: https://sourceware.org/binutils/docs-2.20/binutils/ar-cmdline.html
- 2: https://man7.org/linux/man-pages/man1/ar.1.html
- 3: https://sourceware.org/binutils/docs/binutils.html
- 4: https://www.mankier.com/1/ar
🌐 Web query:
llvm-ar q flag quick append behavior duplicates
💡 Result:
The q (quick append) flag in llvm-ar is used to append files to the end of an archive without checking for or removing duplicates [1][2]. Because it does not perform any deduplication, it is generally faster than the r (replace) operation, which must scan the archive to identify and replace existing members with the same name [1][2]. If the specified archive does not exist, llvm-ar will create it [1][2].
Citations:
- 1: https://llvm.org/docs/CommandGuide/llvm-ar.html
- 2: https://rocm.docs.amd.com/projects/llvm-project/en/latest/LLVM/llvm/html/CommandGuide/llvm-ar.html
🏁 Script executed:
rg "use_llvm|ARFLAGS" tools/ -B 5 -A 5Repository: Redot-Engine/redot-cpp
Length of output: 7173
Fix global ARFLAGS setting.
The q flag applies globally to all ar invocations on Linux, but only the batched path (lines 44-50) requires special handling to work around command-line length limits. Both GNU ar and llvm-ar implement q as quick append without deduplication, so the concern about incompatible behavior across toolchains is unfounded. However, forcing q globally still affects all archive operations; if needed, apply it conditionally only within the batched path to avoid unintended side effects elsewhere.
🤖 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/linux.py` at line 57, The global env.Replace(ARFLAGS=["q"]) should be
removed and the ARFLAGS=["q"] setting applied only inside the batched ar
handling path (the code block that implements the batched/concatenated ar
invocation), so that other archive operations are unaffected; locate the batched
path in tools/linux.py (the block that builds the batched ar command) and set
ARFLAGS or pass the "q" flag locally there before invoking ar, leaving the
global env.ARFLAGS unchanged.
Added logic to linux.py split long `.o` file lists into commands of 512 or less.
Arctis-Fireblight
force-pushed
the
fix-linux-lines-too-long
branch
from
71e44a1 to
db53da2
Compare
… commands. In `my_spawn.py`, the command line length cannot exceed 32000 characters on windows. The previous implementation would break down linking to one `.obj` file at a time which was extremely slow. The new batching logic instead will batch as many `.obj` files as possible into a single linking operation resulting in significantly faster build times when compiling using MinGW on Windows.
Arctis-Fireblight
force-pushed
the
fix-linux-lines-too-long
branch
from
916313e to
c18312f
Compare
…compiling with MinGW and on linux The lines removed were forcing Scons to effectively run single threaded.
1 participant
Added logic to linux.py split long
.ofile lists into commands of 512 or less.Summary by CodeRabbit