Move ~/.git-credential-cache to ~/.cache/git
cover Move ~/.git-credential-cache to ~/.cache/git
Patches for my Microproject. I took into consideration the feedback I recieved on an earlier email thread and now look for a socket at the old default location before creating a new socket in $XDG_CACHE_HOME.
Documentation/git-credential-cache.txt | 3 ++- cache.h | 7 +++++++ credential-cache.c | 15 ++++++++++++++- path.c | 15 +++++++++++++++ 4 files changed, 38 insertions(+), 2 deletions(-)
PATCH 1/3 path.c: Add xdg_cache_home to get paths under XDG_CACHE_HOME
This is necessary to make it posible to use XDG_CACHE_HOME for caches in the XDG standard. I modeled this after the very similar xdg_config_home function for obtaining paths to functions under XDG_CONFIG_HOME
--- cache.h | 7 +++++++ path.c | 15 +++++++++++++++ 2 files changed, 22 insertions(+)
cache.h +7 -0
@@ -1169,6 +1169,13 @@ extern int is_ntfs_dotgit(const char *name);*/ extern char *xdg_config_home (const char *filename); +/** +* Return a newly allocated string with the evaluation of +* "$XDG_CACHE_HOME/git/$filename" if $XDG_CONFIG_HOME is non-empty, otherwise +* "$HOME/.config/git/$filename". Return NULL upon error. +*/ +extern char *xdg_cache_home (const char *filename); +/* object replacement */ #define LOOKUP_REPLACE_OBJECT 1#define LOOKUP_UNKNOWN_OBJECT 2
path.c +15 -0
@@ -1272,6 +1272,21 @@ char *xdg_config_home(const char *filename)return NULL ; } +char *xdg_cache_home (const char *filename) +{ +const char *home, *cache_home; + +assert (filename); + cache_home =getenv ("XDG_CACHE_HOME" ); +if (cache_home && *cache_home) +return mkpathdup ("%s/git/%s" , cache_home, filename); + + home =getenv ("HOME" ); +if (home) +return mkpathdup ("%s/.cache/git/%s" , home, filename); +return NULL ; +} + GIT_PATH_FUNC(git_path_cherry_pick_head, "CHERRY_PICK_HEAD") GIT_PATH_FUNC(git_path_revert_head, "REVERT_HEAD")GIT_PATH_FUNC (git_path_squash_msg,"SQUASH_MSG" )
Signature · 1 line
2.11.0
Re: [GSoC][PATCH 1/3] path.c: Add xdg_cache_home to get paths under XDG_CACHE_HOME
Quoted 1 line · Devin Lehmacher
Subject: Re: [GSoC][PATCH 1/3]
path.c: Addxdg_cache_hometo get paths underXDG_CACHE_HOME
It's nice to keep subject lines succinct, as people will skim through them in "shortlog" or "log --oneline" output for years to come. Of course, you should not make them _too_ succinct, but I think there is a lot of extra detail here.
Probably:
path.c: add xdg_cache_home
would suffice in this case (not also that we usually do not start with a capital letter).
Quoted 3 lines
This is necessary to make it posible to use
XDG_CACHE_HOMEfor caches in the XDG standard. I modeled this after the very similarxdg_config_homefunction for obtaining paths to functions underXDG_CONFIG_HOME
I think this covers what it needs to, which is good. I'd usually try to avoid starting the message with "this", as it it can be a bit vague (and assumes that the body is shown next to the subject, which is not always the case).
I'd have probably written it like:
We already have xdg_config_home() to help us format paths in XDG_CONFIG_HOME. Let's provide a similar xdg_cache_home() to do the same thing for XDG_CACHE_HOME.
or something. I admit this is bikeshedding, but since you're new I feel like I get to spout off about commit message style. :)
Quoted 4 lines
path.c +4
-0+char *xdg_cache_home (const char *filename) +{ +const char *home, *cache_home; +
path.c char *xdg_config_home(const char *filename) · line 1288
Quoted 10 lines
path.c +10
-0+assert (filename); + cache_home =getenv ("XDG_CACHE_HOME" ); +if (cache_home && *cache_home) +return mkpathdup ("%s/git/%s" , cache_home, filename); + + home =getenv ("HOME" ); +if (home) +return mkpathdup ("%s/.cache/git/%s" , home, filename); +return NULL ; +}
This looks fine, as it comes from xdg_config_home(). It does make me wonder if the two should be sharing a common implementation, with a signature like:
char *xdg_generic_home(const char *env,
const char *fallback, const char *filename);
and then the two functions do:
return xdg_generic_home("XDG_CACHE_HOME", ".cache", filename);For two the duplication is not so bad, but I wonder if there are other xdg paths we'd care about.
And one final note. I notice that we return NULL if the user has no HOME. But I'm not sure most callers are prepared to handle this. E.g., if you have no ident set and no HOME, then we will pass NULL to lstat(). On Linux at least that just gets you EFAULT, but I wouldn't be surprised if it's a segfault on other systems (probably at least Windows, where we have an lstat wrapper that calls strlen on the filename).
This is not at all a new thing with your patch, but it might be worth considering while we are thinking about expanding this interface. I'm not sure if the callers should be more careful, of it the function should promise to either die() or return a non-NULL value.
-Peff
Re: [GSoC][PATCH 1/3] path.c: Add xdg_cache_home to get paths under XDG_CACHE_HOME
Quoted 6 lines
And one final note. I notice that we return NULL if the user has no HOME. But I'm not sure most callers are prepared to handle this. E.g., if you have no ident set and no HOME, then we will pass NULL to
lstat(). On Linux at least that just gets you EFAULT, but I wouldn't be surprised if it's a segfault on other systems (probably at least Windows, where we have an lstat wrapper that calls strlen on the filename).
Right now we check the return value from these two functions and if it is NULL we instead use some other configuration location.
That said I agree that this could lead to unexpected behavior in some scenarios.
Devin
PATCH 2/3 credential-cache.c: Make git use XDG_CACHE_HOME for credentials
In reply to Devin Lehmachergit-credential-cache will now use the socket $XDG_CACHE_HOME/git/credential/socket if there is not already a socket at ~/.git-credential-cache/socket. This ensures that if another process already created a socket at the old location it will be used over the new one if it exists.
--- credential-cache.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-)
credential-cache.c +14 -1
@@ -83,6 +83,19 @@ static void do_cache(const char *socket, const char *action, int timeout,strbuf_release (&buf); } +static char *get_socket_path (void ) { +char *home_socket; + + home_socket =expand_user_path ("~/.git_credential_cache/socket" ); +if (home_socket) +if (file_exists (home_socket)) +return home_socket; +else +free (home_socket); + +return xdg_cache_home ("credential/socket" ); +} +int cmd_main (int argc,const char **argv) {char *socket_path =NULL ;
@@ -106,7 +119,7 @@ int cmd_main(int argc, const char **argv) op = argv[0 ];if (!socket_path) - socket_path =expand_user_path ("~/.git-credential-cache/socket" ); + socket_path =get_socket_path ();if (!socket_path)die ("unable to find a suitable socket path; use --socket" );
Signature · 1 line
2.11.0
Re: [GSoC][PATCH 2/3] credential-cache.c: Make git use XDG_CACHE_HOME for credentials
credential-cache.c static void do_cache(const char *socket, const char *action, int timeout, · line 89
Quoted 4 lines · Devin Lehmacher
credential-cache.c +4
-0+static char *get_socket_path (void ) { +char *home_socket; + + home_socket =expand_user_path ("~/.git_credential_cache/socket" );
The typo on this line guarantees that nobody will retain the current location and will be migrated forcibly to the new xdg place ;-)
credential-cache.c static void do_cache(const char *socket, const char *action, int timeout, · line 97
Quoted 8 lines
credential-cache.c +8
-0+if (home_socket) +if (file_exists (home_socket)) +return home_socket; +else +free (home_socket); + +return xdg_cache_home ("credential/socket" ); +}
I somehow feel that the order of precedence should be the other way around, though.
If somebody really wants to use the xdg location and has a socket already there, using that existing socket would be the right thing to do. However, when neither ~/.git-credential-cache/socket nor ~/.cache/git/socket exists, why should we prefer the latter over the former?
Quoted 11 lines
int
cmd_main(int argc, const char **argv)
{credential-cache.c +1
-1char *socket_path =NULL ;@@ -106,7 +119,7 @@ int cmd_main(int argc, const char **argv) op = argv[0 ];if (!socket_path) - socket_path =expand_user_path ("~/.git-credential-cache/socket" ); + socket_path =get_socket_path ();if (!socket_path)die ("unable to find a suitable socket path; use --socket" );
Re: [GSoC][PATCH 2/3] credential-cache.c: Make git use XDG_CACHE_HOME for credentials
Quoted 12 lines · Junio C Hamano
credential-cache.c static void do_cache(const char *socket, const char *action, int timeout, · line 97
Quoted 8 lines
credential-cache.c +8
-0+if (home_socket) +if (file_exists (home_socket)) +return home_socket; +else +free (home_socket); + +return xdg_cache_home ("credential/socket" ); +}I somehow feel that the order of precedence should be the other way around, though.
Quoted 5 lines
If somebody really wants to use the xdg location and has a socket already there, using that existing socket would be the right thing to do. However, when neither ~/.git-credential-cache/socket nor ~/.cache/git/socket exists, why should we prefer the latter over the former?
How would they get a socket in the xdg location if we never create one?
I think the point of this commit is that we should generally prefer the xdg locations to ones that were simply made up by Git.
As an aside, I also wondered if stale socket files might cause us to keep picking the "old" location forever. But it looks like credential-cache--daemon does use a tempfile handler to make sure it cleans up the socket afterwards (and we are checking for the actual socket, not just the presence of the outer directory, which is good).
-Peff
Re: [GSoC][PATCH 2/3] credential-cache.c: Make git use XDG_CACHE_HOME for credentials
Quoted 14 lines · Jeff King
Quoted 12 lines · Junio C Hamano
credential-cache.c static void do_cache(const char *socket, const char *action, int timeout, · line 97
Quoted 8 lines
credential-cache.c +8
-0+if (home_socket) +if (file_exists (home_socket)) +return home_socket; +else +free (home_socket); + +return xdg_cache_home ("credential/socket" ); +}I somehow feel that the order of precedence should be the other way around, though.
Quoted 7 lines
Quoted 5 lines
If somebody really wants to use the xdg location and has a socket already there, using that existing socket would be the right thing to do. However, when neither ~/.git-credential-cache/socket nor ~/.cache/git/socket exists, why should we prefer the latter over the former?
How would they get a socket in the xdg location if we never create one?
They wouldn't. It was my oblique way to say "it is unclear from the patch description and the code why this is a good idea---it needs to be explained better" ;-).
Re: [GSoC][PATCH 2/3] credential-cache.c: Make git use XDG_CACHE_HOME for credentials
Quoted 4 lines
They wouldn't. It was my oblique way to say "it is unclear from the patch description and the code why this is a good idea---it needs to be explained better" ;-).
Ok, I will submit a new patch with a better explanation.
Devin
Re: [GSoC][PATCH 2/3] credential-cache.c: Make git use XDG_CACHE_HOME for credentials
In reply to Junio C HamanoQuoted 8 lines
I somehow feel that the order of precedence should be the other way around, though.
If somebody really wants to use the xdg location and has a socket already there, using that existing socket would be the right thing to do. However, when neither ~/.git-credential-cache/socket nor ~/.cache/git/socket exists, why should we prefer the latter over the former?
At least to me the latter feels superior. It keeps clutter out of the home directory putting it in a directory with all other cache files from other programs. If we continue to put the socket at ~/.git-credential-cache/socket by default then checking ~/.cache/git/credential/socket seems useless since the daemon will check socket locations provided with —-socket anyways and there is no other way to create the socket at the new location.
To me it would make sense to check the old location and if it is not there then use the XDG location to follow that standard.
I guess that this is not the same behavior that git config —-global (it will only put its configuration file there if .gitconfig is unreadable) has right now though so we should stay consistent and reverse the precedence here.
Thanks for the feedback,
Devin
Re: [GSoC][PATCH 2/3] credential-cache.c: Make git use XDG_CACHE_HOME for credentials
In reply to Devin LehmacherQuoted 1 line · Devin Lehmacher
+static char*
get_socket_path(void){
By the way, in our codebase (which is C not C++), the asterisk does not stick to the type, i.e. the above should be:
static char *get_socket_path(void)
{Re: [GSoC][PATCH 2/3] credential-cache.c: Make git use XDG_CACHE_HOME for credentials
In reply to Devin LehmacherQuoted 1 line · Devin Lehmacher
Subject: Re: [GSoC][PATCH 2/3]
credential-cache.c: Make git useXDG_CACHE_HOMEfor credentials
Another subject nitpick. :)
Saying "Make git" is just extraneous. All commits make git do something.
Likewise "for credentials" doesn't say much. Perhaps "for socket" would be more descriptive.
I'm not sure we are entirely consistent here, but I'd probably say just "credential-cache" (not "credential-cache.c"). My rule of thumb is to mention the C file if it's an internal thing: a cleanup, a new function, etc. And use the command-name if it's something user-visible.
So all together:
Subject: credential-cache: use XDG_CACHE_HOME for socket
is shorter and more descriptive (IMHO).
Quoted 5 lines
git-credential-cache will now use the socket $XDG_CACHE_HOME/git/credential/socket if there is not already a socket at ~/.git-credential-cache/socket. This ensures that if another process already created a socket at the old location it will be used over the new one if it exists.
This tells us what, but not much about "why". Probably the reason is something simple, like "using standards is better than not". But the commit message should say that.
credential-cache.c static void do_cache(const char *socket, const char *action, int timeout, · line 94
Quoted 9 lines
credential-cache.c +9
-0+static char *get_socket_path (void ) { +char *home_socket; + + home_socket =expand_user_path ("~/.git_credential_cache/socket" ); +if (home_socket) +if (file_exists (home_socket)) +return home_socket; +else +free (home_socket);
Please use braces for nested ifs. I know that C resolves a dangling-else like this as you've indented it (as opposed to attaching the else to the outer "if"), but it is unnecessarily confusing. Just one set of braces:
if (home_socket) {
if (file_exists(home_socket);
return home_socket;
else
free(home_socket);}
is sufficient.
-Peff
PATCH 3/3 Update documentation to reflect new socket location
In reply to Devin Lehmacher--- Documentation/git-credential-cache.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
Documentation/git-credential-cache.txt +2 -1
@@ -34,7 +34,8 @@ OPTIONS Use `<path>` to contact a running cache daemon (or start a new cache daemon if one is not started). Defaults to - `~/.git-credential-cache/socket`. If your home directory is on a + `~/.git-credential-cache/socket` if it exists and otherwise + `$XDG_CACHE_HOME/git/credential/socket`. If your home directory is on a network-mounted filesystem, you may need to change this to a local filesystem. You must specify an absolute path.
Signature · 1 line
2.11.0
Re: [GSoC][PATCH 3/3] Update documentation to reflect new socket location
Quoted 1 line · Devin Lehmacher
Subject: Re: [GSoC][PATCH 3/3] Update documentation to reflect new socket location
Read the above single line and nothing else, while pretending you do not know what this patch is about at all. That is what readers of "git shortlog" will see.
It may be obvious for you, but can you tell whose socket the series has updated in 6 months after forgetting what you worked on? The title does not give any hint that this is about the credential cache daemon.
Quoted 18 lines
Signed-off-by Devin Lehmacher <[email protected]>--- Documentation/git-credential-cache.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)Documentation/git-credential-cache.txt +2
-1@@ -34,7 +34,8 @@ OPTIONS Use `<path>` to contact a running cache daemon (or start a new cache daemon if one is not started). Defaults to - `~/.git-credential-cache/socket`. If your home directory is on a + `~/.git-credential-cache/socket` if it exists and otherwise + `$XDG_CACHE_HOME/git/credential/socket`. If your home directory is on a network-mounted filesystem, you may need to change this to a local filesystem. You must specify an absolute path.
PATCH v2 1/2 path.c: add xdg_cache_home
In reply to Devin LehmacherWe already have xdg_config_home to format paths relative to XDG_CONFIG_HOME. Let's provide a similar function xdg_cache_home to do the same for paths relative to XDG_CACHE_HOME.
--- cache.h | 7 +++++++ path.c | 15 +++++++++++++++ 2 files changed, 22 insertions(+)
cache.h +7 -0
@@ -1169,6 +1169,13 @@ extern int is_ntfs_dotgit(const char *name);*/ extern char *xdg_config_home (const char *filename); +/** +* Return a newly allocated string with the evaluation of +* "$XDG_CACHE_HOME/git/$filename" if $XDG_CONFIG_HOME is non-empty, otherwise +* "$HOME/.config/git/$filename". Return NULL upon error. +*/ +extern char *xdg_cache_home (const char *filename); +/* object replacement */ #define LOOKUP_REPLACE_OBJECT 1#define LOOKUP_UNKNOWN_OBJECT 2
path.c +15 -0
@@ -1272,6 +1272,21 @@ char *xdg_config_home(const char *filename)return NULL ; } +char *xdg_cache_home (const char *filename) +{ +const char *home, *cache_home; + +assert (filename); + cache_home =getenv ("XDG_CACHE_HOME" ); +if (cache_home && *cache_home) +return mkpathdup ("%s/git/%s" , cache_home, filename); + + home =getenv ("HOME" ); +if (home) +return mkpathdup ("%s/.cache/git/%s" , home, filename); +return NULL ; +} + GIT_PATH_FUNC(git_path_cherry_pick_head, "CHERRY_PICK_HEAD") GIT_PATH_FUNC(git_path_revert_head, "REVERT_HEAD")GIT_PATH_FUNC (git_path_squash_msg,"SQUASH_MSG" )
Signature · 1 line
2.11.0
PATCH v2 2/2 credential-cache: use XDG_CACHE_HOME for socket
git-credential-cache will now use a socket following the XDG base path specification by default. This increases consistency with other applications and helps keep clutter out of users' home directories.
We still check the old socket location, ~/.git-credential-cache/socket first in case the user already has a socket at that location. This ensures that a socket previously created will be used over forcibly switching to the new socket location.
If there is not a socket at that location we create a new one at $XDG_CACHE_HOME/git/credential/socket. This complies with the XDG standard and good for the reasons previously mentioned. We use the subdirectory credential/ in case we later want to store other files under $XDG_CACHE_HOME/git/ and to make the purpose of the socket clear.
I also change to documentation to reflect the new default socket location.
--- Documentation/git-credential-cache.txt | 3 ++- credential-cache.c | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-)
Documentation/git-credential-cache.txt +2 -1
@@ -34,7 +34,8 @@ OPTIONS Use `<path>` to contact a running cache daemon (or start a new cache daemon if one is not started). Defaults to - `~/.git-credential-cache/socket`. If your home directory is on a + `~/.git-credential-cache/socket` if it exists and otherwise + `$XDG_CACHE_HOME/git/credential/socket`. If your home directory is on a network-mounted filesystem, you may need to change this to a local filesystem. You must specify an absolute path.
credential-cache.c +15 -1
@@ -83,6 +83,20 @@ static void do_cache(const char *socket, const char *action, int timeout,strbuf_release (&buf); } +static char *get_socket_path (void ) { +char *home_socket; + + home_socket =expand_user_path ("~/.git-credential-cache/socket" ); +if (home_socket) { +if (file_exists (home_socket)) +return home_socket; +else +free (home_socket); + } + +return xdg_cache_home ("credential/socket" ); +} +int cmd_main (int argc,const char **argv) {char *socket_path =NULL ;
@@ -106,7 +120,7 @@ int cmd_main(int argc, const char **argv) op = argv[0 ];if (!socket_path) - socket_path =expand_user_path ("~/.git-credential-cache/socket" ); + socket_path =get_socket_path ();if (!socket_path)die ("unable to find a suitable socket path; use --socket" );
Signature · 1 line
2.11.0
Re: [GSoC][PATCH v2 2/2] credential-cache: use XDG_CACHE_HOME for socket
Quoted 3 lines · Devin Lehmacher
git-credential-cache will now use a socket following the XDG base path specification by default. This increases consistency with other applications and helps keep clutter out of users' home directories.
We tend to write our log messages in imperative mood, as if you are giving an order to the codebase to "be like so" (alternatively, you can read them as if you are giving an order to the maintainer of the code to "make these changes").
We have been using ~/.git-credential-cache/socket as the location to store the UNIX socket to communicate with the credential daemon process. In order to make it more consistent with other applications and reduce clutter in the home directory, move it to $XDG_CACHE_HOME/git/credential/socket, which matches what XDG base path specification suggests.
Similarly for the other two paragraphs. Instead of "We still check the old location ...", just "Check the old location ...", etc.
Quoted 3 lines
If there is not a socket at that location we create a new one at $XDG_CACHE_HOME/git/credential/socket. This complies with the XDG standard and good for the reasons previously mentioned.
And the second sentence can go; you already said why you think XDG_CACHE_HOME is a good idea.
Quoted 3 lines
We use the
subdirectory credential/ in case we later want to store other files under $XDG_CACHE_HOME/git/ and to make the purpose of the socket clear.
And this probably can disappear (or rolled into the first paragraph, if we really want; personally I think it is obvious why we want the extra "credential" directory under "git" there).
Quoted 2 lines
I also change to documentation to reflect the new default socket location.
This probably does not have to be said, as it is obvious from the diffstat.
Missing sign-off.
Quoted 10 lines
--- Documentation/git-credential-cache.txt | 3 ++- credential-cache.c | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-)Documentation/git-credential-cache.txt +0
-0
Documentation/git-credential-cache.txt OPTIONS · line 38
Quoted 6 lines
Documentation/git-credential-cache.txt +2
-1@@ -34,7 +34,8 @@ OPTIONS Use `<path>` to contact a running cache daemon (or start a new cache daemon if one is not started). Defaults to - `~/.git-credential-cache/socket`. If your home directory is on a + `~/.git-credential-cache/socket` if it exists and otherwise + `$XDG_CACHE_HOME/git/credential/socket`. If your home directory is on a
There is a funny indentation here.
Quoted 8 lines
Documentation/git-credential-cache.txt +0
-0network-mounted filesystem, you may need to change this to a local filesystem. You must specify an absolute path.credential-cache.c +0
-0
credential-cache.c static void do_cache(const char *socket, const char *action, int timeout, · line 91
Quoted 9 lines
credential-cache.c +6
-0@@ -83,6 +83,20 @@ static void do_cache(const char *socket, const char *action, int timeout,strbuf_release (&buf); } +static char *get_socket_path (void ) { +char *home_socket; + + home_socket =expand_user_path ("~/.git-credential-cache/socket" ); +if (home_socket) { +if (file_exists (home_socket))
Don't we want to make sure that this path _is_ a socket? In general I think file_exists() is a poor choice to use here (the existing use are all about having a regular file there, and its definition may be later tightened from "does lstat() succeed?" to something a bit more sensible, and FIFO may start failing the updated test.
Thanks.
v3 cover Fix commit messages, check if socket is socket
I fixed all of the commit messages and the weird indentation. I also now check that the socket is actually a socket.
What do you think of the function is_socket? Is it general enough to be put in dir.h or unix_socket.h for use in other files? Or should it be left as is?
- Devin
PATCH v3 1/3 path.c: add xdg_cache_home
We already have xdg_config_home to format paths relative to XDG_CONFIG_HOME. Let's provide a similar function xdg_cache_home to do the same for paths relative to XDG_CACHE_HOME.
--- cache.h | 7 +++++++ path.c | 15 +++++++++++++++ 2 files changed, 22 insertions(+)
cache.h +7 -0
@@ -1169,6 +1169,13 @@ extern int is_ntfs_dotgit(const char *name);*/ extern char *xdg_config_home (const char *filename); +/** +* Return a newly allocated string with the evaluation of +* "$XDG_CACHE_HOME/git/$filename" if $XDG_CACHE_HOME is non-empty, otherwise +* "$HOME/.cache/git/$filename". Return NULL upon error. +*/ +extern char *xdg_cache_home (const char *filename); +/* object replacement */ #define LOOKUP_REPLACE_OBJECT 1#define LOOKUP_UNKNOWN_OBJECT 2
path.c +15 -0
@@ -1272,6 +1272,21 @@ char *xdg_config_home(const char *filename)return NULL ; } +char *xdg_cache_home (const char *filename) +{ +const char *home, *cache_home; + +assert (filename); + cache_home =getenv ("XDG_CACHE_HOME" ); +if (cache_home && *cache_home) +return mkpathdup ("%s/git/%s" , cache_home, filename); + + home =getenv ("HOME" ); +if (home) +return mkpathdup ("%s/.cache/git/%s" , home, filename); +return NULL ; +} + GIT_PATH_FUNC(git_path_cherry_pick_head, "CHERRY_PICK_HEAD") GIT_PATH_FUNC(git_path_revert_head, "REVERT_HEAD")GIT_PATH_FUNC (git_path_squash_msg,"SQUASH_MSG" )
Signature · 1 line
2.11.0
PATCH v3 2/3 credential-cache: use XDG_CACHE_HOME for socket
In reply to Devin LehmacherMake git-credential-cache follow the XDG base path specification by default. This increases consistency with other applications and helps keep clutter out of users' home directories.
Check the old socket location, ~/.git-credential-cache/socket and use it instead if there is already a socket at that location rather than forcibly creating a new socket at the new location.
If there is not a socket at that location create a new one at $XDG_CACHE_HOME/git/credential/socket following XDG base path specification. Use the subdirectory credential/ in case other files are stored under $XDG_CACHE_HOME/git/ in the future and to make the socket's purpose clear.
--- Documentation/git-credential-cache.txt | 10 ++++++---- credential-cache.c | 16 +++++++++++++++- 2 files changed, 21 insertions(+), 5 deletions(-)
Documentation/git-credential-cache.txt +6 -4
@@ -33,10 +33,12 @@ OPTIONS --socket <path>:: Use `<path>` to contact a running cache daemon (or start a new - cache daemon if one is not started). Defaults to - `~/.git-credential-cache/socket`. If your home directory is on a - network-mounted filesystem, you may need to change this to a - local filesystem. You must specify an absolute path. + cache daemon if one is not started). + Defaults to `~/.git-credential-cache/socket` if it exists and + `$XDG_CACHE_HOME/git/credential/socket` otherwise. + If your home directory is on a network-mounted filesystem, you + may need to change this to a local filesystem. You must specify + an absolute path. CONTROLLING THE DAEMON ----------------------
credential-cache.c +15 -1
@@ -83,6 +83,20 @@ static void do_cache(const char *socket, const char *action, int timeout,strbuf_release (&buf); } +static char *get_socket_path (void ) { +char *home_socket; + + home_socket =expand_user_path ("~/.git-credential-cache/socket" ); +if (home_socket) { +if (file_exists (home_socket)) +return home_socket; +else +free (home_socket); + } + +return xdg_cache_home ("credential/socket" ); +} +int cmd_main (int argc,const char **argv) {char *socket_path =NULL ;
@@ -106,7 +120,7 @@ int cmd_main(int argc, const char **argv) op = argv[0 ];if (!socket_path) - socket_path =expand_user_path ("~/.git-credential-cache/socket" ); + socket_path =get_socket_path ();if (!socket_path)die ("unable to find a suitable socket path; use --socket" );
Signature · 1 line
2.11.0
Re: [GSoC][PATCH/RFC v3 2/3] credential-cache: use XDG_CACHE_HOME for socket
Quoted 2 lines · Devin Lehmacher
Signed-off-by Devin Lehmacher <[email protected]>Reviewed-by Junio C Hamano, Jeff King
The last line is premature; neither of us reviewed this exact version and haven't checked if all the issues we mentioned in the previous review have been addressed.
Quoted 1 line
+static char
*get_socket_path(void){
Compare ths line with how cmd_main() below is declared. Notice any difference? We begin our functions like this:
static char *get_socket_path(void)
{PATCH v3 3/3 credential-cache: only use user_socket if a socket
In reply to Devin LehmacherCreate function is_socket.
Make get_socket_path return check if ~/.git-credential-cache/socket is a socket and not just a file. If file_exists behavior could change in an unexpected way. Additionally a file at ~/.git-credential-cache/socket could cause false positives which would otherwise lead to crashes.
--- credential-cache.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
credential-cache.c +7 -1
@@ -83,12 +83,18 @@ static void do_cache(const char *socket, const char *action, int timeout,strbuf_release (&buf); } +static int is_socket (char *path) { +struct stat sb; +int ret =lstat (path, &sb); +return ret &&S_IFSOCK (sb.st_mode); +} +static char *get_socket_path (void ) {char *home_socket; home_socket =expand_user_path ("~/.git-credential-cache/socket" );if (home_socket) { -if (file_exists (home_socket)) +if (is_socket (home_socket))return home_socket;else free (home_socket);
Signature · 1 line
2.11.0
Re: [GSoC][PATCH/RFC v3 3/3] credential-cache: only use user_socket if a socket
credential-cache.c static void do_cache(const char *socket, const char *action, int timeout, · line 90
Quoted 5 lines
credential-cache.c +5
-0+static int is_socket (char *path) { +struct stat sb; +int ret =lstat (path, &sb); +return ret &&S_IFSOCK (sb.st_mode); +}
This patch won’t even compile. S_IFSOCK(sb.st_mode) should have been S_IFSOCK & sb.st_mode.
(I guess I should have compiled first)
After making that change this patch compiles (currently running tests).
-Devin
Re: [GSoC][PATCH/RFC v3 3/3] credential-cache: only use user_socket if a socket
Quoted 11 lines · Devin Lehmacher
credential-cache.c static void do_cache(const char *socket, const char *action, int timeout, · line 90
Quoted 5 lines
credential-cache.c +5
-0+static int is_socket (char *path) { +struct stat sb; +int ret =lstat (path, &sb); +return ret &&S_IFSOCK (sb.st_mode); +}This patch won’t even compile.
S_IFSOCK(sb.st_mode)should have beenS_IFSOCK& sb.st_mode.(I guess I should have compiled first)
After making that change this patch compiles (currently running tests).
Best practice for submitting patches would be to ensure that each patch compiles without errors (with the DEVELOPER=1 flag set) and that the entire test suite passes with no errors; this is to maintain bisect-ability. Only after you've done this should you send your patches to the mailing list.
Signature · 1 line
Brandon Williams
Re: [GSoC][PATCH/RFC v3 3/3] credential-cache: only use user_socket if a socket
Quoted 5 lines
Best practice for submitting patches would be to ensure that each patch compiles without errors (with the DEVELOPER=1 flag set) and that the entire test suite passes with no errors; this is to maintain bisect-ability. Only after you've done this should you send your patches to the mailing list.
Thanks for the advice. I will be more careful in the future.
-Devin
Re: [GSoC][PATCH/RFC v3 3/3] credential-cache: only use user_socket if a socket
Quoted 7 lines · Devin Lehmacher
Quoted 5 lines
Best practice for submitting patches would be to ensure that each patch compiles without errors (with the DEVELOPER=1 flag set) and that the entire test suite passes with no errors; this is to maintain bisect-ability. Only after you've done this should you send your patches to the mailing list.
Thanks for the advice. I will be more careful in the future.
Of course! And no worries, I just wanted you to be aware of the conventions so that you have an easier time contributing :)
Signature · 1 line
Brandon Williams
Re: [GSoC][PATCH/RFC v3 3/3] credential-cache: only use user_socket if a socket
In reply to Devin LehmacherQuoted 11 lines · Devin Lehmacher
credential-cache.c +3
-0@@ -83,12 +83,18 @@ static void do_cache(const char *socket, const char *action, int timeout,strbuf_release (&buf); } +static int is_socket (char *path) { +struct stat sb; +int ret =lstat (path, &sb);
credential-cache.c static void do_cache(const char *socket, const char *action, int timeout, · line 97
Quoted 10 lines
credential-cache.c +4
-1+return ret &&S_IFSOCK (sb.st_mode); +} +static char *get_socket_path (void ) {char *home_socket; home_socket =expand_user_path ("~/.git-credential-cache/socket" );if (home_socket) { -if (file_exists (home_socket)) +if (is_socket (home_socket))
This should be done as part of 2/3, no? It does not make sense to add 2/3 and then immediately say "oops, the check in 2/3 is wrong, and let's update it like so".
Re: [GSoC][PATCH/RFC v3 3/3] credential-cache: only use user_socket if a socket
Quoted 19 lines · Junio C Hamano
Quoted 17 lines · Devin Lehmacher
credential-cache.c +6
-0@@ -83,12 +83,18 @@ static void do_cache(const char *socket, const char *action, int timeout,strbuf_release (&buf); } +static int is_socket (char *path) { +struct stat sb; +int ret =lstat (path, &sb); +return ret &&S_IFSOCK (sb.st_mode); +} +static char *get_socket_path (void ) {char *home_socket;
Quoted 8 lines
credential-cache.c static void do_cache(const char *socket, const char *action, int timeout, · line 97
Quoted 4 lines
credential-cache.c +1
-1home_socket =expand_user_path ("~/.git-credential-cache/socket" );if (home_socket) { -if (file_exists (home_socket)) +if (is_socket (home_socket))This should be done as part of 2/3, no? It does not make sense to add 2/3 and then immediately say "oops, the check in 2/3 is wrong, and let's update it like so".
Also I think you would want to use S_ISFIFO() and/or S_ISSOCK() macros (I do not offhand recall which one credential cache daemon uses), not the S_IFxxx constant.
Perhaps
1/3 - add xdg_cache similar to xdg_config
2/3 - add is_socket()
3/3 - use xdg_cache location for socket if traditional location
is not in use
would be a better logical ordering of the patches.
Having said that, I do not think ~/.git-credential-cache/socket is the right thing to test. If the ~/.git-credential-cache directory already exists, it is likely that the user has a set-up that works well with "socket" inside it, and it is safer to keep using that location.
On the other hand, if that directory does not exist, we know it is safe to use whatever new location---after all, the lack of the directory tells us that the user has never used the traditional location successfully ;-)
So is_socket() may not even be needed, in which case this will be a two-patch series:
1/2 - add xdg_cache similar to xdg_config
2/2 - use xdg_cache location if ~/.git-credential-cache/ is not there
Re: [GSoC][PATCH/RFC v3 3/3] credential-cache: only use user_socket if a socket
Quoted 3 lines
Also I think you would want to use
S_ISFIFO()and/orS_ISSOCK()macros (I do not offhand recall which one credential cache daemon uses), not theS_IFxxxconstant.
Huh. These macros are not on the man page for lstat.
Quoted 10 lines
Having said that, I do not think ~/.git-credential-cache/socket is the right thing to test. If the ~/.git-credential-cache directory already exists, it is likely that the user has a set-up that works well with "socket" inside it, and it is safer to keep using that location.
On the other hand, if that directory does not exist, we know it is safe to use whatever new location---after all, the lack of the directory tells us that the user has never used the traditional location successfully ;-)
That makes sense
-Devin
v4 cover Moving credential-cache socket to xdg path
In reply to Devin LehmacherI implemented all changes from the previous reviews and also wrote a few tests.
-Devin
PATCH v4 1/4 path.c: add xdg_cache_home
We already have xdg_config_home to format paths relative to XDG_CONFIG_HOME. Let's provide a similar function xdg_cache_home to do the same for paths relative to XDG_CACHE_HOME.
--- cache.h | 7 +++++++ path.c | 15 +++++++++++++++ 2 files changed, 22 insertions(+)
cache.h +7 -0
@@ -1169,6 +1169,13 @@ extern int is_ntfs_dotgit(const char *name);*/ extern char *xdg_config_home (const char *filename); +/** +* Return a newly allocated string with the evaluation of +* "$XDG_CACHE_HOME/git/$filename" if $XDG_CACHE_HOME is non-empty, otherwise +* "$HOME/.cache/git/$filename". Return NULL upon error. +*/ +extern char *xdg_cache_home (const char *filename); +/* object replacement */ #define LOOKUP_REPLACE_OBJECT 1#define LOOKUP_UNKNOWN_OBJECT 2
path.c +15 -0
@@ -1272,6 +1272,21 @@ char *xdg_config_home(const char *filename)return NULL ; } +char *xdg_cache_home (const char *filename) +{ +const char *home, *cache_home; + +assert (filename); + cache_home =getenv ("XDG_CACHE_HOME" ); +if (cache_home && *cache_home) +return mkpathdup ("%s/git/%s" , cache_home, filename); + + home =getenv ("HOME" ); +if (home) +return mkpathdup ("%s/.cache/git/%s" , home, filename); +return NULL ; +} + GIT_PATH_FUNC(git_path_cherry_pick_head, "CHERRY_PICK_HEAD") GIT_PATH_FUNC(git_path_revert_head, "REVERT_HEAD")GIT_PATH_FUNC (git_path_squash_msg,"SQUASH_MSG" )
Signature · 1 line
2.11.0
PATCH v4 2/4 dir: add directory_exists
In reply to Devin Lehmacherfile_exists currently only does an lstat to see if anything is at the path. For some purposes it would be better to ensure that a path is a certain type of file. Add function directory_exists that checks if a path exists and is a directory.
--- dir.c | 7 +++++++ dir.h | 1 + 2 files changed, 8 insertions(+)
dir.c +7 -0
@@ -2055,6 +2055,13 @@ int file_exists(const char *f)return lstat (f, &sb) ==0 ; } +int directory_exists (const char *path) +{ +struct stat sb; +int ret =lstat (path, &sb); +return ret ==0 &&S_ISDIR (sb.st_mode); +} +static int cmp_icase (char a,char b) {if (a == b)
dir.h +1 -0
@@ -247,6 +247,7 @@ extern void add_exclude(const char *string, const char *base,extern void clear_exclude_list (struct exclude_list *el);extern void clear_directory (struct dir_struct *dir);extern int file_exists (const char *); +extern int directory_exists (const char *);extern int is_inside_dir (const char *dir);extern int dir_inside_of (const char *subdir,const char *dir);
Signature · 1 line
2.11.0
Re: [GSoC][PATCH v4 2/4] dir: add directory_exists
dir.c int file_exists(const char *f) · line 2063
Quoted 6 lines · Devin Lehmacher
dir.c +6
-0+int directory_exists (const char *path) +{ +struct stat sb; +int ret =lstat (path, &sb); +return ret ==0 &&S_ISDIR (sb.st_mode); +}
I am not a great fan of using file_exists() [*1*] on anything other than paths in the working tree (i.e. in preparation for checking things out of or into the index), as paths inside .git/ and paths outside have different requirements. One of the difference is if it makes sense to use stat(2) or lstat(2) for a check like this function does. For working tree entities, which file_exists() and friends are designed for, we absolutely should use lstat(2). The "RelNotes" symbolic link at the top level of the project must be known as a symbolic link and a question "is this a directory?" on it must be answered "no", for example.
But there is no fundamental reason ~/.git-credential-cache (or anything outside the working tree) must be S_ISDIR(). If the user wanted to have the real directory elsewhere and point at it with a symbolic link ~/.git-credential-cache, we should allow it.
If we need a helper function to see if a path in the working tree is a directory, adding this new helper function to dir.c (which is about the paths in the working tree) and use lstat(2) in it is the right thing to do. But I do not think it should be used to check if ~/.git-credential-cache directory, which is not a filesystem entity in a working tree, exists.
IOW, I do not think this patch helps the topic of this series. Drop this patch from the series and have a similar code (but use stat(2) instead of lstat(2)) directly inside get_socket_path()'s in the next patch, perhaps?
[Footnote]
*1* ... and friends, like safe_create_leading_directories().
Re: [GSoC][PATCH v4 2/4] dir: add directory_exists
Quoted 15 lines · Junio C Hamano
dir.c int file_exists(const char *f) · line 2063
Quoted 6 lines · Devin Lehmacher
dir.c +6
-0+int directory_exists (const char *path) +{ +struct stat sb; +int ret =lstat (path, &sb); +return ret ==0 &&S_ISDIR (sb.st_mode); +}I am not a great fan of using
file_exists()[*1*] on anything other than paths in the working tree (i.e. in preparation for checking things out of or into the index), as ...
...
[Footnote]
Quoted 1 line
*1* ... and friends, like
safe_create_leading_directories().
I actually got things mixed up. safe-create-leading-directories is to be used only for paths inside .git/ (i.e. not for working tree files, but for things like ".git/refs/heads/foo" when creating a branch called foo/bar).
PATCH v4 3/4 credential-cache: use XDG_CACHE_HOME for socket
In reply to Devin LehmacherMake git-credential-cache follow the XDG base path specification by default. This increases consistency with other applications and helps keep clutter out of users' home directories.
Check the old socket location, ~/.git-credential-cache/, and use ~/.git-credential-cache/socket if that directory exists rather than forcing users who have used git credential-cache before to migrate to the new XDG compliant location.
Otherwise use the socket $XDG_CACHE_HOME/git/credential/socket following XDG base path specification. Use the subdirectory credential/ in case other files are cached under $XDG_CACHE_HOME/git/ in the future and to make the socket's purpose clear.
--- Documentation/git-credential-cache.txt | 11 +++++++---- credential-cache.c | 15 ++++++++++++++- 2 files changed, 21 insertions(+), 5 deletions(-)
Documentation/git-credential-cache.txt +7 -4
@@ -33,10 +33,13 @@ OPTIONS --socket <path>:: Use `<path>` to contact a running cache daemon (or start a new - cache daemon if one is not started). Defaults to - `~/.git-credential-cache/socket`. If your home directory is on a - network-mounted filesystem, you may need to change this to a - local filesystem. You must specify an absolute path. + cache daemon if one is not started). + Defaults to `$XDG_CACHE_HOME/git/credential/socket` unless + `~/.git-credential-cache/` exists in which case + `~/.git-credential-cache/socket` is used instead. + If your home directory is on a network-mounted filesystem, you + may need to change this to a local filesystem. You must specify + an absolute path. CONTROLLING THE DAEMON ----------------------
credential-cache.c +14 -1
@@ -1,5 +1,6 @@#include "cache.h" #include "credential.h" +#include "dir.h" #include "string-list.h" #include "parse-options.h" #include "unix-socket.h"
@@ -83,6 +84,18 @@ static void do_cache(const char *socket, const char *action, int timeout,strbuf_release (&buf); } +static char *get_socket_path (void ) +{ +char *old_credential_dir, *socket; + old_credential_dir =expand_user_path ("~/.git-credential-cache" ); +if (old_credential_dir &&directory_exists (old_credential_dir)) + socket =expand_user_path ("~/.git-credential-cache/socket" ); +else + socket =xdg_cache_home ("credential/socket" ); +free (old_credential_dir); +return socket; +} +int cmd_main (int argc,const char **argv) {char *socket_path =NULL ;
@@ -106,7 +119,7 @@ int cmd_main(int argc, const char **argv) op = argv[0 ];if (!socket_path) - socket_path =expand_user_path ("~/.git-credential-cache/socket" ); + socket_path =get_socket_path ();if (!socket_path)die ("unable to find a suitable socket path; use --socket" );
Signature · 1 line
2.11.0
Re: [GSoC][PATCH v4 3/4] credential-cache: use XDG_CACHE_HOME for socket
Documentation/git-credential-cache.txt OPTIONS · line 42
Quoted 3 lines · Devin Lehmacher
Documentation/git-credential-cache.txt +3
-0+ If your home directory is on a network-mounted filesystem, you + may need to change this to a local filesystem. You must specify + an absolute path.
Nicely explained.
If a socket cannot be created in "~/.git-credential-cache", existing users would have created a symbolic link that points at a local directory. With XDG_CACHE_HOME mechanism, they would just set the variable to directly point at a local directory and there is no need for symlink trick, on the other hand.
Which points us back to the observation I made in my review on the previous step.
credential-cache.c static void do_cache(const char *socket, const char *action, int timeout, · line 97
Quoted 11 lines
credential-cache.c +11
-0+static char *get_socket_path (void ) +{ +char *old_credential_dir, *socket; + old_credential_dir =expand_user_path ("~/.git-credential-cache" ); +if (old_credential_dir &&directory_exists (old_credential_dir)) + socket =expand_user_path ("~/.git-credential-cache/socket" ); +else + socket =xdg_cache_home ("credential/socket" ); +free (old_credential_dir); +return socket; +}
As we do not want to use the dir.c::directory_exists(), which is meant to be used for working tree files, we can do something like this instead:
static char *get_socket_path(void)
{
struct stat st;
char *path;
path = expand_user_path("~/.git-credential-cache");
if (path && !stat(path, &st) && S_ISDIR(st.st_mode))) {
free(path);
path = expand_user_path("~/.git-credential-cache/socket");
} else {
path = xdg_cache_home("credential/socket");
}
return path;
}The duplication of "~/.git-credential-cache" bothers me somewhat and perhaps people can suggest better ways to get rid of the dup.
Other than that, makes sense to me.
Thanks.
Re: [GSoC][PATCH v4 3/4] credential-cache: use XDG_CACHE_HOME for socket
Quoted 15 lines · Junio C Hamano
static char
*get_socket_path(void){ struct stat st; char *path; path = expand_user_path("~/.git-credential-cache"); if (path && !stat(path, &st) && S_ISDIR(st.st_mode))) { free(path); path = expand_user_path("~/.git-credential-cache/socket"); } else { path = xdg_cache_home("credential/socket"); } return path; }
Quoted 2 lines
The duplication of "~/.git-credential-cache" bothers me somewhat and perhaps people can suggest better ways to get rid of the dup.
Maybe:
path = expand_user_path("~/.git-credential-cache");
if (path && !stat(path, &st) && S_ISDIR(st.st_mode))) {
char *new_path = xstrfmt("%s/socket", path);
free(path);
path = new_path;} else ...
-Peff
PATCH v4 4/4 credential-cache: add tests for XDG functionality
In reply to Devin Lehmacher--- t/t0301-credential-cache.sh | 64 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+)
t/t0301-credential-cache.sh +64 -0
@@ -12,6 +12,7 @@ test -z "$NO_UNIX_SOCKETS" || { # don't leave a stale daemon running trap 'code=$?; git credential-cache exit; (exit $code); die' EXIT +# test that the daemon works with no special setup helper_test cache helper_test_timeout cache --timeout=1
@@ -20,4 +21,67 @@ helper_test_timeout cache --timeout=1 # our socket, leaving us with no way to access the daemon. git credential-cache exit +# we need to use rm -rf here since sometimes the daemon hasn't finished +# cleaning up after itself and rmdir fails +test_expect_success 'credential-cache --socket option overrides default location' ' + test_when_finished "rm -rf \"$HOME\"/dir/" && + check approve "cache --socket \"$HOME/dir/socket\"" <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/dir/socket" && + git credential-cache exit +' + +XDG_CACHE_HOME="$HOME/xdg" +export XDG_CACHE_HOME +# test behavior when XDG_CACHE_HOME is set +helper_test cache + +test_expect_success "use custom XDG_CACHE_HOME if set and default sockets are not created" ' + test -S "$XDG_CACHE_HOME/git/credential/socket" && + test_path_is_missing "$HOME/.git-credential-cache/socket" && + test_path_is_missing "$HOME/.cache/git/credential/socket" && + git credential-cache exit +' +unset XDG_CACHE_HOME + +test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" ' + check approve cache <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/.cache/git/credential/socket" && + XDG_CACHE_HOME="$HOME/xdg" && + export XDG_CACHE_HOME && + check approve cache <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/xdg/git/credential/socket" && + git credential-cache exit && + unset XDG_CACHE_HOME +' + +# we need to use rm -rf here since sometimes the daemon hasn't finished +# cleaning up after itself and rmdir fails +test_expect_success 'use user socket if user directory exists' ' + test_when_finished "rm -rf \"$HOME/.git-credential-cache/\"" && + mkdir -p -m 700 "$HOME/.git-credential-cache/" && + check approve cache <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/.git-credential-cache/socket" && + git credential-cache exit +' + test_done
Signature · 1 line
2.11.0
Re: [GSoC][PATCH v4 4/4] credential-cache: add tests for XDG functionality
t/t0301-credential-cache.sh helper_test_timeout cache --timeout=1 · line 25
Quoted 6 lines · Devin Lehmacher
t/t0301-credential-cache.sh +2
-0@@ -20,4 +21,67 @@ helper_test_timeout cache --timeout=1 # our socket, leaving us with no way to access the daemon. git credential-cache exit +# we need to use rm -rf here since sometimes the daemon hasn't finished +# cleaning up after itself and rmdir fails
Hmmmm. Peff, do you have ideas on better ways to do this (or explanation why this is the best we could do)?
Quoted 34 lines
t/t0301-credential-cache.sh +34
-0+test_expect_success 'credential-cache --socket option overrides default location' ' + test_when_finished "rm -rf \"$HOME\"/dir/" && + check approve "cache --socket \"$HOME/dir/socket\"" <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/dir/socket" && + git credential-cache exit +' + +XDG_CACHE_HOME="$HOME/xdg" +export XDG_CACHE_HOME +# test behavior when XDG_CACHE_HOME is set +helper_test cache + +test_expect_success "use custom XDG_CACHE_HOME if set and default sockets are not created" ' + test -S "$XDG_CACHE_HOME/git/credential/socket" && + test_path_is_missing "$HOME/.git-credential-cache/socket" && + test_path_is_missing "$HOME/.cache/git/credential/socket" && + git credential-cache exit +' +unset XDG_CACHE_HOME + +test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" ' + check approve cache <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/.cache/git/credential/socket" && + XDG_CACHE_HOME="$HOME/xdg" &&
t/t0301-credential-cache.sh helper_test_timeout cache --timeout=1 · line 69
Quoted 10 lines
t/t0301-credential-cache.sh +10
-0+ export XDG_CACHE_HOME && + check approve cache <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/xdg/git/credential/socket" && + git credential-cache exit && + unset XDG_CACHE_HOME
This unset will not run if any of the above steps since it was set and exported fails. It probably should be in test_when_finished and should use safe_unset shell function instead.
Quoted 4 lines
t/t0301-credential-cache.sh +4
-0+' + +# we need to use rm -rf here since sometimes the daemon hasn't finished +# cleaning up after itself and rmdir fails
t/t0301-credential-cache.sh helper_test_timeout cache --timeout=1 · line 83
Quoted 10 lines
t/t0301-credential-cache.sh +10
-0+test_expect_success 'use user socket if user directory exists' ' + test_when_finished "rm -rf \"$HOME/.git-credential-cache/\"" && + mkdir -p -m 700 "$HOME/.git-credential-cache/" && + check approve cache <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/.git-credential-cache/socket" &&
We should also test that the XDG location is not touched at the same time that the traditional directory was used for the socket.
Review helper_test_timeout cache --timeout=1 · line 85
Quoted 2 lines
(quoted diff) +2
-0+ git credential-cache exit +'
This last test should be replicated to also check the case where we have a symbolic link at ~/.git-credential-cache that points at a directory.
t/t0301-credential-cache.sh helper_test_timeout cache --timeout=1 · line 87
Quoted 1 line
t/t0301-credential-cache.sh +0
-0test_done
Thanks.
Re: [GSoC][PATCH v4 4/4] credential-cache: add tests for XDG functionality
Quoted 11 lines · Junio C Hamano
t/t0301-credential-cache.sh helper_test_timeout cache --timeout=1 · line 25
Quoted 6 lines · Devin Lehmacher
t/t0301-credential-cache.sh +2
-0@@ -20,4 +21,67 @@ helper_test_timeout cache --timeout=1 # our socket, leaving us with no way to access the daemon. git credential-cache exit +# we need to use rm -rf here since sometimes the daemon hasn't finished +# cleaning up after itself and rmdir failsHmmmm. Peff, do you have ideas on better ways to do this (or explanation why this is the best we could do)?
If you call "git credential-cache exit", that should be deterministic. The client program won't exit until the other side closes the descriptor, which it won't do until it has cleaned up the socket. See the comment at line 130 of credential-cache--daemon.c.
So here:
Quoted 11 lines
t/t0301-credential-cache.sh helper_test_timeout cache --timeout=1 · line 36
Quoted 11 lines
t/t0301-credential-cache.sh +11
-0+test_expect_success 'credential-cache --socket option overrides default location' ' + test_when_finished "rm -rf \"$HOME\"/dir/" && + check approve "cache --socket \"$HOME/dir/socket\"" <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/dir/socket" && + git credential-cache exit +'
This is almost right, except:
- the "exit" needs to be told which socket to use
- we should do the "exit" even when the test fails early (so in test_when_finished)
- the test_when_finished block will interpolate $HOME when setting up the block, which will break if it contains double-quotes or other special characters. It should use \$HOME.
I suspect the "check" invocation needs to do so as well, though it is even trickier (we shove it into a single-quoted "-c" argument).
I think you could get by in both cases with relative paths.
So all together, probably:
test_when_finished "
git credential-cache --socket dir/socket && rmdir dir
" &&
check approve "cache --socket dir/socket" <<-\EOF &&
...
EOF
test -S dir/socket
The final "test -S" should be OK in practice. We're assuming that the cache-daemon is still running, but it has a 900 second timeout by default (and besides, all the other tests have exact same race).
-Peff
PATCH v5 1/3 path.c: add xdg_cache_home
In reply to Devin LehmacherWe already have xdg_config_home to format paths relative to XDG_CONFIG_HOME. Let's provide a similar function xdg_cache_home to do the same for paths relative to XDG_CACHE_HOME.
--- cache.h | 7 +++++++ path.c | 15 +++++++++++++++ 2 files changed, 22 insertions(+)
cache.h +7 -0
@@ -1169,6 +1169,13 @@ extern int is_ntfs_dotgit(const char *name);*/ extern char *xdg_config_home (const char *filename); +/** +* Return a newly allocated string with the evaluation of +* "$XDG_CACHE_HOME/git/$filename" if $XDG_CACHE_HOME is non-empty, otherwise +* "$HOME/.cache/git/$filename". Return NULL upon error. +*/ +extern char *xdg_cache_home (const char *filename); +/* object replacement */ #define LOOKUP_REPLACE_OBJECT 1#define LOOKUP_UNKNOWN_OBJECT 2
path.c +15 -0
@@ -1272,6 +1272,21 @@ char *xdg_config_home(const char *filename)return NULL ; } +char *xdg_cache_home (const char *filename) +{ +const char *home, *cache_home; + +assert (filename); + cache_home =getenv ("XDG_CACHE_HOME" ); +if (cache_home && *cache_home) +return mkpathdup ("%s/git/%s" , cache_home, filename); + + home =getenv ("HOME" ); +if (home) +return mkpathdup ("%s/.cache/git/%s" , home, filename); +return NULL ; +} + GIT_PATH_FUNC(git_path_cherry_pick_head, "CHERRY_PICK_HEAD") GIT_PATH_FUNC(git_path_revert_head, "REVERT_HEAD")GIT_PATH_FUNC (git_path_squash_msg,"SQUASH_MSG" )
Signature · 1 line
2.11.0
PATCH v5 2/3 credential-cache: use XDG_CACHE_HOME for socket
Make git-credential-cache follow the XDG base path specification by default. This increases consistency with other applications and helps keep clutter out of users' home directories.
Check the old socket location, ~/.git-credential-cache/, and use ~/.git-credential-cache/socket if that directory exists rather than forcing users who have used git credential-cache before to migrate to the new XDG compliant location.
Otherwise use the socket $XDG_CACHE_HOME/git/credential/socket following XDG base path specification. Use the subdirectory credential/ in case other files are cached under $XDG_CACHE_HOME/git/ in the future and to make the socket's purpose clear.
--- Documentation/git-credential-cache.txt | 11 +++++++---- credential-cache.c | 15 ++++++++++++++- 2 files changed, 21 insertions(+), 5 deletions(-)
Documentation/git-credential-cache.txt +7 -4
@@ -33,10 +33,13 @@ OPTIONS --socket <path>:: Use `<path>` to contact a running cache daemon (or start a new - cache daemon if one is not started). Defaults to - `~/.git-credential-cache/socket`. If your home directory is on a - network-mounted filesystem, you may need to change this to a - local filesystem. You must specify an absolute path. + cache daemon if one is not started). + Defaults to `$XDG_CACHE_HOME/git/credential/socket` unless + `~/.git-credential-cache/` exists in which case + `~/.git-credential-cache/socket` is used instead. + If your home directory is on a network-mounted filesystem, you + may need to change this to a local filesystem. You must specify + an absolute path. CONTROLLING THE DAEMON ----------------------
credential-cache.c +14 -1
@@ -83,6 +83,19 @@ static void do_cache(const char *socket, const char *action, int timeout,strbuf_release (&buf); } +static char *get_socket_path (void ) +{ +struct stat sb; +char *old_dir, *socket; + old_dir =expand_user_path ("~/.git-credential-cache" ); +if (old_dir && !stat (old_dir, &sb) &&S_ISDIR (sb.st_mode)) + socket =xstrfmt ("%s/socket" , old_dir); +else + socket =xdg_cache_home ("credential/socket" ); +free (old_dir); +return socket; +} +int cmd_main (int argc,const char **argv) {char *socket_path =NULL ;
@@ -106,7 +119,7 @@ int cmd_main(int argc, const char **argv) op = argv[0 ];if (!socket_path) - socket_path =expand_user_path ("~/.git-credential-cache/socket" ); + socket_path =get_socket_path ();if (!socket_path)die ("unable to find a suitable socket path; use --socket" );
Signature · 1 line
2.11.0
Re: [GSoC][PATCH v5 2/3] credential-cache: use XDG_CACHE_HOME for socket
Quoted 24 lines · Devin Lehmacher
Make git-credential-cache follow the XDG base path specification by default. This increases consistency with other applications and helps keep clutter out of users' home directories.
Check the old socket location, ~/.git-credential-cache/, and use ~/.git-credential-cache/socket if that directory exists rather than forcing users who have used
git credential-cachebefore to migrate to the new XDG compliant location.
Otherwise use the socket $XDG_CACHE_HOME/git/credential/socket following XDG base path specification. Use the subdirectory credential/ in case other files are cached under $XDG_CACHE_HOME/git/ in the future and to make the socket's purpose clear.Signed-off-by Devin Lehmacher <[email protected]>--- Documentation/git-credential-cache.txt | 11 +++++++---- credential-cache.c | 15 ++++++++++++++- 2 files changed, 21 insertions(+), 5 deletions(-)Documentation/git-credential-cache.txt +0
-0
Documentation/git-credential-cache.txt OPTIONS · line 37
Quoted 9 lines
Documentation/git-credential-cache.txt +2
-4@@ -33,10 +33,13 @@ OPTIONS --socket <path>:: Use `<path>` to contact a running cache daemon (or start a new - cache daemon if one is not started). Defaults to - `~/.git-credential-cache/socket`. If your home directory is on a - network-mounted filesystem, you may need to change this to a - local filesystem. You must specify an absolute path. + cache daemon if one is not started). + Defaults to `$XDG_CACHE_HOME/git/credential/socket` unless
or $HOME/.cache/git/credential/socket if $XDG_CACHE_HOME is not set. I don't have a good suggestion to re-word this paragraph. (I just spent ten minutes trying!).
Hmm, git-credential-store doesn't mention the $HOME/.config/git/credentials outside of the files section. (In particular, not as part of the --file option description).
Documentation/git-credential-cache.txt OPTIONS · line 42
Quoted 5 lines
Documentation/git-credential-cache.txt +5
-0+ `~/.git-credential-cache/` exists in which case + `~/.git-credential-cache/socket` is used instead. + If your home directory is on a network-mounted filesystem, you + may need to change this to a local filesystem. You must specify + an absolute path.
ATB,
Ramsay Jones
Re: [GSoC][PATCH v5 2/3] credential-cache: use XDG_CACHE_HOME for socket
Quoted 3 lines
or $HOME/.cache/git/credential/socket if $XDG_CACHE_HOME is not set. I don't have a good suggestion to re-word this paragraph. (I just spent ten minutes trying!).
I think it would not be too bad to not include this though because $HOME/.cache/ is what XDG specifies as the default if $XDG_CACHE_HOME is unset.
Devin
PATCH v5 3/3 credential-cache: add tests for XDG functionality
In reply to Devin Lehmacher--- t/t0301-credential-cache.sh | 94 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 94 insertions(+)
t/t0301-credential-cache.sh +94 -0
@@ -12,7 +12,101 @@ test -z "$NO_UNIX_SOCKETS" || { # don't leave a stale daemon running trap 'code=$?; git credential-cache exit; (exit $code); die' EXIT +# test that the daemon works with no special setup helper_test cache + +test_expect_success 'socket defaults to ~/.cache/git/credential/socket' ' + test_when_finished " + git credential-cache exit && + rmdir -p .cache/git/credential/ + " && + test_path_is_missing "$HOME/.git-credential-cache" && + test -S "$HOME/.cache/git/credential/socket" +' + +XDG_CACHE_HOME="$HOME/xdg" +export XDG_CACHE_HOME +# test behavior when XDG_CACHE_HOME is set +helper_test cache + +test_expect_success "use custom XDG_CACHE_HOME if set and default sockets are not created" ' + test_when_finished "git credential-cache exit" && + test -S "$XDG_CACHE_HOME/git/credential/socket" && + test_path_is_missing "$HOME/.git-credential-cache/socket" && + test_path_is_missing "$HOME/.cache/git/credential/socket" +' +unset XDG_CACHE_HOME + +test_expect_success 'credential-cache --socket option overrides default location' ' + test_when_finished " + git credential-cache exit --socket \"\$HOME/dir/socket\" && + rmdir \"\$HOME/dir\" + " && + check approve "cache --socket \"\$HOME/dir/socket\"" <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/dir/socket" +' + +test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" ' + test_when_finished " + git credential-cache exit && + sane_unset XDG_CACHE_HOME + " && + check approve cache <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + find . && + test -S "$HOME/.cache/git/credential/socket" && + XDG_CACHE_HOME="$HOME/xdg" && + export XDG_CACHE_HOME && + check approve cache <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$XDG_CACHE_HOME/git/credential/socket" +' + +test_expect_success 'use user socket if user directory exists' ' + test_when_finished " + git credential-cache exit && + rmdir \"\$HOME/.git-credential-cache/\" + " && + mkdir -p -m 700 "$HOME/.git-credential-cache/" && + check approve cache <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/.git-credential-cache/socket" +' + +test_expect_success 'use user socket if user directory is a symlink to a directory' ' + test_when_finished " + git credential-cache exit && + rmdir \"\$HOME/dir/\" && + rm \"\$HOME/.git-credential-cache\" + " && + mkdir -p -m 700 "$HOME/dir/" && + ln -s "$HOME/dir" "$HOME/.git-credential-cache" && + check approve cache <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/.git-credential-cache/socket" +' + helper_test_timeout cache --timeout=1 # we can't rely on our "trap" above working after test_done,
Signature · 1 line
2.11.0
Re: [GSoC][PATCH v5 3/3] credential-cache: add tests for XDG functionality
t/t0301-credential-cache.sh test -z "$NO_UNIX_SOCKETS" || { · line 25
Quoted 11 lines · Devin Lehmacher
t/t0301-credential-cache.sh +10
-0+# test that the daemon works with no special setup helper_test cache + +test_expect_success 'socket defaults to ~/.cache/git/credential/socket' ' + test_when_finished " + git credential-cache exit && + rmdir -p .cache/git/credential/ + " && + test_path_is_missing "$HOME/.git-credential-cache" && + test -S "$HOME/.cache/git/credential/socket" +'
OK, the clean-up looks quite sensible.
t/t0301-credential-cache.sh test -z "$NO_UNIX_SOCKETS" || { · line 65
Quoted 5 lines
t/t0301-credential-cache.sh +5
-0+test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" ' +... + password=store-pass + EOF + find . &&
A leftover from a debugging session? If so remove the "find" line.
t/t0301-credential-cache.sh test -z "$NO_UNIX_SOCKETS" || {
Quoted 6 lines
t/t0301-credential-cache.sh +4
-0+ test -S "$HOME/.cache/git/credential/socket" && + XDG_CACHE_HOME="$HOME/xdg" && +' +test_expect_success 'use user socket if user directory is a symlink to a directory' '
I think anybody without NO_UNIX_SOCKETS is practically capable of doing a symbolic link, but still it is a good idea to protect this one behind the SYMLINKS prerequisite, i.e.
test_expect_success SYMLINKS 'use ~/.git-credential-cache that is a symlink' ' ...
t/t0301-credential-cache.sh test -z "$NO_UNIX_SOCKETS" || { · line 112
Quoted 7 lines
t/t0301-credential-cache.sh +4
-0+ test_when_finished " +... +' + helper_test_timeout cache --timeout=1 # we can't rely on our "trap" above working after test_done,
Thanks.
PATCH v6 1/3 path.c: add xdg_cache_home
In reply to Devin LehmacherWe already have xdg_config_home to format paths relative to XDG_CONFIG_HOME. Let's provide a similar function xdg_cache_home to do the same for paths relative to XDG_CACHE_HOME.
--- cache.h | 7 +++++++ path.c | 15 +++++++++++++++ 2 files changed, 22 insertions(+)
cache.h +7 -0
@@ -1169,6 +1169,13 @@ extern int is_ntfs_dotgit(const char *name);*/ extern char *xdg_config_home (const char *filename); +/** +* Return a newly allocated string with the evaluation of +* "$XDG_CACHE_HOME/git/$filename" if $XDG_CACHE_HOME is non-empty, otherwise +* "$HOME/.cache/git/$filename". Return NULL upon error. +*/ +extern char *xdg_cache_home (const char *filename); +/* object replacement */ #define LOOKUP_REPLACE_OBJECT 1#define LOOKUP_UNKNOWN_OBJECT 2
path.c +15 -0
@@ -1272,6 +1272,21 @@ char *xdg_config_home(const char *filename)return NULL ; } +char *xdg_cache_home (const char *filename) +{ +const char *home, *cache_home; + +assert (filename); + cache_home =getenv ("XDG_CACHE_HOME" ); +if (cache_home && *cache_home) +return mkpathdup ("%s/git/%s" , cache_home, filename); + + home =getenv ("HOME" ); +if (home) +return mkpathdup ("%s/.cache/git/%s" , home, filename); +return NULL ; +} + GIT_PATH_FUNC(git_path_cherry_pick_head, "CHERRY_PICK_HEAD") GIT_PATH_FUNC(git_path_revert_head, "REVERT_HEAD")GIT_PATH_FUNC (git_path_squash_msg,"SQUASH_MSG" )
Signature · 1 line
2.11.0
PATCH v6 2/3 credential-cache: use XDG_CACHE_HOME for socket
Make git-credential-cache follow the XDG base path specification by default. This increases consistency with other applications and helps keep clutter out of users' home directories.
Check the old socket location, ~/.git-credential-cache/, and use ~/.git-credential-cache/socket if that directory exists rather than forcing users who have used git credential-cache before to migrate to the new XDG compliant location.
Otherwise use the socket $XDG_CACHE_HOME/git/credential/socket following XDG base path specification. Use the subdirectory credential/ in case other files are cached under $XDG_CACHE_HOME/git/ in the future and to make the socket's purpose clear.
--- Documentation/git-credential-cache.txt | 11 +++++++---- credential-cache.c | 15 ++++++++++++++- 2 files changed, 21 insertions(+), 5 deletions(-)
Documentation/git-credential-cache.txt +7 -4
@@ -33,10 +33,13 @@ OPTIONS --socket <path>:: Use `<path>` to contact a running cache daemon (or start a new - cache daemon if one is not started). Defaults to - `~/.git-credential-cache/socket`. If your home directory is on a - network-mounted filesystem, you may need to change this to a - local filesystem. You must specify an absolute path. + cache daemon if one is not started). + Defaults to `$XDG_CACHE_HOME/git/credential/socket` unless + `~/.git-credential-cache/` exists in which case + `~/.git-credential-cache/socket` is used instead. + If your home directory is on a network-mounted filesystem, you + may need to change this to a local filesystem. You must specify + an absolute path. CONTROLLING THE DAEMON ----------------------
credential-cache.c +14 -1
@@ -83,6 +83,19 @@ static void do_cache(const char *socket, const char *action, int timeout,strbuf_release (&buf); } +static char *get_socket_path (void ) +{ +struct stat sb; +char *old_dir, *socket; + old_dir =expand_user_path ("~/.git-credential-cache" ); +if (old_dir && !stat (old_dir, &sb) &&S_ISDIR (sb.st_mode)) + socket =xstrfmt ("%s/socket" , old_dir); +else + socket =xdg_cache_home ("credential/socket" ); +free (old_dir); +return socket; +} +int cmd_main (int argc,const char **argv) {char *socket_path =NULL ;
@@ -106,7 +119,7 @@ int cmd_main(int argc, const char **argv) op = argv[0 ];if (!socket_path) - socket_path =expand_user_path ("~/.git-credential-cache/socket" ); + socket_path =get_socket_path ();if (!socket_path)die ("unable to find a suitable socket path; use --socket" );
Signature · 1 line
2.11.0
Re: [GSoC][PATCH v6 2/3] credential-cache: use XDG_CACHE_HOME for socket
Quoted 4 lines · Devin Lehmacher
Make git-credential-cache follow the XDG base path specification by default. This increases consistency with other applications and helps keep clutter out of users' home directories.
Quoted 8 lines
Check the old socket location, ~/.git-credential-cache/, and use ~/.git-credential-cache/socket if that directory exists rather than forcing users who have used
git credential-cachebefore to migrate to the new XDG compliant location.
Otherwise use the socket $XDG_CACHE_HOME/git/credential/socket following XDG base path specification. Use the subdirectory credential/ in case other files are cached under $XDG_CACHE_HOME/git/ in the future and to make the socket's purpose clear.
Makes sense.
credential-cache.c static void do_cache(const char *socket, const char *action, int timeout, · line 97
Quoted 12 lines
credential-cache.c +12
-0+static char *get_socket_path (void ) +{ +struct stat sb; +char *old_dir, *socket; + old_dir =expand_user_path ("~/.git-credential-cache" ); +if (old_dir && !stat (old_dir, &sb) &&S_ISDIR (sb.st_mode)) + socket =xstrfmt ("%s/socket" , old_dir); +else + socket =xdg_cache_home ("credential/socket" ); +free (old_dir); +return socket; +}
The implementation looks nice and clean.
-Peff
PATCH v6 3/3 credential-cache: add tests for XDG functionality
In reply to Devin Lehmacher--- t/t0301-credential-cache.sh | 93 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+)
t/t0301-credential-cache.sh +93 -0
@@ -12,7 +12,100 @@ test -z "$NO_UNIX_SOCKETS" || { # don't leave a stale daemon running trap 'code=$?; git credential-cache exit; (exit $code); die' EXIT +# test that the daemon works with no special setup helper_test cache + +test_expect_success 'socket defaults to ~/.cache/git/credential/socket' ' + test_when_finished " + git credential-cache exit && + rmdir -p .cache/git/credential/ + " && + test_path_is_missing "$HOME/.git-credential-cache" && + test -S "$HOME/.cache/git/credential/socket" +' + +XDG_CACHE_HOME="$HOME/xdg" +export XDG_CACHE_HOME +# test behavior when XDG_CACHE_HOME is set +helper_test cache + +test_expect_success "use custom XDG_CACHE_HOME if set and default sockets are not created" ' + test_when_finished "git credential-cache exit" && + test -S "$XDG_CACHE_HOME/git/credential/socket" && + test_path_is_missing "$HOME/.git-credential-cache/socket" && + test_path_is_missing "$HOME/.cache/git/credential/socket" +' +unset XDG_CACHE_HOME + +test_expect_success 'credential-cache --socket option overrides default location' ' + test_when_finished " + git credential-cache exit --socket \"\$HOME/dir/socket\" && + rmdir \"\$HOME/dir\" + " && + check approve "cache --socket \"\$HOME/dir/socket\"" <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/dir/socket" +' + +test_expect_success "use custom XDG_CACHE_HOME even if xdg socket exists" ' + test_when_finished " + git credential-cache exit && + sane_unset XDG_CACHE_HOME + " && + check approve cache <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/.cache/git/credential/socket" && + XDG_CACHE_HOME="$HOME/xdg" && + export XDG_CACHE_HOME && + check approve cache <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$XDG_CACHE_HOME/git/credential/socket" +' + +test_expect_success 'use user socket if user directory exists' ' + test_when_finished " + git credential-cache exit && + rmdir \"\$HOME/.git-credential-cache/\" + " && + mkdir -p -m 700 "$HOME/.git-credential-cache/" && + check approve cache <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/.git-credential-cache/socket" +' + +test_expect_success SYMLINKS 'use user socket if user directory is a symlink to a directory' ' + test_when_finished " + git credential-cache exit && + rmdir \"\$HOME/dir/\" && + rm \"\$HOME/.git-credential-cache\" + " && + mkdir -p -m 700 "$HOME/dir/" && + ln -s "$HOME/dir" "$HOME/.git-credential-cache" && + check approve cache <<-\EOF && + protocol=https + host=example.com + username=store-user + password=store-pass + EOF + test -S "$HOME/.git-credential-cache/socket" +' + helper_test_timeout cache --timeout=1 # we can't rely on our "trap" above working after test_done,
Signature · 1 line
2.11.0
Re: [GSoC][PATCH v6 3/3] credential-cache: add tests for XDG functionality
t/t0301-credential-cache.sh test -z "$NO_UNIX_SOCKETS" || { · line 45
Quoted 6 lines · Devin Lehmacher
t/t0301-credential-cache.sh +6
-0+test_expect_success 'credential-cache --socket option overrides default location' ' + test_when_finished " + git credential-cache exit --socket \"\$HOME/dir/socket\" && + rmdir \"\$HOME/dir\" + " && + check approve "cache --socket \"\$HOME/dir/socket\"" <<-\EOF &&
I wondered how this one got through the quoting/eval mess of the "check()" function. But it works because we pass a raw unexpanded $HOME into the config variable, and the credential subsystem then runs "cache --socket ..." using a shell, which is where it gets expanded.
t/t0301-credential-cache.sh test -z "$NO_UNIX_SOCKETS" || { · line 82
Quoted 1 line
t/t0301-credential-cache.sh +1
-0+ mkdir -p -m 700 "$HOME/.git-credential-cache/" &&
This is our first use of "mkdir -m". It's in POSIX, so it should probably be fine. I'm not sure if platforms without POSIXPERM would complain, but I think in practice anything with unix sockets will handle it just fine.
The rest of the tests look OK to me.
-Peff
Re: [GSoC][PATCH v6 1/3] path.c: add xdg_cache_home
In reply to Devin LehmacherQuoted 5 lines · Devin Lehmacher
We already have
xdg_config_hometo format paths relative toXDG_CONFIG_HOME. Let's provide a similar functionxdg_cache_hometo do the same for paths relative toXDG_CACHE_HOME.Signed-off-by Devin Lehmacher <[email protected]>
Looks obviously correct. The whole series looks good to me.
-Peff
Re: [GSoC][PATCH v2 1/2] path.c: add xdg_cache_home
In reply to Devin LehmacherQuoted 3 lines · Devin Lehmacher
We already have
xdg_config_hometo format paths relative toXDG_CONFIG_HOME. Let's provide a similar functionxdg_cache_hometo do the same for paths relative toXDG_CACHE_HOME.
Nicely explained.
Quoted 4 lines
+/**
+ * Return a newly allocated string with the evaluation of
+ * "$XDG_CACHE_HOME/git/$filename" if $XDG_CONFIG_HOME is non-empty, otherwise
+ * "$HOME/.config/git/$filename". Return NULL upon error.
s|CONFIG|CACHE| and s|.config|.cache| are needed, methinks.
Will fix while queuing, so no need to resend only to fix this typo.
PATCH v2 2/2 credential-cache: use XDG_CACHE_HOME for socket
In reply to Devin Lehmachergit-credential-cache will now use a socket following the XDG base path specification by default. This increases consistency with other applications and helps keep clutter out of users' home directories.
We still check the old socket location, ~/.git-credential-cache/socket first in case the user already has a socket at that location. This ensures that a socket previously created will be used over forcibly switching to the new socket location.
If there is not a socket at that location we create a new one at $XDG_CACHE_HOME/git/credential/socket. This complies with the XDG standard and good for the reasons previously mentioned. We use the subdirectory credential/ in case we later want to store other files under $XDG_CACHE_HOME/git/ and to make the purpose of the socket clear.
I also change to documentation to reflect the new default socket location.
--- Documentation/git-credential-cache.txt | 3 ++- credential-cache.c | 16 +++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-)
Documentation/git-credential-cache.txt +2 -1
@@ -34,7 +34,8 @@ OPTIONS Use `<path>` to contact a running cache daemon (or start a new cache daemon if one is not started). Defaults to - `~/.git-credential-cache/socket`. If your home directory is on a + `~/.git-credential-cache/socket` if it exists and otherwise + `$XDG_CACHE_HOME/git/credential/socket`. If your home directory is on a network-mounted filesystem, you may need to change this to a local filesystem. You must specify an absolute path.
credential-cache.c +15 -1
@@ -83,6 +83,20 @@ static void do_cache(const char *socket, const char *action, int timeout,strbuf_release (&buf); } +static char *get_socket_path (void ) { +char *home_socket; + + home_socket =expand_user_path ("~/.git-credential-cache/socket" ); +if (home_socket) { +if (file_exists (home_socket)) +return home_socket; +else +free (home_socket); + } + +return xdg_cache_home ("credential/socket" ); +} +int cmd_main (int argc,const char **argv) {char *socket_path =NULL ;
@@ -106,7 +120,7 @@ int cmd_main(int argc, const char **argv) op = argv[0 ];if (!socket_path) - socket_path =expand_user_path ("~/.git-credential-cache/socket" ); + socket_path =get_socket_path ();if (!socket_path)die ("unable to find a suitable socket path; use --socket" );
Signature · 1 line
2.11.0