Use XDG_DATA_DIRS instead of hardcoding /usr/share

[jmbaur]

When running nvidia-ctk on a system that uses a custom XDG_DATA_DIRS environment variable value, the configuration files for glvnd, vulkan, and egl fail to get passed through from the host to the container. Reading from XDG_DATA_DIRS instead of hardcoding the default value allows for finding said files so they can be mounted in the container.

[elezar]

Thanks for the PR @jmbaur.

I think this is related to #327 where the intent is to make locating these files more robust. What are your thoughts on that PR?

(I will also still give this a proper review).

[jmbaur]

I think this is related to #327 where the intent is to make locating these files more robust. What are your thoughts on that PR?

Looks like a good overall change! I think the existing XDG_DATA_DIRS environment variable may cover more use cases since a user is able to potentially spread out these config files among many directories that can still be captured in a single environment variable since it is a representation of one or more directories, but I don't see anything wrong with allowing for a way to override that altogether. One other nice thing with going the XDG route is that a user would potentially not have to pass any explicit args to nvidia-ctk, assuming the distro is already setting these environment variables correctly (assuming they stray from the default value of /usr/local/share:/usr/share).

[elezar]

I think this is related to #327 where the intent is to make locating these files more robust. What are your thoughts on that PR?

Looks like a good overall change! I think the existing XDG_DATA_DIRS environment variable may cover more use cases since a user is able to potentially spread out these config files among many directories that can still be captured in a single environment variable since it is a representation of one or more directories, but I don't see anything wrong with allowing for a way to override that altogether. One other nice thing with going the XDG route is that a user would potentially not have to pass any explicit args to nvidia-ctk, assuming the distro is already setting these environment variables correctly (assuming they stray from the default value of /usr/local/share:/usr/share).

Yes, I think adding an explicit check for the XDG_DATA_DIRS variable there may make sense. (Does it also make sense to rename Config* in terms of functions and variables to Data* instead?

[elezar]

Thanks @jmbaur.

Given the changes proposed in #327, we could also update this to the following (assuming that we rebase off those):

// xdgDataDirs finds the paths as specified in the environment variable XDG_DATA_DIRS.
// See https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html.
func xdgDataDirs() []string {
	if dirs, exists := os.LookupEnv("XDG_DATA_DIRS"); exists && dirs != "" {
		return normalizeSearchPaths(dirs)
	}

	return []string{"/usr/local/share", "/usr/share"}
}

[jmbaur]

Yes, I think adding an explicit check for the XDG_DATA_DIRS variable there may make sense. (Does it also make sense to rename Config* in terms of functions and variables to Data* instead?

IMO Config* is easier to understand what is being referred to rather than Data*, but to each their own. I know there also exist other XDG environment variables that have CONFIG in the name, it just so happens that the environment variable that is useful in this context doesn't have that.

[jmbaur]

Thanks @jmbaur.

Given the changes proposed in #327, we could also update this to the following (assuming that we rebase off those):

// xdgDataDirs finds the paths as specified in the environment variable XDG_DATA_DIRS.
// See https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html.
func xdgDataDirs() []string {
	if dirs, exists := os.LookupEnv("XDG_DATA_DIRS"); exists && dirs != "" {
		return normalizeSearchPaths(dirs)
	}

	return []string{"/usr/local/share", "/usr/share"}
}

Actually looks like this change can be done now, doesn't seem to be dependent on #327, unless I'm missing something.

[elezar]

Actually looks like this change can be done now, doesn't seem to be dependent on #327, unless I'm missing something.

You're right, it's not dependent on #327 and should be applicable now.

[jmbaur]

Changed to use NormalizeSearchPaths

[elezar]

@jmbaur I got around to merging #327 and have rebased your changes here. Please take a look to see if I missed anything.

[elezar]

Thanks for this contribution @jmbaur.

[jmbaur]

@jmbaur I got around to merging #327 and have rebased your changes here. Please take a look to see if I missed anything.

Looks right to me! Thanks for rebasing