* [PATCH v1 2/2] urlmatch: allow regex-based URL matching
From: Patrick Steinhardt @ 2017-01-23 13:06 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Patrick Steinhardt
In-Reply-To: <20170123130635.29577-1-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.
This commit introduces the ability to have regex-based URL matches. A
user can prefix a configuration key's URL with a question mark ('?') to
use regular expressions instead of exact matches in order to find all
matching URLs. A user can now simply add a key
`http.?http://.*\\.example\\.com.proxy` to set a proxy for all
subdomains of `example.com`. When no question mark is given as a prefix,
then the configuration subsystem will use the old algorithm based on
exact matches.
Signed-off-by: Patrick Steinhardt <patrick.steinhardt@elego.de>
---
Documentation/config.txt | 6 ++++-
t/t1300-repo-config.sh | 31 ++++++++++++++++++++++++++
urlmatch.c | 57 ++++++++++++++++++++++++++++++++++++++----------
3 files changed, 81 insertions(+), 13 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 506431267..23651b19e 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1906,7 +1906,11 @@ http.followRedirects::
http.<url>.*::
Any of the http.* options above can be applied selectively to some URLs.
- For a config key to match a URL, each element of the config key is
+ There are two different modes to match URLs: if the config key's URL is
+ prefixed with a `?`, it allows to make use of regular expressions. An
+ example for this is `http.?http://.*\\.example\\.com.*` to match all
+ subdomains of `example.com`.
+ If the key is not prefixed with a `?`, each element of the config key is
compared to that of the URL, in the following order:
+
--
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 923bfc5a2..fbbc58304 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1177,6 +1177,37 @@ test_expect_success 'urlmatch' '
test_cmp expect actual
'
+test_expect_success 'regex-based urlmatch' '
+ 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 &&
+
+ 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 false >expect &&
+ git config --bool --get-urlmatch http.sslverify https://subdomain.example.com >actual &&
+ test_cmp expect actual &&
+
+ {
+ echo http.cookiefile /tmp/cookie.txt &&
+ echo http.sslverify false
+ } >expect &&
+ git config --get-urlmatch HTTP https://subdomain.example.com >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 132d342bc..8ed5047e3 100644
--- a/urlmatch.c
+++ b/urlmatch.c
@@ -490,18 +490,51 @@ int urlmatch_config_entry(const char *var, const char *value, void *cb)
}
dot = strrchr(key, '.');
if (dot) {
- char *config_url, *norm_url;
- struct url_info norm_info;
-
- config_url = xmemdupz(key, dot - key);
- norm_url = url_normalize(config_url, &norm_info);
- free(config_url);
- if (!norm_url)
- return 0;
- matched_len = match_urls(url, &norm_info, &user_matched);
- free(norm_url);
- if (!matched_len)
- return 0;
+ /*
+ * When the configuration key's URL is prefixed
+ * with a '?', regular expressions are enabled to
+ * match the URL instead of the exact-match
+ * algorithm.
+ */
+ if (starts_with(key, "?")) {
+ char *config_url;
+ regex_t reg;
+ int status;
+
+ config_url = xmemdupz(key + 1, dot - key - 1);
+ if (regcomp(®, config_url, REG_EXTENDED)) {
+ warning(_("Cannot prepare URL regexp %s"),
+ config_url);
+ free(config_url);
+ return 0;
+ }
+
+ status = regexec(®, url->url, 0, NULL, 0);
+ free(config_url);
+ regfree(®);
+
+ if (status) {
+ if (status != REG_NOMATCH)
+ warning(_("regexec returned %d for input '%s'"),
+ status, url->url);
+
+ return 0;
+ }
+ } else {
+ char *config_url, *norm_url;
+ struct url_info norm_info;
+
+ config_url = xmemdupz(key, dot - key);
+ norm_url = url_normalize(config_url, &norm_info);
+ free(config_url);
+ if (!norm_url)
+ return 0;
+ matched_len = match_urls(url, &norm_info, &user_matched);
+ free(norm_url);
+ if (!matched_len)
+ return 0;
+ }
+
key = dot + 1;
}
--
2.11.0
^ permalink raw reply related
* [PATCH v1 1/2] mailmap: add Patrick Steinhardt's work address
From: Patrick Steinhardt @ 2017-01-23 13:06 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Patrick Steinhardt
In-Reply-To: <20170123130635.29577-1-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
* [PATCH v1 0/2] urlmatch: allow regexp-based matches
From: Patrick Steinhardt @ 2017-01-23 13:06 UTC (permalink / raw)
To: git; +Cc: Patrick Steinhardt, Patrick Steinhardt
Hi,
Short disclaimer: this patch results from work for a client at my
day job at elego Software Solutions GmbH. As such, I'm using my
work mail address and added a new mailmap entry. I wasn't exactly
certain if the mailmap entry should've been created in a separate
commit series, as it has nothing to do with the actual topic --
I can re-send it separately if requested.
This patch is mostly a request for comments. The use case is to
be able to configure an HTTP proxy for all subdomains of a
certain domain where there are hundreds of subdomains. The most
flexible way I could imagine was by using regular expressions for
the matching, which is how I implemented it for now. So users can
now create a configuration key like
`http.?http://.*\\.example\\.com.*` to apply settings for all
subdomains of `example.com`.
I tried to make this feature as backwards-compatible as it can be
by having the '?' prefix. Older clients will barf when trying to
normalize the URL as '?' is not in the set of allowed characters
for a URL, and for newer clients there will be no change in
behavior for previously configured `http.<url>.*` keys.
Regards
Patrick Steinhardt
Patrick Steinhardt (2):
mailmap: add Patrick Steinhardt's work address
urlmatch: allow regex-based URL matching
.mailmap | 1 +
Documentation/config.txt | 6 ++++-
t/t1300-repo-config.sh | 31 ++++++++++++++++++++++++++
urlmatch.c | 57 ++++++++++++++++++++++++++++++++++++++----------
4 files changed, 82 insertions(+), 13 deletions(-)
--
2.11.0
^ permalink raw reply
* GSoC 2017: application open, deadline = February 9, 2017
From: Matthieu Moy @ 2017-01-23 15:02 UTC (permalink / raw)
To: git, Pranit Bauva, Lars Schneider, Christian Couder, Jeff King,
Carlos Martín Nieto, Johannes Schindelin, Thomas Gummerer
Hi,
The Google Summer of Code 2017 program is launched
(https://summerofcode.withgoogle.com/).
Last year, Pranit Bauva worked on porting "git bisect" from shell to C,
mentored by Christian and Lars (I didn't follow closely, but essentially
many preparatory steps, cleanups and tests were merged in master, and
patches starting the real port are still queued in pu). The org admins
were Peff and I.
The application deadline is February 9, 2017, i.e. a bit more than 2
weeks from now. So, we should decide quickly whether or not to
participate, and if so work on the application.
On my side, I've been struggling to find some time budget to allocate to
Git since last year and I couldn't even keep up with discussions on this
list :-(. Last summer, I thought that being an org co-admin would help,
but it didn't. So, as much as possible, I'd like to avoid being an org
admin this year. It's not a lot of work (much, much less than being a
mentor!), but if I manage to get some time to work for Git, I'd rather
do that on coding and reviewing this year.
So, a bunch of unanswered questions:
* Does the git.git community want to participate in GSoC this year?
Actually, I have mixed feelings about this: I do like GSoC, but it
seems we lack reviewer time more than coder time, and GSoC does not
make it better. OTOH, a GSoC participant is a potential reviewer in
the long run ...
* Does the libgit2 community want to participate in GSoC? If so, we
should clarify the application process. Last year, I wrote (too late):
> It's essentially too late to change this for this year, but next
> time we should discuss earlier about how we want to organize this
> git.git/libgit2 thing. For example, I think it would make little sense
> to have a git.git microproject and then apply for a libgit2 project
> since we have very different ways of interacting. And honestly, right
> now the application is really git.git-centric so I don't think it
> attracts students towards libgit2. So, if you want to attract more
> students, we should work on that.
If the answer to one of the above question is yes, then:
* Who's willing to mentor? and to admin?
* We need to write the application, i.e. essentially polish and update
the text here: https://git.github.io/SoC-2016-Org-Application/ and
update the list of project ideas and microprojects :
https://git.github.io/SoC-2017-Ideas/
https://git.github.io/SoC-2016-Microprojects/
Cheers,
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
^ permalink raw reply
* Re: sparse checkout : ignore paths
From: Johannes Schindelin @ 2017-01-23 15:13 UTC (permalink / raw)
To: Tushar Kapila; +Cc: git
In-Reply-To: <CALNyQkyy_prP60kp_DpMzG9+affqPW0-z6F81R4OSgHg0QFc+g@mail.gmail.com>
Hi,
On Mon, 23 Jan 2017, Tushar Kapila wrote:
> When we clone/ pull with : config core.sparsecheckout true
>
> We can specify paths to include. Would be good to explicitly specify
> paths to exclude too.
That is already possible by using "negative patterns", i.e. patterns
preceded by an exclamation point.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH v3 08/21] Documentation/git-update-index: talk about core.splitIndex config var
From: Christian Couder @ 2017-01-23 15:55 UTC (permalink / raw)
To: Duy Nguyen
Cc: Junio C Hamano, git, Ævar Arnfjörð Bjarmason,
Christian Couder
In-Reply-To: <CACsJy8D1Pf6zTS8gqv5Gq6xMxNNbDrTpKHGADRMKNqW1FAzZvA@mail.gmail.com>
On Mon, Jan 9, 2017 at 12:18 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Sun, Jan 8, 2017 at 4:38 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> Christian Couder <christian.couder@gmail.com> writes:
>>
>>> It feels strange that when I do things one way, you suggest another
>>> way, and the next time in a similar situation when I do things the way
>>> you suggested previously, then you suggest the way I did it initially
>>> the first time...
>>
>> Perhaps because neither is quite satisfactory and I am being forced
>> to choose between the two unsatifactory designs? In any case, I
>> mostly am and was pointing out the issues and not saying the other
>> one is the most preferred solution in these threads.
>>
>> I think there should just be one authoritative source of the truth,
>
> Either that, or we make sure all sources of truth are consistent. In
> this case, 'update --split-index' could update core.splitIndex if it
> finds that the config tells a different story. As a plumbing though, I
> rather leave update-index do simple things, even if it means the user
> has to clean up after it (or before it) with "git config -unset
> core.splitIndex". Another option is refuse to execute --split-index in
> the presence of (conflicting) core.splitIndex. We kind of force the
> user to keep all sources of truth consistent this way while leaving a
> back door ("git -c core.splitIndex= update-index") for those who need
> tools to recover from a bad case.
>
>> and I have always thought it should be the bit set in the index file
>> when the command line option is used, because that was the way the
>> feature was introduced first and I am superstitious about end-user
>> inertia. And from that point of view, no matter how you make this
>> new "config" thing interact with it, it would always give a strange
>> and unsatifactory end-user experience, at least to me.
>>
>> Perhaps we should declare that config will be the one and only way
>> in the future and start deprecating the command line option way.
>> That will remove the need for two to interact with each other.
That would be my preferred solution as I think it is the simplest in
the end for users.
Also, as Duy wrote above, one can always use something like "git -c
core.splitIndex= ...", which by the way can work for any command, not
just "update-index".
Anyway we would have to introduce core.splitIndex first, and it
wouldn't make sense to have a different behavior between
--[no-]split-index and --[no-]untracked-cache in the meantime before
they are deprecated and eventually removed.
So let's just go with the implementation in this series, which is
similar to --[no-]untracked-cache, and let's plan to deprecate
--[no-]split-index and --[no-]untracked-cache in a later patch series.
^ permalink raw reply
* Re: [PATCH] blame: add option to print tips (--tips)
From: Pranit Bauva @ 2017-01-23 16:33 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Edmundo Carmona Antoranz, Git List
In-Reply-To: <xmqqlgu2u0qx.fsf@gitster.mtv.corp.google.com>
Hey Junio,
On Mon, Jan 23, 2017 at 5:05 AM, Junio C Hamano <gitster@pobox.com> wrote:
> That is too early to tell. At this point we only know there are me
> who won't use it and you who will, among all the other people in the
> world.
We can probably make it useful with some extended efforts. I use
git-blame and I sometimes find that I don't need things like the name
of the author, time, timezone and not even the file name and I have to
use a bigger terminal. If we could somehow remove those fields then
maybe this would be a useful feature.
Idea: Make git-blame understand `format`
git-log has a format option in which we can configure what all things
we need and what we don't. We could probably do the same here also.
After carefully using the format specification with git-log each
person can get a good look at however he seems to view. But I am not
sure whether this is worth the effort. I personally find this `format`
feature useful.
Regards,
Pranit Bauva
^ permalink raw reply
* [PATCH v3 3/5] show-ref: Move --quiet handling into show_one
From: Vladimir Panteleev @ 2017-01-23 18:00 UTC (permalink / raw)
To: git; +Cc: Vladimir Panteleev
In-Reply-To: <20170123180059.4288-1-git@thecybershadow.net>
Do the same with --quiet as was done with -d, to remove the need to
perform this check at show_one's call site from the --verify branch.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
---
builtin/show-ref.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index a72a626b1..ab8e0dc41 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -22,6 +22,9 @@ static void show_one(const char *refname, const struct object_id *oid)
const char *hex;
struct object_id peeled;
+ if (quiet)
+ return;
+
hex = find_unique_abbrev(oid->hash, abbrev);
if (hash_only)
printf("%s\n", hex);
@@ -82,9 +85,6 @@ static int show_ref(const char *refname, const struct object_id *oid,
die("git show-ref: bad ref %s (%s)", refname,
oid_to_hex(oid));
- if (quiet)
- return 0;
-
show_one(refname, oid);
return 0;
@@ -205,8 +205,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
if ((starts_with(*pattern, "refs/") || !strcmp(*pattern, "HEAD")) &&
!read_ref(*pattern, oid.hash)) {
- if (!quiet)
- show_one(*pattern, &oid);
+ show_one(*pattern, &oid);
}
else if (!quiet)
die("'%s' - not a valid ref", *pattern);
--
2.11.0
^ permalink raw reply related
* [PATCH v3 5/5] show-ref: Remove dead `if (verify)' check
From: Vladimir Panteleev @ 2017-01-23 18:00 UTC (permalink / raw)
To: git; +Cc: Vladimir Panteleev
In-Reply-To: <20170123180059.4288-1-git@thecybershadow.net>
As show_ref is only ever called on the path where --verify is not
specified, `verify' can never possibly be true here.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
---
builtin/show-ref.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 107d05fe0..2dfcb5634 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -73,9 +73,6 @@ static int show_ref(const char *refname, const struct object_id *oid,
continue;
if (len == reflen)
goto match;
- /* "--verify" requires an exact match */
- if (verify)
- continue;
if (refname[reflen - len - 1] == '/')
goto match;
}
--
2.11.0
^ permalink raw reply related
* [PATCH v3 1/5] show-ref: Accept HEAD with --verify
From: Vladimir Panteleev @ 2017-01-23 18:00 UTC (permalink / raw)
To: git; +Cc: Vladimir Panteleev
In-Reply-To: <20170123180059.4288-1-git@thecybershadow.net>
Previously, when --verify was specified, show-ref would use a separate
code path which did not handle HEAD and treated it as an invalid
ref. Thus, "git show-ref --verify HEAD" (where "--verify" is used
because the user is not interested in seeing refs/remotes/origin/HEAD)
did not work as expected.
Instead of insisting that the input begins with "refs/", allow "HEAD"
as well in the codepath that handles "--verify", so that all valid
full refnames including HEAD are passed to the same output machinery.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
---
builtin/show-ref.c | 2 +-
t/t1403-show-ref.sh | 11 +++++++++++
2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 6d4e66900..0e53e3da4 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -202,7 +202,7 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)
while (*pattern) {
struct object_id oid;
- if (starts_with(*pattern, "refs/") &&
+ if ((starts_with(*pattern, "refs/") || !strcmp(*pattern, "HEAD")) &&
!read_ref(*pattern, oid.hash)) {
if (!quiet)
show_one(*pattern, &oid);
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index 7e10bcfe3..5932beada 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -164,4 +164,15 @@ test_expect_success 'show-ref --heads, --tags, --head, pattern' '
test_cmp expect actual
'
+test_expect_success 'show-ref --verify HEAD' '
+ echo $(git rev-parse HEAD) HEAD >expect &&
+ git show-ref --verify HEAD >actual &&
+ test_cmp expect actual &&
+
+ >expect &&
+
+ git show-ref --verify -q HEAD >actual &&
+ test_cmp expect actual
+'
+
test_done
--
2.11.0
^ permalink raw reply related
* [PATCH v3 0/5] show-ref: Allow -d, --head to work with --verify
From: Vladimir Panteleev @ 2017-01-23 18:00 UTC (permalink / raw)
To: git
Third iteration, according to Junio's comments. This time we keep
show_ref and show_one separate, accept HEAD with --verify even without
--head, and add tests for dangling ref validation with --verify.
^ permalink raw reply
* [PATCH v3 2/5] show-ref: Allow -d to work with --verify
From: Vladimir Panteleev @ 2017-01-23 18:00 UTC (permalink / raw)
To: git; +Cc: Vladimir Panteleev
In-Reply-To: <20170123180059.4288-1-git@thecybershadow.net>
Move handling of -d into show_one, so that it takes effect when
--verify is present as well as when it is absent. This is useful when
the user wishes to avoid the costly iteration of refs.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
---
builtin/show-ref.c | 23 ++++++++++++-----------
t/t1403-show-ref.sh | 9 +++++++++
2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index 0e53e3da4..a72a626b1 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -19,19 +19,27 @@ static const char *exclude_existing_arg;
static void show_one(const char *refname, const struct object_id *oid)
{
- const char *hex = find_unique_abbrev(oid->hash, abbrev);
+ const char *hex;
+ struct object_id peeled;
+
+ hex = find_unique_abbrev(oid->hash, abbrev);
if (hash_only)
printf("%s\n", hex);
else
printf("%s %s\n", hex, refname);
+
+ if (!deref_tags)
+ return;
+
+ if (!peel_ref(refname, peeled.hash)) {
+ hex = find_unique_abbrev(peeled.hash, abbrev);
+ printf("%s %s^{}\n", hex, refname);
+ }
}
static int show_ref(const char *refname, const struct object_id *oid,
int flag, void *cbdata)
{
- const char *hex;
- struct object_id peeled;
-
if (show_head && !strcmp(refname, "HEAD"))
goto match;
@@ -79,13 +87,6 @@ static int show_ref(const char *refname, const struct object_id *oid,
show_one(refname, oid);
- if (!deref_tags)
- return 0;
-
- if (!peel_ref(refname, peeled.hash)) {
- hex = find_unique_abbrev(peeled.hash, abbrev);
- printf("%s %s^{}\n", hex, refname);
- }
return 0;
}
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index 5932beada..c6872bd96 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -97,6 +97,9 @@ test_expect_success 'show-ref -d' '
git show-ref -d refs/tags/A refs/tags/C >actual &&
test_cmp expect actual &&
+ git show-ref --verify -d refs/tags/A refs/tags/C >actual &&
+ test_cmp expect actual &&
+
echo $(git rev-parse refs/heads/master) refs/heads/master >expect &&
git show-ref -d master >actual &&
test_cmp expect actual &&
@@ -116,6 +119,12 @@ test_expect_success 'show-ref -d' '
test_cmp expect actual &&
test_must_fail git show-ref -d --verify heads/master >actual &&
+ test_cmp expect actual &&
+
+ test_must_fail git show-ref --verify -d A C >actual &&
+ test_cmp expect actual &&
+
+ test_must_fail git show-ref --verify -d tags/A tags/C >actual &&
test_cmp expect actual
'
--
2.11.0
^ permalink raw reply related
* [PATCH v3 4/5] show-ref: Detect dangling refs under --verify as well
From: Vladimir Panteleev @ 2017-01-23 18:00 UTC (permalink / raw)
To: git; +Cc: Vladimir Panteleev
In-Reply-To: <20170123180059.4288-1-git@thecybershadow.net>
Move detection of dangling refs into show_one, so that they are
detected when --verify is present as well as when it is absent.
Signed-off-by: Vladimir Panteleev <git@thecybershadow.net>
---
builtin/show-ref.c | 16 ++++++++--------
t/t1403-show-ref.sh | 22 ++++++++++++++++++++++
2 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/builtin/show-ref.c b/builtin/show-ref.c
index ab8e0dc41..107d05fe0 100644
--- a/builtin/show-ref.c
+++ b/builtin/show-ref.c
@@ -22,6 +22,14 @@ static void show_one(const char *refname, const struct object_id *oid)
const char *hex;
struct object_id peeled;
+ /* This changes the semantics slightly that even under quiet we
+ * detect and return error if the repository is corrupt and
+ * ref points at a nonexistent object.
+ */
+ if (!has_sha1_file(oid->hash))
+ die("git show-ref: bad ref %s (%s)", refname,
+ oid_to_hex(oid));
+
if (quiet)
return;
@@ -77,14 +85,6 @@ static int show_ref(const char *refname, const struct object_id *oid,
match:
found_match++;
- /* This changes the semantics slightly that even under quiet we
- * detect and return error if the repository is corrupt and
- * ref points at a nonexistent object.
- */
- if (!has_sha1_file(oid->hash))
- die("git show-ref: bad ref %s (%s)", refname,
- oid_to_hex(oid));
-
show_one(refname, oid);
return 0;
diff --git a/t/t1403-show-ref.sh b/t/t1403-show-ref.sh
index c6872bd96..30354fd26 100755
--- a/t/t1403-show-ref.sh
+++ b/t/t1403-show-ref.sh
@@ -184,4 +184,26 @@ test_expect_success 'show-ref --verify HEAD' '
test_cmp expect actual
'
+test_expect_success 'show-ref --verify with dangling ref' '
+ sha1_file() {
+ echo "$*" | sed "s#..#.git/objects/&/#"
+ } &&
+
+ remove_object() {
+ file=$(sha1_file "$*") &&
+ test -e "$file" &&
+ rm -f "$file"
+ } &&
+
+ test_when_finished "rm -rf dangling" &&
+ (
+ git init dangling &&
+ cd dangling &&
+ test_commit dangling &&
+ sha=$(git rev-parse refs/tags/dangling) &&
+ remove_object $sha &&
+ test_must_fail git show-ref --verify refs/tags/dangling
+ )
+'
+
test_done
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 25/27] attr: store attribute stacks in hashmap
From: Brandon Williams @ 2017-01-23 18:08 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, pclouds, sbeller
In-Reply-To: <20170118203440.GB10641@google.com>
On 01/18, Brandon Williams wrote:
> On 01/13, Junio C Hamano wrote:
> > Brandon Williams <bmwill@google.com> writes:
> >
> > > The last big hurdle towards a thread-safe API for the attribute system
> > > is the reliance on a global attribute stack that is modified during each
> > > call into the attribute system.
> > >
> > > This patch removes this global stack and instead a stack is retrieved or
> > > constructed locally. Since each of these stacks is only used as a
> > > read-only structure once constructed, they can be stored in a hashmap
> > > and shared between threads.
> >
> > Very good.
> >
> > The reason why the original code used a stack was because it wanted
> > to keep only the info read from releavant files in-core, discarding
> > ones from files no-longer relevant (because the traversal switched
> > to another subdirectory of the same parent directory), to avoid the
> > memory consumption grow unbounded. It probably was a premature
> > "optimization" that we can do without, so keeping everything we have
> > read so far in a hashmap (which is my understanding of what is going
> > on in this patch) is probably OK.
> >
> > I suspect that this hashmap may eventually need to become per
> > attr_check if we want to follow through the optimization envisioned
> > by patch 15/27.
> >
> > Inside fill(), path_matches() is called for the number of match_attr
> > in the entire attribute stack but it is wasteful to check if the
> > path matches with the a.u.pat if none of the a.state[] entries talk
> > about attributes and macros that are eventually get used by the
> > caller of check_attr(). By introducing a wrapping structure, 15/27
> > wanted to make sure that we have a place to store a "reduced"
> > attribute stack that is kept per attr_check that has only entries
> > from the files that talk about the attributes the particular
> > attr_check wants to learn about.
> >
> > I need to think about this a bit more, but I do not offhand think
> > that it makes future such enhancement to make it per-check harder to
> > move from a global stack to a global hashmap, i.e. the above is not
> > an objection to this step.
>
> If we want to continue through and do the optimization you originally
> envisioned then I may need to rethink this patch. One thing we did talk
> about offline was doing another check prior to the path_match() function
> call which looks through the list of state structs to see if one of
> those states would actually have an affect on the array being used to
> collect attributes. Though that may be an optimization which can be
> done in addition to creating a reduced stack.
>
> The one difficulty (which you pointed out in comment form) is if we have
> a reduced attribute stack that is stored per attr_check then handling
> the cleanup when the direction is changed may be messy.
After thinking about this some more I'm going to redo this patch in the
series and instead of storing all of the frames in a shared hashmap,
we'll have an attribute stack stored per attr_check instance like you
originally envisioned. I think that having a hashmap of all the stack
frames may make it more difficult to do optimizations in the future. At
least this way (simply pushing the stack into the attr_check) makes it
more straight forward to do optimizations and doesn't have the potential
for memory to grow unbounded.
I'll try to get out a v2 later today.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH] rebase: pass --signoff option to git am
From: Junio C Hamano @ 2017-01-23 18:13 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: git
In-Reply-To: <20170121104904.15132-1-giuseppe.bilotta@gmail.com>
Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
> Signed-off-by: Giuseppe Bilotta <giuseppe.bilotta@gmail.com>
> ---
> Documentation/git-rebase.txt | 5 +++++
> git-rebase.sh | 3 ++-
> 2 files changed, 7 insertions(+), 1 deletion(-)
Should we plan to extend this to the interactive backend that is
shared between rebase -i and rebase -m, too? Or is this patch
already sufficient to cover them?
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 67d48e6883..e6f0b93337 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -385,6 +385,11 @@ have the long commit hash prepended to the format.
> Recreate merge commits instead of flattening the history by replaying
> commits a merge commit introduces. Merge conflict resolutions or manual
> amendments to merge commits are not preserved.
> +
> +--signoff::
> + This flag is passed to 'git am' to sign off all the rebased
> + commits (see linkgit:git-am[1]).
> +
> +
> This uses the `--interactive` machinery internally, but combining it
> with the `--interactive` option explicitly is generally not a good
> diff --git a/git-rebase.sh b/git-rebase.sh
> index 48d7c5ded4..e468a061f9 100755
> --- a/git-rebase.sh
> +++ b/git-rebase.sh
> @@ -34,6 +34,7 @@
> autosquash move commits that begin with squash!/fixup! under -i
> committer-date-is-author-date! passed to 'git am'
> ignore-date! passed to 'git am'
> +signoff! passed to 'git am'
> whitespace=! passed to 'git apply'
> ignore-whitespace! passed to 'git apply'
> C=! passed to 'git apply'
> @@ -321,7 +322,7 @@ run_pre_rebase_hook ()
> --ignore-whitespace)
> git_am_opt="$git_am_opt $1"
> ;;
> - --committer-date-is-author-date|--ignore-date)
> + --committer-date-is-author-date|--ignore-date|--signoff)
> git_am_opt="$git_am_opt $1"
> force_rebase=t
> ;;
^ permalink raw reply
* Re: [PATCH v3 14/21] read-cache: touch shared index files when used
From: Christian Couder @ 2017-01-23 18:14 UTC (permalink / raw)
To: Junio C Hamano
Cc: Duy Nguyen, git, Ævar Arnfjörð Bjarmason,
Christian Couder
In-Reply-To: <xmqqlgu627uj.fsf@gitster.mtv.corp.google.com>
On Thu, Jan 19, 2017 at 8:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> On Mon, Jan 9, 2017 at 9:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Duy Nguyen <pclouds@gmail.com> writes:
>>>
>>>> On Sun, Jan 8, 2017 at 4:46 AM, Junio C Hamano <gitster@pobox.com> wrote:
>>>>> Christian Couder <christian.couder@gmail.com> writes:
>>>>>
>>>>>> So what should we do if freshen_file() returns 0 which means that the
>>>>>> freshening failed?
>>>>>
>>>>> You tell me ;-) as you are the one who is proposing this feature.
Before the above question I already had given my opinion about what we
should do.
There are the following cases:
- Freshening failed because the shared index file does not exist
anymore. In this case it could have been removed for a good reason
(for example maybe the user wants to remove all the shared index
files), or it could be a bug somewhere else. Anyway we cannot know and
the user will get an error if the shared index file that disappeared
is read from afterwards, so I don't think we need to warn or do
anything.
- Freshening failed because the mtime of the shared index cannot be
changed. You already in a previous email wrote that we shoudn't warn
if the file system is read only, and I agree with that, as anyway if
the file system is read only, the shared index file cannot be deleted,
so there is no risk from the current user. Also the split index mode
is useful to speed up index writing at the cost of making index
reading a little slower, so its use in a read only mode should not be
the primary way it is used.
So my opinion is that there are good reasons not to do anything if
freshening fails.
>>>> My answer is, we are not worse than freshening loose objects case
>>>> (especially since I took the idea from there).
>>>
>>> I do not think so, unfortunately. Loose object files with stale
>>> timestamps are not removed as long as objects are still reachable.
As the current index is read, which freshens its shared index file,
before a new index is created, the number of shared index files
doesn't go below 2. This can be seen in the tests changed in patch
19/21. So the risk of deleting interesting shared index files is quite
low in my opinion.
Also in general the split-index mode is useful when you often write
new indexes, and in this case shared index files that are used will
often be freshened, so the risk of deleting interesting shared index
files should be low.
>> But there are plenty of unreachable loose objects, added in index,
>> then got replaced with new versions. cache-tree can create loose trees
>> too and it's been run more often, behind user's back, to take
>> advantage of the shortcut in unpack-trees.
>
> I am not sure if I follow. Aren't objects reachable from the
> cache-tree in the index protected from gc?
>
> Not that I think freshening would actually fail in a repository
> where you can actually write into to update the index or its refs to
> make a difference (iow, even if we make it die() loudly when shared
> index cannot be "touched" because we are paranoid, no real life
> usage will trigger that die(), and if a repository does trigger the
> die(), I think you would really want to know about it).
As I wrote above, I think if we can actually write the shared index
file but its freshening fails, it probably means that the shared index
file has been removed behind us, and this case is equivalent as when
loose files have been removed behind us.
^ permalink raw reply
* Re: [RFC PATCH] Option to allow cherry-pick to skip empty commits
From: Junio C Hamano @ 2017-01-23 18:15 UTC (permalink / raw)
To: Giuseppe Bilotta; +Cc: Git List
In-Reply-To: <CAOxFTczut_CGGxmbrVFzhn_o4=StTOY6N1mEAw75Ro2Q4tzWgQ@mail.gmail.com>
Giuseppe Bilotta <giuseppe.bilotta@gmail.com> writes:
> By the way, I noticed going over the code that the -allow options are
> not stored, so that in case of interruption they will be reset, is
> this intentional or a bug?
I do not know offhand, but given the history of the two commands, I
would guess it was a bug simply overlooked when people bolted "a
series of commits" mode onto these commands that originally worked
only on a single commit.
^ permalink raw reply
* Re: [PATCH v1] travis-ci: fix Perforce install on macOS
From: Junio C Hamano @ 2017-01-23 18:22 UTC (permalink / raw)
To: larsxschneider; +Cc: git
In-Reply-To: <20170121144245.31702-1-larsxschneider@gmail.com>
larsxschneider@gmail.com writes:
> Could you fast track the patch to `maint` if it works without trouble on
> `next` (as it should!)?
>
> Notes:
> Base Commit: 787f75f056 (master)
I do not think there is any difference between 'maint' and 'master'
for this file right now, but I still would have appreciated if this
line also said 'maint', not 'master', if you want it to go to
'maint' eventually ;-) As https://travis-ci.org/git/git/builds seems
to be doing 'pu', too, hopefully we'll catch any issues there first.
Thanks.
> Diff on Web: https://github.com/larsxschneider/git/commit/ec7106339d
> Checkout: git fetch https://github.com/larsxschneider/git travisci/brew-perforce-fix-v1 && git checkout ec7106339d
>
> .travis.yml | 11 ++---------
> 1 file changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/.travis.yml b/.travis.yml
> index 3843967a69..c6ba8c8ec5 100644
> --- a/.travis.yml
> +++ b/.travis.yml
> @@ -75,20 +75,13 @@ before_install:
> popd
> ;;
> osx)
> - brew_force_set_latest_binary_hash () {
> - FORMULA=$1
> - SHA=$(brew fetch --force $FORMULA 2>&1 | grep ^SHA256: | cut -d ' ' -f 2)
> - sed -E -i.bak "s/sha256 \"[0-9a-f]{64}\"/sha256 \"$SHA\"/g" \
> - "$(brew --repository homebrew/homebrew-binary)/$FORMULA.rb"
> - }
> brew update --quiet
> brew tap homebrew/binary --quiet
> - brew_force_set_latest_binary_hash perforce
> - brew_force_set_latest_binary_hash perforce-server
> # Uncomment this if you want to run perf tests:
> # brew install gnu-time
> - brew install git-lfs perforce-server perforce gettext
> + brew install git-lfs gettext
> brew link --force gettext
> + brew install Caskroom/cask/perforce
> ;;
> esac;
> echo "$(tput setaf 6)Perforce Server Version$(tput sgr0)";
> --
> 2.11.0
^ permalink raw reply
* Re: [PATCH 2/3] stash: introduce push verb
From: Junio C Hamano @ 2017-01-23 18:27 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin
In-Reply-To: <20170121200804.19009-3-t.gummerer@gmail.com>
Thomas Gummerer <t.gummerer@gmail.com> writes:
> + stash_msg="$*"
> +
> + if test -z stash_msg
A dollar-sign is missing here, I think.
> + then
> + push_stash $push_options
> + else
> + push_stash $push_options -m "$stash_msg"
> + fi
> +}
^ permalink raw reply
* Re: [PATCH v1] travis-ci: fix Perforce install on macOS
From: Lars Schneider @ 2017-01-23 18:35 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqq8tq1tz49.fsf@gitster.mtv.corp.google.com>
> On 23 Jan 2017, at 19:22, Junio C Hamano <gitster@pobox.com> wrote:
>
> larsxschneider@gmail.com writes:
>
>> Could you fast track the patch to `maint` if it works without trouble on
>> `next` (as it should!)?
>>
>> Notes:
>> Base Commit: 787f75f056 (master)
>
> I do not think there is any difference between 'maint' and 'master'
> for this file right now, but I still would have appreciated if this
> line also said 'maint', not 'master', if you want it to go to
> 'maint' eventually ;-) As https://travis-ci.org/git/git/builds seems
> to be doing 'pu', too, hopefully we'll catch any issues there first.
You're right! My own automation betrayed me :-)
I made a tiny change, though. Can you queue v2?
http://public-inbox.org/git/20170122225550.28422-1-larsxschneider@gmail.com/
Thanks,
Lars
^ permalink raw reply
* Re: [RFC] Case insensitive Git attributes
From: Junio C Hamano @ 2017-01-23 18:35 UTC (permalink / raw)
To: Dakota Hawkins
Cc: Duy Nguyen, Johannes Schindelin, Stefan Beller, Lars Schneider,
git
In-Reply-To: <CAHnyXxRx_iagZhki1rmVEpw+GZPWBRx7mNmahs3pruy57L3h-Q@mail.gmail.com>
Dakota Hawkins <dakota@dakotahawkins.com> writes:
> Apologies for the delayed bump. I think because we're talking about
> affecting the behavior of .gitattributes that it would be better to
> have a distinct .gitattributes option, whether or not you also have a
> similar config option.
As I know I am on the To: line of the message I am responding to,
let me quicly let you know that I won't be responding to this thread
for a while as I don't recall what the discussion was about. I will
after I'll dig and find out what the thread was about but it won't
happen immediately. Sorry about that.
^ permalink raw reply
* Re: [PATCH] blame: add option to print tips (--tips)
From: Junio C Hamano @ 2017-01-23 18:36 UTC (permalink / raw)
To: Pranit Bauva; +Cc: Edmundo Carmona Antoranz, Git List
In-Reply-To: <CAFZEwPPx2vDJVf=uk0iUJ2sh9DxWwp2Lp1k-APz9n=7NYMN5uQ@mail.gmail.com>
Pranit Bauva <pranit.bauva@gmail.com> writes:
> We can probably make it useful with some extended efforts. I use
> git-blame and I sometimes find that I don't need things like the name
> of the author, time, timezone and not even the file name and I have to
> use a bigger terminal. If we could somehow remove those fields then
> maybe this would be a useful feature.
I admit that I didn't recall the option until somebody else told me,
but I think "blame -s" or something like that for that purpose ;-)
^ permalink raw reply
* Re: [PATCH v2] travis-ci: fix Perforce install on macOS
From: Junio C Hamano @ 2017-01-23 18:39 UTC (permalink / raw)
To: Lars Schneider; +Cc: git
In-Reply-To: <20170122225550.28422-1-larsxschneider@gmail.com>
Lars Schneider <larsxschneider@gmail.com> writes:
> The `perforce` and `perforce-server` package were moved from brew [1][2]
> to cask [3]. Teach TravisCI the new location.
>
> Perforce updates their binaries without version bumps. That made the
> brew install (legitimately!) fail due to checksum mismatches. The
> workaround is not necessary anymore as Cask [4] allows to disable the
> checksum test for individual formulas.
>
> [1] https://github.com/Homebrew/homebrew-binary/commit/1394e42de04d07445f82f9512627e864ff4ca4c6
> [2] https://github.com/Homebrew/homebrew-binary/commit/f8da22d6b8dbcfcfdb2dfa9ac1a5e5d8e05aac2b
> [3] https://github.com/caskroom/homebrew-cask/pull/29180
> [4] https://caskroom.github.io/
>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> ---
>
> Hi,
>
> this small update removes one more unnecessary line and makes the
> formula name lower case (no functional reason - just looks better ;-).
>
> @Junio: Do you prefer such a v2 as "--in-reply-to" to v1 or as separate
> thread? What eases your workflow?
It does not make that much of a difference if the second one comes
within a few days since the first one but in general it probably
would help if the follow-up is threaded _and_ made it clear upfront
that if somebody is reading v2 then v1 can be ignored ;-).
Thanks.
^ permalink raw reply
* Re: [RFC] Case insensitive Git attributes
From: Lars Schneider @ 2017-01-23 18:42 UTC (permalink / raw)
To: Junio C Hamano
Cc: Dakota Hawkins, Duy Nguyen, Johannes Schindelin, Stefan Beller,
git
In-Reply-To: <xmqqvat5sjym.fsf@gitster.mtv.corp.google.com>
> On 23 Jan 2017, at 19:35, Junio C Hamano <gitster@pobox.com> wrote:
>
> Dakota Hawkins <dakota@dakotahawkins.com> writes:
>
>> Apologies for the delayed bump. I think because we're talking about
>> affecting the behavior of .gitattributes that it would be better to
>> have a distinct .gitattributes option, whether or not you also have a
>> similar config option.
>
> As I know I am on the To: line of the message I am responding to,
> let me quicly let you know that I won't be responding to this thread
> for a while as I don't recall what the discussion was about. I will
> after I'll dig and find out what the thread was about but it won't
> happen immediately. Sorry about that.
Problem:
Git attributes for path names are generally case sensitive. However, on
a case insensitive file system (e.g. macOS/Windows) they appear to be
case insensitive (`*.bar` would match `foo.bar` and `foo.BAR`). That
works great until a Git users joins the party with a case sensitive file
system. For this Git user the attributes pattern only matches files with
the exact case (*.bar` would match only `foo.bar`).
Question/Proposal:
Could we introduce some flag to signal that certain attribute patterns
are always case insensitive?
Thread:
http://public-inbox.org/git/C83BE22D-EAC8-49E2-AEE3-22D4A99AE205@gmail.com/#t
Cheers,
Lars
^ permalink raw reply
* Re: [PATCH 3/3] stash: support filename argument
From: Junio C Hamano @ 2017-01-23 18:50 UTC (permalink / raw)
To: Thomas Gummerer
Cc: git, Stephan Beyer, Marc Strapetz, Jeff King, Johannes Schindelin
In-Reply-To: <20170121200804.19009-4-t.gummerer@gmail.com>
Thomas Gummerer <t.gummerer@gmail.com> writes:
> diff --git a/git-stash.sh b/git-stash.sh
> index d6b4ae3290..7dcce629bd 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -41,7 +41,7 @@ no_changes () {
> untracked_files () {
> excl_opt=--exclude-standard
> test "$untracked" = "all" && excl_opt=
> - git ls-files -o -z $excl_opt
> + git ls-files -o -z $excl_opt -- $1
Does $1 need to be quoted to prevent it from split at $IFS?
> @@ -56,6 +56,23 @@ clear_stash () {
> }
>
> create_stash () {
> + files=
> + while test $# != 0
> + do
> + case "$1" in
> + --)
> + shift
> + break
> + ;;
> + --files)
> + ;;
> + *)
> + files="$1 $files"
> + ;;
Hmph. What is this "no-op" option about? Did you mean to say
something like this instead?
case "$1" in
...
--file)
case $# in
1)
die "--file needs a pathspec" ;;
*)
shift
files="$files$1 " ;;
esac
;;
Another thing I noticed. We won't support filenames with embedded
$IFS characters at all?
I somehow had an impression that the script was carefully done
(e.g. by using -z option where appropriate) to add such a
limitation.
Perhaps we have broken it over time and it no longer matters
(i.e. there already may be existing breakages), but this troubles
me somehow.
By the way, in addition to "push" thing that corrects the argument
convention by requiring "-m" before the message, we need to correct
create_stash that is used internally from "stash push" somehow?
^ 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