chore: merge release v0.38.1 #11020
Merged
chore: merge release v0.38.1 #11020
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* upgrade go-ds-pebble to v0.5.2 * Change pebble option `MaxConcurrentCompactions` to `CompactionConcurrencyRange` (cherry picked from commit 9370004)
This fixes a panic that happens when reusing a batch go-datastore Batch. (cherry picked from commit 2b5adee)
* test: add migration tests for Windows and macOS - add dedicated CI workflow for migration tests on Windows/macOS - workflow triggers on migration-related file changes only * build: remove redundant go version checks - remove GO_MIN_VERSION and check_go_version scripts - go.mod already enforces minimum version (go 1.25) - fixes make build on Windows * fix: windows migration panic by reading config into memory fixes migration panic on Windows when upgrading from v0.37 to v0.38 by reading the entire config file into memory before performing atomic operations. this avoids file locking issues on Windows where open files cannot be renamed. also fixes: - TestRepoDir to set USERPROFILE on Windows (not just HOME) - CLI migration tests to sanitize directory names (remove colons) minimal fix that solves the "panic: error can't be dealt with transactionally: Access is denied" error without adding unnecessary platform-specific complexity. * fix: set PATH for CLI migration tests in CI the CLI tests need the built ipfs binary to be in PATH * fix: use ipfs shutdown for graceful daemon termination in tests replaces platform-specific signal handling with ipfs shutdown command which works consistently across all platforms including Windows * fix: isolate PATH modifications in parallel migration tests tests running in parallel with t.Parallel() were interfering with each other through global PATH modifications via os.Setenv(). this caused tests to download real migration binaries instead of using mocks, leading to Windows failures due to path separator issues in external tools. now each test builds its own custom PATH and passes it explicitly to commands, preventing interference between parallel tests. * chore: improve error messages in WithBackup * fix: Windows CI migration test failures - add .exe extension to mock migration binaries on Windows - handle repo lock file properly in mock migration binary - ensure lock is created and removed to prevent conflicts * refactor: align atomicfile error handling with fs-repo-migrations - check close error in Abort() before attempting removal - leave temp file on rename failure for debugging (like fs-repo-15-to-16) - improves consistency with external migration implementations * fix: use req.Context in repo migrate to avoid double-lock The repo migrate command was calling cctx.Context() which has a hidden side effect: it lazily constructs the IPFS node by calling GetNode(), which opens the repository and acquires repo.lock. When migrations then tried to acquire the same lock, it failed with "lock is already held by us" because go4.org/lock tracks locks per-process in a global map. The fix uses req.Context instead, which is a plain context.Context with no side effects. This provides what migrations need (cancellation handling) without triggering node construction or repo opening. Context types explained: - req.Context: Standard Go context for request lifetime, cancellation, and timeouts. No side effects. - cctx.Context(): Kubo-specific method that lazily constructs the full IPFS node (opens repo, acquires lock, initializes subsystems). Returns the node's internal context. Why req.Context is correct here: - Migrations work on raw filesystem (only need ConfigRoot path) - Command has SetDoesNotUseRepo(true) - doesn't need running node - Migrations handle their own locking via lockfile.Lock() - Need cancellation support but not node lifecycle The bug only appeared with embedded migrations (v16+) because they run in-process. External migrations (pre-v16) were separate processes, so each had isolated state. Sequential migrations (forward then backward) in the same process exposed this latent double-lock issue. Also adds repo.lock acquisition to RunEmbeddedMigrations to prevent concurrent migration access, and removes the now-unnecessary daemon lock check from the migrate command handler. * fix: use req.Context for migrations and autoconf in daemon startup daemon.go was incorrectly using cctx.Context() in two critical places: 1. Line 337: migrations call - cctx.Context() triggers GetNode() which opens the repo and acquires repo.lock BEFORE migrations run, causing "lock is already held by us" errors when migrations try to lock 2. Line 390: autoconf client.Start() - uses context for HTTP timeouts and background updater lifecycle, doesn't need node construction Both now use req.Context (plain Go context) which provides: - request lifetime and cancellation - no side effects (doesn't construct node or open repo) - correct lifecycle for HTTP requests and background goroutines (cherry picked from commit f4834e7)
Release v0.38.1
keep -dev version from master
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.