* Re: [PATCH v2 3/3] connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config
From: Johannes Schindelin @ 2017-01-27 10:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Segev Finer, Jeff King
In-Reply-To: <xmqqo9yt4o5i.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Thu, 26 Jan 2017, Junio C Hamano wrote:
> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
>
> > diff --git a/connect.c b/connect.c
> > index 9f750eacb6..7b4437578b 100644
> > --- a/connect.c
> > +++ b/connect.c
> > @@ -691,6 +691,24 @@ static const char *get_ssh_command(void)
> > return NULL;
> > }
> >
> > +static int handle_ssh_variant(int *port_option, int *needs_batch)
> > +{
> > + const char *variant;
> > +
> > + if (!(variant = getenv("GIT_SSH_VARIANT")) &&
> > + git_config_get_string_const("ssh.variant", &variant))
> > + return 0;
> > +
> > + if (!strcmp(variant, "plink") || !strcmp(variant, "putty"))
> > + *port_option = 'P';
> > + else if (!strcmp(variant, "tortoiseplink")) {
> > + *port_option = 'P';
> > + *needs_batch = 1;
> > + }
> > +
> > + return 1;
> > +}
>
> Between handle and get I do not think there is strong reason to
> favor one over the other.
That is correct. "handle" and "get" are two very beautiful words, and none
of them deserves to take a back seat behind the other.
In this case, "get" is inappropriate, though, as the function does not
return the ssh variant, nor does it assign the ssh variant to any variable
to which any of its argument points.
Will try to find the time to address the other issues soon,
Johannes
^ permalink raw reply
* [PATCH v4 5/5] urlmatch: allow globbing for the URL host part
From: Patrick Steinhardt @ 2017-01-27 10:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Patrick Steinhardt,
Philip Oakley
In-Reply-To: <cover.1485512626.git.patrick.steinhardt@elego.de>
The URL matching function computes for two URLs whether they match not.
The match is performed by splitting up the URL into different parts and
then doing an exact comparison with the to-be-matched URL.
The main user of `urlmatch` is the configuration subsystem. It allows to
set certain configurations based on the URL which is being connected to
via keys like `http.<url>.*`. A common use case for this is to set
proxies for only some remotes which match the given URL. Unfortunately,
having exact matches for all parts of the URL can become quite tedious
in some setups. Imagine for example a corporate network where there are
dozens or even hundreds of subdomains, which would have to be configured
individually.
Allow users to write an asterisk '*' in place of any 'host' or
'subdomain' label as part of the host name. For example,
"http.https://*.example.com.proxy" sets "http.proxy" for all direct
subdomains of "https://example.com", e.g. "https://foo.example.com", but
not "https://foo.bar.example.com".
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
Helped-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/config.txt | 5 +++-
t/t1300-repo-config.sh | 72 ++++++++++++++++++++++++++++++++++++++++++++++++
urlmatch.c | 49 +++++++++++++++++++++++++++++---
3 files changed, 121 insertions(+), 5 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 506431267..078e9b490 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1914,7 +1914,10 @@ http.<url>.*::
must match exactly between the config key and the URL.
. Host/domain name (e.g., `example.com` in `https://example.com/`).
- This field must match exactly between the config key and the URL.
+ This field must match between the config key and the URL. It is
+ possible to specify a `*` as part of the host name to match all subdomains
+ at this level. `https://*.example.com/` for example would match
+ `https://foo.example.com/`, but not `https://foo.bar.example.com/`.
. Port number (e.g., `8080` in `http://example.com:8080/`).
This field must match exactly between the config key and the URL.
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 6c844d519..052f12021 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1187,6 +1187,18 @@ test_expect_success 'urlmatch favors more specific URLs' '
cookieFile = /tmp/user.txt
[http "https://averylonguser@example.com/"]
cookieFile = /tmp/averylonguser.txt
+ [http "https://preceding.example.com"]
+ cookieFile = /tmp/preceding.txt
+ [http "https://*.example.com"]
+ cookieFile = /tmp/wildcard.txt
+ [http "https://*.example.com/wildcardwithsubdomain"]
+ cookieFile = /tmp/wildcardwithsubdomain.txt
+ [http "https://trailing.example.com"]
+ cookieFile = /tmp/trailing.txt
+ [http "https://user@*.example.com/"]
+ cookieFile = /tmp/wildcardwithuser.txt
+ [http "https://sub.example.com/"]
+ cookieFile = /tmp/sub.txt
EOF
echo http.cookiefile /tmp/root.txt >expect &&
@@ -1207,6 +1219,66 @@ test_expect_success 'urlmatch favors more specific URLs' '
echo http.cookiefile /tmp/subdirectory.txt >expect &&
git config --get-urlmatch HTTP https://averylonguser@example.com/subdirectory >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/preceding.txt >expect &&
+ git config --get-urlmatch HTTP https://preceding.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/wildcard.txt >expect &&
+ git config --get-urlmatch HTTP https://wildcard.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/sub.txt >expect &&
+ git config --get-urlmatch HTTP https://sub.example.com/wildcardwithsubdomain >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/trailing.txt >expect &&
+ git config --get-urlmatch HTTP https://trailing.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/sub.txt >expect &&
+ git config --get-urlmatch HTTP https://user@sub.example.com >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'urlmatch with wildcard' '
+ cat >.git/config <<-\EOF &&
+ [http]
+ sslVerify
+ [http "https://*.example.com"]
+ sslVerify = false
+ cookieFile = /tmp/cookie.txt
+ EOF
+
+ test_expect_code 1 git config --bool --get-urlmatch doesnt.exist https://good.example.com >actual &&
+ test_must_be_empty actual &&
+
+ echo true >expect &&
+ git config --bool --get-urlmatch http.SSLverify https://example.com >actual &&
+ test_cmp expect actual &&
+
+ echo true >expect &&
+ git config --bool --get-urlmatch http.SSLverify https://good-example.com >actual &&
+ test_cmp expect actual &&
+
+ echo true >expect &&
+ git config --bool --get-urlmatch http.sslverify https://deep.nested.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo false >expect &&
+ git config --bool --get-urlmatch http.sslverify https://good.example.com >actual &&
+ test_cmp expect actual &&
+
+ {
+ echo http.cookiefile /tmp/cookie.txt &&
+ echo http.sslverify false
+ } >expect &&
+ git config --get-urlmatch HTTP https://good.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.sslverify >expect &&
+ git config --get-urlmatch HTTP https://more.example.com.au >actual &&
test_cmp expect actual
'
diff --git a/urlmatch.c b/urlmatch.c
index f35d00a6e..bb5267732 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -63,6 +63,49 @@ static int append_normalized_escapes(struct strbuf *buf,
return 1;
}
+static const char *end_of_token(const char *s, int c, size_t n)
+{
+ const char *next = memchr(s, c, n);
+ if (!next)
+ next = s + n;
+ return next;
+}
+
+static int match_host(const struct url_info *url_info,
+ const struct url_info *pattern_info)
+{
+ const char *url = url_info->url + url_info->host_off;
+ const char *pat = pattern_info->url + pattern_info->host_off;
+ int url_len = url_info->host_len;
+ int pat_len = pattern_info->host_len;
+
+ while (url_len && pat_len) {
+ const char *url_next = end_of_token(url, '.', url_len);
+ const char *pat_next = end_of_token(pat, '.', pat_len);
+
+ if (pat_next == pat + 1 && pat[0] == '*')
+ /* wildcard matches anything */
+ ;
+ else if ((pat_next - pat) == (url_next - url) &&
+ !memcmp(url, pat, url_next - url))
+ /* the components are the same */
+ ;
+ else
+ return 0; /* found an unmatch */
+
+ if (url_next < url + url_len)
+ url_next++;
+ url_len -= url_next - url;
+ url = url_next;
+ if (pat_next < pat + pat_len)
+ pat_next++;
+ pat_len -= pat_next - pat;
+ pat = pat_next;
+ }
+
+ return (!url_len && !pat_len);
+}
+
static char *url_normalize_1(const char *url, struct url_info *out_info, char allow_globs)
{
/*
@@ -467,9 +510,7 @@ static int match_urls(const struct url_info *url,
}
/* check the host */
- if (url_prefix->host_len != url->host_len ||
- strncmp(url->url + url->host_off,
- url_prefix->url + url_prefix->host_off, url->host_len))
+ if (!match_host(url, url_prefix))
return 0; /* host names do not match */
/* check the port */
@@ -528,7 +569,7 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)
struct url_info norm_info;
config_url = xmemdupz(key, dot - key);
- norm_url = url_normalize(config_url, &norm_info);
+ norm_url = url_normalize_1(config_url, &norm_info, 1);
free(config_url);
if (!norm_url)
return 0;
--
2.11.0
^ permalink raw reply related
* [PATCH v4 3/5] urlmatch: split host and port fields in `struct url_info`
From: Patrick Steinhardt @ 2017-01-27 10:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Patrick Steinhardt,
Philip Oakley
In-Reply-To: <cover.1485512626.git.patrick.steinhardt@elego.de>
The `url_info` structure contains information about a normalized URL
with the URL's components being represented by different fields. The
host and port part though are to be accessed by the same `host` field,
so that getting the host and/or port separately becomes more involved
than really necessary.
To make the port more readily accessible, split up the host and port
fields. Namely, the `host_len` will not include the port length anymore
and a new `port_off` field has been added which includes the offset to
the port, if available.
The only user of these fields is `url_normalize_1`. This change makes it
easier later on to treat host and port differently when introducing
globs for domains.
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
---
urlmatch.c | 16 ++++++++++++----
urlmatch.h | 9 +++++----
2 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/urlmatch.c b/urlmatch.c
index d350478c0..e328905eb 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -104,7 +104,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
struct strbuf norm;
size_t spanned;
size_t scheme_len, user_off=0, user_len=0, passwd_off=0, passwd_len=0;
- size_t host_off=0, host_len=0, port_len=0, path_off, path_len, result_len;
+ size_t host_off=0, host_len=0, port_off=0, port_len=0, path_off, path_len, result_len;
const char *slash_ptr, *at_ptr, *colon_ptr, *path_start;
char *result;
@@ -263,6 +263,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
return NULL;
}
strbuf_addch(&norm, ':');
+ port_off = norm.len;
strbuf_add(&norm, url, slash_ptr - url);
port_len = slash_ptr - url;
}
@@ -270,7 +271,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
url = slash_ptr;
}
if (host_off)
- host_len = norm.len - host_off;
+ host_len = norm.len - host_off - (port_len ? port_len + 1 : 0);
/*
@@ -378,6 +379,7 @@ static char *url_normalize_1(const char *url, struct url_info *out_info, char al
out_info->passwd_len = passwd_len;
out_info->host_off = host_off;
out_info->host_len = host_len;
+ out_info->port_off = port_off;
out_info->port_len = port_len;
out_info->path_off = path_off;
out_info->path_len = path_len;
@@ -464,11 +466,17 @@ static int match_urls(const struct url_info *url,
usermatched = 1;
}
- /* check the host and port */
+ /* check the host */
if (url_prefix->host_len != url->host_len ||
strncmp(url->url + url->host_off,
url_prefix->url + url_prefix->host_off, url->host_len))
- return 0; /* host names and/or ports do not match */
+ return 0; /* host names do not match */
+
+ /* check the port */
+ if (url_prefix->port_len != url->port_len ||
+ strncmp(url->url + url->port_off,
+ url_prefix->url + url_prefix->port_off, url->port_len))
+ return 0; /* ports do not match */
/* check the path */
pathmatchlen = url_match_prefix(
diff --git a/urlmatch.h b/urlmatch.h
index 528862adc..0ea812b03 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -18,11 +18,12 @@ struct url_info {
size_t passwd_len; /* length of passwd; if passwd_off != 0 but
passwd_len == 0, an empty passwd was given */
size_t host_off; /* offset into url to start of host name (0 => none) */
- size_t host_len; /* length of host name; this INCLUDES any ':portnum';
+ size_t host_len; /* length of host name;
* file urls may have host_len == 0 */
- size_t port_len; /* if a portnum is present (port_len != 0), it has
- * this length (excluding the leading ':') at the
- * end of the host name (always 0 for file urls) */
+ size_t port_off; /* offset into url to start of port number (0 => none) */
+ size_t port_len; /* if a portnum is present (port_off != 0), it has
+ * this length (excluding the leading ':') starting
+ * from port_off (always 0 for file urls) */
size_t path_off; /* offset into url to the start of the url path;
* this will always point to a '/' character
* after the url has been normalized */
--
2.11.0
^ permalink raw reply related
* [PATCH v4 4/5] urlmatch: include host and port in urlmatch length
From: Patrick Steinhardt @ 2017-01-27 10:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Patrick Steinhardt,
Philip Oakley
In-Reply-To: <cover.1485512626.git.patrick.steinhardt@elego.de>
In order to be able to rank positive matches by `urlmatch`, we inspect
the path length and user part to decide whether a match is better than
another match. As all other parts are matched exactly between both URLs,
this is the right thing to do right now.
In the future, though, we want to introduce wild cards for the domain
part. When doing this, though, it does not make sense anymore to only
compare the path lengths. Instead, we also want to compare the domain
lengths to determine which of both URLs matches the host part more
closely.
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
---
t/t1300-repo-config.sh | 33 ++++++++++++++++++++++++++++
urlmatch.c | 59 +++++++++++++++++++++++++++++---------------------
urlmatch.h | 3 ++-
3 files changed, 69 insertions(+), 26 deletions(-)
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 923bfc5a2..6c844d519 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1177,6 +1177,39 @@ test_expect_success 'urlmatch' '
test_cmp expect actual
'
+test_expect_success 'urlmatch favors more specific URLs' '
+ cat >.git/config <<-\EOF &&
+ [http "https://example.com/"]
+ cookieFile = /tmp/root.txt
+ [http "https://example.com/subdirectory"]
+ cookieFile = /tmp/subdirectory.txt
+ [http "https://user@example.com/"]
+ cookieFile = /tmp/user.txt
+ [http "https://averylonguser@example.com/"]
+ cookieFile = /tmp/averylonguser.txt
+ EOF
+
+ echo http.cookiefile /tmp/root.txt >expect &&
+ git config --get-urlmatch HTTP https://example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/subdirectory.txt >expect &&
+ git config --get-urlmatch HTTP https://example.com/subdirectory >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/subdirectory.txt >expect &&
+ git config --get-urlmatch HTTP https://example.com/subdirectory/nested >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/user.txt >expect &&
+ git config --get-urlmatch HTTP https://user@example.com/ >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/subdirectory.txt >expect &&
+ git config --get-urlmatch HTTP https://averylonguser@example.com/subdirectory >actual &&
+ test_cmp expect actual
+'
+
# good section hygiene
test_expect_failure 'unsetting the last key in a section removes header' '
cat >.git/config <<-\EOF &&
diff --git a/urlmatch.c b/urlmatch.c
index e328905eb..f35d00a6e 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -426,7 +426,7 @@ static size_t url_match_prefix(const char *url,
static int match_urls(const struct url_info *url,
const struct url_info *url_prefix,
- int *exactusermatch)
+ struct urlmatch_item *match)
{
/*
* url_prefix matches url if the scheme, host and port of url_prefix
@@ -445,8 +445,8 @@ static int match_urls(const struct url_info *url,
* contained a user name or false if url_prefix did not have a
* user name. If there is no match *exactusermatch is left untouched.
*/
- int usermatched = 0;
- int pathmatchlen;
+ char usermatched = 0;
+ size_t pathmatchlen;
if (!url || !url_prefix || !url->url || !url_prefix->url)
return 0;
@@ -483,22 +483,38 @@ static int match_urls(const struct url_info *url,
url->url + url->path_off,
url_prefix->url + url_prefix->path_off,
url_prefix->url_len - url_prefix->path_off);
+ if (!pathmatchlen)
+ return 0; /* paths do not match */
- if (pathmatchlen && exactusermatch)
- *exactusermatch = usermatched;
- return pathmatchlen;
+ if (match) {
+ match->hostmatch_len = url_prefix->host_len;
+ match->pathmatch_len = pathmatchlen;
+ match->user_matched = usermatched;
+ }
+
+ return 1;
+}
+
+static int cmp_matches(const struct urlmatch_item *a,
+ const struct urlmatch_item *b)
+{
+ if (a->hostmatch_len != b->hostmatch_len)
+ return a->hostmatch_len < b->hostmatch_len ? -1 : 1;
+ if (a->pathmatch_len != b->pathmatch_len)
+ return a->pathmatch_len < b->pathmatch_len ? -1 : 1;
+ if (a->user_matched != b->user_matched)
+ return b->user_matched ? -1 : 1;
+ return 0;
}
int urlmatch_config_entry(const char *var, const char *value, void *cb)
{
struct string_list_item *item;
struct urlmatch_config *collect = cb;
- struct urlmatch_item *matched;
+ struct urlmatch_item matched;
struct url_info *url = &collect->url;
const char *key, *dot;
struct strbuf synthkey = STRBUF_INIT;
- size_t matched_len = 0;
- int user_matched = 0;
int retval;
if (!skip_prefix(var, collect->section, &key) || *(key++) != '.') {
@@ -516,9 +532,9 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)
free(config_url);
if (!norm_url)
return 0;
- matched_len = match_urls(url, &norm_info, &user_matched);
+ retval = match_urls(url, &norm_info, &matched);
free(norm_url);
- if (!matched_len)
+ if (!retval)
return 0;
key = dot + 1;
}
@@ -528,24 +544,17 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)
item = string_list_insert(&collect->vars, key);
if (!item->util) {
- matched = xcalloc(1, sizeof(*matched));
- item->util = matched;
+ item->util = xcalloc(1, sizeof(matched));
} else {
- matched = item->util;
- /*
- * Is our match shorter? Is our match the same
- * length, and without user while the current
- * candidate is with user? Then we cannot use it.
- */
- if (matched_len < matched->matched_len ||
- ((matched_len == matched->matched_len) &&
- (!user_matched && matched->user_matched)))
+ if (cmp_matches(&matched, item->util) <= 0)
+ /*
+ * Our match is worse than the old one,
+ * we cannot use it.
+ */
return 0;
- /* Otherwise, replace it with this one. */
}
- matched->matched_len = matched_len;
- matched->user_matched = user_matched;
+ memcpy(item->util, &matched, sizeof(matched));
strbuf_addstr(&synthkey, collect->section);
strbuf_addch(&synthkey, '.');
strbuf_addstr(&synthkey, key);
diff --git a/urlmatch.h b/urlmatch.h
index 0ea812b03..37ee5da85 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -34,7 +34,8 @@ struct url_info {
extern char *url_normalize(const char *, struct url_info *);
struct urlmatch_item {
- size_t matched_len;
+ size_t hostmatch_len;
+ size_t pathmatch_len;
char user_matched;
};
--
2.11.0
^ permalink raw reply related
* [PATCH v4 0/5] urlmatch: allow wildcard-based matches
From: Patrick Steinhardt @ 2017-01-27 10:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Patrick Steinhardt,
Philip Oakley
In-Reply-To: <20170123130635.29577-1-patrick.steinhardt@elego.de>
Hi,
so this is part four of my patch series. The previous version can
be found at [1]. The use case is to be able to configure an HTTP
proxy for all subdomains of a domain where there are hundreds of
subdomains.
Changes to the previous version:
- applied Junio's proposed patch to replace `strtok_r` with a
`memchr`-based loop
- applied Junio's proposed rewrite of the commit message of
patch 5
- I realized that with my patches, "ranking" of URLs was broken.
Previously, we've always taken the longest matching URL. As
previously, only the user and path could actually differ, only
these two components were used for the comparison. I've
changed this now to also include the host part so that URLs
with a longer host will take precedence. This resulted in a
the patch 4.
- New tests are included which examine if the precedence-rules
are actually honored correctly. The tests are part of patches
4 and 5.
You can find the interdiff below.
Regards
Patrick
[1]: http://public-inbox.org/git/20170125095648.4116-1-patrick.steinhardt@elego.de/T/#t
Patrick Steinhardt (5):
mailmap: add Patrick Steinhardt's work address
urlmatch: enable normalization of URLs with globs
urlmatch: split host and port fields in `struct url_info`
urlmatch: include host and port in urlmatch length
urlmatch: allow globbing for the URL host part
.mailmap | 1 +
Documentation/config.txt | 5 +-
t/t1300-repo-config.sh | 105 ++++++++++++++++++++++++++++++++++++
urlmatch.c | 138 +++++++++++++++++++++++++++++++++++------------
urlmatch.h | 12 +++--
5 files changed, 220 insertions(+), 41 deletions(-)
--
2.11.0
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index ec545e092..052f12021 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1177,7 +1177,72 @@ test_expect_success 'urlmatch' '
test_cmp expect actual
'
-test_expect_success 'glob-based urlmatch' '
+test_expect_success 'urlmatch favors more specific URLs' '
+ cat >.git/config <<-\EOF &&
+ [http "https://example.com/"]
+ cookieFile = /tmp/root.txt
+ [http "https://example.com/subdirectory"]
+ cookieFile = /tmp/subdirectory.txt
+ [http "https://user@example.com/"]
+ cookieFile = /tmp/user.txt
+ [http "https://averylonguser@example.com/"]
+ cookieFile = /tmp/averylonguser.txt
+ [http "https://preceding.example.com"]
+ cookieFile = /tmp/preceding.txt
+ [http "https://*.example.com"]
+ cookieFile = /tmp/wildcard.txt
+ [http "https://*.example.com/wildcardwithsubdomain"]
+ cookieFile = /tmp/wildcardwithsubdomain.txt
+ [http "https://trailing.example.com"]
+ cookieFile = /tmp/trailing.txt
+ [http "https://user@*.example.com/"]
+ cookieFile = /tmp/wildcardwithuser.txt
+ [http "https://sub.example.com/"]
+ cookieFile = /tmp/sub.txt
+ EOF
+
+ echo http.cookiefile /tmp/root.txt >expect &&
+ git config --get-urlmatch HTTP https://example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/subdirectory.txt >expect &&
+ git config --get-urlmatch HTTP https://example.com/subdirectory >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/subdirectory.txt >expect &&
+ git config --get-urlmatch HTTP https://example.com/subdirectory/nested >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/user.txt >expect &&
+ git config --get-urlmatch HTTP https://user@example.com/ >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/subdirectory.txt >expect &&
+ git config --get-urlmatch HTTP https://averylonguser@example.com/subdirectory >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/preceding.txt >expect &&
+ git config --get-urlmatch HTTP https://preceding.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/wildcard.txt >expect &&
+ git config --get-urlmatch HTTP https://wildcard.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/sub.txt >expect &&
+ git config --get-urlmatch HTTP https://sub.example.com/wildcardwithsubdomain >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/trailing.txt >expect &&
+ git config --get-urlmatch HTTP https://trailing.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.cookiefile /tmp/sub.txt >expect &&
+ git config --get-urlmatch HTTP https://user@sub.example.com >actual &&
+ test_cmp expect actual
+'
+
+test_expect_success 'urlmatch with wildcard' '
cat >.git/config <<-\EOF &&
[http]
sslVerify
@@ -1210,6 +1275,10 @@ test_expect_success 'glob-based urlmatch' '
echo http.sslverify false
} >expect &&
git config --get-urlmatch HTTP https://good.example.com >actual &&
+ test_cmp expect actual &&
+
+ echo http.sslverify >expect &&
+ git config --get-urlmatch HTTP https://more.example.com.au >actual &&
test_cmp expect actual
'
diff --git a/urlmatch.c b/urlmatch.c
index 53ff972a6..bb5267732 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -63,36 +63,47 @@ static int append_normalized_escapes(struct strbuf *buf,
return 1;
}
+static const char *end_of_token(const char *s, int c, size_t n)
+{
+ const char *next = memchr(s, c, n);
+ if (!next)
+ next = s + n;
+ return next;
+}
+
static int match_host(const struct url_info *url_info,
const struct url_info *pattern_info)
{
- char *url = xmemdupz(url_info->url + url_info->host_off, url_info->host_len);
- char *pat = xmemdupz(pattern_info->url + pattern_info->host_off, pattern_info->host_len);
- char *url_tok, *pat_tok, *url_save, *pat_save;
- int matching;
+ const char *url = url_info->url + url_info->host_off;
+ const char *pat = pattern_info->url + pattern_info->host_off;
+ int url_len = url_info->host_len;
+ int pat_len = pattern_info->host_len;
- url_tok = strtok_r(url, ".", &url_save);
- pat_tok = strtok_r(pat, ".", &pat_save);
+ while (url_len && pat_len) {
+ const char *url_next = end_of_token(url, '.', url_len);
+ const char *pat_next = end_of_token(pat, '.', pat_len);
- for (; url_tok && pat_tok; url_tok = strtok_r(NULL, ".", &url_save),
- pat_tok = strtok_r(NULL, ".", &pat_save)) {
- if (!strcmp(pat_tok, "*"))
- continue; /* a simple glob matches everything */
+ if (pat_next == pat + 1 && pat[0] == '*')
+ /* wildcard matches anything */
+ ;
+ else if ((pat_next - pat) == (url_next - url) &&
+ !memcmp(url, pat, url_next - url))
+ /* the components are the same */
+ ;
+ else
+ return 0; /* found an unmatch */
- if (strcmp(url_tok, pat_tok)) {
- /* subdomains do not match */
- matching = 0;
- break;
- }
+ if (url_next < url + url_len)
+ url_next++;
+ url_len -= url_next - url;
+ url = url_next;
+ if (pat_next < pat + pat_len)
+ pat_next++;
+ pat_len -= pat_next - pat;
+ pat = pat_next;
}
- /* matching if both URL and pattern are at their ends */
- matching = (url_tok == NULL && pat_tok == NULL);
-
- free(url);
- free(pat);
-
- return matching;
+ return (!url_len && !pat_len);
}
static char *url_normalize_1(const char *url, struct url_info *out_info, char allow_globs)
@@ -458,7 +469,7 @@ static size_t url_match_prefix(const char *url,
static int match_urls(const struct url_info *url,
const struct url_info *url_prefix,
- int *exactusermatch)
+ struct urlmatch_item *match)
{
/*
* url_prefix matches url if the scheme, host and port of url_prefix
@@ -477,8 +488,8 @@ static int match_urls(const struct url_info *url,
* contained a user name or false if url_prefix did not have a
* user name. If there is no match *exactusermatch is left untouched.
*/
- int usermatched = 0;
- int pathmatchlen;
+ char usermatched = 0;
+ size_t pathmatchlen;
if (!url || !url_prefix || !url->url || !url_prefix->url)
return 0;
@@ -513,22 +524,38 @@ static int match_urls(const struct url_info *url,
url->url + url->path_off,
url_prefix->url + url_prefix->path_off,
url_prefix->url_len - url_prefix->path_off);
+ if (!pathmatchlen)
+ return 0; /* paths do not match */
- if (pathmatchlen && exactusermatch)
- *exactusermatch = usermatched;
- return pathmatchlen;
+ if (match) {
+ match->hostmatch_len = url_prefix->host_len;
+ match->pathmatch_len = pathmatchlen;
+ match->user_matched = usermatched;
+ }
+
+ return 1;
+}
+
+static int cmp_matches(const struct urlmatch_item *a,
+ const struct urlmatch_item *b)
+{
+ if (a->hostmatch_len != b->hostmatch_len)
+ return a->hostmatch_len < b->hostmatch_len ? -1 : 1;
+ if (a->pathmatch_len != b->pathmatch_len)
+ return a->pathmatch_len < b->pathmatch_len ? -1 : 1;
+ if (a->user_matched != b->user_matched)
+ return b->user_matched ? -1 : 1;
+ return 0;
}
int urlmatch_config_entry(const char *var, const char *value, void *cb)
{
struct string_list_item *item;
struct urlmatch_config *collect = cb;
- struct urlmatch_item *matched;
+ struct urlmatch_item matched;
struct url_info *url = &collect->url;
const char *key, *dot;
struct strbuf synthkey = STRBUF_INIT;
- size_t matched_len = 0;
- int user_matched = 0;
int retval;
if (!skip_prefix(var, collect->section, &key) || *(key++) != '.') {
@@ -546,9 +573,9 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)
free(config_url);
if (!norm_url)
return 0;
- matched_len = match_urls(url, &norm_info, &user_matched);
+ retval = match_urls(url, &norm_info, &matched);
free(norm_url);
- if (!matched_len)
+ if (!retval)
return 0;
key = dot + 1;
}
@@ -558,24 +585,17 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)
item = string_list_insert(&collect->vars, key);
if (!item->util) {
- matched = xcalloc(1, sizeof(*matched));
- item->util = matched;
+ item->util = xcalloc(1, sizeof(matched));
} else {
- matched = item->util;
- /*
- * Is our match shorter? Is our match the same
- * length, and without user while the current
- * candidate is with user? Then we cannot use it.
- */
- if (matched_len < matched->matched_len ||
- ((matched_len == matched->matched_len) &&
- (!user_matched && matched->user_matched)))
+ if (cmp_matches(&matched, item->util) <= 0)
+ /*
+ * Our match is worse than the old one,
+ * we cannot use it.
+ */
return 0;
- /* Otherwise, replace it with this one. */
}
- matched->matched_len = matched_len;
- matched->user_matched = user_matched;
+ memcpy(item->util, &matched, sizeof(matched));
strbuf_addstr(&synthkey, collect->section);
strbuf_addch(&synthkey, '.');
strbuf_addstr(&synthkey, key);
diff --git a/urlmatch.h b/urlmatch.h
index 0ea812b03..37ee5da85 100644
--- a/urlmatch.h
+++ b/urlmatch.h
@@ -34,7 +34,8 @@ struct url_info {
extern char *url_normalize(const char *, struct url_info *);
struct urlmatch_item {
- size_t matched_len;
+ size_t hostmatch_len;
+ size_t pathmatch_len;
char user_matched;
};
^ permalink raw reply related
* [PATCH v4 2/5] urlmatch: enable normalization of URLs with globs
From: Patrick Steinhardt @ 2017-01-27 10:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Patrick Steinhardt,
Philip Oakley
In-Reply-To: <cover.1485512626.git.patrick.steinhardt@elego.de>
The `url_normalize` function is used to validate and normalize URLs. As
such, it does not allow for some special characters to be part of the
URLs that are to be normalized. As we want to allow using globs in some
configuration keys making use of URLs, namely `http.<url>.<key>`, but
still normalize them, we need to somehow enable some additional allowed
characters.
To do this without having to change all callers of `url_normalize`,
where most do not actually want globbing at all, we split off another
function `url_normalize_1`. This function accepts an additional
parameter `allow_globs`, which is subsequently called by `url_normalize`
with `allow_globs=0`.
As of now, this function is not used with globbing enabled. A caller
will be added in the following commit.
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
---
urlmatch.c | 14 ++++++++++++--
1 file changed, 12 insertions(+), 2 deletions(-)
diff --git a/urlmatch.c b/urlmatch.c
index 132d342bc..d350478c0 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -63,7 +63,7 @@ static int append_normalized_escapes(struct strbuf *buf,
return 1;
}
-char *url_normalize(const char *url, struct url_info *out_info)
+static char *url_normalize_1(const char *url, struct url_info *out_info, char allow_globs)
{
/*
* Normalize NUL-terminated url using the following rules:
@@ -191,7 +191,12 @@ char *url_normalize(const char *url, struct url_info *out_info)
strbuf_release(&norm);
return NULL;
}
- spanned = strspn(url, URL_HOST_CHARS);
+
+ if (allow_globs)
+ spanned = strspn(url, URL_HOST_CHARS "*");
+ else
+ spanned = strspn(url, URL_HOST_CHARS);
+
if (spanned < colon_ptr - url) {
/* Host name has invalid characters */
if (out_info) {
@@ -380,6 +385,11 @@ char *url_normalize(const char *url, struct url_info *out_info)
return result;
}
+char *url_normalize(const char *url, struct url_info *out_info)
+{
+ return url_normalize_1(url, out_info, 0);
+}
+
static size_t url_match_prefix(const char *url,
const char *url_prefix,
size_t url_prefix_len)
--
2.11.0
^ permalink raw reply related
* [PATCH v4 1/5] mailmap: add Patrick Steinhardt's work address
From: Patrick Steinhardt @ 2017-01-27 10:32 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Junio C Hamano, Patrick Steinhardt,
Philip Oakley
In-Reply-To: <cover.1485512626.git.patrick.steinhardt@elego.de>
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
---
.mailmap | 1 +
1 file changed, 1 insertion(+)
diff --git a/.mailmap b/.mailmap
index 9c87a3840..ea59205b9 100644
--- a/.mailmap
+++ b/.mailmap
@@ -177,6 +177,7 @@ Paolo Bonzini <bonzini@gnu.org> <paolo.bonzini@lu.unisi.ch>
Pascal Obry <pascal@obry.net> <pascal.obry@gmail.com>
Pascal Obry <pascal@obry.net> <pascal.obry@wanadoo.fr>
Pat Notz <patnotz@gmail.com> <pknotz@sandia.gov>
+Patrick Steinhardt <ps@pks.im> <patrick.steinhardt@elego.de>
Paul Mackerras <paulus@samba.org> <paulus@dorrigo.(none)>
Paul Mackerras <paulus@samba.org> <paulus@pogo.(none)>
Peter Baumann <waste.manager@gmx.de> <Peter.B.Baumann@stud.informatik.uni-erlangen.de>
--
2.11.0
^ permalink raw reply related
* Re: [PATCH] mingw: allow hooks to be .exe files
From: Johannes Schindelin @ 2017-01-27 10:29 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
In-Reply-To: <xmqq4m0l64pg.fsf@gitster.mtv.corp.google.com>
Hi Junio,
On Thu, 26 Jan 2017, Junio C Hamano wrote:
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
> > On Wed, 25 Jan 2017, Jeff King wrote:
> >
> >> On Wed, Jan 25, 2017 at 05:58:42PM +0100, Johannes Schindelin wrote:
> >>
> >> > - if (access(path.buf, X_OK) < 0)
> >> > + if (access(path.buf, X_OK) < 0) {
> >> > +#ifdef STRIP_EXTENSION
> >> > + strbuf_addstr(&path, ".exe");
> >>
> >> I think STRIP_EXTENSION is a string. Should this line be:
> >>
> >> strbuf_addstr(&path, STRIP_EXTENSION);
> >
> > Yep.
> >
> > v2 coming,
> > Johannes
>
> I think I've already tweaked it out when I queued the original one.
After digging, I found your SQUASH commit. I had not known about that.
In any case, I much rather prefer to have the final version of any patch
or patch series I contribute to be identical between what you commit and
what I sent to the mailing list. We do disagree from time to time, and I
would like to have the opportunity of reviewing how you tweak my changes.
Ciao,
Johannes
^ permalink raw reply
* [PATCH v3 2/3] refs: add option core.logAllRefUpdates = always
From: cornelius.weig @ 2017-01-27 10:09 UTC (permalink / raw)
To: gitster; +Cc: peff, git, Cornelius Weig
In-Reply-To: <20170127100948.29408-1-cornelius.weig@tngtech.com>
From: Cornelius Weig <cornelius.weig@tngtech.com>
When core.logallrefupdates is true, we only create a new reflog for refs
that are under certain well-known hierarchies. The reason is that we
know that some hierarchies (like refs/tags) are not meant to change, and
that unknown hierarchies might not want reflogs at all (e.g., a
hypothetical refs/foo might be meant to change often and drop old
history immediately).
However, sometimes it is useful to override this decision and simply log
for all refs, because the safety and audit trail is more important than
the performance implications of keeping the log around.
This patch introduces a new "always" mode for the core.logallrefupdates
option which will log updates to everything under refs/, regardless
where in the hierarchy it is (we still will not log things like
ORIG_HEAD and FETCH_HEAD, which are known to be transient).
Based-on-patch-by: Jeff King <peff@peff.net>
Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
Reviewed-by: Jeff King <peff@peff.net>
---
Notes:
Changes wrt v2:
- change wording in commit message s/do not typically/are not meant to/;
- in update_refs_for_switch move refname to the enclosing block, so that
should_autocreate_reflog has access. Thanks Junio for spotting this
potential bug early :)
- add test that asserts reflogs are created for tags if
logAllRefUpdates=always. The case with logAllRefUpdates=true is IMHO already
covered by the default case. To make that clearer, I explicitly added
logAllRefUpdates=true.
When writing the test for git-tag, I realized that the option
--no-create-reflog to git-tag does not take precedence over
logAllRefUpdate=always. IOW the setting cannot be overridden on the command
line. Do you think this is a defect or would it not be desirable to have this
feature anyway?
Documentation/config.txt | 2 ++
Documentation/git-tag.txt | 3 ++-
branch.c | 2 +-
builtin/checkout.c | 7 +++----
builtin/init-db.c | 2 +-
cache.h | 9 ++++++++-
config.c | 7 ++++++-
environment.c | 2 +-
refs.c | 15 ++++++++++-----
refs.h | 2 ++
refs/files-backend.c | 6 +++---
refs/refs-internal.h | 2 --
t/t1400-update-ref.sh | 37 +++++++++++++++++++++++++++++++++++++
t/t7004-tag.sh | 8 ++++++++
14 files changed, 84 insertions(+), 20 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index c7d8a01..d1fab67 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -521,6 +521,8 @@ core.logAllRefUpdates::
file is automatically created for branch heads (i.e. under
`refs/heads/`), remote refs (i.e. under `refs/remotes/`),
note refs (i.e. under `refs/notes/`), and the symbolic ref `HEAD`.
+ If it is set to `always`, then a missing reflog is automatically
+ created for any ref under `refs/`.
+
This information can be used to determine what commit
was the tip of a branch "2 days ago".
diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt
index 5055a96..2ac25a9 100644
--- a/Documentation/git-tag.txt
+++ b/Documentation/git-tag.txt
@@ -150,7 +150,8 @@ This option is only applicable when listing tags without annotation lines.
'strip' removes both whitespace and commentary.
--create-reflog::
- Create a reflog for the tag.
+ Create a reflog for the tag. To globally enable reflogs for tags, see
+ `core.logAllRefUpdates` in linkgit:git-config[1].
<tagname>::
The name of the tag to create, delete, or describe.
diff --git a/branch.c b/branch.c
index c431cbf..b955d4f 100644
--- a/branch.c
+++ b/branch.c
@@ -298,7 +298,7 @@ void create_branch(const char *name, const char *start_name,
start_name);
if (reflog)
- log_all_ref_updates = 1;
+ log_all_ref_updates = LOG_REFS_NORMAL;
if (!dont_change_ref) {
struct ref_transaction *transaction;
diff --git a/builtin/checkout.c b/builtin/checkout.c
index bfe685c..81ea2ed 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -612,14 +612,12 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
const char *old_desc, *reflog_msg;
if (opts->new_branch) {
if (opts->new_orphan_branch) {
- if (opts->new_branch_log && !log_all_ref_updates) {
+ const char *refname = mkpathdup("refs/heads/%s", opts->new_orphan_branch);
+ if (opts->new_branch_log && should_autocreate_reflog(refname)) {
int ret;
- char *refname;
struct strbuf err = STRBUF_INIT;
- refname = mkpathdup("refs/heads/%s", opts->new_orphan_branch);
ret = safe_create_reflog(refname, 1, &err);
- free(refname);
if (ret) {
fprintf(stderr, _("Can not do reflog for '%s': %s\n"),
opts->new_orphan_branch, err.buf);
@@ -628,6 +626,7 @@ static void update_refs_for_switch(const struct checkout_opts *opts,
}
strbuf_release(&err);
}
+ free(refname);
}
else
create_branch(opts->new_branch, new->name,
diff --git a/builtin/init-db.c b/builtin/init-db.c
index 76d68fa..1d4d6a0 100644
--- a/builtin/init-db.c
+++ b/builtin/init-db.c
@@ -262,7 +262,7 @@ static int create_default_files(const char *template_path,
const char *work_tree = get_git_work_tree();
git_config_set("core.bare", "false");
/* allow template config file to override the default */
- if (log_all_ref_updates == -1)
+ if (log_all_ref_updates == LOG_REFS_UNSET)
git_config_set("core.logallrefupdates", "true");
if (needs_work_tree_config(original_git_dir, work_tree))
git_config_set("core.worktree", work_tree);
diff --git a/cache.h b/cache.h
index 00a029a..96eeaaf 100644
--- a/cache.h
+++ b/cache.h
@@ -660,7 +660,6 @@ extern int minimum_abbrev, default_abbrev;
extern int ignore_case;
extern int assume_unchanged;
extern int prefer_symlink_refs;
-extern int log_all_ref_updates;
extern int warn_ambiguous_refs;
extern int warn_on_object_refname_ambiguity;
extern const char *apply_default_whitespace;
@@ -728,6 +727,14 @@ enum hide_dotfiles_type {
};
extern enum hide_dotfiles_type hide_dotfiles;
+enum log_refs_config {
+ LOG_REFS_UNSET = -1,
+ LOG_REFS_NONE = 0,
+ LOG_REFS_NORMAL,
+ LOG_REFS_ALWAYS
+};
+extern enum log_refs_config log_all_ref_updates;
+
enum branch_track {
BRANCH_TRACK_UNSPECIFIED = -1,
BRANCH_TRACK_NEVER = 0,
diff --git a/config.c b/config.c
index b680f79..c6b874a 100644
--- a/config.c
+++ b/config.c
@@ -826,7 +826,12 @@ static int git_default_core_config(const char *var, const char *value)
}
if (!strcmp(var, "core.logallrefupdates")) {
- log_all_ref_updates = git_config_bool(var, value);
+ if (value && !strcasecmp(value, "always"))
+ log_all_ref_updates = LOG_REFS_ALWAYS;
+ else if (git_config_bool(var, value))
+ log_all_ref_updates = LOG_REFS_NORMAL;
+ else
+ log_all_ref_updates = LOG_REFS_NONE;
return 0;
}
diff --git a/environment.c b/environment.c
index 8a83101..c07fb17 100644
--- a/environment.c
+++ b/environment.c
@@ -21,7 +21,6 @@ int ignore_case;
int assume_unchanged;
int prefer_symlink_refs;
int is_bare_repository_cfg = -1; /* unspecified */
-int log_all_ref_updates = -1; /* unspecified */
int warn_ambiguous_refs = 1;
int warn_on_object_refname_ambiguity = 1;
int ref_paranoia = -1;
@@ -64,6 +63,7 @@ int merge_log_config = -1;
int precomposed_unicode = -1; /* see probe_utf8_pathname_composition() */
unsigned long pack_size_limit_cfg;
enum hide_dotfiles_type hide_dotfiles = HIDE_DOTFILES_DOTGITONLY;
+enum log_refs_config log_all_ref_updates = LOG_REFS_UNSET;
#ifndef PROTECT_HFS_DEFAULT
#define PROTECT_HFS_DEFAULT 0
diff --git a/refs.c b/refs.c
index 9bd0bc1..cd36b64 100644
--- a/refs.c
+++ b/refs.c
@@ -638,12 +638,17 @@ int copy_reflog_msg(char *buf, const char *msg)
int should_autocreate_reflog(const char *refname)
{
- if (!log_all_ref_updates)
+ switch (log_all_ref_updates) {
+ case LOG_REFS_ALWAYS:
+ return 1;
+ case LOG_REFS_NORMAL:
+ return starts_with(refname, "refs/heads/") ||
+ starts_with(refname, "refs/remotes/") ||
+ starts_with(refname, "refs/notes/") ||
+ !strcmp(refname, "HEAD");
+ default:
return 0;
- return starts_with(refname, "refs/heads/") ||
- starts_with(refname, "refs/remotes/") ||
- starts_with(refname, "refs/notes/") ||
- !strcmp(refname, "HEAD");
+ }
}
int is_branch(const char *refname)
diff --git a/refs.h b/refs.h
index 6947843..9fbff90 100644
--- a/refs.h
+++ b/refs.h
@@ -64,6 +64,8 @@ int read_ref(const char *refname, unsigned char *sha1);
int ref_exists(const char *refname);
+int should_autocreate_reflog(const char *refname);
+
int is_branch(const char *refname);
extern int refs_init_db(struct strbuf *err);
diff --git a/refs/files-backend.c b/refs/files-backend.c
index f902393..14b17a6 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2682,7 +2682,7 @@ static int files_rename_ref(struct ref_store *ref_store,
}
flag = log_all_ref_updates;
- log_all_ref_updates = 0;
+ log_all_ref_updates = LOG_REFS_NONE;
if (write_ref_to_lockfile(lock, orig_sha1, &err) ||
commit_ref_update(refs, lock, orig_sha1, NULL, &err)) {
error("unable to write current sha1 into %s: %s", oldrefname, err.buf);
@@ -2835,8 +2835,8 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1,
{
int logfd, result, oflags = O_APPEND | O_WRONLY;
- if (log_all_ref_updates < 0)
- log_all_ref_updates = !is_bare_repository();
+ if (log_all_ref_updates == LOG_REFS_UNSET)
+ log_all_ref_updates = is_bare_repository() ? LOG_REFS_NONE : LOG_REFS_NORMAL;
result = log_ref_setup(refname, logfile, err, flags & REF_FORCE_CREATE_REFLOG);
diff --git a/refs/refs-internal.h b/refs/refs-internal.h
index 708b260..25444cf 100644
--- a/refs/refs-internal.h
+++ b/refs/refs-internal.h
@@ -133,8 +133,6 @@ int verify_refname_available(const char *newname,
*/
int copy_reflog_msg(char *buf, const char *msg);
-int should_autocreate_reflog(const char *refname);
-
/**
* Information needed for a single ref update. Set new_sha1 to the new
* value or to null_sha1 to delete the ref. To check the old value
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index d4fb977..b9084ca 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -93,6 +93,42 @@ test_expect_success 'update-ref creates reflogs with --create-reflog' '
git reflog exists $outside
'
+test_expect_success 'core.logAllRefUpdates=true does not create reflog by default' '
+ test_config core.logAllRefUpdates true &&
+ test_when_finished "git update-ref -d $outside" &&
+ git update-ref $outside $A &&
+ git rev-parse $A >expect &&
+ git rev-parse $outside >actual &&
+ test_cmp expect actual &&
+ test_must_fail git reflog exists $outside
+'
+
+test_expect_success 'core.logAllRefUpdates=always creates reflog by default' '
+ test_config core.logAllRefUpdates always &&
+ test_when_finished "git update-ref -d $outside" &&
+ git update-ref $outside $A &&
+ git rev-parse $A >expect &&
+ git rev-parse $outside >actual &&
+ test_cmp expect actual &&
+ git reflog exists $outside
+'
+
+test_expect_success 'core.logAllRefUpdates=always creates no reflog for ORIG_HEAD' '
+ test_config core.logAllRefUpdates always &&
+ git update-ref ORIG_HEAD $A &&
+ test_must_fail git reflog exists ORIG_HEAD
+'
+
+test_expect_success '--no-create-reflog overrides core.logAllRefUpdates=always' '
+ test_config core.logAllRefUpdates true &&
+ test_when_finished "git update-ref -d $outside" &&
+ git update-ref --no-create-reflog $outside $A &&
+ git rev-parse $A >expect &&
+ git rev-parse $outside >actual &&
+ test_cmp expect actual &&
+ test_must_fail git reflog exists $outside
+'
+
test_expect_success \
"create $m (by HEAD)" \
"git update-ref HEAD $A &&
@@ -501,6 +537,7 @@ test_expect_success 'stdin does not create reflogs by default' '
'
test_expect_success 'stdin creates reflogs with --create-reflog' '
+ test_when_finished "git update-ref -d $outside" &&
echo "create $outside $m" >stdin &&
git update-ref --create-reflog --stdin <stdin &&
git rev-parse $m >expect &&
diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
index 1cfa8a2..1bf622d 100755
--- a/t/t7004-tag.sh
+++ b/t/t7004-tag.sh
@@ -71,6 +71,7 @@ test_expect_success 'creating a tag for an unknown revision should fail' '
# commit used in the tests, test_tick is also called here to freeze the date:
test_expect_success 'creating a tag using default HEAD should succeed' '
+ test_config core.logAllRefUpdates true &&
test_tick &&
echo foo >foo &&
git add foo &&
@@ -90,6 +91,13 @@ test_expect_success '--create-reflog does not create reflog on failure' '
test_must_fail git reflog exists refs/tags/mytag
'
+test_expect_success 'option core.logAllRefUpdates=always creates reflog' '
+ test_when_finished "git tag -d tag_with_reflog" &&
+ test_config core.logAllRefUpdates always &&
+ git tag tag_with_reflog &&
+ git reflog exists refs/tags/tag_with_reflog
+'
+
test_expect_success 'listing all tags if one exists should succeed' '
git tag -l &&
git tag
--
2.10.2
^ permalink raw reply related
* [PATCH v3 1/3] config: add markup to core.logAllRefUpdates doc
From: cornelius.weig @ 2017-01-27 10:09 UTC (permalink / raw)
To: gitster; +Cc: peff, git, Cornelius Weig
In-Reply-To: <xmqqvat11k1i.fsf@gitster.mtv.corp.google.com>
From: Cornelius Weig <cornelius.weig@tngtech.com>
Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
---
Notes:
Changes wrt v2:
Remove duplicated line.
Documentation/config.txt | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index af2ae4c..c7d8a01 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -517,10 +517,10 @@ core.logAllRefUpdates::
"`$GIT_DIR/logs/<ref>`", by appending the new and old
SHA-1, the date/time and the reason of the update, but
only when the file exists. If this configuration
- variable is set to true, missing "`$GIT_DIR/logs/<ref>`"
+ variable is set to `true`, missing "`$GIT_DIR/logs/<ref>`"
file is automatically created for branch heads (i.e. under
- refs/heads/), remote refs (i.e. under refs/remotes/),
- note refs (i.e. under refs/notes/), and the symbolic ref HEAD.
+ `refs/heads/`), remote refs (i.e. under `refs/remotes/`),
+ note refs (i.e. under `refs/notes/`), and the symbolic ref `HEAD`.
+
This information can be used to determine what commit
was the tip of a branch "2 days ago".
--
2.10.2
^ permalink raw reply related
* [PATCH v3 3/3] update-ref: add test cases for bare repository
From: cornelius.weig @ 2017-01-27 10:09 UTC (permalink / raw)
To: gitster; +Cc: peff, git, Cornelius Weig
In-Reply-To: <20170127100948.29408-1-cornelius.weig@tngtech.com>
From: Cornelius Weig <cornelius.weig@tngtech.com>
The default behavior of update-ref to create reflogs differs in
repositories with worktree and bare ones. The existing tests cover only
the behavior of repositories with worktree.
This commit adds tests that assert the correct behavior in bare
repositories for update-ref. Two cases are covered:
- If core.logAllRefUpdates is not set, no reflogs should be created
- If core.logAllRefUpdates is true, reflogs should be created
Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
---
Notes:
Changes wrt v2:
Remove bashism 'local' from test function
t/t1400-update-ref.sh | 43 ++++++++++++++++++++++++++++++++++++-------
1 file changed, 36 insertions(+), 7 deletions(-)
diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
index b9084ca..b0ffc0b 100755
--- a/t/t1400-update-ref.sh
+++ b/t/t1400-update-ref.sh
@@ -8,23 +8,33 @@ test_description='Test git update-ref and basic ref logging'
Z=$_z40
-test_expect_success setup '
+m=refs/heads/master
+n_dir=refs/heads/gu
+n=$n_dir/fixes
+outside=refs/foo
+bare=bare-repo
+create_test_commits ()
+{
+ prfx="$1"
for name in A B C D E F
do
test_tick &&
T=$(git write-tree) &&
sha1=$(echo $name | git commit-tree $T) &&
- eval $name=$sha1
+ eval $prfx$name=$sha1
done
+}
+test_expect_success setup '
+ create_test_commits "" &&
+ mkdir $bare &&
+ cd $bare &&
+ git init --bare &&
+ create_test_commits "bare" &&
+ cd -
'
-m=refs/heads/master
-n_dir=refs/heads/gu
-n=$n_dir/fixes
-outside=refs/foo
-
test_expect_success \
"create $m" \
"git update-ref $m $A &&
@@ -93,6 +103,25 @@ test_expect_success 'update-ref creates reflogs with --create-reflog' '
git reflog exists $outside
'
+test_expect_success 'creates no reflog in bare repository' '
+ git -C $bare update-ref $m $bareA &&
+ git -C $bare rev-parse $bareA >expect &&
+ git -C $bare rev-parse $m >actual &&
+ test_cmp expect actual &&
+ test_must_fail git -C $bare reflog exists $m
+'
+
+test_expect_success 'core.logAllRefUpdates=true creates reflog in bare repository' '
+ test_when_finished "git -C $bare config --unset core.logAllRefUpdates && \
+ rm $bare/logs/$m" &&
+ git -C $bare config core.logAllRefUpdates true &&
+ git -C $bare update-ref $m $bareB &&
+ git -C $bare rev-parse $bareB >expect &&
+ git -C $bare rev-parse $m >actual &&
+ test_cmp expect actual &&
+ git -C $bare reflog exists $m
+'
+
test_expect_success 'core.logAllRefUpdates=true does not create reflog by default' '
test_config core.logAllRefUpdates true &&
test_when_finished "git update-ref -d $outside" &&
--
2.10.2
^ permalink raw reply related
* Re: [PATCH 2/2] use absolute_pathdup()
From: Johannes Schindelin @ 2017-01-27 10:21 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Junio C Hamano
In-Reply-To: <d15fdbb9-2a21-eeab-1fee-4a1553bd3bcb@web.de>
[-- Attachment #1: Type: text/plain, Size: 166 bytes --]
Hi René,
On Thu, 26 Jan 2017, René Scharfe wrote:
> Apply the symantic patch for converting callers that duplicate the
s/symantic/semantic/
Ciao,
Dscho
^ permalink raw reply
* Re: git clone problem
From: Jordi Durban @ 2017-01-27 9:18 UTC (permalink / raw)
To: Santiago Torres; +Cc: git
In-Reply-To: <20170125175840.hy7d2f775dxnafpo@LykOS.localdomain>
Hi Santiago!
Thank you for your answer.
What I meant was that the "WHATEVER" directory contained the same files
as the current directory (i.e the directory where I typed "git clone").
Thus, no files from the remote repository were cloned. It seemed really
weird. However I was playing around with git and finally I was able to
clone remote files in a "test" location as you suggested.
Thank you very much.
El 25/01/17 a les 18:58, Santiago Torres ha escrit:
> Hello, Jordi.
>
> Hmm, it should've cloned in the "whatever" directory.
>
> Can you post your git version/configs and maybe the output verbatim of
> the command when you run it?
>
> If you can reproduce in an empty dictionary that'd be better
>
> $ mkdir test && cd test
>
> $ git clone --recursive https://github.com/...
>
> $ ls
>
> Thanks,
> -Santiago
>
> On Wed, Jan 25, 2017 at 05:58:58PM +0100, Jordi Durban wrote:
>> Hi all! Not sure if that will reach the goal, but let's it a try.
>>
>> I have a problem with the git clone command: when I try to clone a remote
>> repository with the following:
>>
>> git clone --recursive https://github.com/whatever.git
>>
>> what I actually obtain is a copy of my own files in the current directory.
>>
>> I mean:
>>
>> In the current directory:
>>
>> $ls
>>
>> -rwxr-xr-x 1 1,6K oct 24 17:29 get_fasta.pl
>> -rwxr-xr-x 1 1,6K set 5 13:05 script_clus_miRNA_c95.pl
>>
>> $git clone --recursive https://github.com/whatever.git WHATEVER
>>
>> $ls
>>
>> -rwxr-xr-x 1 1,6K oct 24 17:29 get_fasta.pl
>> -rwxr-xr-x 1 1,6K set 5 13:05 script_clus_miRNA_c95.pl
>>
>> -rwxr-xr-x 1 1,6K set 5 13:05 WHATEVER
>>
>> $ls WHATEVER
>>
>> -rwxr-xr-x 1 1,6K oct 24 17:29 get_fasta.pl
>> -rwxr-xr-x 1 1,6K set 5 13:05 script_clus_miRNA_c95.pl
>>
>> I am really confused with that.
>>
>> Any help will be appreciated. Thank you
>>
^ permalink raw reply
* Re: Intermittent failure of t1700-split-index.sh
From: Christian Couder @ 2017-01-27 6:38 UTC (permalink / raw)
To: Jeff King; +Cc: Ramsay Jones, Junio C Hamano, GIT Mailing-list
In-Reply-To: <20170127035806.rtvlas7dja5ikvxg@sigill.intra.peff.net>
On Fri, Jan 27, 2017 at 4:58 AM, Jeff King <peff@peff.net> wrote:
> On Fri, Jan 27, 2017 at 02:45:15AM +0000, Ramsay Jones wrote:
>
>> I can't devote any time to looking at this further tonight
>> (it's 2-45am here, I'm off to bed!). Can you reproduce the
>> problem, or is it just me? :)
>
> I can reproduce here with 'pu'. t1700.17 seems to fail reliably with my
> stress script after 5-10 seconds.
>
> It bisects to ff97d10f57c2f99b6d86da8f07c16659979b2780, but take that
> with a moderate grain of salt, as I may have marked a bad commit as
> "good" if it simply got lucky and didn't fail as quickly.
Thanks both for your tests and your report. I will take a look.
^ permalink raw reply
* Re: [PATCH v3 4/4] urlmatch: allow globbing for the URL host part
From: Patrick Steinhardt @ 2017-01-27 6:21 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Patrick Steinhardt, Philip Oakley
In-Reply-To: <20170125095648.4116-5-patrick.steinhardt@elego.de>
[-- Attachment #1: Type: text/plain, Size: 3017 bytes --]
On Thu, Jan 26, 2017 at 12:43:31PM -0800, Junio C Hamano wrote:
> Patrick Steinhardt <patrick.steinhardt@elego.de> writes:
>
> > The URL matching function computes for two URLs whether they match not.
> > The match is performed by splitting up the URL into different parts and
> > then doing an exact comparison with the to-be-matched URL.
> >
> > The main user of `urlmatch` is the configuration subsystem. It allows to
> > set certain configurations based on the URL which is being connected to
> > via keys like `http.<url>.*`. A common use case for this is to set
> > proxies for only some remotes which match the given URL. Unfortunately,
> > having exact matches for all parts of the URL can become quite tedious
> > in some setups. Imagine for example a corporate network where there are
> > dozens or even hundreds of subdomains, which would have to be configured
> > individually.
> >
> > This commit introduces the ability to use globbing in the host-part of
> > the URLs. A user can simply specify a `*` as part of the host name to
> > match all subdomains at this level. For example adding a configuration
> > key `http.https://*.example.com.proxy` will match all subdomains of
> > `https://example.com`.
>
> This is probably a useful improvement.
>
> Having said that, when I mentioned "glob", I meant to also support
> something like this:
>
> https://www[1-4].ibm.com/
The problem with additional extended syntax like proposed by you
is that we would indeed need an escaping mechanism here. '[]' are
already allowed inside the host part to enable IPv6 hosts of the
form 'https://[2001:0db8:]/', so the syntax is now ambiguous. So
we have to be cautios which characters to enable for globbing
syntax. As of now, I think we can only safely include '*' and '?'
here without escaping mechanisms.
If additional use cases come up we might still extend the syntax
later on to allow for more special syntax.
> And when people read "glob", that is what they expect.
>
> So calling this "the ability to use globbing" is misleading.
> The last paragraph in the log message above needs a bit of
> tweaking, perhaps like this:
>
> Allow users to write an asterisk '*' in place of any 'host'
> or 'subdomain' label as part of the host name. For example,
> "http.https://*.example.com.proxy" sets "http.proxy" for all
> direct subdomains of "https://example.com",
> e.g. "https://foo.example.com", but not
> "https://foo.bar.example.com".
>
> Fortunately, your update to config.txt, which is facing the end
> users, does not misuse the word and instead is explicit that the
> only thing the matcher does is to match '*' to a single hierarchy.
> It is clear that even http://www*.ibm.com/ is not supported from
> the description, which is good.
I agree that globbing is the wrong word here. I'll swap in
"wildcard" where applicable.
I'll send a version 4 later on. Thanks again for your feedback
and improvements.
Regards
Patrick
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: vger not relaying some of Junio's messages today?
From: Jeff King @ 2017-01-27 4:01 UTC (permalink / raw)
To: Eric Wong; +Cc: Junio C Hamano, git
In-Reply-To: <20170127035753.GA2604@dcvr>
On Fri, Jan 27, 2017 at 03:57:53AM +0000, Eric Wong wrote:
> I noticed both of these are are missing from my archives
> (which rejects messages unless they come from vger):
>
> <xmqq1svp7lcs.fsf@gitster.mtv.corp.google.com>
> <xmqqefzp1d1x.fsf@gitster.mtv.corp.google.com>
I don't have them either, so presumably vger ate them (I usually delete
my cc copies after reading and keep the list ones, and I now have
neither).
> Not sure if there's something up with vger or Junio's setup because
> other messages (even from Junio) are getting through fine.
Sporadic failures, especially on a single topic, imply that something in
the content may triggered the taboo filter (though often that takes out
replies, too, which this doesn't seem to have done).
-Peff
^ permalink raw reply
* Re: Intermittent failure of t1700-split-index.sh
From: Jeff King @ 2017-01-27 3:58 UTC (permalink / raw)
To: Ramsay Jones; +Cc: Christian Couder, Junio C Hamano, GIT Mailing-list
In-Reply-To: <818851a6-c3ef-618e-4146-518fbe6bd837@ramsayjones.plus.com>
On Fri, Jan 27, 2017 at 02:45:15AM +0000, Ramsay Jones wrote:
> I can't devote any time to looking at this further tonight
> (it's 2-45am here, I'm off to bed!). Can you reproduce the
> problem, or is it just me? :)
I can reproduce here with 'pu'. t1700.17 seems to fail reliably with my
stress script after 5-10 seconds.
It bisects to ff97d10f57c2f99b6d86da8f07c16659979b2780, but take that
with a moderate grain of salt, as I may have marked a bad commit as
"good" if it simply got lucky and didn't fail as quickly.
-Peff
^ permalink raw reply
* vger not relaying some of Junio's messages today?
From: Eric Wong @ 2017-01-27 3:57 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jeff King, git
I noticed both of these are are missing from my archives
(which rejects messages unless they come from vger):
<xmqq1svp7lcs.fsf@gitster.mtv.corp.google.com>
<xmqqefzp1d1x.fsf@gitster.mtv.corp.google.com>
https://public-inbox.org/git/20170125234101.n2pzrp77df4zycv7@genre.crustytoothpaste.net/#r
I only have them because I was Cc:-ed.
gmane doesn't seem to have them, either:
nntp://news.gmane.org/xmqqefzp1d1x.fsf@gitster.mtv.corp.google.com
nntp://news.gmane.org/xmqq1svp7lcs.fsf@gitster.mtv.corp.google.com
Not sure if there's something up with vger or Junio's setup because
other messages (even from Junio) are getting through fine.
^ permalink raw reply
* Intermittent failure of t1700-split-index.sh
From: Ramsay Jones @ 2017-01-27 2:45 UTC (permalink / raw)
To: Christian Couder; +Cc: Junio C Hamano, GIT Mailing-list
Hi Christian,
I noticed the intermittent failure of t1700-split-index.sh
(tests #17 and #18) yesterday. It failed in a full test-suite
run, but would not fail when run by hand, until I ran it
like so:
$ cd t
$ for i in `seq 100`; do
> echo "== $i =="
> ./t1700-split-index.sh -i -v || break
> done
... when it failed on the 82nd run!
I only had a brief look in the trash directory, but I could
see that the 'twelve' file did not exist, 'eleven' did exist
and was in the index (well, git ls-files showed it), and that
there were only two '.git/sharedindex.*' files and that their
timestamps had not been changed.
I can't devote any time to looking at this further tonight
(it's 2-45am here, I'm off to bed!). Can you reproduce the
problem, or is it just me? :)
ATB,
Ramsay Jones
^ permalink raw reply
* Re: [RFC 02/14] upload-pack: allow ref name and glob requests
From: Junio C Hamano @ 2017-01-27 1:54 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git
In-Reply-To: <dc09e446-6d29-8b94-f440-6aa094ab9dc9@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
>> I am not sure if this "at the conclusion of" is sensible. It is OK
>> to assume that what the client side has is fixed, and it is probably
>> OK to desire that what the server side has can change, but at the
>> same time, it feels quite fragile to move the goalpost in between.
>
> Do you have any specific concerns as to this fragility? Peff mentioned
> some concerns with the client making some decisions based on the
> initial SHA-1 vs the SHA-1 reported by "wanted-ref", to which I
> replied [1].
There were two but I think you are aware of both. One is what Peff
already mentioned, the client may want to make the decision before
going through the negotiation. The other is "moving the goalpost",
the history the last server has may violate the view of the history
common between the server and the client that is established during
the negotiation with previous servers.
^ permalink raw reply
* Re: [RFC 03/14] upload-pack: test negotiation with changing repo
From: Jonathan Tan @ 2017-01-27 0:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq8tpx30zq.fsf@gitster.mtv.corp.google.com>
On 01/26/2017 02:33 PM, Junio C Hamano wrote:
> Jonathan Tan <jonathantanmy@google.com> writes:
>
>> diff --git a/t/lib-httpd/one-time-sed.sh b/t/lib-httpd/one-time-sed.sh
>> new file mode 100644
>> index 000000000..060ec0300
>> --- /dev/null
>> +++ b/t/lib-httpd/one-time-sed.sh
>> @@ -0,0 +1,8 @@
>> +#!/bin/sh
>> +
>> +if [ -e one-time-sed ]; then
>> + "$GIT_EXEC_PATH/git-http-backend" | sed "$(cat one-time-sed)"
>> + rm one-time-sed
>> +else
>> + "$GIT_EXEC_PATH/git-http-backend"
>> +fi
>
> CodingGuidelines?
Thanks - done locally and will send out in the next reroll.
>> +inconsistency() {
>> + # Simulate that the server initially reports $2 as the ref
>> + # corresponding to $1, and after that, $1 as the ref corresponding to
>> + # $1. This corresponds to the real-life situation where the server's
>> + # repository appears to change during negotiation, for example, when
>> + # different servers in a load-balancing arrangement serve (stateless)
>> + # RPCs during a single negotiation.
>> + printf "s/%s/%s/" \
>> + $(git -C "$REPO" rev-parse $1 | tr -d "\n") \
>> + $(git -C "$REPO" rev-parse $2 | tr -d "\n") \
>> + >"$HTTPD_ROOT_PATH/one-time-sed"
>
> I'd prefer for the printf'd result to have a final LF (i.e. not
> leaving the resulting one-time-sed with a final incomplete line).
> Also, do you really need the pipe to tr-d? Doesn't the result of
> $(command substitution) omit the final LF anyway?
>
> $ printf '1 %s 2 %s 3\n' "$(echo foo)" "$(echo bar)"; echo OK
> 1 foo 2 bar 3
> OK
Done.
>> diff --git a/upload-pack.c b/upload-pack.c
>> index b88ed8e26..0678c53d6 100644
>> --- a/upload-pack.c
>> +++ b/upload-pack.c
>> @@ -862,9 +862,13 @@ static void receive_needs(struct string_list *wanted_ns_refs)
>> } else if (skip_prefix(line, "want ", &arg) &&
>> !get_sha1_hex(arg, sha1_buf)) {
>> o = parse_object(sha1_buf);
>> - if (!o)
>> + if (!o) {
>> + packet_write_fmt(1,
>> + "ERR upload-pack: not our ref %s",
>> + sha1_to_hex(sha1_buf));
>> die("git upload-pack: not our ref %s",
>> sha1_to_hex(sha1_buf));
>> + }
>
> This somehow looks like a good thing to do even in production. Am I
> mistaken?
Yes, that's true. If this patch set stalls (for whatever reason), I'll
spin this off into an independent patch.
^ permalink raw reply
* Re: [PATCH] Documentation: implement linkgit macro for Asciidoctor
From: brian m. carlson @ 2017-01-27 0:40 UTC (permalink / raw)
To: Eric Wong
Cc: Junio C Hamano, Jeff King, git, Johannes Schindelin,
Øyvind A. Holm
In-Reply-To: <20170126191841.GA6060@dcvr.yhbt.net>
[-- Attachment #1: Type: text/plain, Size: 1277 bytes --]
On Thu, Jan 26, 2017 at 07:18:41PM +0000, Eric Wong wrote:
> > Eric Wong <e@80x24.org> writes:
> Junio C Hamano <gitster@pobox.com> wrote:
> > + "<citerefentry>\n"
> > + "<refentrytitle>#{target}</refentrytitle>"
> > + "<manvolnum>#{attrs[1]}</manvolnum>\n"
> > + "</citerefentry>\n"
> > end
>
> You need the '\' at the end of those strings, it's not like C
> since Ruby doesn't require semi-colons to terminate lines.
> In other words, that should be:
>
> "<citerefentry>\n" \
> "<refentrytitle>#{target}</refentrytitle>" \
> "<manvolnum>#{attrs[1]}</manvolnum>\n" \
> "</citerefentry>\n"
>
This change is fine with me.
For the record, I don't have a strong opinion one way or the other.
Since this code is related to Asciidoctor and Git has no existing Ruby
style standards, I picked the Asciidoctor house style, which uses
multi-line %(). We could pick [0] as an option, or just argue it out
when someone cares, like here.
[0] https://github.com/bbatsov/ruby-style-guide
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
* Re: [RFC 02/14] upload-pack: allow ref name and glob requests
From: Jonathan Tan @ 2017-01-27 0:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqd1f931g7.fsf@gitster.mtv.corp.google.com>
On 01/26/2017 02:23 PM, Junio C Hamano wrote:
> Jonathan Tan <jonathantanmy@google.com> writes:
>
>> Currently, while performing packfile negotiation [1], upload-pack allows
>> clients to specify their desired objects only as SHA-1s. This causes:
>> (a) vulnerability to failure when an object turns non-existent during
>> negotiation, which may happen if, for example, upload-pack is
>> provided by multiple Git servers in a load-balancing arrangement,
>> and
>> (b) dependence on the server first publishing a list of refs with
>> associated objects.
>>
>> To eliminate (a) and take a step towards eliminating (b), teach
>> upload-pack to support requests in the form of ref names and globs (in
>> addition to the existing support for SHA-1s) through a new line of the
>> form "want-ref <ref>" where ref is the full name of a ref, a glob
>> pattern, or a SHA-1. At the conclusion of negotiation, the server will
>> write "wanted-ref <SHA-1> <name>" for all requests that have been
>> specified this way.
>
> I am not sure if this "at the conclusion of" is sensible. It is OK
> to assume that what the client side has is fixed, and it is probably
> OK to desire that what the server side has can change, but at the
> same time, it feels quite fragile to move the goalpost in between.
Do you have any specific concerns as to this fragility? Peff mentioned
some concerns with the client making some decisions based on the initial
SHA-1 vs the SHA-1 reported by "wanted-ref", to which I replied [1].
> Stepping back a bit, in an environment that involves multiple server
> instances that have inconsistent set of refs, can the negotiation
> even be sensibly and safely implemented? The first server the
> client contacts may, in response to a "have", say "I do have that
> commit so you do not have to send its ancestors to me. We found one
> cut-off point. Please do explore other lines of histories." The
> next server that concludes the negotiation exchange may not have
> that commit and will be unable to produce a pack that excludes the
> objects reachable from that commit---wouldn't that become a problem?
It's true that this patch set wouldn't solve this problem. This problem
only occurs when there is a commit that the client knows but only a few
of the servers know (maybe because the client just pushed it to one of
them). If, for example, the client does not know a commit and only a few
of the servers know it (for example, because another user just pushed
it), this patch set does help. The latter scenario seems like it would
occur relatively commonly.
> One way to prevent such a problem from hurting clients may be for
> these multiple server instances to coordinate and make sure they
> have a shared perception of the common history among them. Some
> pushes may have come to one instance but may not have propagated to
> other instances, and such a commit cannot be accepted as usable
> "have" if the servers anticipate that the final client request would
> go to any of the servers. Otherwise the multiple server arrangement
> would not work safely, methinks.
>
> And if the servers are ensuring the safety using such a mechanism,
> they can use the same mechanism to restrain "faster" instances from
> sending too fresh state of refs that other instances haven't caught
> up to, which would mean they can present a consistent set of refs to
> the client in the first place, no?
>
> So I am not sure if the mechanism to request history by refname
> instead of the tip commit would help the multi-server environment as
> advertised. It may help solving other problems, though (e.g. like
> "somebody pushed to update after the initial advertisement was sent
> out" which can happen even in a single server environment).
This patch set would solve the problem you describe (whether in a single
server environment or the coordination between multiple servers that
provides "strong consistency"). (Although it may not be an important
problem to solve, since it is probably OK if the client got a "slow"
version of the state of the refs.)
>> To be flexible with respect to client needs, the server does not
>> indicate an error if a "want-ref" line corresponds to no refs, but
>> instead relies on the client to ensure that what the user needs has been
>> fetched. For example, a client could reasonably expand an abbreviated
>> name "foo" to "want-ref foo", "want-ref refs/heads/foo", "want-ref
>> refs/tags/foo", among others, and ensure that at least one such ref has
>> been fetched.
>
> Cute. This may be one way to implement the DWIM thing within the
> constraint of eventually wanting to go to "client speaks first, the
> server does not advertise things the client is not interested in"
> world.
>
> But at the same time it may end up bloating the set of refs the
> client asks instead. Instead of receiving the advertisement and
> then sending one request after picking the matching one from it,
> the client needs to send "refs/{heads,tags,whatever}/foo".
That is true, although I think that the client will typically send only
a few ref names (with or without globs), so the request packet is still
not that large.
[1] <67afbb3b-5d0b-8c0d-3f6e-3f559c68f4bd@google.com>
^ permalink raw reply
* Re: [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)
From: Jonathan Tan @ 2017-01-27 0:26 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20170126230046.aknesybfyzxhx3ia@sigill.intra.peff.net>
Thanks for your comments.
On 01/26/2017 03:00 PM, Jeff King wrote:
> On Wed, Jan 25, 2017 at 02:02:53PM -0800, Jonathan Tan wrote:
>
>> Negotiation currently happens by upload-pack initially sending a list of
>> refs with names and SHA-1 hashes, and then several request/response
>> pairs in which the request from fetch-pack consists of SHA-1 hashes
>> (selected from the initial list). Allowing the request to consist of
>> names instead of SHA-1 hashes increases tolerance to refs changing
>> (due to time, and due to having load-balanced servers without strong
>> consistency),
>
> Interesting. My big question is: what happens when a ref _does_ change?
> How does the client handle this?
>
> The existing uploadpack.allowReachableSHA1InWant is there to work around
> the problem that an http client may get a ref advertisement in one step,
> and then come back later to do the want/have negotiation, at which point
> the server has moved on (or maybe it's even a different server). There
> the client says "I want sha1 X", and the server needs to say "well, X
> isn't my tip now, but it's still acceptable for you to fetch".
>
> But this seems to go in the opposite direction. After the advertisement,
> the client decides "OK, I want to fetch refs/heads/master which is at
> SHA1 X". It connects to the server and says "I want refs/heads/master".
> Let's say the server has moved its version of the ref to SHA1 Y.
>
> What happens? I think the server will say "wanted-ref master Y". Does
> the client just decide to use "Y" then? How does that interact with any
> decisions the client might have made about X? I guess things like
> fast-forwards have to be decided after we fetch the objects anyway
> (since we cannot compute them until we get the pack), so maybe there
> aren't any such decisions. I haven't checked.
Yes, the server will say "wanted-ref master Y". The relevant code
regarding the decisions the client makes regarding X or Y is in do_fetch
in builtin/fetch.c.
There, I can see these decisions done using X:
- check_not_current_branch (forbidding fetching that modifies the
current branch) (I just noticed that this has to be done for Y too,
and will do so)
- prune refs [*]
- automatic tag following [*]
[*] X and Y may differ in that one relevant ref appears in one set but
not in the other (because a ref was added or removed in the meantime),
causing a different result if these decisions were to be done using Y,
but I think that it is OK either way.
Fetch optimizations (for example, everything_local in fetch-pack.c) that
check if the client really needs to fetch are also done using X, of
course (and if the optimization succeeds, there is no Y).
Fast-forwards (and everything else in store_updated_refs) are decided
using Y.
>> and is a step towards eliminating the need for the server
>> to send the list of refs first (possibly improving performance).
>
> I'm not sure it is all that useful towards that end. You still have to
> break compatibility so that the client tells the server to suppress the
> ref advertisement. After that, it is just a question of asking for the
> refs. And you have two options:
>
> 1. Ask the server to tell you the values of some subset of the refs,
> pick what you want, and then do the want/have as normal.
>
> 2. Go straight to the want/have, but tell the server the refs you want
> instead of their sha1s.
>
> I think your approach here would lead to (2).
>
> But (1), besides being closer to how the protocol works now, seems like
> it's more flexible. I can ask about the ref state without necessarily
> having to retrieve the objects. How would you write git-ls-remote with
> such a system?
Assuming a new protocol with the appropriate backwards compatibility
(which would have to be done for both options), (2) does save a
request/response pair (because we can send the ref names directly as
"want-ref" in the initial request instead of sending ref names in the
initial request and then confirming them using "want <SHA-1>" in the
subsequent request). Also, (2) is more tolerant towards refs changing
over time. (1) might be closer to the current protocol, but I think that
the difference is not so significant (only in "want-ref" vs "want"
request and the "wanted-ref" response).
As for git-ls-remote, I currently think that it would have to use the
existing protocol.
>> [1] There has been some discussion about whether the server should
>> accept partial ref names, e.g. [2]. In this patch set, I have made the
>> server only accept full names, and it is the responsibility of the
>> client to send the multiple patterns which it wants to match. Quoting
>> from the commit message of the second patch:
>>
>> For example, a client could reasonably expand an abbreviated
>> name "foo" to "want-ref foo", "want-ref refs/heads/foo", "want-ref
>> refs/tags/foo", among others, and ensure that at least one such ref has
>> been fetched.
>
> That has a cost that scales linearly with the number of refs, because
> you have to ask for each name 6 times. After the discussion you linked,
> I think my preference is more like:
>
> 1. Teach servers to accept a list of patterns from the client
> which will be resolved in order. Unlike your system, the client
> only needs to specify the list once per session, rather than once
> per ref.
>
> 2. (Optional) Give a shorthand for the stock patterns that git has had
> in place for years. That saves some bytes over specifying the
> patterns completely (though it's really not _that_ many bytes, so
> perhaps the complication isn't a big deal).
I envision that most fetches would be done on a single name (with or
without globs), that expands to 5 (so it is not so crucial to factor out
the list of patterns). Having said that, I am open to changing this.
^ permalink raw reply
* Re: [PATCH v2 3/3] update-ref: add test cases for bare repository
From: Junio C Hamano @ 2017-01-26 23:41 UTC (permalink / raw)
To: cornelius.weig; +Cc: peff, git
In-Reply-To: <20170126223159.16439-3-cornelius.weig@tngtech.com>
cornelius.weig@tngtech.com writes:
> From: Cornelius Weig <cornelius.weig@tngtech.com>
>
> The default behavior of update-ref to create reflogs differs in
> repositories with worktree and bare ones. The existing tests cover only
> the behavior of repositories with worktree.
>
> This commit adds tests that assert the correct behavior in bare
> repositories for update-ref. Two cases are covered:
>
> - If core.logAllRefUpdates is not set, no reflogs should be created
> - If core.logAllRefUpdates is true, reflogs should be created
>
> Signed-off-by: Cornelius Weig <cornelius.weig@tngtech.com>
> ---
> t/t1400-update-ref.sh | 43 ++++++++++++++++++++++++++++++++++++-------
> 1 file changed, 36 insertions(+), 7 deletions(-)
>
> diff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh
> index b9084ca..bad88c8 100755
> --- a/t/t1400-update-ref.sh
> +++ b/t/t1400-update-ref.sh
> @@ -8,23 +8,33 @@ test_description='Test git update-ref and basic ref logging'
>
> Z=$_z40
>
> -test_expect_success setup '
> +m=refs/heads/master
> +n_dir=refs/heads/gu
> +n=$n_dir/fixes
> +outside=refs/foo
> +bare=bare-repo
>
> +create_test_objects ()
> +{
> + local T, sha1, prfx="$1"
CodingGuidelines. Do not use bash-ism "local" (besides, I do not
think you want to have comma here).
> for name in A B C D E F
> do
> test_tick &&
> T=$(git write-tree) &&
> sha1=$(echo $name | git commit-tree $T) &&
> - eval $name=$sha1
> + eval $prfx$name=$sha1
> done
> +}
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox