Use XDG Base Directories #96
Conversation
aDogCalledSpot
force-pushed
the
xdg_base_directory
branch
from
c406e4b to
e6a0930
Compare
aDogCalledSpot
force-pushed
the
xdg_base_directory
branch
from
e6a0930 to
4399b02
Compare
aDogCalledSpot
force-pushed
the
xdg_base_directory
branch
from
4399b02 to
22bedbb
Compare
|
Thank you for your contribution. How much did you test this already? What I don't like that much is that even when the xdg variables aren't defined it defaults to the "recommended" paths. This would also require documentation about each file and where it would be located. Which is currently quite easy: Everything is in |
So far I have made sure that all the path files are set correctly. I have not extensively tested functionality. I'll make sure to run more tests once we have agreed on where everything should go.
It is what is stated in the specification and is followed by many programs. No distributions I am aware of set the environment variables by default yet ~/.config is used by various programs.
Where would you suggest we put this? Would it be sufficient to write it into the README? |
One thing I've noticed is that you can't use it anymore when the base directories don't exist already. (
Probably DOCUMENTATION.md or something the like, (doesn't exist yet, but very much needed in the future) which would be for a technical documentation. |
| @@ -80,13 +80,15 @@ def cli(): | |||
| """Run user's input command.""" | |||
|
|
|||
| # Initial log values | |||
| change_file_owner(os.path.join(CONFIG_DIR, "pvpn-cli.log")) | |||
| change_file_owner(os.path.join(CACHE_DIR, "pvpn-cli.log")) | |||
There was a problem hiding this comment.
There currently isn't a LOG_DIR as there isn't one listed in the specification. As log files are not necessary to run the program they are placed into the CACHE_DIR. I could make a separate LOG_DIR and have that be the same as CACHE_DIR to make the code clearer.
There was a problem hiding this comment.
Yeah, I just see that XDG standard is incomplete here: https://stackoverflow.com/questions/25897836/where-should-i-write-a-user-specific-log-file-to-and-be-xdg-base-directory-comp
There was a problem hiding this comment.
The specification does state the following
The
$XDG_STATE_HOMEcontains state data that should persist between (application) restarts, but that is not important or portable enough to the user that it should be stored in$XDG_DATA_HOME. It may contain:
- actions history (logs, history, recently used files, …)
- [......]
This was was already stated in the stack overflow link above as well.
Admittedly I have not defined this environment variable in my own implementation. (I only have XDG_DATA_HOME and XDG_CONFIG_HOME environment variable defined).
While it may not be set in stone, I think the XDG Base Directories is the best and most widely accepted standard that we have. Implementing this will further encourage adoption, keep $HOME directories clean and easier to maintain. This change also is somewhat trivial and provides more value than the pain of adding it.
While I understand that this seems to scatter the various files needed by protonvpn_cli among the different directories, keep in mind that this is the case with system applications as well (/etc for config files, /var/log for logs, etc....). Having the $HOME organized thus does not increase its complexity. Rather it only improves upon organization facilitating automated maintenance/archival tasks.
They would be created by I could check in order whether the configuration files can be found in the XDG directory (if the environment variable is set or the default if it is not set) and then the home directory. If no files are found the user is told to run
Would this only be an internal documentation? Where configuration files are stored should also be available to users, so a man page would be pretty good. Though this should probably be a separate issue. |
Not in the current form. It's basically
Not sure what you mean by internal. It would just be another file in this git repository. |
| xdg_config_str = os.getenv("XDG_CONFIG_HOME") | ||
| if not xdg_config_str: | ||
| xdg_config = os.path.join(home_path, ".config") | ||
| else: | ||
| xdg_config = os.path.realpath(xdg_config_str) |
There was a problem hiding this comment.
nit: perhaps you can simplify it by writing
xdg_config = os.path.realpath(
os.getenv("XDG_CONFIG_HOME", os.path.join(home_path, ".config"))
)There was a problem hiding this comment.
You could also use the XDG module (last updated April of 2021) to facilitate the base directory specification.
The XDG Base Directories are currently unsupported. You can find the specification here. A brief summary is available on the ArchWiki.
Using the specification gives the user more flexibility in where certain files should go. Programs can also take advantage of the directories being used by, for example, having backup programs exclude
$XDG_CACHE_HOME.Which files go where should be discussed. My proposal would be:
TEMPLATE_FILEdoesn't appear provide any user-specific content so it would be placed into$XDG_CACHE_HOMECONFIG_FILEis a configuration file and should go into$XDG_CONFIG_HOMESERVER_INFO_FILEgoes into$XDG_CACHE_HOMESPLIT_TUNNEL_FILEgoes into$XDG_DATA_HOMEOVPN_FILEgoes into$XDG_DATA_HOMEPASSFILEgoes into$XDG_DATA_HOME$XDG_CACHE_HOME. It should be noted that there are programs which handle this differently.$XDG_DATA_HOMEIn order to not break any current configurations, it will first be checked if
"$HOME/.pvpn-cli"exists and if it does it will be used as the directory in which all files would be placed (as before).The function
purge_configurationcurrently deletes everything in the config directory. For now, I have kept it that it only deletes the configuration file, this could be changed to also delete everything in the data and/or cache directory.