-
Notifications
You must be signed in to change notification settings - Fork 1
Update Setup guide #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
I have to test on fresh machine again. Because seems like it requires MSVC libs which are obtainable via Visual Studio Installer |
|
I can't really comment on this procedure. I've never done it myself. Is the workflow still working stable? |
📝 WalkthroughWalkthroughSETUP.md documentation expanded with clarified installation instructions for Clang, Visual Studio, and GCC via Chocolatey. Added new Patcher section, build script usage details, and expanded testing and debugging guidance. Removed references to MSYS2-based setup. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
|
Okay lets see what this AI says |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@SETUP.md`:
- Line 8: Update the sentence in SETUP.md to use the possessive "its" instead of
the contraction "it's": change "requires it's components" to "requires its
components" so the line reads "You will need GCC and Clang compilers and some
components of Visual Studio (Yes, patching game engine that was originally built
with Visual Studio requires its components)."
- Line 30: Update the informal phrasing in the README/SETUP text by replacing
"gonna be installed at `C:\ProgramData\mingw64`" with the formal wording "will
be installed at `C:\ProgramData\mingw64`" so the sentence reads: "Run `choco
install mingw --x86 -y --no-progress` which will be installed at
`C:\ProgramData\mingw64`."
- Around line 29-31: Update the incorrect Chocolatey installation path on line
containing "C:\ProgramData\mingw64" to the correct Chocolatey install location
"C:\ProgramData\chocolatey\lib\mingw\tools\install\mingw32\bin\" and explicitly
reference the full executable paths
"C:\ProgramData\chocolatey\lib\mingw\tools\install\mingw32\bin\g++.exe" and
"C:\ProgramData\chocolatey\lib\mingw\tools\install\mingw32\bin\ld.exe"; also
replace the informal phrase "gonna be installed" with "will be installed" in
that same line.
🧹 Nitpick comments (1)
SETUP.md (1)
39-41: Consider adding parameter descriptions for clarity.The command syntax example lists four paths, but adding brief inline descriptions could help users avoid mistakes when setting up the build script.
📝 Suggested enhancement
```bat -python main.py [Path to patches folder] [Path to clang++.exe] [Path to ld.exe] [Path to g++.exe] +python main.py [Path to patches folder] [Path to clang++.exe] [Path to ld.exe from GCC] [Path to g++.exe from GCC]Or add a brief example: ```markdown ```bat python main.py [Path to patches folder] [Path to clang++.exe] [Path to ld.exe] [Path to g++.exe]Example:
python main.py C:\patches C:\LLVM\bin\clang++.exe C:\ProgramData\mingw64\mingw32\bin\ld.exe C:\ProgramData\mingw64\mingw32\bin\g++.exe
| # Compilers | ||
|
|
||
| You will need GCC and Clang compilers. | ||
| You will need GCC and Clang compilers and some components of Visual Studio (Yes, patching game engine that was originally built with Visual Studio requires it's components). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix grammar: use possessive "its" instead of contraction "it's".
The phrase "requires it's components" should be "requires its components" (possessive form, not contraction).
✏️ Proposed fix
-You will need GCC and Clang compilers and some components of Visual Studio (Yes, patching game engine that was originally built with Visual Studio requires it's components).
+You will need GCC and Clang compilers and some components of Visual Studio (Yes, patching game engine that was originally built with Visual Studio requires its components).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| You will need GCC and Clang compilers and some components of Visual Studio (Yes, patching game engine that was originally built with Visual Studio requires it's components). | |
| You will need GCC and Clang compilers and some components of Visual Studio (Yes, patching game engine that was originally built with Visual Studio requires its components). |
🤖 Prompt for AI Agents
In `@SETUP.md` at line 8, Update the sentence in SETUP.md to use the possessive
"its" instead of the contraction "it's": change "requires it's components" to
"requires its components" so the line reads "You will need GCC and Clang
compilers and some components of Visual Studio (Yes, patching game engine that
was originally built with Visual Studio requires its components)."
| * Install [chocolatey](https://chocolatey.org/) | ||
| * Run `choco install mingw --x86 -y --no-progress` which is gonna be installed at `C:\ProgramData\mingw64`. | ||
| * You need paths to `/mingw32/bin/g++.exe` and `/mingw32/bin/ld.exe` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, locate and read the SETUP.md file to understand the full context
fd -t f "SETUP.md" | head -5Repository: FAForever/fa-python-binary-patcher
Length of output: 86
🏁 Script executed:
# Read the SETUP.md file to see lines 29-31 in context
cat -n SETUP.mdRepository: FAForever/fa-python-binary-patcher
Length of output: 2331
🌐 Web query:
Chocolatey mingw package installation path --x86 flag C:\ProgramData
💡 Result:
With Chocolatey installed in the default location, %ChocolateyInstall is C:\ProgramData\chocolatey. (docs.chocolatey.org)
For the community mingw (MinGW-w64) package, the package script extracts the archive into an install folder inside the package’s tools directory, i.e. typically:
- 64-bit default install path (most common):
C:\ProgramData\chocolatey\lib\mingw\tools\install\...(community.chocolatey.org) - The
--x86flag tells Chocolatey to use the 32-bit download (url) instead of the 64-bit download (url64bit), but it still extracts under that same...\lib\mingw\tools\install\folder. (community.chocolatey.org)
To confirm on your machine, check:
%ChocolateyInstall%\lib\mingw\tools\install- or run in PowerShell:
Write-Host $env:ChocolateyInstall(docs.chocolatey.org)
Citations:
- 1: https://docs.chocolatey.org/en-us/faqs/?utm_source=openai
- 2: https://community.chocolatey.org/packages/mingw?utm_source=openai
- 3: https://community.chocolatey.org/packages/mingw?utm_source=openai
- 4: https://docs.chocolatey.org/en-us/choco/setup/?utm_source=openai
🌐 Web query:
Chocolatey mingw package tools install directory structure mingw32 mingw64
💡 Result:
For Chocolatey packages, the extracted “real” files typically live under:
%ChocolateyInstall%\lib\<package>\tools\install\...(default%ChocolateyInstall%isC:\ProgramData\chocolatey).
mingw / MinGW-w64 (Chocolatey package name: mingw)
The toolchain is placed under the package’s tools\install\mingw64\..., e.g.:
C:\ProgramData\chocolatey\lib\mingw\tools\install\mingw64\bin(compiler/binutils, etc.) [1]- You may also see subtrees like
...\mingw64\opt\bin\...referenced by Chocolatey shims (e.g.,python.exe) [2] - GCC’s versioned libs/headers are under paths like
...\mingw64\lib\gcc\x86_64-w64-mingw32\<ver>\...[3]
So, for this package, mingw64 is the directory you normally add to PATH (or let Chocolatey shims handle).
Packages that provide both mingw32 and mingw64
Some Chocolatey packages (e.g., WinLibs-based ones) extract both architectures side-by-side and then use:
<installDir>\mingw32\bin<installDir>\mingw64\bin[4]
This is where the “mingw32 and mingw64 directory structure” usually comes from.
Sources: [1] [2] [3] [4]
🌐 Web query:
Chocolatey mingw --x86 flag 32-bit binaries mingw32 directory
💡 Result:
--x86(aka--forcex86) tells Chocolatey to force the 32‑bit build even on a 64‑bit Windows machine. [1]- For the Chocolatey
mingwpackage, the install script unpacks into the package’stools\installfolder (under Chocolatey’slib\mingwdirectory). [2]- Typical path:
C:\ProgramData\chocolatey\lib\mingw\tools\install\
- Typical path:
- The 32‑bit MinGW-w64 binaries will then be under the
mingw32tree, i.e.:C:\ProgramData\chocolatey\lib\mingw\tools\install\mingw32\bin\[2]
- On a 64‑bit machine, using
--x86makes themingwpackage use its i686 (32‑bit) download (url), instead of theurl64bit(x86_64) download. [2]
Sources: [1] Chocolatey install command docs (option --x86). [2] Chocolatey mingw package install script showing tools\install location and 32/64-bit URLs.
Line 30: Incorrect installation path.
choco install mingw --x86 does not install to C:\ProgramData\mingw64. The actual installation path is C:\ProgramData\chocolatey\lib\mingw\tools\install\mingw32\bin\ (when using the --x86 flag for 32-bit binaries). Update line 30 to reflect the correct Chocolatey installation location and provide the full path to the executables.
Additionally, replace the informal "gonna be" with "will be" on line 30.
🧰 Tools
🪛 LanguageTool
[style] ~30-~30: The word ‘gonna’ is informal.
Context: ... mingw --x86 -y --no-progresswhich is gonna be installed atC:\ProgramData\mingw64...
(GONNA)
🤖 Prompt for AI Agents
In `@SETUP.md` around lines 29 - 31, Update the incorrect Chocolatey installation
path on line containing "C:\ProgramData\mingw64" to the correct Chocolatey
install location
"C:\ProgramData\chocolatey\lib\mingw\tools\install\mingw32\bin\" and explicitly
reference the full executable paths
"C:\ProgramData\chocolatey\lib\mingw\tools\install\mingw32\bin\g++.exe" and
"C:\ProgramData\chocolatey\lib\mingw\tools\install\mingw32\bin\ld.exe"; also
replace the informal phrase "gonna be installed" with "will be installed" in
that same line.
| * x64: pacman -S mingw-w64-x86_64-gcc | ||
|
|
||
| * Install [chocolatey](https://chocolatey.org/) | ||
| * Run `choco install mingw --x86 -y --no-progress` which is gonna be installed at `C:\ProgramData\mingw64`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use formal language in documentation.
Replace the informal "gonna be" with "will be" for better documentation quality.
✏️ Proposed fix
-* Run `choco install mingw --x86 -y --no-progress` which is gonna be installed at `C:\ProgramData\mingw64`.
+* Run `choco install mingw --x86 -y --no-progress` which will be installed at `C:\ProgramData\mingw64`.As per coding guidelines: The word 'gonna' is informal and should be avoided in documentation.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| * Run `choco install mingw --x86 -y --no-progress` which is gonna be installed at `C:\ProgramData\mingw64`. | |
| * Run `choco install mingw --x86 -y --no-progress` which will be installed at `C:\ProgramData\mingw64`. |
🧰 Tools
🪛 LanguageTool
[style] ~30-~30: The word ‘gonna’ is informal.
Context: ... mingw --x86 -y --no-progresswhich is gonna be installed atC:\ProgramData\mingw64...
(GONNA)
🤖 Prompt for AI Agents
In `@SETUP.md` at line 30, Update the informal phrasing in the README/SETUP text
by replacing "gonna be installed at `C:\ProgramData\mingw64`" with the formal
wording "will be installed at `C:\ProgramData\mingw64`" so the sentence reads:
"Run `choco install mingw --x86 -y --no-progress` which will be installed at
`C:\ProgramData\mingw64`."
Seems like
MSYS2got some unreliable updates which just caused my fresh setup to fail to build patches. With this guide it worked properly.Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.