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
Ensure creation of ~/.osc_cookiejar
adheres to XDG Base Directory Specification
#940
Conversation
First of all, thanks for the PR!
On 2021-08-11 18:04:12 -0700, Edwin Kofler wrote:
In a similar theme to #349, this ensures the file is created according to the XDG Base Directory specification. Just like that change, it uses the XDG Base Directly specification on windows (usually I prefer to use the spec only on Unix-like platlforms). In this case, `XDG_CACHE_HOME` seemed to be a better fit compared to `XDG_STATE_HOME`
Why?:)
Naively, I would say that we should store it in $XDG_STATE_HOME
because:
- it should persist between (application) restarts
- it is not important enough to store it in $XDG_DATA_HOME
What are your arguments for $XDG_CACHE_HOME?
A subtle difference compared to #349 is that `XDG_STATE_HOME` is not used if it is defined, but empty (in the other change, the variable is used if it is empty). This is more correct, according to the spec.
Good catch! (You are talking about $XDG_CACHE_HOME, right?)
Are you in the mood to create a new PR that fixes this for
$XDG_CONFIG_HOME?:)
diff --git a/osc/conf.py b/osc/conf.py
index 245dce76..adcc314a 100644
--- a/osc/conf.py
+++ b/osc/conf.py
@@ -44,6 +44,7 @@
import ssl
import warnings
import getpass
+import platform
Hmm unused.
try:
from http.cookiejar import LWPCookieJar, CookieJar
@@ -94,6 +95,21 @@ def _get_processors():
except ValueError as e:
return 1
+
+def _identify_osccookiejar():
+ if os.path.isfile(os.path.join(os.path.expanduser("~"), '.osc_cookiejar')):
+ # For backwards compatibility, use the old location if it exists
+ return '~/.osc_cookiejar'
+ else:
Minor: we could get rid of the explicit else (and save one level of
indention).
+ if os.getenv('XDG_CACHE_HOME', '') != '':
+ osc_cache_dir = os.path.join(os.getenv('XDG_CACHE_HOME'), 'osc')
+ else:
+ osc_cache_dir = os.path.join(os.path.expanduser("~"), '.cache', 'osc')
+
+ if not os.path.exists(osc_cache_dir):
+ os.makedirs(osc_cache_dir)
We should create the dirs with mode=0o700. It is probably better to do the
directory creation in "def init_basicauth" (that's where the cookiejar stuff
is used).
… + return os.path.join(osc_cache_dir, 'cookiejar')
+
|
Ahhh yeah, let's do
I originally went with it because I second guessed myself with
Yeah
Yup! I'll soon update my branch with the code improvements you mentioned :) |
On 2021-08-13 14:10:49 -0700, Edwin Kofler wrote:
I'll soon update my branch with the code improvements you mentioned :)
Thanks a bunch! The patches look good to me!
Can you squash both commits into one?
|
Awesome! - now squashed into a single commit |
On 2021-08-14 17:26:35 -0700, Edwin Kofler wrote:
diff --git a/osc/conf.py b/osc/conf.py
index 245dce76..33522dad 100644
--- a/osc/conf.py
+++ b/osc/conf.py
@@ -94,6 +94,19 @@ def _get_processors():
except ValueError as e:
return 1
+
+def _identify_osccookiejar():
+ if os.path.isfile(os.path.join(os.path.expanduser("~"), '.osc_cookiejar')):
+ # For backwards compatibility, use the old location if it exists
+ return '~/.osc_cookiejar'
+
+ if os.getenv('XDG_STATE_HOME', '') != '':
+ osc_state_dir = os.path.join(os.getenv('XDG_STATE_HOME'), 'osc')
+ else:
+ osc_state_dir = os.path.join(os.path.expanduser("~"), '.cache', 'osc')
+
This should be
osc_state_dir = os.path.join(os.path.expanduser("~"), '.local', 'state', 'osc')
… + return os.path.join(osc_state_dir, 'cookiejar')
+
|
…ation The order is now: - ~/.osc_cookiejar, if it exists - $XDG_STATE_HOME/osc/cookiejar if XDG_STATE_HOME neither null nor empty - ~/.local/state/osc/cookiejar
Oops! Sorry about that |
Merged. Thanks a lot!
|
In a similar theme to #349, this ensures the file is created according to the XDG Base Directory specification. Just like that change, it uses the XDG Base Directly specification on windows (usually I prefer to use the spec only on Unix-like platlforms). In this case,
XDG_CACHE_HOME
seemed to be a better fit compared toXDG_STATE_HOME
A subtle difference compared to #349 is that
XDG_STATE_HOME
is not used if it is defined, but empty (in the other change, the variable is used if it is empty). This is more correct, according to the spec.Test suite results (passes)