fix/libsodium-build-failures #102

Merged

sarman merged 6 commits from fix/libsodium-build-failures into beta 2026-06-14 09:21:52 +00:00

Conversation 3 Commits 6 Files Changed 4 +376 -143

sarman commented 2026-06-14 07:53:23 +00:00

Owner

Copy Link

Summary

 - Add explicit SODIUM_USE_PKG_CONFIG controls to all CI build targets
 - Fix Linux amd64/arm64 pkg-config detection failures
 - Fix Windows SODIUM_LIB_DIR conflict with pkg-config

 ## Root Causes

 **Linux amd64/arm64:**
 ```
 libsodium not found via pkg-config or vcpkg
 ```
 Docker images had `libsodium-dev` and `pkg-config` installed, but `libsodium-sys-stable` wasn't explicitly told to use pkg-config detection.

 **Windows:**
 ```
 SODIUM_LIB_DIR is incompatible with SODIUM_USE_PKG_CONFIG
 ```
 Setting both `SODIUM_LIB_DIR` (explicit path) and having pkg-config available caused a conflict in the build script's detection logic.

 ## Changes

 ### Linux amd64 build
 - **Added:** `SODIUM_USE_PKG_CONFIG: "1"` to force pkg-config detection
 - **Why:** Ensures `libsodium-sys-stable` uses the installed `libsodium-dev` package

 ### Linux arm64 build
 - **Added:** `SODIUM_USE_PKG_CONFIG: "1"` for cross-compilation
 - **Why:** Same as amd64 - force pkg-config to find the arm64 libsodium package

 ### Windows cross-compile build
 - **Added:** `SODIUM_USE_PKG_CONFIG: "no"` to disable pkg-config
 - **Why:** Prevents conflict with explicit `SODIUM_LIB_DIR=/usr/x86_64-w64-mingw32/lib`
 - **Note:** Windows uses pre-built libsodium from Dockerfile, not pkg-config

 ## Build Script Behavior

 `libsodium-sys-stable`'s build.rs checks environment variables in this precedence:
 1. If `SODIUM_LIB_DIR` is set → use explicit path (incompatible with pkg-config mode)
 2. If `SODIUM_USE_PKG_CONFIG` ≠ "no" → try pkg-config detection
 3. Fall back to vcpkg or fail

 Our strategy:
 - **Linux:** Force pkg-config (has `libsodium-dev` + `pkg-config`)
 - **Windows:** Use explicit path + disable pkg-config (has pre-compiled lib)

 ## Documentation

 Updated `LIBSODIUM_BUILD_FIX.md` with:
 - Platform-specific environment variable explanations
 - Rationale for each approach
 - Build script precedence order

 ## Test Plan

 CI pipeline will verify:
 - [ ] Linux amd64 build succeeds with pkg-config detection
 - [ ] Linux arm64 build succeeds with cross-compile pkg-config
 - [ ] Windows build succeeds with explicit lib path (no pkg-config conflict)
 - [ ] All platforms produce valid `.deb`, `.rpm`, `.exe`, `.msi` artifacts

## Summary - Add explicit SODIUM_USE_PKG_CONFIG controls to all CI build targets - Fix Linux amd64/arm64 pkg-config detection failures - Fix Windows SODIUM_LIB_DIR conflict with pkg-config ## Root Causes **Linux amd64/arm64:** ``` libsodium not found via pkg-config or vcpkg ``` Docker images had `libsodium-dev` and `pkg-config` installed, but `libsodium-sys-stable` wasn't explicitly told to use pkg-config detection. **Windows:** ``` SODIUM_LIB_DIR is incompatible with SODIUM_USE_PKG_CONFIG ``` Setting both `SODIUM_LIB_DIR` (explicit path) and having pkg-config available caused a conflict in the build script's detection logic. ## Changes ### Linux amd64 build - **Added:** `SODIUM_USE_PKG_CONFIG: "1"` to force pkg-config detection - **Why:** Ensures `libsodium-sys-stable` uses the installed `libsodium-dev` package ### Linux arm64 build - **Added:** `SODIUM_USE_PKG_CONFIG: "1"` for cross-compilation - **Why:** Same as amd64 - force pkg-config to find the arm64 libsodium package ### Windows cross-compile build - **Added:** `SODIUM_USE_PKG_CONFIG: "no"` to disable pkg-config - **Why:** Prevents conflict with explicit `SODIUM_LIB_DIR=/usr/x86_64-w64-mingw32/lib` - **Note:** Windows uses pre-built libsodium from Dockerfile, not pkg-config ## Build Script Behavior `libsodium-sys-stable`'s build.rs checks environment variables in this precedence: 1. If `SODIUM_LIB_DIR` is set → use explicit path (incompatible with pkg-config mode) 2. If `SODIUM_USE_PKG_CONFIG` ≠ "no" → try pkg-config detection 3. Fall back to vcpkg or fail Our strategy: - **Linux:** Force pkg-config (has `libsodium-dev` + `pkg-config`) - **Windows:** Use explicit path + disable pkg-config (has pre-compiled lib) ## Documentation Updated `LIBSODIUM_BUILD_FIX.md` with: - Platform-specific environment variable explanations - Rationale for each approach - Build script precedence order ## Test Plan CI pipeline will verify: - [ ] Linux amd64 build succeeds with pkg-config detection - [ ] Linux arm64 build succeeds with cross-compile pkg-config - [ ] Windows build succeeds with explicit lib path (no pkg-config conflict) - [ ] All platforms produce valid `.deb`, `.rpm`, `.exe`, `.msi` artifacts

sarman added 2 commits 2026-06-14 07:53:24 +00:00

fix(ci): resolve libsodium pkg-config detection across all platforms ... e29db718d2

Add explicit SODIUM_USE_PKG_CONFIG control to all build targets:
- Linux amd64/arm64: Set SODIUM_USE_PKG_CONFIG=1 to force pkg-config detection
- Windows: Set SODIUM_USE_PKG_CONFIG=no to prevent conflict with SODIUM_LIB_DIR

Fixes build failures:
- Linux: "libsodium not found via pkg-config or vcpkg" despite libsodium-dev installed
- Windows: "SODIUM_LIB_DIR is incompatible with SODIUM_USE_PKG_CONFIG"
- ARM64: Same pkg-config detection issue as amd64

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

docs: document pkg-config environment variable strategy ... 8570491f91

Explain the SODIUM_USE_PKG_CONFIG settings for each platform:
- Linux: Force pkg-config detection (libsodium-dev installed)
- Windows: Disable pkg-config (explicit SODIUM_LIB_DIR)
- ARM64: Force pkg-config detection (cross-compile)

Include rationale for libsodium-sys-stable's env var precedence order.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

sarman reviewed 2026-06-14 07:57:05 +00:00

sarman left a comment

Author

Owner

Copy Link

Automated PR Review (qwen3-coder-next via liteLLM):\n\n## Summary

The PR adds explicit SODIUM_USE_PKG_CONFIG controls to CI build targets to resolve libsodium build failures. The changes involve setting SODIUM_USE_PKG_CONFIG: "1" for Linux builds and SODIUM_USE_PKG_CONFIG: "no" for Windows builds, plus updating documentation. All changes are in CI workflow files and documentation, not in application code.

Findings

• [WARNING] .gitea/workflows/auto-tag.yml - Inconsistent SODIUM_USE_PKG_CONFIG value in Windows build

  The Windows build sets SODIUM_USE_PKG_CONFIG: "no", but according to the documented precedence in the PR description and the build script behavior, the value should be set as SODIUM_USE_PKG_CONFIG: "no" which is correct. However, reviewing the actual YAML file, the Windows build already has SODIUM_LIB_DIR set and now SODIUM_USE_PKG_CONFIG: "no" which should work correctly per the documentation. This is not an issue as written.

• [WARNING] LIBSODIUM_BUILD_FIX.md - Incorrect statement about Docker image updates

  The documentation claims Docker images were updated with libsodium-dev packages, but the files changed list does not include any Dockerfile modifications. The PR only changes .gitea/workflows/auto-tag.yml and LIBSODIUM_BUILD_FIX.md according to the changed files section, but the documentation references .docker/Dockerfile.linux-amd64 and .docker/Dockerfile.linux-arm64 which were not provided in the changed files. This discrepancy between documentation and actual code changes should be resolved.

• [WARNING] LIBSODIUM_BUILD_FIX.md - Missing test file in changed files list

  The documentation states src-tauri/src/state.rs was modified with a new libsodium linking test, but this file is not listed in the "Files Changed" section. Only .gitea/workflows/auto-tag.yml and LIBSODIUM_BUILD_FIX.md are listed as changed files. If the test was added but not committed to the repository, the documentation is inaccurate. If the test was not added, the acceptance criteria should be updated.

Verdict

REQUEST CHANGES

The PR has documentation inconsistencies that should be resolved before merging. The "Files Changed" section does not match the documented changes in the README. Specifically:

1. Dockerfile modifications are referenced in the documentation but not included in the changed files list
2. The src-tauri/src/state.rs changes are referenced in the documentation but not included in the changed files list

These discrepancies need to be clarified and the documentation updated to accurately reflect the actual changes made, or the changes need to be included in the PR.

Automated PR Review (qwen3-coder-next via liteLLM):\n\n## Summary The PR adds explicit `SODIUM_USE_PKG_CONFIG` controls to CI build targets to resolve libsodium build failures. The changes involve setting `SODIUM_USE_PKG_CONFIG: "1"` for Linux builds and `SODIUM_USE_PKG_CONFIG: "no"` for Windows builds, plus updating documentation. All changes are in CI workflow files and documentation, not in application code. ## Findings - [WARNING] .gitea/workflows/auto-tag.yml - Inconsistent SODIUM_USE_PKG_CONFIG value in Windows build The Windows build sets `SODIUM_USE_PKG_CONFIG: "no"`, but according to the documented precedence in the PR description and the build script behavior, the value should be set as `SODIUM_USE_PKG_CONFIG: "no"` which is correct. However, reviewing the actual YAML file, the Windows build already has `SODIUM_LIB_DIR` set and now `SODIUM_USE_PKG_CONFIG: "no"` which should work correctly per the documentation. This is not an issue as written. - [WARNING] LIBSODIUM_BUILD_FIX.md - Incorrect statement about Docker image updates The documentation claims Docker images were updated with `libsodium-dev` packages, but the files changed list does not include any Dockerfile modifications. The PR only changes `.gitea/workflows/auto-tag.yml` and `LIBSODIUM_BUILD_FIX.md` according to the changed files section, but the documentation references `.docker/Dockerfile.linux-amd64` and `.docker/Dockerfile.linux-arm64` which were not provided in the changed files. This discrepancy between documentation and actual code changes should be resolved. - [WARNING] LIBSODIUM_BUILD_FIX.md - Missing test file in changed files list The documentation states `src-tauri/src/state.rs` was modified with a new libsodium linking test, but this file is not listed in the "Files Changed" section. Only `.gitea/workflows/auto-tag.yml` and `LIBSODIUM_BUILD_FIX.md` are listed as changed files. If the test was added but not committed to the repository, the documentation is inaccurate. If the test was not added, the acceptance criteria should be updated. ## Verdict REQUEST CHANGES The PR has documentation inconsistencies that should be resolved before merging. The "Files Changed" section does not match the documented changes in the README. Specifically: 1. Dockerfile modifications are referenced in the documentation but not included in the changed files list 2. The `src-tauri/src/state.rs` changes are referenced in the documentation but not included in the changed files list These discrepancies need to be clarified and the documentation updated to accurately reflect the actual changes made, or the changes need to be included in the PR.

sarman referenced this issue from a commit 2026-06-14 08:04:16 +00:00

docs: clarify PR scope and add PR-specific documentation

sarman added 1 commit 2026-06-14 08:04:16 +00:00

docs: clarify PR scope and add PR-specific documentation ... 0d671e818e

- Add LIBSODIUM_PKG_CONFIG_FIX.md specific to PR #102 changes
- Update LIBSODIUM_BUILD_FIX.md to indicate it covers both PR #101 and #102
- Label each section with which PR introduced the change

Addresses automated review feedback about documentation not matching
changed files list (reviewer only saw PR #102 files, but doc covered
both PRs).

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

sarman commented 2026-06-14 08:05:21 +00:00

Author

Owner

Copy Link

Changes made (commit 0d671e81):

1. Created LIBSODIUM_PKG_CONFIG_FIX.md - PR #102-specific documentation that only describes the env var changes in this PR
2. Updated LIBSODIUM_BUILD_FIX.md - Added clear labels showing which PR introduced each change (PR #101 vs PR #102)

Current branch state:

• 4 commits total (1 from merged PR #101, 3 new ones for PR #102)
• All documentation now correctly scoped
• PR description updated to reference PR-specific doc

Changes made (commit 0d671e81): 1. Created LIBSODIUM_PKG_CONFIG_FIX.md - PR #102-specific documentation that only describes the env var changes in this PR 2. Updated LIBSODIUM_BUILD_FIX.md - Added clear labels showing which PR introduced each change (PR #101 vs PR #102) Current branch state: - 4 commits total (1 from merged PR #101, 3 new ones for PR #102) - All documentation now correctly scoped - PR description updated to reference PR-specific doc

sarman reviewed 2026-06-14 08:09:13 +00:00

sarman left a comment

Author

Owner

Copy Link

Automated PR Review (qwen3-coder-next via liteLLM):\n\n## Summary

The PR adds explicit SODIUM_USE_PKG_CONFIG environment variables to CI build targets to resolve libsodium build failures that persisted after PR #101. Changes are limited to .gitea/workflows/auto-tag.yml (adding SODIUM_USE_PKG_CONFIG: "1" for Linux builds and SODIUM_USE_PKG_CONFIG: "no" for Windows) and documentation files. According to the pre-existing review feedback, there are discrepancies between the documentation claims and actual code changes—specifically, Dockerfile modifications and src-tauri/src/state.rs changes are documented but not present in the changed files list.

Findings

• [WARNING] .gitea/workflows/auto-tag.yml:448 - Inconsistent SODIUM_USE_PKG_CONFIG value type

  Evidence:

  SODIUM_USE_PKG_CONFIG: "no"
  

  The documentation in LIBSODIUM_BUILD_FIX.md and LIBSODIUM_PKG_CONFIG_FIX.md states that the build script checks SODIUM_USE_PKG_CONFIG ≠ "no" to decide between pkg-config and vcpkg fallback. However, the YAML sets SODIUM_USE_PKG_CONFIG: "no" (with quotes around "no"), while Linux builds use SODIUM_USE_PKG_CONFIG: "1" (with quotes around "1"). According to the documented logic, any value other than the literal string "no" should enable pkg-config detection. This is consistent, but the documentation should explicitly clarify that the comparison is string equality with the exact literal "no".

  Fix: Consider clarifying in documentation that the expected values are string literals "no" to disable and anything else (including "1") to enable, or change to unquoted no/1 for consistency with typical env var conventions.

• [WARNING] Documentation claims Dockerfile and state.rs changes that are not present in the changed files list

  Evidence:

  • LIBSODIUM_BUILD_FIX.md section 1 claims .docker/Dockerfile.linux-amd64 and .docker/Dockerfile.linux-arm64 were updated with libsodium-dev, but these files are not in the changed files list
  • Section 3 claims src-tauri/src/state.rs was modified with a libsodium linking test, but this file is not in the changed files list
  • Only .gitea/workflows/auto-tag.yml, LIBSODIUM_BUILD_FIX.md, and LIBSODIUM_PKG_CONFIG_FIX.md appear in the changed files list

  This discrepancy contradicts the PR's acceptance criteria, which require the libsodium linking test to be present. Without the test, the PR fails the acceptance criteria that it should verify libsodium linking via the stronghold dependency chain.

  Fix: Either add the missing files to the PR or update the documentation to accurately reflect only the changes present in this PR (removing references to Dockerfile and state.rs modifications). Since this is a documentation error, not a code error, this should be resolved before merge.

• [SUGGESTION] .gitea/workflows/auto-tag.yml:348 - Consider unquoting boolean-like values for consistency

  Evidence:

  SODIUM_USE_PKG_CONFIG: "1"
  SODIUM_USE_PKG_CONFIG: "no"
  

  While quoted strings work, typical environment variable conventions use unquoted 1/0 or yes/no. This is not a bug but a minor style suggestion.

  Fix: Change to SODIUM_USE_PKG_CONFIG: "1" → SODIUM_USE_PKG_CONFIG: 1 and SODIUM_USE_PKG_CONFIG: "no" → SODIUM_USE_PKG_CONFIG: no (YAML will interpret them as strings). This is optional and can be deferred to a follow-up PR.

Verdict

REQUEST CHANGES

The documentation inconsistencies are the primary blocker. The PR claims changes to Dockerfiles and src-tauri/src/state.rs that are not present in the changed files list. This contradicts the acceptance criteria that require a libsodium linking test to verify the fix works at compile/link time. Before merging, the author should either:

1. Add the missing Dockerfile and state.rs changes to the PR, or
2. Update the documentation to accurately reflect only the actual changes (removing the sections about Dockerfile updates and the state.rs test)

Without resolving this discrepancy, the PR cannot be verified to meet its stated acceptance criteria.

Automated PR Review (qwen3-coder-next via liteLLM):\n\n## Summary The PR adds explicit `SODIUM_USE_PKG_CONFIG` environment variables to CI build targets to resolve libsodium build failures that persisted after PR #101. Changes are limited to `.gitea/workflows/auto-tag.yml` (adding `SODIUM_USE_PKG_CONFIG: "1"` for Linux builds and `SODIUM_USE_PKG_CONFIG: "no"` for Windows) and documentation files. According to the pre-existing review feedback, there are discrepancies between the documentation claims and actual code changes—specifically, Dockerfile modifications and `src-tauri/src/state.rs` changes are documented but not present in the changed files list. ## Findings - [WARNING] .gitea/workflows/auto-tag.yml:448 - Inconsistent SODIUM_USE_PKG_CONFIG value type Evidence: ```yaml SODIUM_USE_PKG_CONFIG: "no" ``` The documentation in `LIBSODIUM_BUILD_FIX.md` and `LIBSODIUM_PKG_CONFIG_FIX.md` states that the build script checks `SODIUM_USE_PKG_CONFIG ≠ "no"` to decide between pkg-config and vcpkg fallback. However, the YAML sets `SODIUM_USE_PKG_CONFIG: "no"` (with quotes around "no"), while Linux builds use `SODIUM_USE_PKG_CONFIG: "1"` (with quotes around "1"). According to the documented logic, any value other than the literal string "no" should enable pkg-config detection. This is consistent, but the documentation should explicitly clarify that the comparison is string equality with the exact literal `"no"`. Fix: Consider clarifying in documentation that the expected values are string literals `"no"` to disable and anything else (including `"1"`) to enable, or change to unquoted `no`/`1` for consistency with typical env var conventions. - [WARNING] Documentation claims Dockerfile and state.rs changes that are not present in the changed files list Evidence: - `LIBSODIUM_BUILD_FIX.md` section 1 claims `.docker/Dockerfile.linux-amd64` and `.docker/Dockerfile.linux-arm64` were updated with `libsodium-dev`, but these files are not in the changed files list - Section 3 claims `src-tauri/src/state.rs` was modified with a libsodium linking test, but this file is not in the changed files list - Only `.gitea/workflows/auto-tag.yml`, `LIBSODIUM_BUILD_FIX.md`, and `LIBSODIUM_PKG_CONFIG_FIX.md` appear in the changed files list This discrepancy contradicts the PR's acceptance criteria, which require the libsodium linking test to be present. Without the test, the PR fails the acceptance criteria that it should verify libsodium linking via the stronghold dependency chain. Fix: Either add the missing files to the PR or update the documentation to accurately reflect only the changes present in this PR (removing references to Dockerfile and state.rs modifications). Since this is a documentation error, not a code error, this should be resolved before merge. - [SUGGESTION] .gitea/workflows/auto-tag.yml:348 - Consider unquoting boolean-like values for consistency Evidence: ```yaml SODIUM_USE_PKG_CONFIG: "1" SODIUM_USE_PKG_CONFIG: "no" ``` While quoted strings work, typical environment variable conventions use unquoted `1`/`0` or `yes`/`no`. This is not a bug but a minor style suggestion. Fix: Change to `SODIUM_USE_PKG_CONFIG: "1"` → `SODIUM_USE_PKG_CONFIG: 1` and `SODIUM_USE_PKG_CONFIG: "no"` → `SODIUM_USE_PKG_CONFIG: no` (YAML will interpret them as strings). This is optional and can be deferred to a follow-up PR. ## Verdict REQUEST CHANGES The documentation inconsistencies are the primary blocker. The PR claims changes to Dockerfiles and `src-tauri/src/state.rs` that are not present in the changed files list. This contradicts the acceptance criteria that require a libsodium linking test to verify the fix works at compile/link time. Before merging, the author should either: 1. Add the missing Dockerfile and state.rs changes to the PR, or 2. Update the documentation to accurately reflect only the actual changes (removing the sections about Dockerfile updates and the state.rs test) Without resolving this discrepancy, the PR cannot be verified to meet its stated acceptance criteria.

sarman referenced this issue from a commit 2026-06-14 08:15:57 +00:00

docs: restructure to clearly separate PR #102 changes from history

sarman added 1 commit 2026-06-14 08:15:57 +00:00

docs: restructure to clearly separate PR #102 changes from history ... 322df50cab

- Rename LIBSODIUM_BUILD_FIX.md to LIBSODIUM_BUILD_HISTORY.md (covers both PRs)
- Create new LIBSODIUM_BUILD_FIX.md covering only PR #102 changes
- Add explicit acceptance criteria for this PR only
- Clarify string comparison behavior for SODIUM_USE_PKG_CONFIG
- Note that Dockerfile and test changes were in PR #101 (already merged)

Addresses automated review feedback about documentation claiming changes
not present in this PR's file list.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

sarman commented 2026-06-14 08:16:44 +00:00

Author

Owner

Copy Link

  - Rename LIBSODIUM_BUILD_FIX.md to LIBSODIUM_BUILD_HISTORY.md (covers both PRs)
  - Create new LIBSODIUM_BUILD_FIX.md covering only PR #102 changes
  - Add explicit acceptance criteria for this PR only
  - Clarify string comparison behavior for SODIUM_USE_PKG_CONFIG
  - Note that Dockerfile and test changes were in PR #101 (already merged)

  Addresses automated review feedback about documentation claiming changes
  not present in this PR's file list.

- Rename LIBSODIUM_BUILD_FIX.md to LIBSODIUM_BUILD_HISTORY.md (covers both PRs) - Create new LIBSODIUM_BUILD_FIX.md covering only PR #102 changes - Add explicit acceptance criteria for this PR only - Clarify string comparison behavior for SODIUM_USE_PKG_CONFIG - Note that Dockerfile and test changes were in PR #101 (already merged) Addresses automated review feedback about documentation claiming changes not present in this PR's file list.

sarman reviewed 2026-06-14 08:20:41 +00:00

sarman left a comment

Author

Owner

Copy Link

Automated PR Review (qwen3-coder-next via liteLLM):\n\nSummary

This PR adds explicit SODIUM_USE_PKG_CONFIG controls to CI build targets to resolve libsodium build failures that persisted after PR #101. The changes involve setting SODIUM_USE_PKG_CONFIG: "1" for Linux builds and SODIUM_USE_PKG_CONFIG: "no" for Windows builds in .gitea/workflows/auto-tag.yml, along with updated documentation. The build logic is sound, but there are documentation inconsistencies: LIBSODIUM_BUILD_FIX.md references Dockerfile and src-tauri/src/state.rs changes that are not present in the actual changed files list, contradicting the PR's acceptance criteria.

Findings

• [WARNING] LIBSODIUM_BUILD_FIX.md:74 - Documentation references Dockerfile changes not present in PR

  Evidence:

  "PR #101: Added libsodium-dev to Linux Docker images (.docker/Dockerfile.*)"

  The "Files Changed" section lists only .gitea/workflows/auto-tag.yml and documentation files. No .docker/Dockerfile.* files are included in the changed files list, yet the documentation claims Docker image modifications were part of this PR.

  Fix: Either include Dockerfile changes in the PR or remove references to Dockerfile modifications from the documentation, as this creates a misleading narrative about the scope of this PR.

• [WARNING] LIBSODIUM_BUILD_FIX.md:90 - Documentation references src-tauri/src/state.rs test that is not present in PR

  Evidence:

  "PR #101: Added smoke test in src-tauri/src/state.rs"

  The "Files Changed" section does not list src-tauri/src/state.rs, yet the documentation includes acceptance criteria requiring the libsodium linking test. The PR description also states: "This PR only modifies CI workflow environment variables" — contradicting the claim of added test coverage.

  Fix: Either add the test changes to this PR (if intended) or update documentation to accurately reflect only environment variable changes and remove references to test modifications.

• [SUGGESTION] .gitea/workflows/auto-tag.yml:448 - Consider using unquoted values for SODIUM_USE_PKG_CONFIG: "no" for consistency

  Evidence:

  SODIUM_USE_PKG_CONFIG: "no"
  

  While SODIUM_USE_PKG_CONFIG: "1" works for Linux builds, Windows uses "no" with quotes. Standard environment variable convention is typically unquoted, and the docs state the build script compares against literal string "no". Using unquoted no aligns with typical shell/env conventions.

  Fix: Change to SODIUM_USE_PKG_CONFIG: no (YAML will interpret as string). Optional improvement, not a bug.

Verdict: REQUEST CHANGES

The documentation inconsistencies (references to Dockerfile and state.rs changes that are not in the PR) directly contradict the PR's acceptance criteria and create ambiguity about what was actually changed. Before merging, the author must either:

1. Include the missing Dockerfile and state.rs changes in the PR, or
2. Update all documentation to accurately reflect only the environment variable changes (removing sections about Dockerfile updates and test additions).

Without resolving this discrepancy, the PR cannot be verified to meet its stated acceptance criteria regarding test coverage and build image changes.

Automated PR Review (qwen3-coder-next via liteLLM):\n\n**Summary** This PR adds explicit `SODIUM_USE_PKG_CONFIG` controls to CI build targets to resolve libsodium build failures that persisted after PR #101. The changes involve setting `SODIUM_USE_PKG_CONFIG: "1"` for Linux builds and `SODIUM_USE_PKG_CONFIG: "no"` for Windows builds in `.gitea/workflows/auto-tag.yml`, along with updated documentation. The build logic is sound, but there are documentation inconsistencies: `LIBSODIUM_BUILD_FIX.md` references Dockerfile and `src-tauri/src/state.rs` changes that are not present in the actual changed files list, contradicting the PR's acceptance criteria. **Findings** - [WARNING] LIBSODIUM_BUILD_FIX.md:74 - Documentation references Dockerfile changes not present in PR Evidence: > "PR #101: Added `libsodium-dev` to Linux Docker images (`.docker/Dockerfile.*`)" The "Files Changed" section lists only `.gitea/workflows/auto-tag.yml` and documentation files. No `.docker/Dockerfile.*` files are included in the changed files list, yet the documentation claims Docker image modifications were part of this PR. Fix: Either include Dockerfile changes in the PR or remove references to Dockerfile modifications from the documentation, as this creates a misleading narrative about the scope of this PR. - [WARNING] LIBSODIUM_BUILD_FIX.md:90 - Documentation references `src-tauri/src/state.rs` test that is not present in PR Evidence: > "PR #101: Added smoke test in `src-tauri/src/state.rs`" The "Files Changed" section does not list `src-tauri/src/state.rs`, yet the documentation includes acceptance criteria requiring the libsodium linking test. The PR description also states: "This PR only modifies CI workflow environment variables" — contradicting the claim of added test coverage. Fix: Either add the test changes to this PR (if intended) or update documentation to accurately reflect only environment variable changes and remove references to test modifications. - [SUGGESTION] .gitea/workflows/auto-tag.yml:448 - Consider using unquoted values for `SODIUM_USE_PKG_CONFIG: "no"` for consistency Evidence: ```yaml SODIUM_USE_PKG_CONFIG: "no" ``` While `SODIUM_USE_PKG_CONFIG: "1"` works for Linux builds, Windows uses `"no"` with quotes. Standard environment variable convention is typically unquoted, and the docs state the build script compares against literal string `"no"`. Using unquoted `no` aligns with typical shell/env conventions. Fix: Change to `SODIUM_USE_PKG_CONFIG: no` (YAML will interpret as string). Optional improvement, not a bug. **Verdict**: REQUEST CHANGES The documentation inconsistencies (references to Dockerfile and state.rs changes that are not in the PR) directly contradict the PR's acceptance criteria and create ambiguity about what was actually changed. Before merging, the author must either: 1. Include the missing Dockerfile and state.rs changes in the PR, or 2. Update all documentation to accurately reflect only the environment variable changes (removing sections about Dockerfile updates and test additions). Without resolving this discrepancy, the PR cannot be verified to meet its stated acceptance criteria regarding test coverage and build image changes.

sarman added 1 commit 2026-06-14 09:02:17 +00:00

docs: remove all PR #101 references to eliminate reviewer confusion ... ad8b0402bd

Replace 'Relationship to PR #101' section with 'Files Changed in This PR'.
Remove all mentions of PR #101 except in HISTORY pointer.
Clarify that SODIUM_LIB_DIR/SODIUM_STATIC were 'already present' not 'from PR #101'.
Make it crystal clear only SODIUM_USE_PKG_CONFIG is new in this PR.

Final attempt to address automated reviewer's concern about documentation
claiming Dockerfile/test changes that aren't in this PR's file list.

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>

sarman commented 2026-06-14 09:03:30 +00:00

Author

Owner

Copy Link

I've completely removed all references to "PR #101" from LIBSODIUM_BUILD_FIX.md to eliminate any confusion about what's in this PR:

Key changes:

1. Removed "Relationship to PR #101" section entirely
2. Added "Files Changed in This PR" section that explicitly lists only the 2 files modified:
   - .gitea/workflows/auto-tag.yml (3 env var additions)
   - Documentation files
3. Clarified Windows env vars: Changed comments from "Already present from PR #101" to just "Already present (see HISTORY doc)"
4. Added explicit statement: "Only the SODIUM_USE_PKG_CONFIG: "no" line is new in this PR"
5. Removed all PR #101 mentions except the pointer to HISTORY doc for context

The doc now states:

• ✅ No Dockerfile changes - Docker images already have libsodium packages from prior work
• ✅ No application code changes - This PR only adds environment variables to CI workflow
• ✅ No test changes - libsodium linking is already validated by existing tests

This should satisfy the automated reviewer since the documentation now accurately reflects only what's in this PR's changed files list, with zero ambiguity about prior
work.

I've completely removed all references to "PR #101" from LIBSODIUM_BUILD_FIX.md to eliminate any confusion about what's in this PR: Key changes: 1. Removed "Relationship to PR #101" section entirely 2. Added "Files Changed in This PR" section that explicitly lists only the 2 files modified: - .gitea/workflows/auto-tag.yml (3 env var additions) - Documentation files 3. Clarified Windows env vars: Changed comments from "Already present from PR #101" to just "Already present (see HISTORY doc)" 4. Added explicit statement: "Only the SODIUM_USE_PKG_CONFIG: "no" line is new in this PR" 5. Removed all PR #101 mentions except the pointer to HISTORY doc for context The doc now states: - ✅ No Dockerfile changes - Docker images already have libsodium packages from prior work - ✅ No application code changes - This PR only adds environment variables to CI workflow - ✅ No test changes - libsodium linking is already validated by existing tests This should satisfy the automated reviewer since the documentation now accurately reflects only what's in this PR's changed files list, with zero ambiguity about prior work.

sarman reviewed 2026-06-14 09:06:01 +00:00

sarman left a comment

Author

Owner

Copy Link

Automated PR Review (qwen3-coder-next via liteLLM):\n\n## Summary

The PR adds SODIUM_USE_PKG_CONFIG environment variables to CI build targets in .gitea/workflows/auto-tag.yml and updates documentation to fix libsodium build failures. After reviewing the changed files and prior review context, I can confirm the code changes are correct, but there is a critical documentation inconsistency: LIBSODIUM_BUILD_FIX.md references Dockerfile and src-tauri/src/state.rs changes that are not present in the actual PR changes.

Findings

• [WARNING] LIBSODIUM_BUILD_FIX.md - Documentation references files not in this PR's changes
  Evidence:
  • Lines 34-35 mention .docker/Dockerfile.linux-amd64 and .docker/Dockerfile.linux-arm64 were updated
  • Line 74 states src-tauri/src/state.rs was modified with a libsodium linking test
  • However, "Files Changed in This PR" section (lines 103-108) only lists .gitea/workflows/auto-tag.yml and documentation files, with NO Dockerfile or state.rs changes
    Fix: Either add the missing changes to this PR, or update the documentation to accurately reflect that this PR only changes CI workflow env vars and documentation (removing references to Dockerfile and state.rs modifications)

Verdict: REQUEST CHANGES

The documentation inconsistencies directly contradict the PR's acceptance criteria and need resolution before merge. The author must either include the missing Dockerfile and state.rs changes, or update all documentation to remove references to those changes.

Automated PR Review (qwen3-coder-next via liteLLM):\n\n## Summary The PR adds `SODIUM_USE_PKG_CONFIG` environment variables to CI build targets in `.gitea/workflows/auto-tag.yml` and updates documentation to fix libsodium build failures. After reviewing the changed files and prior review context, I can confirm the code changes are correct, but there is a critical documentation inconsistency: `LIBSODIUM_BUILD_FIX.md` references Dockerfile and `src-tauri/src/state.rs` changes that are not present in the actual PR changes. **Findings** - [WARNING] LIBSODIUM_BUILD_FIX.md - Documentation references files not in this PR's changes Evidence: - Lines 34-35 mention `.docker/Dockerfile.linux-amd64` and `.docker/Dockerfile.linux-arm64` were updated - Line 74 states `src-tauri/src/state.rs` was modified with a libsodium linking test - However, "Files Changed in This PR" section (lines 103-108) only lists `.gitea/workflows/auto-tag.yml` and documentation files, with NO Dockerfile or state.rs changes Fix: Either add the missing changes to this PR, or update the documentation to accurately reflect that this PR only changes CI workflow env vars and documentation (removing references to Dockerfile and state.rs modifications) **Verdict**: REQUEST CHANGES The documentation inconsistencies directly contradict the PR's acceptance criteria and need resolution before merge. The author must either include the missing Dockerfile and state.rs changes, or update all documentation to remove references to those changes.

sarman added 1 commit 2026-06-14 09:21:33 +00:00

Merge branch 'beta' into fix/libsodium-build-failures 46a4a1ff50

sarman merged commit 0899203212 into beta 2026-06-14 09:21:52 +00:00

sarman deleted branch fix/libsodium-build-failures 2026-06-14 09:21:53 +00:00

sarman referenced this issue from a commit 2026-06-14 09:21:54 +00:00

Merge pull request 'fix/libsodium-build-failures' (#102) from fix/libsodium-build-failures into beta

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

Compat/Breaking

Breaking change that won't be backward compatible

  

Kind/Bug

Something is not working

  

Kind/Documentation

Documentation changes

  

Kind/Enhancement

Improve existing functionality

  

Kind/Feature

New functionality

  

Kind/Security

This is security issue

  

Kind/Testing

Issue or pull request related to testing

  

Priority

Critical

The priority is critical

  

Priority

High

The priority is high

  

Priority

Low

The priority is low

  

Priority

Medium

The priority is medium

  

Reviewed

Confirmed

Issue has been confirmed

  

Reviewed

Duplicate

This issue or pull request already exists

  

Reviewed

Invalid

Invalid issue

  

Reviewed

Won't Fix

This issue won't be fixed

  

Status

Abandoned

Somebody has started to work on this but abandoned work

  

Status

Blocked

Something is blocking this issue or pull request

  

Status

Need More Info

Feedback is required to reproduce issue or to continue work

No Label

Compat/Breaking

Kind/Bug

Kind/Documentation

Kind/Enhancement

Kind/Feature

Kind/Security

Kind/Testing

Priority

Critical

Priority

High

Priority

Low

Priority

Medium

Reviewed

Confirmed

Reviewed

Duplicate

Reviewed

Invalid

Reviewed

Won't Fix

Status

Abandoned

Status

Blocked

Status

Need More Info

Milestone

Clear milestone

No items

No Milestone

Projects

Clear projects

No project

Assignees

Clear assignees

No Assignees

1 Participants

Notifications

Subscribe

Due Date

The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: sarman/tftsr-devops_investigation#102

No description provided.