* [PATCH v2 0/4] git-credential-store: XDG user-specific config file support
@ 2015-03-08 7:58 Paul Tan
2015-03-08 7:58 ` [PATCH v2 1/4] git-credential-store: support multiple credential files Paul Tan
` (3 more replies)
0 siblings, 4 replies; 8+ messages in thread
From: Paul Tan @ 2015-03-08 7:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Matthieu Moy, Jeff King, Paul Tan
The previous patch series can be found at [1].
[1] http://thread.gmane.org/gmane.comp.version-control.git/264682
The changes are as follows:
* Code refactor: instead of a gigantic blob of code in main(),
store_credential(), remove_credential() and lookup_credential() have been
modified to take a string_list of precedence-ordered file paths. Although in
this patch only support for XDG_CONFIG_HOME (user-specific config files) are
implemented, this opens the door for support of XDG_CONFIG_DIRS (system-wide
config files) to be implemented as well.
* parse_credential_file() returns the value of found_credential at all times.
(Thanks Junio for pointing this out)
* parse_credential_file(), and thus "get" ignores unreadable/non-existing files
instead of warning the user. This is done to follow the XDG base dir spec,
which states that: "if for any reason a file in a certain directory is
unaccessible, e.g. because the directory is non-existant, the file is
non-existant or the user is not authorized to open the file, then the
processing of the file in that directory should be skipped."[2]
[2] http://standards.freedesktop.org/basedir-spec/basedir-spec-latest.html
* "store" now only write to a single file instead of writing to one and erasing
from the rest, unlike the behavior in the 1st patch, as the complexity
introduced by implementing this behavior is probably unnecessary. If the user
really wanted all matching credentials to be erased, the user can simply just
issue an erase. (Thanks Matthieu, Junio and Jeff for this suggestion)
* ~/.git-credentials now has greater precedence than the xdg credentials file.
This is to be consistent with the behavior of git-config.
* Support for $XDG_CONFIG_HOME/git/credentials has been documented in
git-credentials-store.
The changes for the tests are as follows:
* Instead of repeating ${XDG_CONFIG_HOME:-$HOME/.config} all over
the place, add tests that test for $HOME/.config/git/credentials (when
XDG_CONFIG_HOME is unset) and for $XDG_CONFIG_HOME/git/credentials with a
custom $XDG_CONFIG_HOME directory set. This is to test that the new code
respects the $XDG_CONFIG_HOME environment variable. (Thanks Junio)
* Use test_path_is_missing to test if files exist. (Thanks Matthieu)
* All code is now within test_expect_success and will now fail if any
error occurs in the code block through the use of "&&". Furthermore, tests do
not rely on previous tests passing as the credential files are overwritten in
each test. (Thanks Matthieu and Junio for your code review)
The most current version of the patch queue is published in the xdg branch
at [3]. I try to push -f regularly.
For your feedback, please.
[3] https://github.com/pyokagan/git
Paul Tan (4):
git-credential-store: support multiple credential files
git-credential-store: support XDG_CONFIG_HOME
docs/git-credential-store: document XDG file and precedence
t0302: test credential-store support for XDG_CONFIG_HOME
Documentation/git-credential-store.txt | 37 +++++++++++++-
credential-store.c | 86 +++++++++++++++++++++----------
t/t0302-credential-store.sh | 92 ++++++++++++++++++++++++++++++++++
3 files changed, 186 insertions(+), 29 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 1/4] git-credential-store: support multiple credential files
2015-03-08 7:58 [PATCH v2 0/4] git-credential-store: XDG user-specific config file support Paul Tan
@ 2015-03-08 7:58 ` Paul Tan
2015-03-08 7:58 ` [PATCH v2 2/4] git-credential-store: support XDG_CONFIG_HOME Paul Tan
` (2 subsequent siblings)
3 siblings, 0 replies; 8+ messages in thread
From: Paul Tan @ 2015-03-08 7:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Matthieu Moy, Jeff King, 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
default_index, thus creating it. For backwards compatibility,
~/.git-credentials should be the file specified by default_index.
Helped-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
credential-store.c | 77 ++++++++++++++++++++++++++++++++++++------------------
1 file changed, 52 insertions(+), 25 deletions(-)
diff --git a/credential-store.c b/credential-store.c
index 925d3f4..3455d7a 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 = STRBUF_INIT;
- /*
- * Sanity check that what we are storing is actually sensible.
- * In particular, we can't make a URL without a protocol field.
- * Without either a host or pathname (depending on the scheme),
- * we have no primary key. And without a username and password,
- * we are not actually storing a credential.
- */
- if (!c->protocol || !(c->host || c->path) ||
- !c->username || !c->password)
- return;
-
strbuf_addf(&buf, "%s://", c->protocol);
strbuf_addstr_urlencode(&buf, c->username, 1);
strbuf_addch(&buf, ':');
@@ -95,8 +87,34 @@ static void store_credential(const char *fn, struct credential *c)
strbuf_release(&buf);
}
-static void remove_credential(const char *fn, struct credential *c)
+static void store_credential(const struct string_list *fns, struct credential *c,
+ unsigned int default_index)
{
+ struct string_list_item *fn;
+
+ /*
+ * Sanity check that what we are storing is actually sensible.
+ * In particular, we can't make a URL without a protocol field.
+ * Without either a host or pathname (depending on the scheme),
+ * we have no primary key. And without a username and password,
+ * we are not actually storing a credential.
+ */
+ if (!c->protocol || !(c->host || c->path) || !c->username || !c->password)
+ return;
+
+ for_each_string_list_item(fn, fns)
+ if (!access(fn->string, F_OK)) {
+ store_credential_file(fn->string, c);
+ return;
+ }
+ /* Write credential to the filename at default_index, creating it */
+ store_credential_file(fns->items[default_index].string, c);
+}
+
+static void remove_credential(const struct string_list *fns, struct credential *c)
+{
+ struct string_list_item *fn;
+
/*
* Sanity check that we actually have something to match
* against. The input we get is a restrictive pattern,
@@ -105,14 +123,20 @@ static void remove_credential(const char *fn, struct credential *c)
* to empty input. So explicitly disallow it, and require that the
* pattern have some actual content to match.
*/
- if (c->protocol || c->host || c->path || c->username)
- rewrite_credential_file(fn, c, NULL);
+ if (!c->protocol && !c->host && !c->path && !c->username)
+ return;
+ for_each_string_list_item(fn, fns)
+ if (!access(fn->string, F_OK))
+ rewrite_credential_file(fn->string, c, NULL);
}
-static int lookup_credential(const char *fn, struct credential *c)
+static void lookup_credential(const struct string_list *fns, struct credential *c)
{
- parse_credential_file(fn, c, print_entry, NULL);
- return c->username && c->password;
+ struct string_list_item *fn;
+
+ for_each_string_list_item(fn, fns)
+ if (parse_credential_file(fn->string, c, print_entry, NULL))
+ return; /* Found credential */
}
int main(int argc, char **argv)
@@ -123,6 +147,7 @@ int main(int argc, char **argv)
};
const char *op;
struct credential c = CREDENTIAL_INIT;
+ struct string_list fns = STRING_LIST_INIT_NODUP;
char *file = NULL;
struct option options[] = {
OPT_STRING(0, "file", &file, "path",
@@ -139,18 +164,20 @@ int main(int argc, char **argv)
if (!file)
file = expand_user_path("~/.git-credentials");
- if (!file)
+ if (file)
+ string_list_append_nodup(&fns, file);
+ else
die("unable to set up default path; use --file");
if (credential_read(&c, stdin) < 0)
die("unable to read credential");
if (!strcmp(op, "get"))
- lookup_credential(file, &c);
+ lookup_credential(&fns, &c);
else if (!strcmp(op, "erase"))
- remove_credential(file, &c);
+ remove_credential(&fns, &c);
else if (!strcmp(op, "store"))
- store_credential(file, &c);
+ store_credential(&fns, &c, fns.nr - 1);
else
; /* Ignore unknown operation. */
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 2/4] git-credential-store: support XDG_CONFIG_HOME
2015-03-08 7:58 [PATCH v2 0/4] git-credential-store: XDG user-specific config file support Paul Tan
2015-03-08 7:58 ` [PATCH v2 1/4] git-credential-store: support multiple credential files Paul Tan
@ 2015-03-08 7:58 ` Paul Tan
2015-03-10 13:43 ` Paul Tan
2015-03-08 7:58 ` [PATCH v2 3/4] docs/git-credential-store: document XDG file and precedence Paul Tan
2015-03-08 7:58 ` [PATCH v2 4/4] t0302: test credential-store support for XDG_CONFIG_HOME Paul Tan
3 siblings, 1 reply; 8+ messages in thread
From: Paul Tan @ 2015-03-08 7:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Matthieu Moy, Jeff King, Paul Tan
Add $XDG_CONFIG_HOME/git/credentials to the default credential search
path of git-credential-store. This allows git-credential-store to
support user-specific configuration files in accordance with the XDG
base directory specification[1].
[1] http://standards.freedesktop.org/basedir-spec/basedir-spec-0.7.html
~/.git-credentials has a higher precedence than
$XDG_CONFIG_HOME/git/credentials when looking up credentials. This
means that if any duplicate matching credentials are found in the xdg
file (due to ~/.git-credentials being updated by old versions of git or
outdated tools), they will not be used at all. This is to give the user
some leeway in switching to old versions of git while keeping the xdg
directory. This is consistent with the behavior of git-config.
However, the higher precedence of ~/.git-credentials means that as long
as ~/.git-credentials exist, all credentials will be written to the
~/.git-credentials file even if the user has an xdg file as having a
~/.git-credentials file indicates that the user wants to preserve
backwards-compatibility. This is also consistent with the behavior of
git-config.
Since the xdg file will not be used unless it actually exists, to
prevent the situation where some credentials are present in the xdg file
while some are present in the home file, users are recommended to not
create the xdg file if they require compatibility with old versions of
git or outdated tools. Note, though, that "erase" can be used to
explicitly erase matching credentials from all files.
Helped-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Jeff King <peff@peff.net>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
credential-store.c | 15 ++++++++++-----
1 file changed, 10 insertions(+), 5 deletions(-)
diff --git a/credential-store.c b/credential-store.c
index 3455d7a..7b22a3a 100644
--- a/credential-store.c
+++ b/credential-store.c
@@ -162,11 +162,16 @@ int main(int argc, char **argv)
usage_with_options(usage, options);
op = argv[0];
- if (!file)
- file = expand_user_path("~/.git-credentials");
- if (file)
+ if (file) {
string_list_append_nodup(&fns, file);
- else
+ } else {
+ if ((file = expand_user_path("~/.git-credentials")))
+ string_list_append_nodup(&fns, file);
+ home_config_paths(NULL, &file, "credentials");
+ if (file)
+ string_list_append_nodup(&fns, file);
+ }
+ if (!fns.nr)
die("unable to set up default path; use --file");
if (credential_read(&c, stdin) < 0)
@@ -177,7 +182,7 @@ int main(int argc, char **argv)
else if (!strcmp(op, "erase"))
remove_credential(&fns, &c);
else if (!strcmp(op, "store"))
- store_credential(&fns, &c, fns.nr - 1);
+ store_credential(&fns, &c, 0);
else
; /* Ignore unknown operation. */
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 3/4] docs/git-credential-store: document XDG file and precedence
2015-03-08 7:58 [PATCH v2 0/4] git-credential-store: XDG user-specific config file support Paul Tan
2015-03-08 7:58 ` [PATCH v2 1/4] git-credential-store: support multiple credential files Paul Tan
2015-03-08 7:58 ` [PATCH v2 2/4] git-credential-store: support XDG_CONFIG_HOME Paul Tan
@ 2015-03-08 7:58 ` Paul Tan
2015-03-08 7:58 ` [PATCH v2 4/4] t0302: test credential-store support for XDG_CONFIG_HOME Paul Tan
3 siblings, 0 replies; 8+ messages in thread
From: Paul Tan @ 2015-03-08 7:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Matthieu Moy, Jeff King, Paul Tan
git-credential-store now supports an additional default credential file
at $XDG_CONFIG_HOME/git/credentials. However, ~/.git-credentials takes
precedence over it for backwards compatibility. To make the precedence
ordering explicit, add a new section FILES that lists out the credential
file paths in their order of precedence, and explains how the ordering
affects the lookup, storage and erase operations.
Also update documentation for --store to briefly explain the operations
on multiple files if the --store option is not provided.
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
Documentation/git-credential-store.txt | 37 ++++++++++++++++++++++++++++++++--
1 file changed, 35 insertions(+), 2 deletions(-)
diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt
index bc97071..451c4fa 100644
--- a/Documentation/git-credential-store.txt
+++ b/Documentation/git-credential-store.txt
@@ -31,10 +31,43 @@ OPTIONS
--file=<path>::
- Use `<path>` to store credentials. The file will have its
+ Use `<path>` to lookup and store credentials. The file will have its
filesystem permissions set to prevent other users on the system
from reading it, but will not be encrypted or otherwise
- protected. Defaults to `~/.git-credentials`.
+ protected. If not specified, credentials will be searched for from
+ `~/.git-credentials` and `$XDG_CONFIG_HOME/git/credentials`, and
+ credentials will be written to `~/.git-credentials` if it exists, or
+ `$XDG_CONFIG_HOME/git/credentials` if it exists and the former does
+ not. See also <<FILES>>.
+
+[[FILES]]
+FILES
+-----
+
+If not set explicitly with '--file', there are two files where
+git-credential-store will search for credentials in order of precedence:
+
+~/.git-credentials::
+ User-specific credentials file.
+
+$XDG_CONFIG_HOME/git/credentials::
+ Second user-specific credentials file. If '$XDG_CONFIG_HOME' is not set
+ or empty, `$HOME/.config/git/credentials` will be used. Any credentials
+ stored in this file will not be used if `~/.git-credentials` has a
+ matching credential as well. It is a good idea not to create this file
+ if you sometimes use older versions of Git, as support for this file
+ was added fairly recently.
+
+
+For credential lookups, the files are read in the order given above, with the
+first matching credential found taking precedence over credentials found in
+files further down the list.
+
+Credential storage will per default write to the first existing file in the
+list. If none of these files exist, `~/.git-credentials` will be created and
+written to.
+
+When erasing credentials, matching credentials will be erased from all files.
EXAMPLES
--------
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v2 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
2015-03-08 7:58 [PATCH v2 0/4] git-credential-store: XDG user-specific config file support Paul Tan
` (2 preceding siblings ...)
2015-03-08 7:58 ` [PATCH v2 3/4] docs/git-credential-store: document XDG file and precedence Paul Tan
@ 2015-03-08 7:58 ` Paul Tan
2015-03-09 12:36 ` Matthieu Moy
3 siblings, 1 reply; 8+ messages in thread
From: Paul Tan @ 2015-03-08 7:58 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Matthieu Moy, Jeff King, Paul Tan
t0302 now tests git-credential-store's support for the XDG user-specific
configuration file $XDG_CONFIG_HOME/git/credentials. Specifically:
* Ensure that the XDG file is strictly opt-in. It should not be created
by git at all times if it does not exist.
* On the flip side, if the XDG file exists, ~/.git-credentials should
not be created at all times.
* If both the XDG file and ~/.git-credentials exists, then both files
should be used for credential lookups. However, credentials should
only be written to ~/.git-credentials.
* Credentials must be erased from both files.
* $XDG_CONFIG_HOME can be a custom directory set by the user as per the
XDG base directory specification. Test that git-credential-store
respects that, but defaults to "~/.config/git/credentials" if it does
not exist or is empty.
Helped-by: Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>
Helped-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Paul Tan <pyokagan@gmail.com>
---
t/t0302-credential-store.sh | 92 +++++++++++++++++++++++++++++++++++++++++++++
1 file changed, 92 insertions(+)
diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
index f61b40c..7fe832d 100755
--- a/t/t0302-credential-store.sh
+++ b/t/t0302-credential-store.sh
@@ -5,5 +5,97 @@ test_description='credential-store tests'
. "$TEST_DIRECTORY"/lib-credential.sh
helper_test store
+test_expect_success '~/.git-credentials is written to when xdg credentials file does not exist' '
+ test -s "$HOME/.git-credentials"
+'
+test_expect_success 'xdg credentials file will not be created if it does not exist' '
+ test_path_is_missing "$HOME/.config/git/credentials"
+'
+test_expect_success 'create $XDG_CONFIG_HOME/git/credentials file' '
+ test_might_fail rm "$HOME/.git-credentials" &&
+ mkdir -p "$HOME/.config/git" &&
+ >"$HOME/.config/git/credentials"
+'
+helper_test store
+test_expect_success 'xdg credentials file will be written to if it exists' '
+ test -s "$HOME/.config/git/credentials"
+'
+test_expect_success '~/.git-credentials will not be created if xdg credentials file exists' '
+ test_path_is_missing "$HOME/.git-credentials"
+'
+test_expect_success 'set up custom XDG_CONFIG_HOME credential file' '
+ XDG_CONFIG_HOME="$HOME/xdg" && export XDG_CONFIG_HOME &&
+ mkdir -p "$HOME/xdg/git" &&
+ >"$HOME/xdg/git/credentials" &&
+ >"$HOME/.config/git/credentials"
+'
+helper_test store
+test_expect_success 'custom XDG_CONFIG_HOME credentials file will be written to if it exists' '
+ unset XDG_CONFIG_HOME &&
+ test -s "$HOME/xdg/git/credentials"
+'
+test_expect_success '~/.config/git/credentials file will not be written to if a custom XDG_CONFIG_HOME is set' '
+ test_must_be_empty "$HOME/.config/git/credentials"
+'
+test_expect_success '~/.git-credentials will not be created if xdg credentials file exists' '
+ test_path_is_missing "$HOME/.git-credentials"
+'
+test_expect_success 'get: return credentials from home file if matches are found in both home and xdg files' '
+ mkdir -p "$HOME/.config/git" &&
+ echo "https://xdg-user:xdg-pass@example.com" >"$HOME/.config/git/credentials" &&
+ echo "https://home-user:home-pass@example.com" >"$HOME/.git-credentials" &&
+ check fill store <<-\EOF
+ protocol=https
+ host=example.com
+ --
+ protocol=https
+ host=example.com
+ username=home-user
+ password=home-pass
+ --
+ EOF
+'
+test_expect_success 'get: return credentials from xdg file if the home files do not have them' '
+ mkdir -p "$HOME/.config/git" &&
+ >"$HOME/.git-credentials" &&
+ echo "https://home-user:home-pass@example.com" >"$HOME/.config/git/credentials" &&
+ check fill store <<-\EOF
+ protocol=https
+ host=example.com
+ --
+ protocol=https
+ host=example.com
+ username=home-user
+ password=home-pass
+ --
+ EOF
+'
+test_expect_success 'get: return credentials from home file if xdg files are unreadable' '
+ mkdir -p "$HOME/.config/git" &&
+ echo "https://xdg-user:xdg-pass@example.com" >"$HOME/.config/git/credentials" &&
+ echo "https://home-user:home-pass@example.com" >"$HOME/.git-credentials" &&
+ chmod -r "$HOME/.config/git/credentials" &&
+ check fill store <<-\EOF
+ protocol=https
+ host=example.com
+ --
+ protocol=https
+ host=example.com
+ username=home-user
+ password=home-pass
+ --
+ EOF
+'
+test_expect_success 'erase: erase matching credentials from both xdg and home files' '
+ mkdir -p "$HOME/.config/git" &&
+ echo "https://xdg-user:xdg-pass@example.com" >"$HOME/.config/git/credentials" &&
+ echo "https://home-user:home-pass@example.com" >"$HOME/.git-credentials" &&
+ check reject store <<-\EOF
+ protocol=https
+ host=example.com
+ EOF
+ test_must_be_empty "$HOME/.config/git/credentials" &&
+ test_must_be_empty "$HOME/.git-credentials"
+'
test_done
--
2.1.4
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
2015-03-08 7:58 ` [PATCH v2 4/4] t0302: test credential-store support for XDG_CONFIG_HOME Paul Tan
@ 2015-03-09 12:36 ` Matthieu Moy
2015-03-10 13:55 ` Paul Tan
0 siblings, 1 reply; 8+ messages in thread
From: Matthieu Moy @ 2015-03-09 12:36 UTC (permalink / raw)
To: Paul Tan; +Cc: git, Junio C Hamano, Jeff King
Paul Tan <pyokagan@gmail.com> writes:
> diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
> index f61b40c..7fe832d 100755
> --- a/t/t0302-credential-store.sh
> +++ b/t/t0302-credential-store.sh
> @@ -5,5 +5,97 @@ test_description='credential-store tests'
> . "$TEST_DIRECTORY"/lib-credential.sh
>
> helper_test store
> +test_expect_success '~/.git-credentials is written to when xdg credentials file does not exist' '
> + test -s "$HOME/.git-credentials"
> +'
> +test_expect_success 'xdg credentials file will not be created if it does not exist' '
We usually put a blank line between tests.
> +helper_test store
That seems a bit "brute force" to me to re-run all tests several times.
It's probably OK since the tests are quite fast though.
> +test_expect_success 'set up custom XDG_CONFIG_HOME credential file' '
> + XDG_CONFIG_HOME="$HOME/xdg" && export XDG_CONFIG_HOME &&
> + mkdir -p "$HOME/xdg/git" &&
I'd spell that "$XDG_CONFIG_HOME"/git instead, but that's not really
important.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 2/4] git-credential-store: support XDG_CONFIG_HOME
2015-03-08 7:58 ` [PATCH v2 2/4] git-credential-store: support XDG_CONFIG_HOME Paul Tan
@ 2015-03-10 13:43 ` Paul Tan
0 siblings, 0 replies; 8+ messages in thread
From: Paul Tan @ 2015-03-10 13:43 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Matthieu Moy, Jeff King, Paul Tan
On Sun, Mar 8, 2015 at 3:58 PM, Paul Tan <pyokagan@gmail.com> wrote:
> remove_credential(&fns, &c);
> else if (!strcmp(op, "store"))
> - store_credential(&fns, &c, fns.nr - 1);
> + store_credential(&fns, &c, 0);
> else
> ; /* Ignore unknown operation. */
Whoops, seems like I squashed some commits in the wrong order (but
there is no functional difference). Will re-roll the patch series.
Also, store_credential() should take a string as the default filename
instead of an index into the filename string_list for extra API
flexibility.
Thanks.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 4/4] t0302: test credential-store support for XDG_CONFIG_HOME
2015-03-09 12:36 ` Matthieu Moy
@ 2015-03-10 13:55 ` Paul Tan
0 siblings, 0 replies; 8+ messages in thread
From: Paul Tan @ 2015-03-10 13:55 UTC (permalink / raw)
To: Matthieu Moy; +Cc: git, Junio C Hamano, Jeff King
On Mon, Mar 9, 2015 at 8:36 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
> Paul Tan <pyokagan@gmail.com> writes:
>> +'
>> +test_expect_success 'xdg credentials file will not be created if it does not exist' '
>
> We usually put a blank line between tests.
Okay, will do that.
>> +helper_test store
>
> That seems a bit "brute force" to me to re-run all tests several times.
> It's probably OK since the tests are quite fast though.
The credential helper tests are re-run because I believe that tests
should make no assumptions on the underlying implementation. e.g. the
presence of the xdg file only may activate a code path that causes
credential storage to succeed but retrieval to fail.
>> +test_expect_success 'set up custom XDG_CONFIG_HOME credential file' '
>> + XDG_CONFIG_HOME="$HOME/xdg" && export XDG_CONFIG_HOME &&
>> + mkdir -p "$HOME/xdg/git" &&
>
> I'd spell that "$XDG_CONFIG_HOME"/git instead, but that's not really
> important.
Okay, will do that.
Thanks for the review.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-03-10 13:55 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-03-08 7:58 [PATCH v2 0/4] git-credential-store: XDG user-specific config file support Paul Tan
2015-03-08 7:58 ` [PATCH v2 1/4] git-credential-store: support multiple credential files Paul Tan
2015-03-08 7:58 ` [PATCH v2 2/4] git-credential-store: support XDG_CONFIG_HOME Paul Tan
2015-03-10 13:43 ` Paul Tan
2015-03-08 7:58 ` [PATCH v2 3/4] docs/git-credential-store: document XDG file and precedence Paul Tan
2015-03-08 7:58 ` [PATCH v2 4/4] t0302: test credential-store support for XDG_CONFIG_HOME Paul Tan
2015-03-09 12:36 ` Matthieu Moy
2015-03-10 13:55 ` Paul Tan
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).