diff --git a/internal/appdir/appdir.go b/internal/appdir/appdir.go index fa2cd076..ab036140 100644 --- a/internal/appdir/appdir.go +++ b/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 } } diff --git a/internal/appdir/appdir_test.go b/internal/appdir/appdir_test.go index 71772588..1a435af1 100644 --- a/internal/appdir/appdir_test.go +++ b/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) }