Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dcrutil: Use os.UserHomeDir in appDataDir. #3196

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

peterzen
Copy link
Member

@peterzen peterzen commented Oct 4, 2023

This PR swaps out the user.Current().HomeDir with os.UserHomeDir(), which has been the recommended way of obtaining the user's home directory path since go 1.12

Rationale for the change: packaging systems like Flatpak and Snap provide sandboxed environments for applications and don't allow binaries to directly write to the home directory. Instead, they designate a separate directory per app and set $HOME accordingly. user.Current().HomeDir parses /etc/passwd, os.UserHomeDir uses environment variables, thus the latter enables consumers of appDataDir to run inside these sandboxes.

This change is specifically required for the snap and flatpak packaging effort underway for the DCRDEX desktop app.

Obtain user home directory from os.UserHomeDir()
instead of user.Current().HomeDir
Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code change looks fine, but I'll need to test this. I'm always concerned about changes like this because any change to the default location will break all existing software.

If it turns out that there is a difference that causes issues, an "AppDataDirV2" that accepts a flag to opt in to the new behavior will probably be required.

@peterzen
Copy link
Member Author

peterzen commented Oct 4, 2023

The code change looks fine, but I'll need to test this. I'm always concerned about changes like this because any change to the default location will break all existing software.

If it turns out that there is a difference that causes issues, an "AppDataDirV2" that accepts a flag to opt in to the new behavior will probably be required.

Thank you for looking into this.

I ran the below test on Linux, Windows 10 and MacOS:

package main

import (
	"fmt"
	"os"
	"os/user"
	"path/filepath"
	"runtime"
	"strings"
	"unicode"
)

func appDataDir(goos, appName string, roaming bool) string {
	if appName == "" || appName == "." {
		return "."
	}

	// The caller really shouldn't prepend the appName with a period, but
	// if they do, handle it gracefully by stripping it.
	appName = strings.TrimPrefix(appName, ".")
	appNameUpper := string(unicode.ToUpper(rune(appName[0]))) + appName[1:]
	appNameLower := string(unicode.ToLower(rune(appName[0]))) + appName[1:]

	// Get the OS specific home directory via the Go standard lib.
	var homeDir string
	usr, err := user.Current()
	if err == nil {
		homeDir = usr.HomeDir
	}
	// Fall back to standard HOME environment variable that works
	// for most POSIX OSes if the directory from the Go standard
	// lib failed.
	if err != nil || homeDir == "" {
		homeDir = os.Getenv("HOME")
	}

	switch goos {
	// Attempt to use the LOCALAPPDATA or APPDATA environment variable on
	// Windows.
	case "windows":
		// Windows XP and before didn't have a LOCALAPPDATA, so fallback
		// to regular APPDATA when LOCALAPPDATA is not set.
		appData := os.Getenv("LOCALAPPDATA")
		if roaming || appData == "" {
			appData = os.Getenv("APPDATA")
		}

		if appData != "" {
			return filepath.Join(appData, appNameUpper)
		}

	case "darwin":
		if homeDir != "" {
			return filepath.Join(homeDir, "Library",
				"Application Support", appNameUpper)
		}

	case "plan9":
		if homeDir != "" {
			return filepath.Join(homeDir, appNameLower)
		}

	default:
		if homeDir != "" {
			return filepath.Join(homeDir, "."+appNameLower)
		}
	}

	// Fall back to the current directory if all else fails.
	return "."
}

func appDataDirNew(goos, appName string, roaming bool) string {
	if appName == "" || appName == "." {
		return "."
	}

	// The caller really shouldn't prepend the appName with a period, but
	// if they do, handle it gracefully by stripping it.
	appName = strings.TrimPrefix(appName, ".")
	appNameUpper := string(unicode.ToUpper(rune(appName[0]))) + appName[1:]
	appNameLower := string(unicode.ToLower(rune(appName[0]))) + appName[1:]

	// Get the OS specific home directory via the Go standard lib.
	homeDir, err := os.UserHomeDir()
	// Fall back to standard HOME environment variable that works
	// for most POSIX OSes if the directory from the Go standard
	// lib failed.
	if err != nil || homeDir == "" {
		homeDir = os.Getenv("HOME")
	}

	switch goos {
	// Attempt to use the LOCALAPPDATA or APPDATA environment variable on
	// Windows.
	case "windows":
		// Windows XP and before didn't have a LOCALAPPDATA, so fallback
		// to regular APPDATA when LOCALAPPDATA is not set.
		appData := os.Getenv("LOCALAPPDATA")
		if roaming || appData == "" {
			appData = os.Getenv("APPDATA")
		}

		if appData != "" {
			return filepath.Join(appData, appNameUpper)
		}

	case "darwin":
		if homeDir != "" {
			return filepath.Join(homeDir, "Library",
				"Application Support", appNameUpper)
		}

	case "plan9":
		if homeDir != "" {
			return filepath.Join(homeDir, appNameLower)
		}

	default:
		if homeDir != "" {
			return filepath.Join(homeDir, "."+appNameLower)
		}
	}

	// Fall back to the current directory if all else fails.
	return "."
}
func main() {
	homedir, _ := os.UserHomeDir()

	fmt.Printf("OS:                 %s\n", runtime.GOOS)
	fmt.Printf("UserHomeDir:        %s\n", homedir)
	fmt.Printf("appDataDir:         %s\n", appDataDir(runtime.GOOS, "dexc", false))
	fmt.Printf("appDataDir_roaming: %s\n", appDataDir(runtime.GOOS, "dexc", true))
	fmt.Println("------------------------------------")
	fmt.Printf("appDataDir:         %s\n", appDataDirNew(runtime.GOOS, "dexc", false))
	fmt.Printf("appDataDir_roaming: %s\n", appDataDirNew(runtime.GOOS, "dexc", true))
}

Copy link
Member

@davecgh davecgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I looked at the code for os.UserHomeDir and how HomeDir is set in user.Current() and see that there should not be any differences between the defaults. The differences only surface when environment variables are overridden and when the OS doesn't follow the XDG base directory specification. Neither of those apply to our officially supported operating systems nor any of the BSDs or plan9.

In addition, to further confirm, I compared the results of os.UserHomeDir and HomeDir via user.Current() with defaults on Windows, Linux, OpenBSD, FreeBSD, and plan9. I don't have a Mac to test unfortunately, but given my review of the standard library code and knowing the path that is currently returned, I'm pretty confident there is no change there either.

Given the above, the only potential cases of differences are on entirely unsupported OSes or in scenarios where the user has intentionally made custom modifications. For the latter case, which is certainly exceedingly rare, any user capable of making such modifications are certainly capable of moving the config and data files around accordingly.

@davecgh davecgh changed the title dcrutil: Obtain home directory path from os.UserHomeDir in appDataDir dcrutil: Use os.UserHomeDir in appDataDir. Oct 5, 2023
@davecgh davecgh merged commit c102e54 into decred:master Oct 5, 2023
2 checks passed
@peterzen peterzen deleted the appdatadir-userhomedir branch October 21, 2023 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants