appdir: a data folder beside magpie in the home folder, Downloads, Desktop or Documents is its portable data only when it holds magpie's files and the installed folder doesn't (or with .portable), so magpie.exe run from Downloads no longer writes into a user's own Downloads\data (tangle778 on X)

internal/appdir/appdir.go

@@ -20,10 +20,14 @@
 // keychain, the folders the OS reads for autostart and link handling, and
 // temporary folders.
 //
-// A data folder in a folder every app shares (/Applications on a Mac) is
-// not taken for magpie's own while magpie's installed folder holds its
-// files (#989): a "data" folder there is as likely another app's, and
-// taking it showed a user who had never asked for portable an empty magpie.
+// A data folder in a folder every app or download shares (/Applications on
+// a Mac; the home folder, Downloads, Desktop and Documents everywhere) is
+// taken for magpie's own only when it already holds magpie's files and the
+// installed folder doesn't, or with a ".portable" marker: a "data" folder
+// there is as likely another app's or the user's own. Taking one showed a
+// user who had never asked for portable an empty magpie (#989), and wrote
+// magpie's files into a Downloads\data folder a Windows user's archiver
+// empties, so a provider just added was gone (tangle778 on X).
 //
 // The decision is made once, from the executable as it was when magpie
 // started: an update moving the running copy aside (into .magpie-update on
@@ -65,7 +69,7 @@ func Portable() string {
 		portable = Resolve(exe)
 		if portable == "" {
 			if data := Passed(exe); data != "" {
-				log.Printf("appdir: %s is beside magpie but not taken for its portable data: it is in a folder every app shares, and %s has magpie's files (#989)", data, installed())
+				log.Printf("appdir: %s is beside magpie but not taken for its portable data: it is in a shared folder and %s has magpie's files too; a .portable file beside magpie makes it portable", data, installed())
 			}
 		}
 	})
@@ -93,61 +97,75 @@ func Resolve(exe string) string {
 	if fi, err := os.Stat(filepath.Join(base, MarkerName)); err == nil && !fi.IsDir() {
 		return absolute(data)
 	}
-	if fi, err := os.Stat(data); err == nil && fi.IsDir() && !shared(base) {
+	if fi, err := os.Stat(data); err == nil && fi.IsDir() && !shared(base, data) {
 		return absolute(data)
 	}
 	return ""
 }
 
-// Passed is the data folder beside a magpie run from exe that Resolve
-// doesn't take for its own because it is in a folder every app shares
-// while the installed folder has magpie's files, or "".
+// Passed is the data folder beside a magpie run from exe that holds
+// magpie's files but that Resolve doesn't take for its own, because it is
+// in a shared folder while the installed folder has magpie's files too, or
+// "". A data folder holding none of them is someone else's and isn't named.
 func Passed(exe string) string {
 	if real, err := filepath.EvalSymlinks(exe); err == nil {
 		exe = real
 	}
 	base := Beside(exe)
 	data := filepath.Join(base, DataName)
-	if fi, err := os.Stat(data); err == nil && fi.IsDir() && Resolve(exe) == "" {
+	if fi, err := os.Stat(data); err == nil && fi.IsDir() && hasFiles(data) && Resolve(exe) == "" {
 		return absolute(data)
 	}
 	return ""
 }
 
-// shared says whether a data folder in dir can't be taken for magpie's:
-// dir is a folder every app shares (/Applications or ~/Applications on a
-// Mac), where a folder named data is as likely another app's, and the
-// installed folder holds magpie's files. One who keeps magpie portable
-// there never ran it installed, and keeps it portable; a ".portable"
-// marker beside it makes it portable whatever is installed.
-func shared(dir string) bool {
+// shared says whether the data folder in dir can't be taken for magpie's:
+// dir is a folder every app or download shares, where a folder named data
+// is as likely another app's or the user's own, and either data holds none
+// of magpie's files or the installed folder holds them too. One who keeps
+// magpie portable there has its files in data and never ran it installed,
+// and keeps it portable; a ".portable" marker beside it makes it portable
+// whatever is there.
+func shared(dir, data string) bool {
 	if !sharedFolder(dir) {
 		return false
 	}
-	in := installed()
-	if in == "" {
-		return false
+	if !hasFiles(data) {
+		return true
 	}
+	in := installed()
+	return in != "" && hasFiles(in)
+}
+
+// hasFiles says whether dir holds magpie's own files.
+func hasFiles(dir string) bool {
 	for _, f := range []string{"providers.json", "logins.json", "settings.json"} {
-		if fi, err := os.Stat(filepath.Join(in, f)); err == nil && !fi.IsDir() {
+		if fi, err := os.Stat(filepath.Join(dir, f)); err == nil && !fi.IsDir() {
 			return true
 		}
 	}
 	return false
 }
 
-// sharedFolder says whether dir is a folder every app shares; tests change
-// it.
+// sharedFolder says whether dir is a folder every app or download shares:
+// /Applications and ~/Applications on a Mac, and the home folder, its
+// Downloads, Desktop and Documents everywhere. Tests change it.
 var sharedFolder = func(dir string) bool {
-	if runtime.GOOS != "darwin" {
-		return false
+	var dirs []string
+	if runtime.GOOS == "darwin" {
+		dirs = append(dirs, "/Applications")
 	}
-	dirs := []string{"/Applications"}
 	if h, err := Home(); err == nil {
-		dirs = append(dirs, filepath.Join(h, "Applications"))
+		dirs = append(dirs, h)
+		for _, d := range []string{"Downloads", "Desktop", "Documents"} {
+			dirs = append(dirs, filepath.Join(h, d))
+		}
+		if runtime.GOOS == "darwin" {
+			dirs = append(dirs, filepath.Join(h, "Applications"))
+		}
 	}
 	for _, d := range dirs {
-		if strings.EqualFold(filepath.Clean(dir), d) {
+		if strings.EqualFold(filepath.Clean(dir), filepath.Clean(d)) {
 			return true
 		}
 	}

internal/appdir/appdir_test.go

@@ -182,6 +182,12 @@ func TestResolveSharedFolder(t *testing.T) {
 	mkdir(t, filepath.Join(data, "nodes"))
 	touch(t, filepath.Join(data, "install-id"))
 
+	// another app's data folder, nothing installed: not taken
+	if got := Resolve(exe); got != "" {
+		t.Errorf("no magpie files in data: %q, want installed", got)
+	}
+	touch(t, filepath.Join(data, "settings.json"))
+
 	// nothing installed: one who keeps magpie portable there
 	if got := Resolve(exe); got != data {
 		t.Errorf("nothing installed: %q, want %q", got, data)
@@ -216,6 +222,59 @@ func TestResolveSharedFolder(t *testing.T) {
 	}
 }
 
+// tangle778 on X: magpie.exe run from Downloads beside a data folder the
+// user's archiver empties wrote its providers there and lost them. A data
+// folder in Downloads holding none of magpie's files isn't its portable
+// data; one that does is, and .portable always makes it portable.
+func TestResolveDownloads(t *testing.T) {
+	t.Setenv("APPIMAGE", "")
+	r := root(t)
+	h := filepath.Join(r, "home")
+	t.Setenv("HOME", h)
+	t.Setenv("USERPROFILE", h)
+	t.Setenv("XDG_CONFIG_HOME", "")
+	dl := filepath.Join(h, "Downloads")
+	exe := touch(t, filepath.Join(dl, "magpie.exe"))
+	data := filepath.Join(dl, "data")
+	touch(t, filepath.Join(data, "report.json"))
+
+	if got := Resolve(exe); got != "" {
+		t.Errorf("user's data folder: %q, want installed", got)
+	}
+	if got := Passed(exe); got != "" {
+		t.Errorf("user's data folder: Passed %q, want none", got)
+	}
+	touch(t, filepath.Join(data, "providers.json"))
+	if got := Resolve(exe); got != data {
+		t.Errorf("portable run before: %q, want %q", got, data)
+	}
+	touch(t, filepath.Join(h, ".config", "magpie", "logins.json"))
+	if got := Resolve(exe); got != "" {
+		t.Errorf("installed has files: %q, want installed", got)
+	}
+	if got := Passed(exe); got != data {
+		t.Errorf("Passed %q, want %q", got, data)
+	}
+	touch(t, filepath.Join(dl, ".portable"))
+	if got := Resolve(exe); got != data {
+		t.Errorf(".portable: %q, want %q", got, data)
+	}
+}
+
+func TestSharedFolders(t *testing.T) {
+	h, _ := Home()
+	for _, d := range []string{h, filepath.Join(h, "Downloads"), filepath.Join(h, "Downloads") + string(filepath.Separator), filepath.Join(h, "Desktop"), filepath.Join(h, "Documents")} {
+		if !sharedFolder(d) {
+			t.Errorf("%s not shared", d)
+		}
+	}
+	for _, d := range []string{filepath.Join(h, "Downloads", "magpie"), filepath.Join(h, "tools")} {
+		if sharedFolder(d) {
+			t.Errorf("%s shared", d)
+		}
+	}
+}
+
 func TestSharedFolderIsApplications(t *testing.T) {
 	if runtime.GOOS != "darwin" {
 		if sharedFolder("/Applications") {
@@ -229,7 +288,7 @@ func TestSharedFolderIsApplications(t *testing.T) {
 			t.Errorf("%s not shared", d)
 		}
 	}
-	for _, d := range []string{"/Volumes/USB", filepath.Join(h, "Downloads"), "/Applications/Utilities"} {
+	for _, d := range []string{"/Volumes/USB", "/Applications/Utilities"} {
 		if sharedFolder(d) {
 			t.Errorf("%s shared", d)
 		}