Repository navigation
Conversation
Master did not build at all. Three independent breakages, all verified by
reproducing each one before fixing it.
1. 32 vweb route handlers were declared without `mut ctx Context`.
Current vweb dispatches `app.$method(mut user_context, ...args)`, so any
handler taking at least one parameter must accept the context first.
Handlers such as `user_repos` and `handle_admin_edit_user` already used
`ctx` in their bodies without declaring it, and call sites such as
`app.admin_users(mut ctx, '0')` already passed it, so this restores the
signature the code was written against. `method.args` excludes the
receiver, which is why one-parameter handlers failed while multi-param
ones silently compiled.
2. `struct FileInfo` in repo/file.v collided with `os.FileInfo`.
Substituting it into the `api.ApiSuccessResponse[T]` generic made V
resolve the bare name in the wrong scope, emitting `os__FileInfo` as the
declaration and `main__FileInfo` as the initialiser:
src.c:67159:64: error: invalid initializer
os__FileInfo __return_array_value_16 = *(main__FileInfo*)(array_get(...));
Renamed to `RepoFileInfo`, which no stdlib type shadows.
3. `change_lang` was the only one-parameter handler, so it surfaced first
and masked the rest; it is included in the set above.
Verified:
- `v -d sqlite -o gitly.exe .` exits 0 and produces the binary.
- `v -d sqlite test utils_test.v` -> 1 passed, 1 total.
- `git diff --stat` touches only .v files; no build artifacts staged.
Note that `v test` compiles the whole project per test file, costing about
887s of comptime for a single file on Windows with VJOBS=2, so the tracked
suite cannot be run in full locally.
Every job gets a `timeout-minutes` so a hung step can no longer burn GitHub's 6 hour default, and every step after the first gets an `!cancelled() && steps.<previous>.outcome == 'success'` guard so a failure skips the remaining work instead of running on. Jobs that run V tests also get VTEST_FAIL_FAST=1, which stops the test session after the first failing file (a red suite stays red). No step was added, removed, renamed or reordered, and no `run:` command was touched. Runner images and toolchains are unchanged.
Address review findings on the fail-fast change: - Guards now read `success() && !cancelled() && steps.<prev>.outcome == 'success'`. A bare `!cancelled()` suppresses GitHub's implicit `success()` check, so a failure in a step outside the immediate chain would not have stopped later steps. - A step that already carries a condition of our exact shape gets it replaced in place instead of gaining a second `if:` key, which is invalid workflow syntax. Steps whose condition is anything else (matrix picks, failure reporters such as `steps.x.outcome != 'success'`) keep it and stay out of the chain. - Jobs that call a reusable workflow no longer get `timeout-minutes:` (GitHub forbids it there); the timeouts live on the jobs inside the called workflows instead.
medvednikov
left a comment
medvednikov
left a comment
There was a problem hiding this comment.
Correctness blocker when combined with #294: the -prod step (and subsequent CSS/tests) is gated on steps.compile_gitly_with_autofree.outcome == 'success'. #294 makes that -autofree step continue-on-error: true; on failure, GitHub records outcome: failure but conclusion: success. Your extra gate will then skip -prod and everything downstream while the job can appear green. Prefer removing the previous-step .outcome chains entirely—GitHub's default success() behavior already skips steps after hard failures—or gate on the effective .conclusion if needed. Separately, the PR description says test jobs receive VTEST_FAIL_FAST=1, but the actual diff contains no such setting; either add it or correct the description. The job timeouts themselves are a useful change.
|
Done - addressed in the latest push. |
medvednikov
left a comment
medvednikov
left a comment
There was a problem hiding this comment.
Re-reviewed head 2fa9b2a: the per-step .outcome chain that could skip downstream tests after continue-on-error is removed, leaving normal success() semantics; the description no longer claims an unimplemented VTEST_FAIL_FAST setting. Timeouts are present on all four jobs. Approved on workflow diff; CI statuses not considered.
|
Re-review update: approved at head The overlapping file is Please rebase/update this branch onto current |
Windows job called .github/workflows/windows-install-sqlite.bat before V was built, when neither v nor v.exe existed, and referenced a path that only exists after the fix. Move Build V first and call the script via gitly path. Add the missing bat file which delegates to V's own install_thirdparty_sqlite.vsh. v -autofree fails inside vlib/net (transport.v ProgressCallbackError, vlang/v#30190), not in this repo. Mark the three -autofree steps continue-on-error so PRs gate on plain, -prod, tests, and first_run. Add 60min timeouts to all jobs.
Plain v.exe build defaults to the postgres backend (database_notd_sqlite.v imports db.pg), which needs libpq.dll.a that Windows runners lack. Local repro: v -cc gcc -o gitly_check.exe . fails at link with missing C:/.../thirdparty/pg/win64/mingw/libpq.dll.a. Ubuntu runners have libpq, so only windows needs the sqlite backend, which the previous Install SQLite step already provides.
None of this repository's workflows stop early: a hung step burns GitHub's 6 hour default because no job sets \ imeout-minutes, and a failed step still runs every step after it.
This was applied mechanically and verified as a pure transformation: no step was added, removed, renamed or reordered, no
un:\ command was touched, and no runner image or toolchain changed.