@cryptotaxi247 / kubo / commits / b48f7ae42

fix: migrations for Windows (#11010)

* 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 f4834e797db174e0630aeee4b6dc68d133106f48)

Marcin Rataj committed Oct 8, 2025 at 18:02 UTC b48f7ae429cf2a2195f1061cf854a0b35b8567d9

15 files changed +693 -267

.github/workflows/test-migrations.yml new

+85

@@ -0,0 +1,85 @@

1 +name: Migrations

2 +

3 +on:

4 + workflow_dispatch:

5 + pull_request:

6 + paths:

7 + # Migration implementation files

8 + - 'repo/fsrepo/migrations/**'

9 + - 'test/cli/migrations/**'

10 + # Config and repo handling

11 + - 'repo/fsrepo/**'

12 + # This workflow file itself

13 + - '.github/workflows/test-migrations.yml'

14 + push:

15 + branches:

16 + - 'master'

17 + - 'release-*'

18 + paths:

19 + - 'repo/fsrepo/migrations/**'

20 + - 'test/cli/migrations/**'

21 + - 'repo/fsrepo/**'

22 + - '.github/workflows/test-migrations.yml'

23 +

24 +concurrency:

25 + group: ${{ github.workflow }}-${{ github.event_name }}-${{ github.event_name == 'push' && github.sha || github.ref }}

26 + cancel-in-progress: true

27 +

28 +jobs:

29 + test:

30 + strategy:

31 + fail-fast: false

32 + matrix:

33 + os: [ubuntu-latest, windows-latest, macos-latest]

34 + runs-on: ${{ matrix.os }}

35 + timeout-minutes: 20

36 + env:

37 + TEST_VERBOSE: 1

38 + IPFS_CHECK_RCMGR_DEFAULTS: 1

39 + defaults:

40 + run:

41 + shell: bash

42 + steps:

43 + - name: Check out Kubo

44 + uses: actions/checkout@v5

45 +

46 + - name: Set up Go

47 + uses: actions/setup-go@v6

48 + with:

49 + go-version-file: 'go.mod'

50 +

51 + - name: Build kubo binary

52 + run: |

53 + make build

54 + echo "Built ipfs binary at $(pwd)/cmd/ipfs/"

55 +

56 + - name: Add kubo to PATH

57 + run: |

58 + echo "$(pwd)/cmd/ipfs" >> $GITHUB_PATH

59 +

60 + - name: Verify ipfs in PATH

61 + run: |

62 + which ipfs || echo "ipfs not in PATH"

63 + ipfs version || echo "Failed to run ipfs version"

64 +

65 + - name: Run migration unit tests

66 + run: |

67 + go test ./repo/fsrepo/migrations/...

68 +

69 + - name: Run CLI migration tests

70 + env:

71 + IPFS_PATH: ${{ runner.temp }}/ipfs-test

72 + run: |

73 + export PATH="${{ github.workspace }}/cmd/ipfs:$PATH"

74 + which ipfs || echo "ipfs not found in PATH"

75 + ipfs version || echo "Failed to run ipfs version"

76 + go test ./test/cli/migrations/...

77 +

78 + - name: Upload test results

79 + if: always()

80 + uses: actions/upload-artifact@v4

81 + with:

82 + name: ${{ matrix.os }}-test-results

83 + path: |

84 + test/**/*.log

85 + ${{ runner.temp }}/ipfs-test/

bin/check_go_version deleted

-44

@@ -1,44 +0,0 @@

1 -#!/bin/sh

2 -#

3 -# Check that the go version is at least equal to a minimum version

4 -# number.

5 -#

6 -# Call it for example like this:

7 -#

8 -# $ check_go_version "1.5.2"

9 -#

10 -

11 -USAGE="$0 GO_MIN_VERSION"

12 -

13 -die() {

14 - printf >&2 "fatal: %s\n" "$@"

15 - exit 1

16 -}

17 -

18 -# Get arguments

19 -

20 -test "$#" -eq "1" || die "This program must be passed exactly 1 arguments" "Usage: $USAGE"

21 -

22 -GO_MIN_VERSION="$1"

23 -

24 -UPGRADE_MSG="Please take a look at https://golang.org/doc/install to install or upgrade go."

25 -

26 -# Get path to the directory containing this file

27 -# If $0 has no slashes, uses "./"

28 -PREFIX=$(expr "$0" : "$.*\/$") || PREFIX='./'

29 -# Include the 'check_at_least_version' function

30 -. ${PREFIX}check_version

31 -

32 -# Check that the go binary exists and is in the path

33 -

34 -GOCC=${GOCC="go"}

35 -

36 -type ${GOCC} >/dev/null 2>&1 || die_upgrade "go is not installed or not in the PATH!"

37 -

38 -# Check the go binary version

39 -

40 -VERS_STR=$(${GOCC} version 2>&1) || die "'go version' failed with output: $VERS_STR"

41 -

42 -GO_CUR_VERSION=$(expr "$VERS_STR" : ".*go version.* go$[^[:space:]]*$ .*") || die "Invalid 'go version' output: $VERS_STR"

43 -

44 -check_at_least_version "$GO_MIN_VERSION" "$GO_CUR_VERSION" "${GOCC}"

bin/check_version deleted

-77

@@ -1,77 +0,0 @@

1 -#!/bin/sh

2 -

3 -if test "x$UPGRADE_MSG" = "x"; then

4 - printf >&2 "fatal: Please set '"'$UPGRADE_MSG'"' before sourcing this script\n"

5 - exit 1

6 -fi

7 -

8 -die_upgrade() {

9 - printf >&2 "fatal: %s\n" "$@"

10 - printf >&2 "=> %s\n" "$UPGRADE_MSG"

11 - exit 1

12 -}

13 -

14 -major_number() {

15 - vers="$1"

16 -

17 - # Hack around 'expr' exiting with code 1 when it outputs 0

18 - case "$vers" in

19 - 0) echo "0" ;;

20 - 0.*) echo "0" ;;

21 - *) expr "$vers" : "$[^.]*$.*" || return 1

22 - esac

23 -}

24 -

25 -check_at_least_version() {

26 - MIN_VERS="$1"

27 - CUR_VERS="$2"

28 - PROG_NAME="$3"

29 -

30 - # Get major, minor and fix numbers for each version

31 - MIN_MAJ=$(major_number "$MIN_VERS") || die "No major version number in '$MIN_VERS' for '$PROG_NAME'"

32 - CUR_MAJ=$(major_number "$CUR_VERS") || die "No major version number in '$CUR_VERS' for '$PROG_NAME'"

33 -

34 - # We expect a version to be of form X.X.X

35 - # if the second dot doesn't match, we consider it a prerelease

36 -

37 - if MIN_MIN=$(expr "$MIN_VERS" : "[^.]*\.$[0-9][0-9]*$"); then

38 - # this captured digit is necessary, since expr returns code 1 if the output is empty

39 - if expr "$MIN_VERS" : "[^.]*\.[0-9]*$[0-9]\.\|[0-9]\$$" >/dev/null; then

40 - MIN_PRERELEASE="0"

41 - else

42 - MIN_PRERELEASE="1"

43 - fi

44 - MIN_FIX=$(expr "$MIN_VERS" : "[^.]*\.[0-9][0-9]*[^0-9][^0-9]*$[0-9][0-9]*$") || MIN_FIX="0"

45 - else

46 - MIN_MIN="0"

47 - MIN_PRERELEASE="0"

48 - MIN_FIX="0"

49 - fi

50 - if CUR_MIN=$(expr "$CUR_VERS" : "[^.]*\.$[0-9][0-9]*$"); then

51 - # this captured digit is necessary, since expr returns code 1 if the output is empty

52 - if expr "$CUR_VERS" : "[^.]*\.[0-9]*$[0-9]\.\|[0-9]\$$" >/dev/null; then

53 - CUR_PRERELEASE="0"

54 - else

55 - CUR_PRERELEASE="1"

56 - fi

57 - CUR_FIX=$(expr "$CUR_VERS" : "[^.]*\.[0-9][0-9]*[^0-9][^0-9]*$[0-9][0-9]*$") || CUR_FIX="0"

58 - else

59 - CUR_MIN="0"

60 - CUR_PRERELEASE="0"

61 - CUR_FIX="0"

62 - fi

63 -

64 - # Compare versions

65 - VERS_LEAST="$PROG_NAME version '$CUR_VERS' should be at least '$MIN_VERS'"

66 - test "$CUR_MAJ" -lt "$MIN_MAJ" && die_upgrade "$VERS_LEAST"

67 - test "$CUR_MAJ" -gt "$MIN_MAJ" || {

68 - test "$CUR_MIN" -lt "$MIN_MIN" && die_upgrade "$VERS_LEAST"

69 - test "$CUR_MIN" -gt "$MIN_MIN" || {

70 - test "$CUR_PRERELEASE" -gt "$MIN_PRERELEASE" && die_upgrade "$VERS_LEAST"

71 - test "$CUR_PRERELEASE" -lt "$MIN_PRERELEASE" || {

72 - test "$CUR_FIX" -lt "$MIN_FIX" && die_upgrade "$VERS_LEAST"

73 - true

74 - }

75 - }

76 - }

77 -}

cmd/ipfs/kubo/daemon.go

+4 -2

@@ -334,7 +334,8 @@ func daemonFunc(req *cmds.Request, re cmds.ResponseEmitter, env cmds.Environment

334 }

335

336 // Use hybrid migration strategy that intelligently combines external and embedded migrations

337 - err = migrations.RunHybridMigrations(cctx.Context(), version.RepoVersion, cctx.ConfigRoot, false)

337 + // Use req.Context instead of cctx.Context() to avoid attempting repo open before migrations complete

338 + err = migrations.RunHybridMigrations(req.Context, version.RepoVersion, cctx.ConfigRoot, false)

339 if err != nil {

340 fmt.Println("Repository migration failed:")

341 fmt.Printf(" %s\n", err)

@@ -387,7 +388,8 @@ func daemonFunc(req *cmds.Request, re cmds.ResponseEmitter, env cmds.Environment

388 log.Errorf("failed to create autoconf client: %v", err)

389 } else {

390 // Start primes cache and starts background updater

390 - if _, err := client.Start(cctx.Context()); err != nil {

391 + // Use req.Context for background updater lifecycle (node doesn't exist yet)

392 + if _, err := client.Start(req.Context); err != nil {

393 log.Errorf("failed to start autoconf updater: %v", err)

394 }

395 }

core/commands/repo.go

+3 -10

@@ -423,19 +423,12 @@ migration. Versions below 16 require external migration tools.

423 return fmt.Errorf("downgrade from version %d to %d requires --allow-downgrade flag", currentVersion, targetVersion)

424 }

425

426 - // Check if repo is locked by daemon before running migration

427 - locked, err := fsrepo.LockedByOtherProcess(cctx.ConfigRoot)

428 - if err != nil {

429 - return fmt.Errorf("could not check repo lock: %w", err)

430 - }

431 - if locked {

432 - return fmt.Errorf("cannot run migration while daemon is running (repo.lock exists)")

433 - }

434 -

426 fmt.Printf("Migrating repository from version %d to %d...\n", currentVersion, targetVersion)

427

428 // Use hybrid migration strategy that intelligently combines external and embedded migrations

438 - err = migrations.RunHybridMigrations(cctx.Context(), targetVersion, cctx.ConfigRoot, allowDowngrade)

429 + // Use req.Context instead of cctx.Context() to avoid opening the repo before migrations run,

430 + // which would acquire the lock that migrations need

431 + err = migrations.RunHybridMigrations(req.Context, targetVersion, cctx.ConfigRoot, allowDowngrade)

432 if err != nil {

433 fmt.Println("Repository migration failed:")

434 fmt.Printf(" %s\n", err)

docs/changelogs/v0.38.md

+18

@@ -5,6 +5,7 @@

5 This release was brought to you by the [Shipyard](https://ipshipyard.com/) team.

7 - [v0.38.0](#v0380)

8 +- [v0.38.1](#v0381)

10 ## v0.38.0

@@ -290,3 +291,20 @@ The new [`Internal.MFSNoFlushLimit`](https://github.com/ipfs/kubo/blob/master/do

291 | Jakub Sztandera | 1 | +67/-15 | 3 |

292 | Masih H. Derkani | 1 | +1/-2 | 2 |

293 | Dominic Della Valle | 1 | +2/-1 | 1 |

294 +

295 +## v0.38.1

296 +

297 +Fixes migration panic on Windows when upgrading from v0.37 to v0.38 ("panic: error can't be dealt with transactionally: Access is denied").

298 +

299 +### Changelog

300 +

301 +<details>

302 +<summary>Full Changelog</summary>

303 +

304 +</details>

305 +

306 +### 👨‍👩‍👧‍👦 Contributors

307 +

309 +|-------------|---------|---------|---------------|

310 +| TBD | | | |

mk/golang.mk

+1 -5

@@ -1,5 +1,4 @@

1 # golang utilities

2 -GO_MIN_VERSION = 1.25

2 export GO111MODULE=on

@@ -74,11 +73,8 @@ test_go_lint: test/bin/golangci-lint

74 test_go: $(TEST_GO)

77 -check_go_version:

78 - @$(GOCC) version

79 - bin/check_go_version $(GO_MIN_VERSION)

76 +# Version check is no longer needed - go.mod enforces minimum version

77 .PHONY: check_go_version

81 -DEPS_GO += check_go_version

79 TEST += $(TEST_GO)

80 TEST_SHORT += test_go_fmt test_go_short

repo/fsrepo/migrations/atomicfile/atomicfile.go

+17 -12

@@ -1,6 +1,7 @@

1 package atomicfile

3 import (

4 + "fmt"

5 "io"

6 "os"

7 "path/filepath"

@@ -34,23 +35,27 @@ func New(path string, mode os.FileMode) (*File, error) {

36 // Close atomically replaces the target file with the temporary file

37 func (f *File) Close() error {

37 - if err := f.File.Close(); err != nil {

38 - os.Remove(f.File.Name())

39 - return err

38 + closeErr := f.File.Close()

39 + if closeErr != nil {

40 + // Try to cleanup temp file, but prioritize close error

41 + _ = os.Remove(f.File.Name())

42 + return closeErr

43 }

41 -

42 - if err := os.Rename(f.File.Name(), f.path); err != nil {

43 - os.Remove(f.File.Name())

44 - return err

45 - }

46 -

47 - return nil

44 + return os.Rename(f.File.Name(), f.path)

45 }

47 // Abort removes the temporary file without replacing the target

48 func (f *File) Abort() error {

52 - f.File.Close()

53 - return os.Remove(f.File.Name())

49 + closeErr := f.File.Close()

50 + removeErr := os.Remove(f.File.Name())

51 +

52 + if closeErr != nil && removeErr != nil {

53 + return fmt.Errorf("abort failed: close: %w, remove: %v", closeErr, removeErr)

54 + }

55 + if closeErr != nil {

56 + return closeErr

57 + }

58 + return removeErr

59 }

61 // ReadFrom reads from the given reader into the atomic file

repo/fsrepo/migrations/atomicfile/atomicfile_test.go new

+208

@@ -0,0 +1,208 @@

1 +package atomicfile

2 +

3 +import (

4 + "bytes"

5 + "fmt"

6 + "os"

7 + "path/filepath"

8 + "runtime"

9 + "testing"

10 +

11 + "github.com/stretchr/testify/assert"

12 + "github.com/stretchr/testify/require"

13 +)

14 +

15 +// TestNew_Success verifies atomic file creation

16 +func TestNew_Success(t *testing.T) {

17 + dir := t.TempDir()

18 + path := filepath.Join(dir, "test.txt")

19 +

20 + af, err := New(path, 0644)

21 + require.NoError(t, err)

22 + defer func() { _ = af.Abort() }()

23 +

24 + // Verify temp file exists

25 + assert.FileExists(t, af.File.Name())

26 +

27 + // Verify temp file is in same directory

28 + assert.Equal(t, dir, filepath.Dir(af.File.Name()))

29 +}

30 +

31 +// TestClose_Success verifies atomic replacement

32 +func TestClose_Success(t *testing.T) {

33 + dir := t.TempDir()

34 + path := filepath.Join(dir, "test.txt")

35 +

36 + af, err := New(path, 0644)

37 + require.NoError(t, err)

38 +

39 + content := []byte("test content")

40 + _, err = af.Write(content)

41 + require.NoError(t, err)

42 +

43 + tempName := af.File.Name()

44 +

45 + require.NoError(t, af.Close())

46 +

47 + // Verify target file exists with correct content

48 + data, err := os.ReadFile(path)

49 + require.NoError(t, err)

50 + assert.Equal(t, content, data)

51 +

52 + // Verify temp file removed

53 + assert.NoFileExists(t, tempName)

54 +}

55 +

56 +// TestAbort_Success verifies cleanup

57 +func TestAbort_Success(t *testing.T) {

58 + dir := t.TempDir()

59 + path := filepath.Join(dir, "test.txt")

60 +

61 + af, err := New(path, 0644)

62 + require.NoError(t, err)

63 +

64 + tempName := af.File.Name()

65 +

66 + require.NoError(t, af.Abort())

67 +

68 + // Verify temp file removed

69 + assert.NoFileExists(t, tempName)

70 +

71 + // Verify target not created

72 + assert.NoFileExists(t, path)

73 +}

74 +

75 +// TestAbort_ErrorHandling tests error capture

76 +func TestAbort_ErrorHandling(t *testing.T) {

77 + dir := t.TempDir()

78 + path := filepath.Join(dir, "test.txt")

79 +

80 + af, err := New(path, 0644)

81 + require.NoError(t, err)

82 +

83 + // Close file to force close error

84 + af.File.Close()

85 +

86 + // Remove temp file to force remove error

87 + os.Remove(af.File.Name())

88 +

89 + err = af.Abort()

90 + // Should get both errors

91 + require.Error(t, err)

92 + assert.Contains(t, err.Error(), "abort failed")

93 +}

94 +

95 +// TestClose_CloseError verifies cleanup on close failure

96 +func TestClose_CloseError(t *testing.T) {

97 + dir := t.TempDir()

98 + path := filepath.Join(dir, "test.txt")

99 +

100 + af, err := New(path, 0644)

101 + require.NoError(t, err)

102 +

103 + tempName := af.File.Name()

104 +

105 + // Close file to force close error

106 + af.File.Close()

107 +

108 + err = af.Close()

109 + require.Error(t, err)

110 +

111 + // Verify temp file cleaned up even on error

112 + assert.NoFileExists(t, tempName)

113 +}

114 +

115 +// TestReadFrom verifies io.Copy integration

116 +func TestReadFrom(t *testing.T) {

117 + dir := t.TempDir()

118 + path := filepath.Join(dir, "test.txt")

119 +

120 + af, err := New(path, 0644)

121 + require.NoError(t, err)

122 + defer func() { _ = af.Abort() }()

123 +

124 + content := []byte("test content from reader")

125 + n, err := af.ReadFrom(bytes.NewReader(content))

126 + require.NoError(t, err)

127 + assert.Equal(t, int64(len(content)), n)

128 +}

129 +

130 +// TestFilePermissions verifies mode is set correctly

131 +func TestFilePermissions(t *testing.T) {

132 + dir := t.TempDir()

133 + path := filepath.Join(dir, "test.txt")

134 +

135 + af, err := New(path, 0600)

136 + require.NoError(t, err)

137 +

138 + _, err = af.Write([]byte("test"))

139 + require.NoError(t, err)

140 +

141 + require.NoError(t, af.Close())

142 +

143 + info, err := os.Stat(path)

144 + require.NoError(t, err)

145 +

146 + // On Unix, check exact permissions

147 + if runtime.GOOS != "windows" {

148 + mode := info.Mode().Perm()

149 + assert.Equal(t, os.FileMode(0600), mode)

150 + }

151 +}

152 +

153 +// TestMultipleAbortsSafe verifies calling Abort multiple times is safe

154 +func TestMultipleAbortsSafe(t *testing.T) {

155 + dir := t.TempDir()

156 + path := filepath.Join(dir, "test.txt")

157 +

158 + af, err := New(path, 0644)

159 + require.NoError(t, err)

160 +

161 + tempName := af.File.Name()

162 +

163 + // First abort should succeed

164 + require.NoError(t, af.Abort())

165 + assert.NoFileExists(t, tempName, "temp file should be removed after first abort")

166 +

167 + // Second abort should handle gracefully (file already gone)

168 + err = af.Abort()

169 + // Error is acceptable since file is already removed, but it should not panic

170 + t.Logf("Second Abort() returned: %v", err)

171 +}

172 +

173 +// TestNoTempFilesAfterOperations verifies no .tmp-* files remain after operations

174 +func TestNoTempFilesAfterOperations(t *testing.T) {

175 + const testIterations = 5

176 +

177 + tests := []struct {

178 + name string

179 + operation func(*File) error

180 + }{

181 + {"close", (*File).Close},

182 + {"abort", (*File).Abort},

183 + }

184 +

185 + for _, tt := range tests {

186 + t.Run(tt.name, func(t *testing.T) {

187 + dir := t.TempDir()

188 +

189 + // Perform multiple operations

190 + for i := 0; i < testIterations; i++ {

191 + path := filepath.Join(dir, fmt.Sprintf("test%d.txt", i))

192 +

193 + af, err := New(path, 0644)

194 + require.NoError(t, err)

195 +

196 + _, err = af.Write([]byte("test data"))

197 + require.NoError(t, err)

198 +

199 + require.NoError(t, tt.operation(af))

200 + }

201 +

202 + // Check for any .tmp-* files

203 + tmpFiles, err := filepath.Glob(filepath.Join(dir, ".tmp-*"))

204 + require.NoError(t, err)

205 + assert.Empty(t, tmpFiles, "should be no temp files after %s", tt.name)

206 + })

207 + }

208 +}

repo/fsrepo/migrations/common/utils.go

+26 -21

@@ -1,6 +1,7 @@

1 package common

3 import (

4 + "bytes"

5 "encoding/json"

6 "fmt"

7 "io"

@@ -40,47 +41,51 @@ func Must(err error) {

42 // WithBackup performs a config file operation with automatic backup and rollback on error

43 func WithBackup(configPath string, backupSuffix string, fn func(in io.ReadSeeker, out io.Writer) error) error {

43 - in, err := os.Open(configPath)

44 + // Read the entire file into memory first

45 + // This allows us to close the file before doing atomic operations,

46 + // which is necessary on Windows where open files can't be renamed

47 + data, err := os.ReadFile(configPath)

48 if err != nil {

45 - return err

49 + return fmt.Errorf("failed to read config file %s: %w", configPath, err)

50 }

47 - defer in.Close()

49 - // Create backup

50 - backup, err := atomicfile.New(configPath+backupSuffix, 0600)

52 + // Create an in-memory reader for the data

53 + in := bytes.NewReader(data)

54 +

55 + // Create backup atomically to prevent partial backup on interruption

56 + backupPath := configPath + backupSuffix

57 + backup, err := atomicfile.New(backupPath, 0600)

58 if err != nil {

52 - return err

59 + return fmt.Errorf("failed to create backup file for %s: %w", backupPath, err)

60 }

54 -

55 - // Copy to backup

56 - if _, err := backup.ReadFrom(in); err != nil {

61 + if _, err := backup.Write(data); err != nil {

62 Must(backup.Abort())

58 - return err

63 + return fmt.Errorf("failed to write backup data: %w", err)

64 }

60 -

61 - // Reset input for reading

62 - if _, err := in.Seek(0, io.SeekStart); err != nil {

65 + if err := backup.Close(); err != nil {

66 Must(backup.Abort())

64 - return err

67 + return fmt.Errorf("failed to finalize backup: %w", err)

68 }

67 - // Create output file

70 + // Create output file atomically

71 out, err := atomicfile.New(configPath, 0600)

72 if err != nil {

70 - Must(backup.Abort())

71 - return err

73 + // Clean up backup on error

74 + os.Remove(backupPath)

75 + return fmt.Errorf("failed to create atomic file for %s: %w", configPath, err)

76 }

78 // Run the conversion function

79 if err := fn(in, out); err != nil {

80 Must(out.Abort())

77 - Must(backup.Abort())

78 - return err

81 + // Clean up backup on error

82 + os.Remove(backupPath)

83 + return fmt.Errorf("config conversion failed: %w", err)

84 }

81 - // Close everything on success

86 + // Close the output file atomically

87 Must(out.Close())

83 - Must(backup.Close())

88 + // Backup remains for potential revert

90 return nil

91 }

repo/fsrepo/migrations/embedded.go

@@ -6,6 +6,7 @@ import (

6 "log"

7 "os"

9 + lockfile "github.com/ipfs/go-fs-lock"

10 "github.com/ipfs/kubo/repo/fsrepo/migrations/common"

11 mg16 "github.com/ipfs/kubo/repo/fsrepo/migrations/fs-repo-16-to-17/migration"

12 mg17 "github.com/ipfs/kubo/repo/fsrepo/migrations/fs-repo-17-to-18/migration"

@@ -109,6 +110,13 @@ func RunEmbeddedMigrations(ctx context.Context, targetVer int, ipfsDir string, a

110 return err

111 }

112

113 + // Acquire lock once for all embedded migrations to prevent concurrent access

114 + lk, err := lockfile.Lock(ipfsDir, "repo.lock")

115 + if err != nil {

116 + return fmt.Errorf("failed to acquire repo lock: %w", err)

117 + }

118 + defer lk.Close()

119 +

120 fromVer, err := RepoVersion(ipfsDir)

121 if err != nil {

122 return fmt.Errorf("could not get repo version: %w", err)

repo/fsrepo/migrations/ipfsdir_test.go

@@ -11,6 +11,8 @@ import (

11 func TestRepoDir(t *testing.T) {

12 fakeHome := t.TempDir()

13 t.Setenv("HOME", fakeHome)

14 + // On Windows, os.UserHomeDir() uses USERPROFILE, not HOME

15 + t.Setenv("USERPROFILE", fakeHome)

16 fakeIpfs := filepath.Join(fakeHome, ".ipfs")

17 t.Setenv(config.EnvDir, fakeIpfs)

test/cli/migrations/migration_16_to_latest_test.go

+166 -3

@@ -21,6 +21,7 @@ import (

22 ipfs "github.com/ipfs/kubo"

23 "github.com/ipfs/kubo/test/cli/harness"

24 + "github.com/stretchr/testify/assert"

25 "github.com/stretchr/testify/require"

26 )

@@ -52,6 +53,13 @@ func TestMigration16ToLatest(t *testing.T) {

53 // Comparison tests using 'ipfs repo migrate' command

54 t.Run("repo migrate: forward migration with auto values", testRepoMigrationWithAuto)

55 t.Run("repo migrate: backward migration", testRepoBackwardMigration)

56 +

57 + // Temp file and backup cleanup tests

58 + t.Run("daemon migrate: no temp files after successful migration", testNoTempFilesAfterSuccessfulMigration)

59 + t.Run("daemon migrate: no temp files after failed migration", testNoTempFilesAfterFailedMigration)

60 + t.Run("daemon migrate: backup files persist after successful migration", testBackupFilesPersistAfterSuccessfulMigration)

61 + t.Run("repo migrate: backup files can revert migration", testBackupFilesCanRevertMigration)

62 + t.Run("repo migrate: conversion failure cleans up temp files", testConversionFailureCleanup)

63 }

65 // =============================================================================

@@ -392,10 +400,15 @@ func setupStaticV16Repo(t *testing.T) *harness.Node {

400 v16FixturePath := "testdata/v16-repo"

401

402 // Create a temporary test directory - each test gets its own copy

395 - // Use ./tmp.DELETEME/ as requested by user instead of /tmp/

396 - tmpDir := filepath.Join("tmp.DELETEME", "migration-test-"+t.Name())

403 + // Sanitize test name for Windows - replace invalid characters

404 + sanitizedName := strings.Map(func(r rune) rune {

405 + if strings.ContainsRune(`<>:"/\|?*`, r) {

406 + return '_'

407 + }

408 + return r

409 + }, t.Name())

410 + tmpDir := filepath.Join(t.TempDir(), "migration-test-"+sanitizedName)

411 require.NoError(t, os.MkdirAll(tmpDir, 0755))

398 - t.Cleanup(func() { os.RemoveAll(tmpDir) })

412

413 // Convert to absolute path for harness

414 absTmpDir, err := filepath.Abs(tmpDir)

@@ -559,6 +572,8 @@ func testRepoBackwardMigration(t *testing.T) {

572

573 // First run forward migration to get to v17

574 result := node.RunIPFS("repo", "migrate")

575 + t.Logf("Forward migration stdout:\n%s", result.Stdout.String())

576 + t.Logf("Forward migration stderr:\n%s", result.Stderr.String())

577 require.Empty(t, result.Stderr.String(), "Forward migration should succeed")

578

579 // Verify we're at the latest version

@@ -569,6 +584,8 @@ func testRepoBackwardMigration(t *testing.T) {

584

585 // Now run reverse migration back to v16

586 result = node.RunIPFS("repo", "migrate", "--to=16", "--allow-downgrade")

587 + t.Logf("Backward migration stdout:\n%s", result.Stdout.String())

588 + t.Logf("Backward migration stderr:\n%s", result.Stderr.String())

589 require.Empty(t, result.Stderr.String(), "Reverse migration should succeed")

590

591 // Verify version was downgraded to 16

@@ -753,3 +770,149 @@ func runDaemonWithMultipleMigrationMonitoring(t *testing.T, node *harness.Node,

770 }

771 }

772 }

773 +

774 +// =============================================================================

775 +// TEMP FILE AND BACKUP CLEANUP TESTS

776 +// =============================================================================

777 +

778 +// Helper functions for test cleanup assertions

779 +func assertNoTempFiles(t *testing.T, dir string, msgAndArgs ...interface{}) {

780 + t.Helper()

781 + tmpFiles, err := filepath.Glob(filepath.Join(dir, ".tmp-*"))

782 + require.NoError(t, err)

783 + assert.Empty(t, tmpFiles, msgAndArgs...)

784 +}

785 +

786 +func backupPath(configPath string, fromVer, toVer int) string {

787 + return fmt.Sprintf("%s.%d-to-%d.bak", configPath, fromVer, toVer)

788 +}

789 +

790 +func setupDaemonCmd(ctx context.Context, node *harness.Node, args ...string) *exec.Cmd {

791 + cmd := exec.CommandContext(ctx, node.IPFSBin, args...)

792 + cmd.Dir = node.Dir

793 + for k, v := range node.Runner.Env {

794 + cmd.Env = append(cmd.Env, k+"="+v)

795 + }

796 + return cmd

797 +}

798 +

799 +func testNoTempFilesAfterSuccessfulMigration(t *testing.T) {

800 + node := setupStaticV16Repo(t)

801 +

802 + // Run successful migration

803 + _, migrationSuccess := runDaemonMigrationWithMonitoring(t, node)

804 + require.True(t, migrationSuccess, "migration should succeed")

805 +

806 + assertNoTempFiles(t, node.Dir, "no temp files should remain after successful migration")

807 +}

808 +

809 +func testNoTempFilesAfterFailedMigration(t *testing.T) {

810 + node := setupStaticV16Repo(t)

811 +

812 + // Corrupt config to force migration failure

813 + configPath := filepath.Join(node.Dir, "config")

814 + corruptedJson := `{"Bootstrap": ["auto",` // Invalid JSON

815 + require.NoError(t, os.WriteFile(configPath, []byte(corruptedJson), 0644))

816 +

817 + // Attempt migration (should fail)

818 + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)

819 + defer cancel()

820 +

821 + cmd := setupDaemonCmd(ctx, node, "daemon", "--migrate")

822 + output, _ := cmd.CombinedOutput()

823 + t.Logf("Failed migration output: %s", output)

824 +

825 + assertNoTempFiles(t, node.Dir, "no temp files should remain after failed migration")

826 +}

827 +

828 +func testBackupFilesPersistAfterSuccessfulMigration(t *testing.T) {

829 + node := setupStaticV16Repo(t)

830 +

831 + // Run migration from v16 to latest (v18)

832 + _, migrationSuccess := runDaemonMigrationWithMonitoring(t, node)

833 + require.True(t, migrationSuccess, "migration should succeed")

834 +

835 + // Check for backup files from each migration step

836 + configPath := filepath.Join(node.Dir, "config")

837 + backup16to17 := backupPath(configPath, 16, 17)

838 + backup17to18 := backupPath(configPath, 17, 18)

839 +

840 + // Both backup files should exist

841 + assert.FileExists(t, backup16to17, "16-to-17 backup should exist")

842 + assert.FileExists(t, backup17to18, "17-to-18 backup should exist")

843 +

844 + // Verify backup files contain valid JSON

845 + data16to17, err := os.ReadFile(backup16to17)

846 + require.NoError(t, err)

847 + var config16to17 map[string]interface{}

848 + require.NoError(t, json.Unmarshal(data16to17, &config16to17), "16-to-17 backup should be valid JSON")

849 +

850 + data17to18, err := os.ReadFile(backup17to18)

851 + require.NoError(t, err)

852 + var config17to18 map[string]interface{}

853 + require.NoError(t, json.Unmarshal(data17to18, &config17to18), "17-to-18 backup should be valid JSON")

854 +}

855 +

856 +func testBackupFilesCanRevertMigration(t *testing.T) {

857 + node := setupStaticV16Repo(t)

858 +

859 + configPath := filepath.Join(node.Dir, "config")

860 + versionPath := filepath.Join(node.Dir, "version")

861 +

862 + // Read original v16 config

863 + originalConfig, err := os.ReadFile(configPath)

864 + require.NoError(t, err)

865 +

866 + // Migrate to v17 only

867 + result := node.RunIPFS("repo", "migrate", "--to=17")

868 + require.Empty(t, result.Stderr.String(), "migration to v17 should succeed")

869 +

870 + // Verify backup exists

871 + backup16to17 := backupPath(configPath, 16, 17)

872 + assert.FileExists(t, backup16to17, "16-to-17 backup should exist")

873 +

874 + // Manually revert using backup

875 + backupData, err := os.ReadFile(backup16to17)

876 + require.NoError(t, err)

877 + require.NoError(t, os.WriteFile(configPath, backupData, 0600))

878 + require.NoError(t, os.WriteFile(versionPath, []byte("16"), 0644))

879 +

880 + // Verify config matches original

881 + revertedConfig, err := os.ReadFile(configPath)

882 + require.NoError(t, err)

883 + assert.JSONEq(t, string(originalConfig), string(revertedConfig), "reverted config should match original")

884 +

885 + // Verify version is back to 16

886 + versionData, err := os.ReadFile(versionPath)

887 + require.NoError(t, err)

888 + assert.Equal(t, "16", strings.TrimSpace(string(versionData)), "version should be reverted to 16")

889 +}

890 +

891 +func testConversionFailureCleanup(t *testing.T) {

892 + // This test verifies that when a migration's conversion function fails,

893 + // all temporary files are cleaned up properly

894 + node := setupStaticV16Repo(t)

895 +

896 + configPath := filepath.Join(node.Dir, "config")

897 +

898 + // Create a corrupted config that will cause conversion to fail during JSON parsing

899 + // The migration will read this, attempt to parse as JSON, and fail

900 + corruptedJson := `{"Bootstrap": ["auto",` // Invalid JSON - missing closing bracket

901 + require.NoError(t, os.WriteFile(configPath, []byte(corruptedJson), 0644))

902 +

903 + // Attempt migration (should fail during conversion)

904 + result := node.RunIPFS("repo", "migrate")

905 + require.NotEmpty(t, result.Stderr.String(), "migration should fail with error")

906 +

907 + assertNoTempFiles(t, node.Dir, "no temp files should remain after conversion failure")

908 +

909 + // Verify no backup files were created (failure happened before backup)

910 + backupFiles, err := filepath.Glob(filepath.Join(node.Dir, "config.*.bak"))

911 + require.NoError(t, err)

912 + assert.Empty(t, backupFiles, "no backup files should be created on conversion failure")

913 +

914 + // Verify corrupted config is unchanged (atomic operations prevented overwrite)

915 + currentConfig, err := os.ReadFile(configPath)

916 + require.NoError(t, err)

917 + assert.Equal(t, corruptedJson, string(currentConfig), "corrupted config should remain unchanged")

918 +}

test/cli/migrations/migration_concurrent_test.go new

+55

@@ -0,0 +1,55 @@

1 +package migrations

2 +

3 +// NOTE: These concurrent migration tests require the local Kubo binary (built with 'make build') to be in PATH.

4 +//

5 +// To run these tests successfully:

6 +// export PATH="$(pwd)/cmd/ipfs:$PATH"

7 +// go test ./test/cli/migrations/

8 +

9 +import (

10 + "context"

11 + "testing"

12 + "time"

13 +

14 + "github.com/stretchr/testify/require"

15 +)

16 +

17 +const daemonStartupWait = 2 * time.Second

18 +

19 +// TestConcurrentMigrations tests concurrent daemon --migrate attempts

20 +func TestConcurrentMigrations(t *testing.T) {

21 + t.Parallel()

22 +

23 + t.Run("concurrent daemon migrations prevented by lock", testConcurrentDaemonMigrations)

24 +}

25 +

26 +func testConcurrentDaemonMigrations(t *testing.T) {

27 + node := setupStaticV16Repo(t)

28 +

29 + // Start first daemon --migrate in background (holds repo.lock)

30 + ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)

31 + defer cancel()

32 +

33 + firstDaemon := setupDaemonCmd(ctx, node, "daemon", "--migrate")

34 + require.NoError(t, firstDaemon.Start())

35 + defer func() {

36 + // Shutdown first daemon

37 + shutdownCmd := setupDaemonCmd(context.Background(), node, "shutdown")

38 + _ = shutdownCmd.Run()

39 + _ = firstDaemon.Wait()

40 + }()

41 +

42 + // Wait for first daemon to start and acquire lock

43 + time.Sleep(daemonStartupWait)

44 +

45 + // Attempt second daemon --migrate (should fail due to lock)

46 + secondDaemon := setupDaemonCmd(context.Background(), node, "daemon", "--migrate")

47 + output, err := secondDaemon.CombinedOutput()

48 + t.Logf("Second daemon output: %s", output)

49 +

50 + // Should fail with lock error

51 + require.Error(t, err, "second daemon should fail when first daemon holds lock")

52 + require.Contains(t, string(output), "lock", "error should mention lock")

53 +

54 + assertNoTempFiles(t, node.Dir, "no temp files should be created when lock fails")

55 +}

test/cli/migrations/migration_mixed_15_to_latest_test.go

+100 -93

@@ -23,8 +23,9 @@ import (

23 "os"

24 "os/exec"

25 "path/filepath"

26 + "runtime"

27 + "slices"

28 "strings"

27 - "syscall"

29 "testing"

30 "time"

@@ -61,7 +62,8 @@ func testDaemonMigration15ToLatest(t *testing.T) {

62 node := setupStaticV15Repo(t)

64 // Create mock migration binary for 15→16 (16→17 will use embedded migration)

64 - createMockMigrationBinary(t, "15", "16")

65 + mockBinDir := createMockMigrationBinary(t, "15", "16")

66 + customPath := buildCustomPath(mockBinDir)

68 configPath := filepath.Join(node.Dir, "config")

69 versionPath := filepath.Join(node.Dir, "version")

@@ -80,7 +82,7 @@ func testDaemonMigration15ToLatest(t *testing.T) {

82 originalPeerID := getNestedValue(originalConfig, "Identity.PeerID")

84 // Run dual migration using daemon --migrate

83 - stdoutOutput, migrationSuccess := runDaemonWithLegacyMigrationMonitoring(t, node)

85 + stdoutOutput, migrationSuccess := runDaemonWithLegacyMigrationMonitoring(t, node, customPath)

87 // Debug output

88 t.Logf("Daemon output:\n%s", stdoutOutput)

@@ -124,7 +126,8 @@ func testRepoMigration15ToLatest(t *testing.T) {

126 node := setupStaticV15Repo(t)

127

128 // Create mock migration binary for 15→16 (16→17 will use embedded migration)

127 - createMockMigrationBinary(t, "15", "16")

129 + mockBinDir := createMockMigrationBinary(t, "15", "16")

130 + customPath := buildCustomPath(mockBinDir)

131

132 configPath := filepath.Join(node.Dir, "config")

133 versionPath := filepath.Join(node.Dir, "version")

@@ -135,16 +138,7 @@ func testRepoMigration15ToLatest(t *testing.T) {

138 require.Equal(t, "15", strings.TrimSpace(string(versionData)), "Should start at version 15")

139

140 // Run migration using 'ipfs repo migrate' with custom PATH

138 - result := node.Runner.Run(harness.RunRequest{

139 - Path: node.IPFSBin,

140 - Args: []string{"repo", "migrate"},

141 - CmdOpts: []harness.CmdOpt{

142 - func(cmd *exec.Cmd) {

143 - // Ensure the command inherits our modified PATH with mock binaries

144 - cmd.Env = append(cmd.Env, "PATH="+os.Getenv("PATH"))

145 - },

146 - },

147 - })

141 + result := runMigrationWithCustomPath(node, customPath, "repo", "migrate")

142 require.Empty(t, result.Stderr.String(), "Migration should succeed without errors")

143

144 // Verify final version is latest

@@ -184,10 +178,10 @@ func setupStaticV15Repo(t *testing.T) *harness.Node {

178 }

179

180 // runDaemonWithLegacyMigrationMonitoring monitors for hybrid migration patterns

187 -func runDaemonWithLegacyMigrationMonitoring(t *testing.T, node *harness.Node) (string, bool) {

181 +func runDaemonWithLegacyMigrationMonitoring(t *testing.T, node *harness.Node, customPath string) (string, bool) {

182 // Monitor for hybrid migration completion - use "Hybrid migration completed successfully" as success pattern

183 stdoutOutput, daemonStarted := runDaemonWithMigrationMonitoringCustomEnv(t, node, "Using hybrid migration strategy", "Hybrid migration completed successfully", map[string]string{

190 - "PATH": os.Getenv("PATH"), // Pass current PATH which includes our mock binaries

184 + "PATH": customPath, // Pass custom PATH with our mock binaries

185 })

186

187 // Check for hybrid migration patterns in output

@@ -271,17 +265,59 @@ func runDaemonWithMigrationMonitoringCustomEnv(t *testing.T, node *harness.Node,

265 t.Log("Daemon startup timed out")

266 }

267

274 - // Stop the daemon

268 + // Stop the daemon using ipfs shutdown command for graceful shutdown

269 if cmd.Process != nil {

276 - _ = cmd.Process.Signal(syscall.SIGTERM)

270 + shutdownCmd := exec.Command(node.IPFSBin, "shutdown")

271 + shutdownCmd.Dir = node.Dir

272 + for k, v := range node.Runner.Env {

273 + shutdownCmd.Env = append(shutdownCmd.Env, k+"="+v)

274 + }

275 +

276 + if err := shutdownCmd.Run(); err != nil {

277 + // If graceful shutdown fails, force kill

278 + _ = cmd.Process.Kill()

279 + }

280 +

281 + // Wait for process to exit

282 _ = cmd.Wait()

283 }

284

285 return outputBuffer.String(), daemonReady && migrationStarted && migrationCompleted

286 }

287

283 -// createMockMigrationBinary creates a platform-agnostic Go binary for migration on PATH

284 -func createMockMigrationBinary(t *testing.T, fromVer, toVer string) {

288 +// buildCustomPath creates a custom PATH with mock migration binaries prepended.

289 +// This is necessary for test isolation when running tests in parallel with t.Parallel().

290 +// Without isolated PATH handling, parallel tests can interfere with each other through

291 +// global PATH modifications, causing tests to download real migration binaries instead

292 +// of using the test mocks.

293 +func buildCustomPath(mockBinDirs ...string) string {

294 + // Prepend mock directories to ensure they're found first

295 + pathElements := append(mockBinDirs, os.Getenv("PATH"))

296 + return strings.Join(pathElements, string(filepath.ListSeparator))

297 +}

298 +

299 +// runMigrationWithCustomPath runs a migration command with a custom PATH environment.

300 +// This ensures the migration uses our mock binaries instead of downloading real ones.

301 +func runMigrationWithCustomPath(node *harness.Node, customPath string, args ...string) *harness.RunResult {

302 + return node.Runner.Run(harness.RunRequest{

303 + Path: node.IPFSBin,

304 + Args: args,

305 + CmdOpts: []harness.CmdOpt{

306 + func(cmd *exec.Cmd) {

307 + // Remove existing PATH entries using slices.DeleteFunc

308 + cmd.Env = slices.DeleteFunc(cmd.Env, func(s string) bool {

309 + return strings.HasPrefix(s, "PATH=")

310 + })

311 + // Add custom PATH

312 + cmd.Env = append(cmd.Env, "PATH="+customPath)

313 + },

314 + },

315 + })

316 +}

317 +

318 +// createMockMigrationBinary creates a platform-agnostic Go binary for migration testing.

319 +// Returns the directory containing the binary to be added to PATH.

320 +func createMockMigrationBinary(t *testing.T, fromVer, toVer string) string {

321 // Create bin directory for migration binaries

322 binDir := t.TempDir()

323

@@ -289,73 +325,60 @@ func createMockMigrationBinary(t *testing.T, fromVer, toVer string) {

325 scriptName := fmt.Sprintf("fs-repo-%s-to-%s", fromVer, toVer)

326 sourceFile := filepath.Join(binDir, scriptName+".go")

327 binaryPath := filepath.Join(binDir, scriptName)

328 + if runtime.GOOS == "windows" {

329 + binaryPath += ".exe"

330 + }

331

332 + // Generate minimal mock migration binary code

333 goSource := fmt.Sprintf(`package main

294 -

295 -import (

296 - "fmt"

297 - "os"

298 - "path/filepath"

299 - "strings"

300 -)

301 -

334 +import ("fmt"; "os"; "path/filepath"; "strings"; "time")

335 func main() {

303 - // Parse command line arguments - real migration binaries expect -path=<repo-path>

304 - var repoPath string

336 + var path string

337 var revert bool

306 - for _, arg := range os.Args[1:] {

307 - if strings.HasPrefix(arg, "-path=") {

308 - repoPath = strings.TrimPrefix(arg, "-path=")

309 - } else if arg == "-revert" {

310 - revert = true

311 - }

338 + for _, a := range os.Args[1:] {

339 + if strings.HasPrefix(a, "-path=") { path = a[6:] }

340 + if a == "-revert" { revert = true }

341 }

313 -

314 - if repoPath == "" {

315 - fmt.Fprintf(os.Stderr, "Usage: %%s -path=<repo-path> [-verbose=true] [-revert]\n", os.Args[0])

342 + if path == "" { fmt.Fprintln(os.Stderr, "missing -path="); os.Exit(1) }

343 +

344 + from, to := "%s", "%s"

345 + if revert { from, to = to, from }

346 + fmt.Printf("fake applying %%s-to-%%s repo migration\n", from, to)

347 +

348 + // Create and immediately remove lock file to simulate proper locking behavior

349 + lockPath := filepath.Join(path, "repo.lock")

350 + lockFile, err := os.Create(lockPath)

351 + if err != nil && !os.IsExist(err) {

352 + fmt.Fprintf(os.Stderr, "Error creating lock: %%v\n", err)

353 os.Exit(1)

354 }

318 -

319 - // Determine source and target versions based on revert flag

320 - var sourceVer, targetVer string

321 - if revert {

322 - // When reverting, we go backwards: fs-repo-15-to-16 with -revert goes 16→15

323 - sourceVer = "%s"

324 - targetVer = "%s"

325 - } else {

326 - // Normal forward migration: fs-repo-15-to-16 goes 15→16

327 - sourceVer = "%s"

328 - targetVer = "%s"

355 + if lockFile != nil {

356 + lockFile.Close()

357 + defer os.Remove(lockPath)

358 }

330 -

331 - // Print migration message (same format as real migrations)

332 - fmt.Printf("fake applying %%s-to-%%s repo migration\n", sourceVer, targetVer)

333 -

334 - // Update version file

335 - versionFile := filepath.Join(repoPath, "version")

336 - err := os.WriteFile(versionFile, []byte(targetVer), 0644)

337 - if err != nil {

338 - fmt.Fprintf(os.Stderr, "Error updating version: %%v\n", err)

359 +

360 + // Small delay to simulate migration work

361 + time.Sleep(10 * time.Millisecond)

362 +

363 + if err := os.WriteFile(filepath.Join(path, "version"), []byte(to), 0644); err != nil {

364 + fmt.Fprintf(os.Stderr, "Error: %%v\n", err)

365 os.Exit(1)

366 }

341 -}

342 -`, toVer, fromVer, fromVer, toVer)

367 +}`, fromVer, toVer)

368

369 require.NoError(t, os.WriteFile(sourceFile, []byte(goSource), 0644))

370

371 // Compile the Go binary

347 - require.NoError(t, os.Setenv("CGO_ENABLED", "0")) // Ensure static binary

348 - require.NoError(t, exec.Command("go", "build", "-o", binaryPath, sourceFile).Run())

349 -

350 - // Add bin directory to PATH for this test

351 - currentPath := os.Getenv("PATH")

352 - newPath := binDir + string(filepath.ListSeparator) + currentPath

353 - require.NoError(t, os.Setenv("PATH", newPath))

354 - t.Cleanup(func() { os.Setenv("PATH", currentPath) })

372 + cmd := exec.Command("go", "build", "-o", binaryPath, sourceFile)

373 + cmd.Env = append(os.Environ(), "CGO_ENABLED=0") // Ensure static binary

374 + require.NoError(t, cmd.Run())

375

376 // Verify the binary exists and is executable

377 _, err := os.Stat(binaryPath)

378 require.NoError(t, err, "Mock binary should exist")

379 +

380 + // Return the bin directory to be added to PATH

381 + return binDir

382 }

383

384 // expectedMigrationSteps generates the expected migration step strings for a version range.

@@ -416,26 +439,19 @@ func testRepoReverseHybridMigrationLatestTo15(t *testing.T) {

439 // Start with v15 fixture and migrate forward to latest to create proper backup files

440 node := setupStaticV15Repo(t)

441

419 - // Create mock migration binary for 15→16 (needed for forward migration)

420 - createMockMigrationBinary(t, "15", "16")

421 - // Create mock migration binary for 16→15 (needed for downgrade)

422 - createMockMigrationBinary(t, "16", "15")

442 + // Create mock migration binaries for both forward and reverse migrations

443 + mockBinDirs := []string{

444 + createMockMigrationBinary(t, "15", "16"), // for forward migration

445 + createMockMigrationBinary(t, "16", "15"), // for downgrade

446 + }

447 + customPath := buildCustomPath(mockBinDirs...)

448

449 configPath := filepath.Join(node.Dir, "config")

450 versionPath := filepath.Join(node.Dir, "version")

451

452 // Step 1: Forward migration from v15 to latest to create backup files

453 t.Logf("Step 1: Forward migration v15 → v%d", ipfs.RepoVersion)

429 - result := node.Runner.Run(harness.RunRequest{

430 - Path: node.IPFSBin,

431 - Args: []string{"repo", "migrate"},

432 - CmdOpts: []harness.CmdOpt{

433 - func(cmd *exec.Cmd) {

434 - // Ensure the command inherits our modified PATH with mock binaries

435 - cmd.Env = append(cmd.Env, "PATH="+os.Getenv("PATH"))

436 - },

437 - },

438 - })

454 + result := runMigrationWithCustomPath(node, customPath, "repo", "migrate")

455

456 // Debug: print the output to see what happened

457 t.Logf("Forward migration stdout:\n%s", result.Stdout.String())

@@ -459,16 +475,7 @@ func testRepoReverseHybridMigrationLatestTo15(t *testing.T) {

475

476 // Step 2: Reverse hybrid migration from latest to v15

477 t.Logf("Step 2: Reverse hybrid migration v%d → v15", ipfs.RepoVersion)

462 - result = node.Runner.Run(harness.RunRequest{

463 - Path: node.IPFSBin,

464 - Args: []string{"repo", "migrate", "--to=15", "--allow-downgrade"},

465 - CmdOpts: []harness.CmdOpt{

466 - func(cmd *exec.Cmd) {

467 - // Ensure the command inherits our modified PATH with mock binaries

468 - cmd.Env = append(cmd.Env, "PATH="+os.Getenv("PATH"))

469 - },

470 - },

471 - })

478 + result = runMigrationWithCustomPath(node, customPath, "repo", "migrate", "--to=15", "--allow-downgrade")

479 require.Empty(t, result.Stderr.String(), "Reverse hybrid migration should succeed without errors")

480

481 // Debug output