From b940ce64c7653814934f7e29682f1d0cf64c4cfe Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Tue, 1 Sep 2026 12:42:42 +0100 Subject: [PATCH] operations: fix making a directory with modtime asking twice with --interactive MkdirModTime checked --interactive/--dry-run itself and then called MkdirMetadata or Mkdir which check again, so --interactive asked twice about making the same directory and --dry-run skipped before the operation could be shown in the progress display or counted as a check. Now MkdirModTime decides how to make the directory first and delegates entirely to MkdirMetadata, or Mkdir followed by SetDirModTime, each of which does its own --interactive/--dry-run check exactly once. This also means the modtime setting fallback shows in the progress display, respects --no-update-dir-modtime and has its errors counted. --- fs/operations/operations.go | 30 +++++++++++------------------- fs/operations/operations_test.go | 16 +++++++++++++++- 2 files changed, 26 insertions(+), 20 deletions(-) diff --git a/fs/operations/operations.go b/fs/operations/operations.go index 6cae43104..3221c63a1 100644 --- a/fs/operations/operations.go +++ b/fs/operations/operations.go @@ -1115,30 +1115,22 @@ func MkdirMetadata(ctx context.Context, f fs.Fs, dir string, metadata fs.Metadat // If the directory was created with MkDir then it will attempt to use // Fs.DirSetModTime to update the directory modtime if available. func MkdirModTime(ctx context.Context, f fs.Fs, dir string, modTime time.Time) (newDst fs.Directory, err error) { - logName := fs.LogDirName(f, dir) - if SkipDestructive(ctx, logName, "make directory") { - return nil, nil + if f.Features().MkdirMetadata != nil { + // Make the directory with the modtime as metadata + metadata := fs.Metadata{ + "mtime": modTime.Format(time.RFC3339Nano), + } + return MkdirMetadata(ctx, f, dir, metadata) } - metadata := fs.Metadata{ - "mtime": modTime.Format(time.RFC3339Nano), - } - newDst, err = MkdirMetadata(ctx, f, dir, metadata) + // Otherwise make the directory then set the modtime if possible + err = Mkdir(ctx, f, dir) if err != nil { return nil, err } - if newDst != nil { - // The directory was created and we have logged already - return newDst, nil + if f.Features().DirSetModTime == nil { + return nil, nil } - // The directory was created with Mkdir then we should try to set the time - if do := f.Features().DirSetModTime; do != nil { - err = do(ctx, dir, modTime) - if err == nil { - accounting.Stats(ctx).UpdatedDirs(1) - } - } - fs.Infof(logName, "Made directory with modification time %v", modTime) - return newDst, err + return SetDirModTime(ctx, f, nil, dir, modTime) } // TryRmdir removes a container but not if not empty. It doesn't diff --git a/fs/operations/operations_test.go b/fs/operations/operations_test.go index 85afb5eb8..6eed8c742 100644 --- a/fs/operations/operations_test.go +++ b/fs/operations/operations_test.go @@ -1895,15 +1895,17 @@ func TestMkdirMetadata(t *testing.T) { func TestMkdirModTime(t *testing.T) { const name = "directory with modtime" - ctx := context.Background() + ctx, ci := fs.AddConfig(context.Background()) r := fstest.NewRun(t) if r.Fremote.Features().DirSetModTime == nil && r.Fremote.Features().MkdirMetadata == nil { t.Skip("Skipping test as remote does not support DirSetModTime or MkdirMetadata") } updatedDirs := statsUpdatedDirs(t) + checks := accounting.GlobalStats().GetChecks() newDst, err := operations.MkdirModTime(ctx, r.Fremote, name, t2) require.NoError(t, err) assert.Equal(t, updatedDirs+1, statsUpdatedDirs(t), "MkdirModTime should be counted as an updated dir") + realChecks := accounting.GlobalStats().GetChecks() - checks // Check the returned directory and one read from the listing // newDst may be nil here depending on how the modtime was set @@ -1911,6 +1913,18 @@ func TestMkdirModTime(t *testing.T) { fstest.CheckDirModTime(ctx, t, r.Fremote, newDst, t2) } fstest.CheckDirModTime(ctx, t, r.Fremote, fstest.NewDirectory(ctx, t, r.Fremote, name), t2) + + // Check that --dry-run counts the same number of checks as the + // real run but doesn't update any dirs + updatedDirs = statsUpdatedDirs(t) + checks = accounting.GlobalStats().GetChecks() + ci.DryRun = true + newDst, err = operations.MkdirModTime(ctx, r.Fremote, "dry run "+name, t2) + ci.DryRun = false + require.NoError(t, err) + require.Nil(t, newDst) + assert.Equal(t, realChecks, accounting.GlobalStats().GetChecks()-checks, "--dry-run should count the same checks as the real run") + assert.Equal(t, updatedDirs, statsUpdatedDirs(t), "--dry-run should not count updated dirs") } // statsUpdatedDirs reads the updatedDirs stat from the global stats