adrg/xdg

DataDirs no longer used

gwillem opened this issue · 7 comments

gwillem commented

I was upgrading today from v0.0.0-20190319220657-88e5137d2444 to v0.4.0 and my app broke, because DataDirs is still present but doesn't seem to be used anymore (removed here: 1be1d4f#diff-5c0868fc869fb66c2b33ba837118f18c43f5002d0a3f8b9e11aceabe703d4b86L98)

I suggest dropping it altogether to prevent further confusion.

adrg commented

Hi @gwillem

I suggest dropping it altogether to prevent further confusion.

Can you please explain the confusion part? The xdg.DataDirs (which is represented by the XDG_DATA_DIRS environment variable) contains additional paths which can be used by applications to store data files in. The main path relative to which applications should store data files is xdg.DataHome (represented by the XDG_DATA_HOME environment variable). Both are defined by the XDG spec so neither can be removed. Also, both xdg.DataHome and xdg.DataDirs are documented in the package.

xdg.DataDirs can be used by itself and the same paths are also used by the xdg.DataFile function as fallbacks for cases in which xdg.DataHome is not a writable location, for any reason.

I was upgrading today from v0.0.0-20190319220657-88e5137d2444 to v0.4.0 and my app broke, because DataDirs is still present but doesn't seem to be used anymore (removed here: 1be1d4f#diff-5c0868fc869fb66c2b33ba837118f18c43f5002d0a3f8b9e11aceabe703d4b86L98)

The commit you are referencing is before the first ever release/tag of the library. I don't do breaking changes between minor releases of the application unless I absolutely have to, in which case, I mention it in the release notes. However, in this case, you were not even using a released version.

Also, the commit you are referencing does not remove the use of xdg.DataDirs. The same paths are still used. See:

It is unfortunate that your app stopped working after the upgrade and while I don't know the exact reason for that, again, you were using an unreleased version of the library.

Also, you opened an issue which implies that I made a breaking change (which I did not) and your proposed solution is that I make a real breaking change, by removing xdg.DataDirs.

This is not a real issue and needs to be closed.

gwillem commented

For the record, I am not accusing anyone, I am only grateful that you made this package, thanks for that!

while I don't know the exact reason for that

Commit 1be1d4f1c3bb00cccaa31a0a8044278c1c0d1cc6 changes from using DataDirs to using baseDirs.dataFile (which does not use DataDirs). So, AFAICT, DataDirs is no longer used internally, only exported.

func DataFile(name string) (string, error) {
-	return createPath(name, DataDirs)
+	return baseDirs.dataFile(relPath)
}
gwillem commented

xdg.DataDirs can be used by itself and the same paths are also used by the xdg.DataFile function as fallbacks for cases in which xdg.DataHome is not a writable location, for any reason.

Maybe I misunderstand?

func Test_xdgDataDir(t *testing.T) {
	xdg.DataDirs = []string{"/tmp"}
	xdg.DataHome = "/nonexistent_location"
	path, err := xdg.DataFile("test.txt")
	assert.NoError(t, err)
	assert.Equal(t, "/tmp/test.txt", path)
	// /Users/willem/Library/Application Support/test.txt
}
adrg commented

For the record, I am not accusing anyone, I am only grateful that you made this package, thanks for that!

while I don't know the exact reason for that

Commit 1be1d4f1c3bb00cccaa31a0a8044278c1c0d1cc6 changes from using DataDirs to using baseDirs.dataFile (which does not use DataDirs).

func DataFile(name string) (string, error) {
-	return createPath(name, DataDirs)
+	return baseDirs.dataFile(relPath)
}

That is what I'm saying:

This is what xdg.DataDirs was in the version you were using:

DataDirs = xdgPaths("DATA_DIRS", DataHome,
    []string{"/usr/local/share", "/usr/share"})

This is xdg.DataFile:

func DataFile(name string) (string, error) {
	return createPath(name, DataDirs)
}

In the "new" version, xdg.DataDirs is:

DataDirs = baseDirs.Data

which is:

baseDirs.Data = xdgPaths(envDataDirs, "/usr/local/share", "/usr/share")

This is xdg.DataFile:

func DataFile(relPath string) (string, error) {
	return baseDirs.dataFile(relPath)
}

which is:

func (bd baseDirectories) dataFile(relPath string) (string, error) {
	return createPath(relPath, append([]string{bd.DataHome}, bd.Data...))
}

In both versions, xdg.DataFile and xdg.SearchDataFile do the exact same thing.

adrg commented

xdg.DataDirs can be used by itself and the same paths are also used by the xdg.DataFile function as fallbacks for cases in which xdg.DataHome is not a writable location, for any reason.

Maybe I misunderstand?

func Test_xdgDataDir(t *testing.T) {
	xdg.DataDirs = []string{"/tmp"}
	xdg.DataHome = "/nonexistent_location"
	path, err := xdg.DataFile("test.txt")
	assert.NoError(t, err)
	assert.Equal(t, "/tmp/test.txt", path)
	// /Users/willem/Library/Application Support/test.txt
}

Oh, I see now. Your application changes xdg.DataHome and/or xdg.DataDirs. They are not meant to be changed. They are "read-only" in that sense. Yes, in the version you are referencing, that would impact xdg.DataFile. In any case, they are not used internally in xdg.DataFile or xdg.SearchDataFile in any of the released versions of the package.

Why do you want to change xdg.DataHome/xdg.DataDirs in your application anyway? xdg.DataHome and xdg.DataDirs default to the values of the $XDG_DATA_HOME and $XDG_DATA_DIRS environment variables, which are usually set by the system and you are supposed to use those paths. If they are not defined, the library provides appropriate locations for them.

If you really want to change them, although you should not need to, you can do something like:

os.Setenv("XDG_DATA_HOME", "/nonexistent_location")
os.Setenv("XDG_DATA_DIRS", "/tmp")
xdg.Reload()

path, err := xdg.DataFile("test.txt")
gwillem commented

Thanks for checking!

Why do you want to change xdg.DataHome/xdg.DataDirs in your application

To run unit tests.. Setting the OS envs works indeed, it just took me a while to figure that out :)

adrg commented

Thanks for checking!

Sure, no problem.

Why do you want to change xdg.DataHome/xdg.DataDirs in your application

To run unit tests.. Setting the OS envs works indeed, it just took me a while to figure that out :)

Ah, ok. You were changing them in unit tests. That's fair enough.