add $DPATH environment variable for file locations #2281

WebFreak001 · 2022-06-21T00:17:34Z

I chose not to add a setting for this to make it easier for other apps
in the ecosystem to understand how to locate and change the dub folder,
to parse packages, manipulate them, maintain them, etc.

Having a setting would require other users to potentially locate the
config files (which has a bunch of exceptions), parse a JSON and find
an optional value. Limiting it to an environment variable means that
also tools like scripts can easily find the location of this.

Geod24 · 2022-06-21T00:28:01Z

But an environment variable is prone to other kind of issues. For one it's not persistent unless you edit your shell config, which looses locality. Also names are prone to collision. IMO if one wants to get this value in script, we should have a dub config command.

WebFreak001 · 2022-06-21T07:45:42Z

imo if you set your environment variable only locally then you probably only want DUB to store its data where you specified in that session. (maybe set to /tmp/dub to quickly try out some package without downloading it, or different stores for every different user)

This is similar to the JAVAHOME or GOPATH variables. (go works in a similar way with $GOPATH/pkg/... for downloaded packages)

I think $DPATH is a good fit not only for dub, but also for other tools that we might want to include, if we start with some convention there. In that case it would definitely need to be an environment variable to not be dependent on a single tool.

WebFreak001 · 2022-06-21T10:18:59Z

if we don't plan to add other tools, it might also make sense to define a DUB_HOME environment variable instead, which defaults to ~/.dub / %APPDATA%/dub, so the folder can be overwritten in whole.

WebFreak001 · 2022-06-28T11:56:09Z

source/dub/dub.d

+
+	@property string dubHome()
+	const {
+		if(auto pv = "dubHome" in m_data)


should the setting for this be called dubHome or something else?

Can we make things a bit more hierarchical, now that we have the ability to (easily) do so ?
Like core.author in git, you know.

WebFreak001 · 2022-06-28T11:58:32Z

after discussing with @Geod24 I changed this to also include a setting instead of only an environment variable. This is so that system-wide changes can be made, e.g. by linux package maintainers, but also kept an environment variable so that it's possible to create a quick temporary clean local dub package storage or in general if it might be needed for some specific user's projects on the system.

To support changing the path of the packages (and also the config), the config in /etc/dub/settings.json (or /usr/...) will be read first and if the dubHome property is set in there, loading of further configs will be redirected to there, as well as the packages being stored there.

This introduces the $DPATH environment variable to override the user's home directory just for D tools. This is also intended to be used by DUB (dlang/dub#2281) when there is not the more explicit `$DUB_HOME` variable set on the system. This allows users who install their D compiler through the install.sh script to both have the compiler installations as well as the dub packages be stored elsewhere in the system than in their home folder. In case we have other tools that download their dependencies into the user's home directory, we should also make them support the same $DPATH environment variable.

dk949 · 2022-06-30T11:37:31Z

Hi 👋

I've been working on something similar (I guess now I'll just wait for this to merge), and just wanted to share some thoughts from my implementation.

I also considered something like DUB_HOME or DUB_PATH, but came to the conclusion that it's far too generic, which means users may have it set to control some other part of their system (DPATH would suffer the most from this).

I am not sure how common this is on Windows or Mac, but a lot of Linux programs use environment variables to control the location of their config/cache/etc files (see this page for some examples).

In the end I settled for DUB_SYSTEM_SETTINGS_PATH, DUB_USER_SETTINGS_PATH and DUB_LOCAL_REPOSITORY_PATH. As I understand you intend other D tooling to use this directory, so maybe something like DLANG_HOME (in line with JDK_HOME)?

As for "...it's not persistent unless you edit your shell config", well yes, but users are already used to doing this. zsh even has a special config file pretty much for this purpose (.zshenv).

My impl for reference.

WebFreak001 · 2022-07-06T09:10:12Z

I made it now so that environment variables can only change both folders at once, but you can use the settings for finer control of user settings and repository path, or use the general "dubHome" setting to set both.

Additionally for package maintainers and sysadmins it's probably interesting to have environment variables in the settings, for example to have a different user store for each user, so I added that as well.

See the changelog entry for details https://github.com/dlang/dub/blob/5addac0e22036cf6b328bda72ac87c47d55d0331/changelog/dpath.dd

Geod24

I just realized we read /etc/ before /var/lib...

The idea of a "dub home" is a bit weird: We don't have a unified "root location". We have the badly-named Repository (see the private struct in packagemanager.d) and we tackled on config files on top of that.

Also what's the relationship with customCachePaths ?

changelog/dpath.dd

Geod24 · 2022-07-28T16:57:42Z

changelog/dpath.dd

+```json
+{
+    "dubHome": "/path/to/dub", // sets both package store and config location
+
+    // OR
+
+    "userSettings": "/path/to/dub/settings/folder",
+    "localRepository": "/path/to/dub/repository/folder"
+}
+```
+
+Additionally, these config paths will have environment variables using the `$VARIABLE` syntax resolved.
+
+The following list describes which path is going to be picked, from top to bottom, stopping whenever one is found:
+
+- `$DUB_HOME` environment variable
+- `$DPATH` environment variable
+- system-wide settings.json: `"userSettings"` property
+- system-wide settings.json: `"dubHome"` property (only for userSettings)
+- through all settings.json: `"localRepository"` property
+- through all settings.json: `"dubHome"` property (only for localRepository)


Wait isn't that what customCachePaths is for ?

this overrides the built-in dub location, so it would never even attempt to read anything from the built-in path, while for customCachePaths it's just an addition of paths.

source/dub/dub.d

Geod24 · 2022-07-28T16:59:09Z

source/dub/dub.d

+		// override default userSettings + localRepository if a $DPATH or
+		// $DUB_HOME environment variable is set.
+		bool overrideUserSettings;
+		bool overrideLocalRepository;
+		{
+			string dpathOverride = environment.get("DUB_HOME");
+			if (!dpathOverride.length) {
+				dpathOverride = environment.get("DPATH");
+				if (dpathOverride.length)
+					dpathOverride = (NativePath(dpathOverride) ~ "dub/").toNativeString();
+
+			}
+			if (dpathOverride.length) {
+				overrideUserSettings = true;
+				overrideLocalRepository = true;
+
+				m_dirs.userSettings = NativePath(dpathOverride);
+				m_dirs.localRepository = m_dirs.userSettings;
+			}
+		}


Should we move that logic in SpecialDirs.make ?

I think that's difficult as the state variables overrideUserSettings and overrideLocalRepository are later used to determine if custom path logic should be applied.

changelog/dpath.dd

Geod24 · 2022-07-28T17:03:13Z

source/dub/dub.d

+		// Override user + local package path from /etc/dub/settings.json
+		// Then continues loading local settings from these folders. (keeping
+		// global /etc/dub/settings.json settings intact)
+		//
+		// Don't use it if either $DPATH or $DUB_HOME are set.
+		if (!overrideUserSettings) {
+			if (m_config.userSettings.length) {
+				m_dirs.userSettings = NativePath(m_config.userSettings.expandEnvironmentVariables);
+
+				overrideUserSettings = true;
+			} else if (m_config.dubHome.length) {
+				m_dirs.userSettings = NativePath(m_config.dubHome.expandEnvironmentVariables);
+
+				overrideUserSettings = true;
+			}
+		}


The circular dependency is IMO weird. I need a bit of time to think about it.

it's a little weird in the code, but I think it's great for user flexibility.

A use case here: package maintainer creates global settings.json in /var/lib/dub which then specifies where the user-overridable config is located. On linux distros that have read-only filesystems with some special paths for writable things that might for example be useful. Or for users using non-standard paths like storing all dub things on a separate flash drive or something that just isn't mounted in /etc.

Additionally it's possible to make the config only per-user and use an environment variable like $HOME inside the settings file so each user can have different dub settings.

Geod24 · 2022-07-28T17:05:41Z

Any chance we could get #2343 before, since it overhaul this part of the code and simplifies the logic ? I can then rebase your branch on top of it, if you'd like.

WebFreak001 · 2022-07-28T17:51:47Z

I just realized we read /etc/ before /var/lib...

I don't think that's the case, the systemSettings path is the very first config that is read, everything after that overrides the configs.

Geod24 · 2022-07-29T06:15:36Z

I don't think that's the case, the systemSettings path is the very first config that is read, everything after that overrides the configs.

My bad, I meant the other way around 🤦
I think we should read /etc/ first, but we currently don't.

source/dub/dub.d

WebFreak001 · 2022-07-29T06:44:01Z

I don't think that's the case, the systemSettings path is the very first config that is read, everything after that overrides the configs.

My bad, I meant the other way around facepalm I think we should read /etc/ first, but we currently don't.

I strongly feel that we should not read /etc first, but /var/lib, like currently already done. I think the idea behind that is:

consistency with other programs (e.g. systemd)
the /var/lib/... path is maintained by the distro package maintainers and should never be updated by the user, this implies:
- any setting in /var/lib should not override the user-set settings, thus it must not be loaded after the user settings in /etc

Geod24 · 2022-07-29T06:57:27Z

One use-case that might be important is the default placement path.
Currently the the localRepository (user HOME on POSIX) is the default placement.
Some users might want to use system, and some might want to use local (e.g. in the package itself). Currently the always have to pass an argument to DUB.

If you prefer we can tackle it in a separate PR, but for me it makes much more sense to think of them together (I thought the scope of this PR was originally bigger).

WebFreak001 · 2022-07-29T07:09:43Z

I think a new setting to set the default placement location should be part of a separate PR.

dk949 · 2022-07-29T07:18:19Z

I though you were planning to tie the location of localRrpository (aka $HOME/.dub, aka default placement) to $DPATH. If this is the case, wouldn't it make it somewhat inseparable from this PR?

(I too have possibly misunderstood the scope of these changes).

WebFreak001 · 2022-07-29T07:18:26Z

rebased to new yaml loader

WebFreak001 · 2022-07-29T07:19:56Z

I though you were planning to tie the location of localRrpository (aka $HOME/.dub, aka default placement) to $DPATH. If this is the case, wouldn't it make it somewhat inseparable from this PR?

(I too have possibly misunderstood the scope of these changes).

no the default placement location setting I was talking about would set a default for the dub --cache=* setting (can set it to local, system or user)

dk949 · 2022-07-29T07:22:51Z

Ohh, that makes sense. My bad.

WebFreak001 · 2022-08-15T19:39:39Z

@Geod24 what's your opinion on this?

Geod24

The env approach is sound, the settings code still makes me uneasy but makes much more sense now, made a few suggestions.

source/dub/dub.d

test/dpath-variable/dub.json

test/dpath-variable/source/app.d

source/dub/dub.d

fix dlang#229

WebFreak001 · 2022-08-22T22:55:02Z

adjusted to review, can rebase if it's fine now

source/dub/dub.d

changelog/dpath.dd

WebFreak001 · 2022-08-22T23:41:39Z

fixed left-overs

WebFreak001 · 2022-08-23T08:45:42Z

thanks!

WebFreak001 requested a review from s-ludwig June 21, 2022 00:17

WebFreak001 force-pushed the dpath branch from 0b8a176 to c515971 Compare June 28, 2022 11:55

WebFreak001 commented Jun 28, 2022

View reviewed changes

WebFreak001 mentioned this pull request Jun 28, 2022

support customizing install directory with $DPATH dlang/installer#510

Open

WebFreak001 force-pushed the dpath branch from c515971 to 5addac0 Compare July 6, 2022 09:08

WebFreak001 force-pushed the dpath branch 2 times, most recently from 74ff66d to 09df591 Compare July 28, 2022 15:23

WebFreak001 requested a review from Geod24 July 28, 2022 16:35

Geod24 reviewed Jul 28, 2022

View reviewed changes

WebFreak001 force-pushed the dpath branch from 09df591 to 5b5d0af Compare July 28, 2022 17:55

Geod24 reviewed Jul 29, 2022

View reviewed changes

source/dub/dub.d Outdated Show resolved Hide resolved

WebFreak001 force-pushed the dpath branch 2 times, most recently from 8ec873b to 43bd843 Compare July 29, 2022 06:46

WebFreak001 force-pushed the dpath branch from 5724ea0 to 48376f6 Compare July 29, 2022 07:18

Geod24 reviewed Aug 22, 2022

View reviewed changes

WebFreak001 force-pushed the dpath branch from 48376f6 to 4ee64c5 Compare August 22, 2022 22:29

WebFreak001 added 2 commits August 23, 2022 00:31

error out on unknown environment variables

e28052f

add env + config for package + settings locations

ec4372d

fix dlang#229

WebFreak001 force-pushed the dpath branch from 4ee64c5 to ec4372d Compare August 22, 2022 22:38

adjust to review, extend $DPATH test

29626c6

Geod24 reviewed Aug 22, 2022

View reviewed changes

source/dub/dub.d Outdated Show resolved Hide resolved

Geod24 reviewed Aug 22, 2022

View reviewed changes

source/dub/dub.d Outdated Show resolved Hide resolved

Geod24 reviewed Aug 22, 2022

View reviewed changes

source/dub/dub.d Outdated Show resolved Hide resolved

Geod24 reviewed Aug 22, 2022

View reviewed changes

source/dub/dub.d Outdated Show resolved Hide resolved

Geod24 reviewed Aug 22, 2022

View reviewed changes

changelog/dpath.dd Outdated Show resolved Hide resolved

review round 2

d35755d

Geod24 approved these changes Aug 23, 2022

View reviewed changes

Geod24 merged commit 27e28cf into dlang:master Aug 23, 2022

WebFreak001 deleted the dpath branch August 23, 2022 08:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add $DPATH environment variable for file locations #2281

add $DPATH environment variable for file locations #2281

WebFreak001 commented Jun 21, 2022

Geod24 commented Jun 21, 2022

WebFreak001 commented Jun 21, 2022 •

edited

Loading

WebFreak001 commented Jun 21, 2022

WebFreak001 Jun 28, 2022

Geod24 Jul 29, 2022

WebFreak001 commented Jun 28, 2022

dk949 commented Jun 30, 2022

WebFreak001 commented Jul 6, 2022

Geod24 left a comment

Geod24 Jul 28, 2022

WebFreak001 Jul 28, 2022

Geod24 Jul 28, 2022

WebFreak001 Jul 28, 2022

Geod24 Jul 28, 2022

WebFreak001 Jul 28, 2022

Geod24 commented Jul 28, 2022

WebFreak001 commented Jul 28, 2022

Geod24 commented Jul 29, 2022

WebFreak001 commented Jul 29, 2022

Geod24 commented Jul 29, 2022

WebFreak001 commented Jul 29, 2022

dk949 commented Jul 29, 2022

WebFreak001 commented Jul 29, 2022

WebFreak001 commented Jul 29, 2022

dk949 commented Jul 29, 2022

WebFreak001 commented Aug 15, 2022

Geod24 left a comment

WebFreak001 commented Aug 22, 2022

WebFreak001 commented Aug 22, 2022

WebFreak001 commented Aug 23, 2022

add $DPATH environment variable for file locations #2281

add $DPATH environment variable for file locations #2281

Conversation

WebFreak001 commented Jun 21, 2022

Geod24 commented Jun 21, 2022

WebFreak001 commented Jun 21, 2022 • edited Loading

WebFreak001 commented Jun 21, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WebFreak001 commented Jun 28, 2022

dk949 commented Jun 30, 2022

WebFreak001 commented Jul 6, 2022

Geod24 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Geod24 commented Jul 28, 2022

WebFreak001 commented Jul 28, 2022

Geod24 commented Jul 29, 2022

WebFreak001 commented Jul 29, 2022

Geod24 commented Jul 29, 2022

WebFreak001 commented Jul 29, 2022

dk949 commented Jul 29, 2022

WebFreak001 commented Jul 29, 2022

WebFreak001 commented Jul 29, 2022

dk949 commented Jul 29, 2022

WebFreak001 commented Aug 15, 2022

Geod24 left a comment

Choose a reason for hiding this comment

WebFreak001 commented Aug 22, 2022

WebFreak001 commented Aug 22, 2022

WebFreak001 commented Aug 23, 2022

WebFreak001 commented Jun 21, 2022 •

edited

Loading