mirror of https://github.com/open-goal/jak-project synced 2026-08-20 06:16:35 -04:00

Files

T

Alexander J. Semenuk 5d5e35fb9b fix: Windows toolchain compatibility (curl 8.21 re-vendor, endless reconfigure loop) (#4355 )

## Problem

Windows builds break with a current local toolchain (Scoop LLVM 22.1.8,
CMake 4.4.0, VS 2026), in two independent ways:

1. The build stops at curl's deliberate guard: `#error "no non-blocking
method was found/used/set"` in `third-party/curl/lib/nonblock.c`.
2. From the second configure onward, `cmake --build` re-runs CMake in an
endless loop (observed 42 consecutive reconfigure cycles in a single
build). Likely the same mechanism behind the "endlessly building" VS
2026 note in `docs/setup/dev/vs.md`.

## Root cause

1. `third-party/curl/CMake/CurlTests.c` passes `int *` to
`ioctlsocket()`, whose third parameter is `u_long *`. Clang 22 promotes
`-Wincompatible-pointer-types` to a hard error in C, so the
`HAVE_IOCTLSOCKET_FIONBIO` try_compile silently fails and
`curl_config.h` never defines it. Upstream CI does not see this because
the windows-2022 runner image ships an older LLVM. GCC 14 promotes the
same warning to a hard error, which is very likely the `CurlTests.c.obj`
failure reported from MSYS2 in open-goal/jak-project#3551. Upstream curl
hit the identical problem with GCC 14 and fixed the probe in curl 8.8.0
(curl/curl#13578).
2. The root CMakeLists copies the build tree's `compile_commands.json`
into `<src>/build/` for clangd using `configure_file()`, which registers
its input as a configure dependency. CMake rewrites
`compile_commands.json` late in every generation, after
`CTestTestfile.cmake` and `cmake_install.cmake` (outputs of the same
Ninja regen rule), so once the dependency is registered the rule is
deterministically dirty and every `ninja` invocation re-runs CMake. A
pristine first configure is safe (the file does not exist yet, so the
`if(EXISTS ...)` guard skips the copy), which is why the loop looks
machine- or IDE-specific.

## Fix

1. Per review, re-vendor `third-party/curl` at the `curl-8_21_0` tag
(previously `curl-8_3_0`), which carries the upstream probe fix plus two
years of upstream development; `vendor.yaml` updated to match.
Adjustments the version jump forced:
- curl 8.15 removed the native macOS Secure Transport backend
(`CURL_USE_SECTRANSP`), so macOS now builds curl against OpenSSL like
Linux. The two macOS workflows install Homebrew `openssl@3` and export
`OPENSSL_ROOT_DIR` (keg-only), and the macOS setup docs gained the same
two lines.
- `CURL_BROTLI` / `CURL_ZSTD` switched to AUTO-detection in curl 8.10;
pinned OFF to keep the previous no-compression behavior and avoid
silently linking whatever the CI images happen to have.
- curl's new top-level `BUILD_EXAMPLES` cache option (default ON) leaked
into discord-rpc's identically named option and broke configure at a
nonexistent `examples/send-presence` directory; pinned OFF ahead of the
third-party subdirectories.

The diff is dominated by the mechanical tag-tree swap under
`third-party/curl` (linguist-vendored, collapsed in review). The
hand-written changes are `CMakeLists.txt`, the two macOS workflows,
`docs/setup/system/macos.md`, and `vendor.yaml`.
2. Swap `configure_file()` for `file(COPY ...)`: the same clangd copy
with no configure dependency registered. (`file(COPY_FILE ...
ONLY_IF_DIFFERENT)` would be cleaner still but requires CMake 3.21,
above the declared `cmake_minimum_required(VERSION 3.10)`.)

## Test plan

- [x] Fresh `cmake --preset Release-windows-clang` (LLVM 22, no cache
seeding) completes and logs `Enabled SSL backends: Schannel`; the
FIONBIO probe passes without the previous `#error`
- [x] Full Windows Release build from scratch in the branch worktree
(all 1422 targets)
- [x] goalc-test suite: 1509 passed, 0 failed
- [x] Second consecutive configure with `compile_commands.json` present:
the regen rule in `build.ninja` has no `compile_commands.json` input;
`<src>/build/compile_commands.json` is still refreshed for clangd
- [x] Repeated `ninja` invocations after a full build no longer re-run
CMake
- [x] macOS Intel and ARM CI green (first exercise of the OpenSSL
backend switch)

---

I work off a self-hosted forge, so this GitHub account is quiet; the
configure logs and ninja dirty-node traces from the investigation are
available if anyone wants the raw data.

(AI-assisted)

2026-07-27 19:19:18 -04:00

6.7 KiB

Vendored

Generated

Raw Blame History

checksrc

This is the tool we use within the curl project to scan C source code and check that it adheres to our Source Code Style guide.

Usage

checksrc.pl [options] [file1] [file2] ...

Command line options

-W[file] skip that file and exclude it from being checked. Helpful when, for example, one of the files is generated.

-D[dir] directory name to prepend to filenames when accessing them.

-h shows the help output, that also lists all recognized warnings

What does `checksrc` warn for?

checksrc does not check and verify the code against the entire style guide. The script is an effort to detect the most common mistakes and syntax mistakes that contributors make before they get accustomed to our code style. Heck, many of us regulars do the mistakes too and this script helps us keep the code in shape.

checksrc.pl -h

Lists how to use the script and it lists all existing warnings it has and problems it detects. At the time of this writing, the existing checksrc warnings are:

ASSIGNWITHINCONDITION: Assignment within a conditional expression. The code style mandates the assignment to be done outside of it.
ASTERISKNOSPACE: A pointer was declared like char* name instead of the more appropriate char *name style. The asterisk should sit next to the name.
ASTERISKSPACE: A pointer was declared like char * name instead of the more appropriate char *name style. The asterisk should sit right next to the name without a space in between.
BADCOMMAND: There is a bad checksrc instruction in the code. See the Ignore certain warnings section below for details.
BANNEDFUNC: A banned function was used. The functions sprintf, vsprintf, strcat, strncat, gets are never allowed in curl source code.
BRACEELSE: '} else' on the same line. The else is supposed to be on the following line.
BRACEPOS: wrong position for an open brace ({).
BRACEWHILE: more than once space between end brace and while keyword
COMMANOSPACE: a comma without following space
COPYRIGHT: the file is missing a copyright statement
CPPCOMMENTS: // comment detected, that is not C89 compliant
DOBRACE: only use one space after do before open brace
EMPTYLINEBRACE: found empty line before open brace
EQUALSNOSPACE: no space after = sign
EQUALSNULL: comparison with == NULL used in if/while. We use !var.
EXCLAMATIONSPACE: space found after exclamations mark
FOPENMODE: curlx_fopen(), curlx_freopen() need a macro for the mode string, use it
INDENTATION: detected a wrong start column for code. Note that this warning only checks some specific places and can certainly miss many bad indentations.
LONGLINE: A line is longer than 79 columns.
MULTISPACE: Multiple spaces were found where only one should be used.
NOSPACEEQUALS: An equals sign was found without preceding space. We prefer a = 2 and not a=2.
NOTEQUALSZERO: check found using != 0. We use plain if(var).
ONELINECONDITION: do not put the conditional block on the same line as if()
OPENCOMMENT: File ended with a comment (/*) still "open".
PARENBRACE: ){ was used without sufficient space in between.
RETURNNOSPACE: return was used without space between the keyword and the following value.
SEMINOSPACE: There was no space (or newline) following a semicolon.
SIZEOFNOPAREN: Found use of sizeof without parentheses. We prefer sizeof(int) style.
SNPRINTF - Found use of snprintf(). Since we use an internal replacement with a different return code etc, we prefer curl_msnprintf().
SPACEAFTERPAREN: there was a space after open parenthesis, ( text.
SPACEBEFORECLOSE: there was a space before a close parenthesis, text ).
SPACEBEFORECOMMA: there was a space before a comma, one , two.
SPACEBEFOREPAREN: there was a space before an open parenthesis, if (, where one was not expected
SPACESEMICOLON: there was a space before semicolon, ;.
TABS: TAB characters are not allowed
TRAILINGSPACE: Trailing whitespace on the line
TYPEDEFSTRUCT: we frown upon (most) typedefed structs
UNUSEDIGNORE: a checksrc inlined warning ignore was asked for but not used, that is an ignore that should be removed or changed to get used.
USESAFEFREE: there was a curlx_free(var) call made right before assigning NULL to var. We prefer replacing that with curlx_safefree(), which is doing these two operations in a single call.

Extended warnings

Some warnings are computationally expensive to perform, so they are turned off by default. To enable these warnings, place a .checksrc file in the directory where they should be activated with commands to enable the warnings you are interested in. The format of the file is to enable one warning per line like so: enable <EXTENDEDWARNING>

Currently these are the extended warnings which can be enabled:

COPYRIGHTYEAR: the current changeset has not updated the copyright year in the source file
STRERROR: use of banned function strerror()
STDERR: use of banned variable stderr

Ignore certain warnings

Due to the nature of the source code and the flaws of the checksrc tool, there is sometimes a need to ignore specific warnings. checksrc allows a few different ways to do this.

Inline ignore

You can control what to ignore within a specific source file by providing instructions to checksrc in the source code itself. See examples below. The instruction can ask to ignore a specific warning a specific number of times or you ignore all of them until you mark the end of the ignored section.

Inline ignores are only done for that single specific source code file.

Example

/* !checksrc! disable LONGLINE all */

This ignores the warning for overly long lines until it is re-enabled with:

/* !checksrc! enable LONGLINE */

If the enabling is not performed before the end of the file, it is enabled again automatically for the next file.

You can also opt to ignore N violations so that if you have a single long line you cannot shorten and is agreed to be fine anyway:

/* !checksrc! disable LONGLINE 1 */

... and the warning for long lines is enabled again automatically after it has ignored that single warning. The number 1 can of course be changed to any other integer number. It can be used to make sure only the exact intended instances are ignored and nothing extra.

Directory wide ignore patterns

This is a method we have transitioned away from. Use inline ignores as far as possible.

Make a checksrc.skip file in the directory of the source code with the false positive, and include the full offending line into this file.

6.7 KiB Vendored Generated Raw Blame History