fix(fsi.DeleteFiles): attempt to remove files, even on early return

early-bails were skipping the call to os.Remove, which meant ErrNoHistory ref removal wasn't trying to remove a (very possibly) empty directory.

cmd/remove.go

@@ -120,15 +120,15 @@ func (o *RemoveOptions) Run() (err error) {
 		}
 		if res.NumDeleted == rev.AllGenerations {
 			printSuccess(o.Out, "removed entire dataset '%s'", res.Ref)
-		} else {
+		} else if res.NumDeleted != 0 {
 			printSuccess(o.Out, "removed %d revisions of dataset '%s'", res.NumDeleted, res.Ref)
 		}
-		if res.Unlinked {
-			printSuccess(o.Out, "removed dataset link")
-		}
 		if res.DeletedFSIFiles {
 			printSuccess(o.Out, "deleted dataset files")
 		}
+		if res.Unlinked {
+			printSuccess(o.Out, "removed dataset link")
+		}
 	}
 	return nil
 }

fsi/fsi.go

@@ -143,17 +143,20 @@ func (fsi *FSI) Unlink(dirPath, refStr string) error {
 		log.Debugf("removing link file: %s", removeLinkErr.Error())
 	}
 
+	defer func() {
+		// always attempt to remove the directory, ignoring "directory not empty" errors
+		// os.Remove will fail if the directory isn't empty, which is the behaviour
+		// we want
+		if err := os.Remove(dirPath); err != nil && !strings.Contains(err.Error(), "directory not empty") {
+			log.Errorf("removing directory: %s", err.Error())
+		}
+	}()
+
 	if err = repo.CanonicalizeDatasetRef(fsi.repo, &ref); err != nil {
 		if err == repo.ErrNoHistory {
 			// if we're unlinking a ref without history, delete it
 			return fsi.repo.DeleteRef(ref)
 		}
-
-		return err
-	}
-
-	// attempt to remove the directory, ignoring "directory not empty" errors
-	if err := os.Remove(dirPath); err != nil && !strings.Contains(err.Error(), "directory not empty") {
 		return err
 	}