done in a hacky way and only as a POC, but at least works, if we
are still thinking that warning should be at least comprehensive
for the first iteration.
didn't change the callback as you suggested but I think this
doesn't look that bad either.
squash for final v10 or ..?
---
Documentation/git-credential-store.txt | 7 -------
credential-store.c | 19 +++++++++----------
t/t0302-credential-store.sh | 20 +++++++++++++++++---
3 files changed, 26 insertions(+), 20 deletions(-)
diff --git a/Documentation/git-credential-store.txt b/Documentation/git-credential-store.txt
index 18ba8b6984..d5841cffad 100644
--- a/Documentation/git-credential-store.txt
+++ b/Documentation/git-credential-store.txt
@@ -105,13 +105,6 @@ like spaces, line wrapping or text encoding.
An unparseable or otherwise invalid line is ignored, and a warning
message points out the problematic file and line number it appears in.
-Note that because of deficiencies of the current implementation, these
-warnings will be only generated for the entries that were processed
-while reading the credential files during a get operation, and so they
-are not comprehensive, and will require you to use an editor (or
-better a sed/awk/perl one liner) to edit the credential file and remove
-them.
-
When Git needs authentication for a particular URL context,
credential-store will consider that context a pattern to match against
each entry in the credentials file. If the protocol, hostname, and
diff --git a/credential-store.c b/credential-store.c
index c2778eff8a..236df8e7bf 100644
--- a/credential-store.c
+++ b/credential-store.c
@@ -4,8 +4,6 @@
#include "string-list.h"
#include "parse-options.h"
-#define PARSE_VERBOSE 0x01
-
static struct lock_file credential_lock;
static int valid_credential(struct credential *entry)
@@ -17,7 +15,7 @@ static int valid_credential(struct credential *entry)
static int parse_credential_file(const char *fn,
struct credential *c,
- int flags,
+ int initial_line,
void (*match_cb)(struct credential *),
void (*other_cb)(struct strbuf *))
{
@@ -25,7 +23,7 @@ static int parse_credential_file(const char *fn,
struct strbuf line = STRBUF_INIT;
struct credential entry = CREDENTIAL_INIT;
int found_credential = 0;
- int lineno = 0;
+ int lineno = initial_line;
fh = fopen(fn, "r");
if (!fh) {
@@ -40,8 +38,7 @@ static int parse_credential_file(const char *fn,
if (strchr(line.buf, '\r') ||
credential_from_url_gently(&entry, line.buf, 1) ||
!valid_credential(&entry)) {
- if (flags & PARSE_VERBOSE)
- warning(_("%s:%d: ignoring invalid credential"),
+ warning(_("%s:%d: ignoring invalid credential"),
fn, lineno);
} else if (credential_match(c, &entry)) {
found_credential = 1;
@@ -77,11 +74,14 @@ static void print_line(struct strbuf *buf)
static void rewrite_credential_file(const char *fn, struct credential *c,
struct strbuf *extra)
{
+ int initial_line = 0;
if (hold_lock_file_for_update(&credential_lock, fn, 0) < 0)
die_errno("unable to get credential storage lock");
- if (extra)
+ if (extra) {
print_line(extra);
- parse_credential_file(fn, c, 0, NULL, print_line);
+ initial_line++;
+ }
+ parse_credential_file(fn, c, initial_line, NULL, print_line);
if (commit_lock_file(&credential_lock) < 0)
die_errno("unable to write credential store");
}
@@ -158,8 +158,7 @@ static void lookup_credential(const struct string_list *fns, struct credential *
struct string_list_item *fn;
for_each_string_list_item(fn, fns)
- if (parse_credential_file(fn->string, c, PARSE_VERBOSE,
- print_entry, NULL))
+ if (parse_credential_file(fn->string, c, 0, print_entry, NULL))
return; /* Found credential */
}
diff --git a/t/t0302-credential-store.sh b/t/t0302-credential-store.sh
index 00608e365a..78010590f4 100755
--- a/t/t0302-credential-store.sh
+++ b/t/t0302-credential-store.sh
@@ -218,13 +218,27 @@ test_expect_success 'get: store file can contain empty/bogus lines' '
test_line_count = 3 stderr
'
-test_expect_success 'store: store succeeds silently in corrupted file' '
+test_expect_success 'store: succeeds in corrupted file but correctly warn' '
echo "#comment" >"$HOME/.git-credentials" &&
- check approve store <<-\EOF &&
+ test_config credential.helper store &&
+ git credential approve <<-\EOF &&
url=https://user:pass@xxxxxxxxxxx
EOF
grep "https://user:pass@xxxxxxxxxxx"; "$HOME/.git-credentials" &&
- test_must_be_empty stderr
+ test_i18ngrep "git-credentials:2: ignoring invalid credential" stderr
+'
+
+test_expect_success 'erase: succeeds in corrupted file but correctly warn' '
+ cat <<-\CONFIG >"$HOME/.git-credentials" &&
+ #comment
+ https://user:pass@xxxxxxxxxxx
+ CONFIG
+ test_config credential.helper store &&
+ git credential reject <<-\EOF &&
+ url=https://example.com
+ EOF
+ ! grep "https://user:pass@xxxxxxxxxxx"; "$HOME/.git-credentials" &&
+ test_i18ngrep "git-credentials:1: ignoring invalid credential" stderr
'
test_done
--
2.26.2.3.g90bd2f7136
