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

@@ -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 f.Features().MkdirMetadata != nil {
-	if SkipDestructive(ctx, logName, "make directory") {
+		// Make the directory with the modtime as metadata
-		return nil, nil
-	}
 		metadata := fs.Metadata{
 			"mtime": modTime.Format(time.RFC3339Nano),
 		}
-	newDst, err = MkdirMetadata(ctx, f, dir, metadata)
+		return 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 {
+	if f.Features().DirSetModTime == nil {
-		// The directory was created and we have logged already
+		return nil, nil
-		return newDst, nil
 	}
-	// The directory was created with Mkdir then we should try to set the time
+	return SetDirModTime(ctx, f, nil, dir, modTime)
-	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
 }
 
 // TryRmdir removes a container but not if not empty.  It doesn't

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