Re: [PATCH v4 1/4] git-credential-store: support multiple credential files

2015-03-18 Thread Matthieu Moy
Paul Tan pyoka...@gmail.com writes:

 + /* Write credential to the filename specified by fns-items[0], thus
 +  * creating it */

Just to show I'm following: misformatted multi-line comment.

Other than that, good job!

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 1/4] git-credential-store: support multiple credential files

2015-03-18 Thread Paul Tan
Previously, git-credential-store only supported storing credentials in a
single file: ~/.git-credentials. In order to support the XDG base
directory specification[1], git-credential-store needs to be able to
lookup and erase credentials from multiple files, as well as to pick the
appropriate file to write to so that the credentials can be found on
subsequent lookups.

[1] http://standards.freedesktop.org/basedir-spec/basedir-spec-0.7.html

Note that some credential storage files may not be owned, readable or
writable by the user, as they may be system-wide files that are meant to
apply to every user.

Instead of a single file path, lookup_credential(), remove_credential()
and store_credential() now take a precedence-ordered string_list of
file paths. lookup_credential() expects both user-specific and
system-wide credential files to be provided to support the use case of
system administrators setting default credentials for users.
remove_credential() and store_credential() expect only the user-specific
credential files to be provided as usually the only config files that
users are allowed to edit are their own user-specific ones.

lookup_credential() will read these (user-specific and system-wide) file
paths in order until it finds the 1st matching credential and print it.
As some files may be private and thus unreadable, any file which cannot
be read will be ignored silently.

remove_credential() will erase credentials from all (user-specific)
files in the list.  This is because if credentials are only erased from
the file with the highest precedence, a matching credential may still be
found in a file further down the list. (Note that due to the lockfile
code, this requires the directory to be writable, which should be so for
user-specific config files)

store_credential() will write the credentials to the first existing
(user-specific) file in the list. If none of the files in the list
exist, store_credential() will write to the filename specified by the
first item of the filename list. For backwards compatibility, this
filename should be ~/.git-credentials.

Helped-by: Matthieu Moy matthieu@grenoble-inp.fr
Helped-by: Junio C Hamano gits...@pobox.com
Helped-by: Jeff King p...@peff.net
Signed-off-by: Paul Tan pyoka...@gmail.com
---

The previous version can be found at [1].

[1] http://thread.gmane.org/gmane.comp.version-control.git/265305/focus=265309

The changes are as follows:

* Remove default_fn argument from store_credential(). It now uses the
first item of the filenames string_list. Thanks Jeff, Matthieu and
Junio for your input in this discussion.

* Change filename string_list fns to duplicate strings by default
(STRING_LIST_INIT_DUP). This is to make memory ownership explicit --
when the string_list receives the file paths from xdg_config_home()
and expand_user_path(), it is responsible for freeing those strings.
Thanks Jeff for pointing this out.

 credential-store.c | 79 +-
 1 file changed, 54 insertions(+), 25 deletions(-)

diff --git a/credential-store.c b/credential-store.c
index 925d3f4..8dad479 100644
--- a/credential-store.c
+++ b/credential-store.c
@@ -6,7 +6,7 @@
 
 static struct lock_file credential_lock;
 
-static void parse_credential_file(const char *fn,
+static int parse_credential_file(const char *fn,
  struct credential *c,
  void (*match_cb)(struct credential *),
  void (*other_cb)(struct strbuf *))
@@ -14,18 +14,20 @@ static void parse_credential_file(const char *fn,
FILE *fh;
struct strbuf line = STRBUF_INIT;
struct credential entry = CREDENTIAL_INIT;
+   int found_credential = 0;
 
fh = fopen(fn, r);
if (!fh) {
-   if (errno != ENOENT)
+   if (errno != ENOENT  errno != EACCES)
die_errno(unable to open %s, fn);
-   return;
+   return found_credential;
}
 
while (strbuf_getline(line, fh, '\n') != EOF) {
credential_from_url(entry, line.buf);
if (entry.username  entry.password 
credential_match(c, entry)) {
+   found_credential = 1;
if (match_cb) {
match_cb(entry);
break;
@@ -38,6 +40,7 @@ static void parse_credential_file(const char *fn,
credential_clear(entry);
strbuf_release(line);
fclose(fh);
+   return found_credential;
 }
 
 static void print_entry(struct credential *c)
@@ -64,21 +67,10 @@ static void rewrite_credential_file(const char *fn, struct 
credential *c,
die_errno(unable to commit credential store);
 }
 
-static void store_credential(const char *fn, struct credential *c)
+static void store_credential_file(const char *fn, struct credential *c)
 {
struct strbuf buf =