* Re: [RFC 00/14] Allow fetch-pack to send ref names (globs allowed)
From: Stefan Beller @ 2017-01-26 22:15 UTC (permalink / raw)
To: Jonathan Tan, Jeff King; +Cc: git@vger.kernel.org
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
On Wed, Jan 25, 2017 at 2:02 PM, Jonathan Tan <jonathantanmy@google.com> wrote:
> Hello everyone - this is a proposal for a protocol change to allow the
> fetch-pack/upload-pack to converse in terms of ref names (globs
> allowed), and also an implementation of the server (upload-pack) and
> fetch-from-HTTP client (fetch-pack invoked through fetch).
>
> 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), and is a step towards eliminating the need for the server
> to send the list of refs first (possibly improving performance).
>
> The protocol is extended by allowing fetch-pack to send
> "want-ref <name>", where <name> is a full name (refs/...) [1], possibly
> including glob characters. Upload-pack will inform the client of the
> refs actually matched by sending "wanted-ref <SHA-1> <name>" before
> sending the last ACK or NAK.
I have reviewed the patches and think they are a good idea,
cc'ing Jeff who you linked to and who had some ideas about the protocol as well.
Stefan
^ permalink raw reply
* Re: [RFC 02/14] upload-pack: allow ref name and glob requests
From: Junio C Hamano @ 2017-01-26 22:23 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git
In-Reply-To: <d0d42b3bb4cf755f122591e191354c53848f197d.1485381677.git.jonathantanmy@google.com>
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.
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?
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).
> The server indicates that it supports this feature by advertising the
> capability "ref-in-want". Advertisement of this capability is by default
> disabled, but can be enabled through a configuration option.
OK.
> 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".
^ permalink raw reply
* Re: HEAD's reflog entry for a renamed branch
From: Jeff King @ 2017-01-26 21:53 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Kyle Meyer, git
In-Reply-To: <xmqqk29h33w1.fsf@gitster.mtv.corp.google.com>
On Thu, Jan 26, 2017 at 01:30:54PM -0800, Junio C Hamano wrote:
> > - "git branch -m" does seem to realize when we are renaming HEAD,
> > because it updates HEAD to point to the new branch name. But it
> > should probably insert another reflog entry mentioning the rename
> > (we do for "git checkout foo", even when "foo" has the same sha1 as
> > the current HEAD).
>
> This one I care less (not in the sense that I prefer it not done,
> but in the sense that I do not mind it is left unfixed than the
> other one you pointed out).
I wondered if it might affect how "git checkout -" works. But that
feature looks for reflogs like "checkout: moving from X to Y" to know to
move back to X. So we are fine here. Even though the HEAD reflog does
not show us going _to_ new-master, we would see it in a later entry as
"from new-master to Y". What we are missing is "rename from master to
new-master", but that entry does not matter. There is no "master" to
go back to anymore. :)
-Peff
^ permalink raw reply
* Re: [PATCH] git-bisect: allow running in a working tree subdirectory
From: Johannes Sixt @ 2017-01-26 21:46 UTC (permalink / raw)
To: marcandre.lureau; +Cc: git, chriscool
In-Reply-To: <20170126183030.28632-1-marcandre.lureau@redhat.com>
Am 26.01.2017 um 19:30 schrieb marcandre.lureau@redhat.com:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> It looks like it can do it.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> git-bisect.sh | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/git-bisect.sh b/git-bisect.sh
> index ae3cb013e..b0bd604d4 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -1,5 +1,6 @@
> #!/bin/sh
>
> +SUBDIRECTORY_OK=Yes
> USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'
> LONG_USAGE='git bisect help
> print this long help message.
>
Does it also work to drive git bisect from a subdirectory and pass a
file name (or pathspec) that is relative to that subdirectory rather
than relative to the root of the worktree? Can `git bisect good` or `git
bisect bad` of later bisection steps be invoked from different
subdirectories or the root?
-- Hannes
^ permalink raw reply
* Re: HEAD's reflog entry for a renamed branch
From: Junio C Hamano @ 2017-01-26 21:30 UTC (permalink / raw)
To: Jeff King; +Cc: Kyle Meyer, git
In-Reply-To: <20170126211205.5gz3zsrptop7n34n@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> It's unfortunate that there's no message. This is because the rename
> calls delete_ref() under the hood, but that function doesn't take a
> reflog message argument at all. It usually doesn't matter because
> deleting the ref will also delete the reflog.
>
> But as your example shows, deletions _can_ get logged in the HEAD
> reflog; the low-level ref code detects when HEAD points to the updated
> ref and logs an entry there, too.
> ...
> I'd say there are two potential improvements:
>
> - delete_ref() should take a reflog message argument, in case it
> updates the HEAD reflog (as a bonus, this will future-proof us for a
> day when we might keep reflogs for deleted refs).
This sounds sensible.
> - "git branch -m" does seem to realize when we are renaming HEAD,
> because it updates HEAD to point to the new branch name. But it
> should probably insert another reflog entry mentioning the rename
> (we do for "git checkout foo", even when "foo" has the same sha1 as
> the current HEAD).
This one I care less (not in the sense that I prefer it not done,
but in the sense that I do not mind it is left unfixed than the
other one you pointed out).
^ permalink raw reply
* Re: "git fetch -p" incorrectly deletes branches
From: Jeff King @ 2017-01-26 21:18 UTC (permalink / raw)
To: Reimar Döffinger; +Cc: git
In-Reply-To: <20170117060428.nanqz5lr4hi6dum6@reimardoeffinger.de>
On Tue, Jan 17, 2017 at 07:04:28AM +0100, Reimar Döffinger wrote:
> Deletes refs/heads/test every second time when run repeatedly:
>
> $ git fetch -p -v origin master:refs/heads/test
> From https://github.com/git/git
> * [new branch] master -> test
> = [up to date] master -> origin/master
> $ git fetch -p -v origin master:refs/heads/test
> From https://github.com/git/git
> - [deleted] (none) -> test
> = [up to date] master -> test
> = [up to date] master -> origin/master
Hmm. It seems like the problem is that "-p" is saying "the other side
does not have refs/heads/test; we must prune it". But I think it is
probably nonsense to apply pruning to a non-wildcard refspec.
> Also note that this behaviour appears also when fetch.prune=yes
> is set in the config (instead of -p on the command-line),
> which makes it much less obvious and there is no option to turn
> of prune just for that command to work-around this.
There is a separate issue of whether it is sane to apply fetch.prune to
a refspec given on the command line; I can imagine it as surprising, to
say the least.
I think "--no-prune" would disable it, though.
-Peff
^ permalink raw reply
* Re: [PATCH v3 4/4] urlmatch: allow globbing for the URL host part
From: Junio C Hamano @ 2017-01-26 21:12 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Patrick Steinhardt, Philip Oakley
In-Reply-To: <xmqq37g54kdz.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Junio C Hamano <gitster@pobox.com> writes:
>
>> + 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);
>> + ...
>> }
>>
>> + return 1;
>
> Embarrassing. The last one must be "have they both run out?" i.e.
>
> return (!url_len && !pat_len);
OK, here is my second try. The added test piece is to catch the
silly mistake I made above.
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index ec545e0929..33fd59fbb3 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1177,7 +1177,7 @@ test_expect_success 'urlmatch' '
test_cmp expect actual
'
-test_expect_success 'glob-based urlmatch' '
+test_expect_success 'urlmatch with wildcard' '
cat >.git/config <<-\EOF &&
[http]
sslVerify
@@ -1210,6 +1210,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 53ff972a60..0e007a3e07 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;
-
- url_tok = strtok_r(url, ".", &url_save);
- pat_tok = strtok_r(pat, ".", &pat_save);
-
- 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 (strcmp(url_tok, pat_tok)) {
- /* subdomains do not match */
- matching = 0;
- break;
- }
+ 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;
}
- /* 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)
^ permalink raw reply related
* Re: HEAD's reflog entry for a renamed branch
From: Jeff King @ 2017-01-26 21:12 UTC (permalink / raw)
To: Kyle Meyer; +Cc: git
In-Reply-To: <87pojmwq5y.fsf@kyleam.com>
On Mon, Jan 16, 2017 at 06:17:29PM -0500, Kyle Meyer wrote:
> I noticed that, after renaming the current branch, the corresponding
> message in .git/logs/HEAD is empty.
>
> For example, running
>
> $ mkdir test-repo
> $ cd test-repo
> $ git init
> $ echo abc >file.txt
> $ git add file.txt
> $ git commit -m"Add file.txt"
> $ git branch -m master new-master
Thanks for providing a simple reproduction recipe.
> resulted in the following reflogs:
>
> $ cat .git/logs/refs/heads/new-master
> 00000... 68730... Kyle Meyer <kyle@kyleam.com> 1484607020 -0500 commit (initial): Add file.txt
> 68730... 68730... Kyle Meyer <kyle@kyleam.com> 1484607020 -0500 Branch: renamed refs/heads/master to refs/heads/new-master
>
> $ cat .git/logs/HEAD
> 00000... 68730... Kyle Meyer <kyle@kyleam.com> 1484607020 -0500 commit (initial): Add file.txt
> 68730... 00000... Kyle Meyer <kyle@kyleam.com> 1484607020 -0500
>
> I expected the second line of .git/logs/HEAD to mirror the second line
> of .git/logs/refs/heads/new-master. Are the empty message and null sha1
> in HEAD's entry intentional?
The null sha1 is correct, I think. The branch we were on went away, and
we use the null sha1 to indicate "no value" in both the creation and
deletion cases.
It's unfortunate that there's no message. This is because the rename
calls delete_ref() under the hood, but that function doesn't take a
reflog message argument at all. It usually doesn't matter because
deleting the ref will also delete the reflog.
But as your example shows, deletions _can_ get logged in the HEAD
reflog; the low-level ref code detects when HEAD points to the updated
ref and logs an entry there, too.
You can see the same thing with a simpler example:
$ git init
$ git commit -m foo --allow-empty
$ git update-ref -d refs/heads/master
$ cat .git/logs/HEAD
00000... 8ffd1... Jeff King <peff@peff.net> 1485464779 -0500 commit (initial): foo
8ffd1... 00000... Jeff King <peff@peff.net> 1485464787 -0500
This doesn't come up much because most porcelain commands will refuse to
delete the current branch (notice I had to use "update-ref" and not
"branch -d").
I'd say there are two potential improvements:
- delete_ref() should take a reflog message argument, in case it
updates the HEAD reflog (as a bonus, this will future-proof us for a
day when we might keep reflogs for deleted refs).
- "git branch -m" does seem to realize when we are renaming HEAD,
because it updates HEAD to point to the new branch name. But it
should probably insert another reflog entry mentioning the rename
(we do for "git checkout foo", even when "foo" has the same sha1 as
the current HEAD).
-Peff
^ permalink raw reply
* Re: SubmittingPatches: drop temporal reference for PGP signing
From: Philip Oakley @ 2017-01-26 20:58 UTC (permalink / raw)
To: Junio C Hamano, Cornelius Weig
Cc: Stefan Beller, Johannes Sixt, bitte.keine.werbung.einwerfen, git,
thomas.braun, John Keeping
In-Reply-To: <xmqqpoj965yf.fsf@gitster.mtv.corp.google.com>
From: "Junio C Hamano" <gitster@pobox.com>
> Cornelius Weig <cornelius.weig@tngtech.com> writes:
>
>> How about something along these lines? Does the forward reference
>> break the main line of thought too severly?
>
> I find it a bit distracting for those who know PGP signing has
> nothing to do with signing off your patch, but I think that is OK
> because they are not the primary target audience of this part of the
> document.
Agreed. I this case the target audience was those weren't aware of that.
>
> I however am more worried that it may be misleading to mention these
> two in the same sentence. Those who skim these paragraphs without
> knowing the difference between the two may get a false impression
> that these two may somehow be related because they are mentioned in
> the same sentence.
>
> The retitling of section (5) you did, without any other change,
> might be sufficient. It may also help to be even more explicit in
> the updated title, i.e. s/by signing off/by adding Signed-off-by:/
Maybe even s/by signing off/by adding your Signed-off-by:/ to be sure that
the reader knows that it is _their certification_ that is being sought. Even
if it does double up on the 'your'.
>
> Thanks.
>
>> diff --git a/Documentation/SubmittingPatches
>> b/Documentation/SubmittingPatches
>> index 08352de..c2b0cbe 100644
>> --- a/Documentation/SubmittingPatches
>> +++ b/Documentation/SubmittingPatches
>> @@ -216,12 +216,12 @@ that it will be postponed.
>> Exception: If your mailer is mangling patches then someone may ask
>> you to re-send them using MIME, that is OK.
>>
>> -Do not PGP sign your patch, at least for now. Most likely, your
>> -maintainer or other people on the list would not have your PGP
>> -key and would not bother obtaining it anyway. Your patch is not
>> -judged by who you are; a good patch from an unknown origin has a
>> -far better chance of being accepted than a patch from a known,
>> -respected origin that is done poorly or does incorrect things.
>> +Do not PGP sign your patch, but do sign-off your work as explained in
>> (5).
>> +Most likely, your maintainer or other people on the list would not have
>> your
>> +PGP key and would not bother obtaining it anyway. Your patch is not
>> judged by
>> +who you are; a good patch from an unknown origin has a far better chance
>> of
>> +being accepted than a patch from a known, respected origin that is done
>> poorly
>> +or does incorrect things.
>>
>> If you really really really really want to do a PGP signed
>> patch, format it as "multipart/signed", not a text/plain message
>> @@ -246,7 +246,7 @@ patch.
>> *2* The mailing list: git@vger.kernel.org
>>
>>
>> -(5) Sign your work
>> +(5) Certify your work by signing off
>>
>> To improve tracking of who did what, we've borrowed the
>> "sign-off" procedure from the Linux kernel project on patches
>
^ permalink raw reply
* Re: Force Confirmation for Dropping Changed Lines
From: Junio C Hamano @ 2017-01-26 20:51 UTC (permalink / raw)
To: Hilco Wijbenga; +Cc: Jacob Keller, Git Users
In-Reply-To: <CAE1pOi0foJpZXSpHrbWqvOuG1+VoNKTCMjuLK5TCVcJuGMSOoQ@mail.gmail.com>
Hilco Wijbenga <hilco.wijbenga@gmail.com> writes:
> On 25 January 2017 at 18:32, Junio C Hamano <gitster@pobox.com> wrote:
>> I think you should be able to do something like
>>
>> $ cat >$HOME/bin/fail-3way <<\EOF
>> #!/bin/sh
>> git merge-file "$@"
>> exit 1
>> EOF
>> $ chmod +x $HOME/bin/fail-3way
>> $ cat >>$HOME/.gitconfig <<\EOF
>> [merge "fail"]
>> name = always fail 3-way merge
>> driver = $HOME/bin/fail-3way %A %O %B
>> recursive = text
>> EOF
>> $ echo pom.xml merge=fail >>.gitattributes
>>
>> to define a custom merge driver whose name is "fail", that runs the
>> fail-3way program, which runs the bog standard 3-way merge we use
>> (so that it will do the best-effort textual merge) but always return
>> with a non-zero status to signal that the result is conflicting and
>> needs manual resolution.
>
> Thank you, that's an improvement. :-) Unfortunately, it still
> completes the merge. Is there any way to simply get the
>>>>>>>>>/<<<<<<<< markers?
You can, but you need to write one yourself without relying on "git
merge-file", because the whole point of the 3way merge we implement
(including in that program) is "do not bother the user when both
sides made the same change."
^ permalink raw reply
* Re: [PATCH v3 4/4] urlmatch: allow globbing for the URL host part
From: Junio C Hamano @ 2017-01-26 20:49 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Patrick Steinhardt, Philip Oakley
In-Reply-To: <xmqq7f5h4kng.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> + 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);
> + ...
> }
>
> + return 1;
Embarrassing. The last one must be "have they both run out?" i.e.
return (!url_len && !pat_len);
^ permalink raw reply
* Re: [PATCH v3 4/4] urlmatch: allow globbing for the URL host part
From: Junio C Hamano @ 2017-01-26 20:43 UTC (permalink / raw)
To: Patrick Steinhardt; +Cc: git, Patrick Steinhardt, Philip Oakley
In-Reply-To: <20170125095648.4116-5-patrick.steinhardt@elego.de>
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/
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.
> . 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/`.
This is good as-is.
> . 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 923bfc5a2..ec545e092 100755
> --- a/t/t1300-repo-config.sh
> +++ b/t/t1300-repo-config.sh
> @@ -1177,6 +1177,42 @@ test_expect_success 'urlmatch' '
> test_cmp expect actual
> '
>
> +test_expect_success 'glob-based urlmatch' '
This is not "glob". A more generic term "wildcard" is OK.
> + cat >.git/config <<-\EOF &&
> + [http]
> + sslVerify
> ...
> +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;
> +
> + url_tok = strtok_r(url, ".", &url_save);
> + pat_tok = strtok_r(pat, ".", &pat_save);
Hmph, this will be the first use of strtok_r() in our codebase.
Does everybody have it?
For a use like this where your delimiter set is a singleton, it may
be simpler to do the usual strchrnul() or memchr() based loop. The
attached is my attempt to do so on top of this patch.
> +
> + 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 */
s/glob/asterisk/
Other than that, the patch looks OK.
diff --git a/urlmatch.c b/urlmatch.c
index 53ff972a60..8dfc7fd28a 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;
-
- url_tok = strtok_r(url, ".", &url_save);
- pat_tok = strtok_r(pat, ".", &pat_save);
-
- 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 (strcmp(url_tok, pat_tok)) {
- /* subdomains do not match */
- matching = 0;
- break;
- }
+ 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;
}
- /* matching if both URL and pattern are at their ends */
- matching = (url_tok == NULL && pat_tok == NULL);
-
- free(url);
- free(pat);
-
- return matching;
+ return 1;
}
static char *url_normalize_1(const char *url, struct url_info *out_info, char allow_globs)
^ permalink raw reply related
* Re: [PATCH] git-bisect: allow running in a working tree subdirectory
From: Junio C Hamano @ 2017-01-26 19:34 UTC (permalink / raw)
To: Stefan Beller
Cc: marcandre.lureau, Duy Nguyen, git@vger.kernel.org,
Christian Couder
In-Reply-To: <CAGZ79kZkxnoKCyk9teQnEPjsnS7iEorZALY4dXE8Fy78ifur7g@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> + Duy, main author of the worktree feature.
>
> On Thu, Jan 26, 2017 at 10:30 AM, <marcandre.lureau@redhat.com> wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> It looks like it can do it.
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> ---
I do not think the OP meant by "a working tree subdirectory" using
the command in a secondary worktree. SUBDIRECTORY_OK is about "can
the command be started in a subdirectory (as opposed to requiring to
be run only at the toplevel)?"
I am slightly negative on this change, though. The subdirectory you
are sitting in when you start your bisection may disappear and reappear
as you dig the history, and I do not think the code makes anything
special to prevent the disappearing current directory from getting
in the way of bisection process.
>> git-bisect.sh | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/git-bisect.sh b/git-bisect.sh
>> index ae3cb013e..b0bd604d4 100755
>> --- a/git-bisect.sh
>> +++ b/git-bisect.sh
>> @@ -1,5 +1,6 @@
>> #!/bin/sh
>>
>> +SUBDIRECTORY_OK=Yes
>> USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'
>> LONG_USAGE='git bisect help
>> print this long help message.
>> --
>> 2.11.0.295.gd7dffce1c.dirty
>>
^ permalink raw reply
* Re: [PATCH v2 3/3] connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config
From: Junio C Hamano @ 2017-01-26 19:27 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Segev Finer, Jeff King
In-Reply-To: <3d451f2c357a3fd7f0b0e4b427548553d7d05306.1485442231.git.johannes.schindelin@gmx.de>
Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index af2ae4cc02..f2c210f0a0 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -1949,6 +1949,13 @@ Environment variable settings always override any matches. The URLs that are
> matched against are those given directly to Git commands. This means any URLs
> visited as a result of a redirection do not participate in matching.
>
> +ssh.variant::
> + Override the autodetection of plink/tortoiseplink in the SSH
> + command that 'git fetch' and 'git push' use. It can be set to
> + either `ssh`, `plink`, `putty` or `tortoiseplink`. Any other
> + value will be treated as normal ssh. This is useful in case
> + that Git gets this wrong.
> +
I do like the fact that this now sits under ssh.* hierarchy (not core.*).
It should mention core.sshCommand, GIT_SSH_COMMAND, etc., because
reading this alone would mislead users to set only this one and expect
that their plink will be used without setting core.sshCommand etc.
It also should say that GIT_SSH_VARIANT environment variable will
override this variable.
> diff --git a/Documentation/git.txt b/Documentation/git.txt
> index 4f208fab92..c322558aa7 100644
> --- a/Documentation/git.txt
> +++ b/Documentation/git.txt
> @@ -1020,6 +1020,13 @@ Usually it is easier to configure any desired options through your
> personal `.ssh/config` file. Please consult your ssh documentation
> for further details.
>
> +`GIT_SSH_VARIANT`::
> + If this environment variable is set, it overrides the autodetection
> + of plink/tortoiseplink in the SSH command that 'git fetch' and 'git
> + push' use. It can be set to either `ssh`, `plink`, `putty` or
> + `tortoiseplink`. Any other value will be treated as normal ssh. This
> + is useful in case that Git gets this wrong.
Similarly this should mention GIT_SSH_COMMAND at least. It is crazy
to set something that will cause misdetection to core.sshCommand and
use this environment variable to fix it (instead of using ssh.variant),
so I think it is OK not to mention core.sshCommand (but it would not
hurt to do so).
> 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. Unlike the one I sent yesterday, this is
not overriding but is getting, so we should keep the original name
Segev gave it, which is shorter, I would think, rather than renaming
it to "handle".
The string that came from the configuration must be freed, no? That
is what I recall in Peff's comment yesterday.
> @@ -817,7 +835,8 @@ struct child_process *git_connect(int fd[2], const char *url,
> ssh_argv0 = xstrdup(ssh);
> }
>
> - if (ssh_argv0) {
> + if (!handle_ssh_variant(&port_option, &needs_batch) &&
> + ssh_argv0) {
I like the placement of this "if the user explicitly told us" much
better.
My patch yesterday was deliberately being lazy, because the next
thought that will come to us after comparing the two is "this way,
we avoid having to do the auto-detection altogether, which is way
better than wasting the effort to auto-detect, only to override".
And that reasoning will lead to a realization "there is no reason to
even do the split_cmdline() if the user explicitly told us". While
that reasoning is correct and we _should_ further refactor, I didn't
want to spend braincycles on that myself.
^ permalink raw reply
* Re: Force Confirmation for Dropping Changed Lines
From: Hilco Wijbenga @ 2017-01-26 19:19 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Jacob Keller, Git Users
In-Reply-To: <xmqqd1fa7dqf.fsf@gitster.mtv.corp.google.com>
On 25 January 2017 at 18:32, Junio C Hamano <gitster@pobox.com> wrote:
> I think you should be able to do something like
>
> $ cat >$HOME/bin/fail-3way <<\EOF
> #!/bin/sh
> git merge-file "$@"
> exit 1
> EOF
> $ chmod +x $HOME/bin/fail-3way
> $ cat >>$HOME/.gitconfig <<\EOF
> [merge "fail"]
> name = always fail 3-way merge
> driver = $HOME/bin/fail-3way %A %O %B
> recursive = text
> EOF
> $ echo pom.xml merge=fail >>.gitattributes
>
> to define a custom merge driver whose name is "fail", that runs the
> fail-3way program, which runs the bog standard 3-way merge we use
> (so that it will do the best-effort textual merge) but always return
> with a non-zero status to signal that the result is conflicting and
> needs manual resolution.
Thank you, that's an improvement. :-) Unfortunately, it still
completes the merge. Is there any way to simply get the
>>>>>>>>/<<<<<<<< markers?
^ permalink raw reply
* Re: [PATCH] Documentation: implement linkgit macro for Asciidoctor
From: Eric Wong @ 2017-01-26 19:18 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jeff King, brian m. carlson, git, Johannes Schindelin,
Øyvind A. Holm
In-Reply-To: <xmqq1svp7lcs.fsf@gitster.mtv.corp.google.com>
> Eric Wong <e@80x24.org> writes:
> > You can use '\' to continue long lines with any Ruby version:
> >
> > "<citerefentry>" \
> > "<refentrytitle>#{target}</refentrytitle>" \
> > "<manvolnum>#{attrs[1]}</manvolnum>" \
> > "</citerefentry>"
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"
^ permalink raw reply
* Re: [PATCH 2/2] fsck: lazily load types under --connectivity-only
From: Jeff King @ 2017-01-26 19:12 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqziid4puz.fsf@gitster.mtv.corp.google.com>
On Thu, Jan 26, 2017 at 10:51:00AM -0800, Junio C Hamano wrote:
> > 2. It serves as a cross-check that the coercion in (1a) is
> > correct (i.e., we'll complain about a parent link that
> > points to a blob). But we get most of this for free
> > already, because right after coercing, we'll parse any
> > non-blob objects. So we'd notice then if we expected a
> > commit and got a blob.
> >
> > The one exception is when we expect a blob, in which
> > case we never actually read the object contents.
> >
> > So this is a slight weakening, but given that the whole
> > point of --connectivity-only is to sacrifice some data
> > integrity checks for speed, this seems like an
> > acceptable tradeoff.
>
> The only weakening is that a non-blob (or a corrupt blob) object
> that sits where we expect a blob (because the containing tree or the
> tag says so) would not be diagnosed as an error, then? I think that
> is in line with the spirit of --connectivity-only and is a good
> trade-off.
Correct. The corrupt-blob case we always knew was a tradeoff (that's the
whole point of --connectivity-only). We could add back in the "we expect
a blob, is it really one?" at the moment we traverse to it, but IMHO
it's not interesting enough to even be worth the sha1_object_info()
lookup time.
-Peff
^ permalink raw reply
* Re: [PATCH 2/2] ref-filter: add function to parse atoms from a nul-terminated string
From: Junio C Hamano @ 2017-01-26 18:54 UTC (permalink / raw)
To: SZEDER Gábor; +Cc: git
In-Reply-To: <CAM0VKjntATMwDTdu1fSmjeLbwVe73iTo2NQizNXjZchBzqG44w@mail.gmail.com>
SZEDER Gábor <szeder.dev@gmail.com> writes:
> On Wed, Dec 7, 2016 at 5:09 PM, SZEDER Gábor <szeder.dev@gmail.com> wrote:
>> ref-filter's parse_ref_filter_atom() function parses an atom between
>> the start and end pointers it gets as arguments. This is fine for two
>> of its callers, which process '%(atom)' format specifiers and the end
>> pointer comes directly from strchr() looking for the closing ')'.
>> However, it's not quite so straightforward for its other two callers,
>> which process sort specifiers given as plain nul-terminated strings.
>> Especially not for ref_default_sorting(), which has the default
>> hard-coded as a string literal, but can't use it directly, because a
>> pointer to the end of that string literal is needed as well.
>> The next patch will add yet another caller using a string literal.
>>
>> Add a helper function around parse_ref_filter_atom() to parse an atom
>> up to the terminating nul, and call this helper in those two callers.
>>
>> Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
>> ---
>> ref-filter.c | 8 ++------
>> ref-filter.h | 4 ++++
>> 2 files changed, 6 insertions(+), 6 deletions(-)
>
> Ping?
>
> It looks like that this little two piece cleanup series fell on the floor.
I am still waiting for somebody else to comment on the changes, and
the final reroll after such a review discussion to address your own
comment in <CAM0VKjk1mnNzQX6LThq1t7keesBz_fjE9x2e0ywsBKSNKP9SCw@mail.gmail.com>
^ permalink raw reply
* Re: [PATCH] rebase: pass --signoff option to git am
From: Giuseppe Bilotta @ 2017-01-26 18:18 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Git List
In-Reply-To: <CAOxFTcyuLkvgPOxQuzaDUVuDRu_KJg=JrYtU84pQyjLstChbLg@mail.gmail.com>
On Mon, Jan 23, 2017 at 9:03 PM, Giuseppe Bilotta
<giuseppe.bilotta@gmail.com> wrote:
> On Mon, Jan 23, 2017 at 7:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> 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?
>
> AFAIK this is sufficient for both, in the sense that I've used it with
> git rebase -i and it works.
Hm, something very strange is going on, I've just tested the patch on
top of current next and for some reason the signoff line does not get
added. The command-line option gets passed to git am, but I get no
signoff for some reason, so something is failing down the line, I'll
have to investigate.
--
Giuseppe "Oblomov" Bilotta
^ permalink raw reply
* Re: [PATCH] mingw: allow hooks to be .exe files
From: Junio C Hamano @ 2017-01-26 18:44 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jeff King, git
In-Reply-To: <alpine.DEB.2.20.1701261321010.3469@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Hi Peff,
>
> 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.
^ permalink raw reply
* Re: [PATCH 2/2] fsck: lazily load types under --connectivity-only
From: Junio C Hamano @ 2017-01-26 18:51 UTC (permalink / raw)
To: Jeff King; +Cc: git
In-Reply-To: <20170126041206.5qfv7r7czbwfqvaa@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> The recent fixes to "fsck --connectivity-only" load all of
> the objects with their correct types. This keeps the
> connectivity-only code path close to the regular one, but it
> also introduces some unnecessary inefficiency. While getting
> the type of an object is cheap compared to actually opening
> and parsing the object (as the non-connectivity-only case
> would do), it's still not free.
>
> For reachable non-blob objects, we end up having to parse
> them later anyway (to see what they point to), making our
> type lookup here redundant.
>
> For unreachable objects, we might never hit them at all in
> the reachability traversal, making the lookup completely
> wasted. And in some cases, we might have quite a few
> unreachable objects (e.g., when alternates are used for
> shared object storage between repositories, it's normal for
> there to be objects reachable from other repositories but
> not the one running fsck).
>
> The comment in mark_object_for_connectivity() claims two
> benefits to getting the type up front:
>
> 1. We need to know the types during fsck_walk(). (And not
> explicitly mentioned, but we also need them when
> printing the types of broken or dangling commits).
>
> We can address this by lazy-loading the types as
> necessary. Most objects never need this lazy-load at
> all, because they fall into one of these categories:
>
> a. Reachable from our tips, and are coerced into the
> correct type as we traverse (e.g., a parent link
> will call lookup_commit(), which converts OBJ_NONE
> to OBJ_COMMIT).
>
> b. Unreachable, but not at the tip of a chunk of
> unreachable history. We only mention the tips as
> "dangling", so an unreachable commit which links
> to hundreds of other objects needs only report the
> type of the tip commit.
>
> 2. It serves as a cross-check that the coercion in (1a) is
> correct (i.e., we'll complain about a parent link that
> points to a blob). But we get most of this for free
> already, because right after coercing, we'll parse any
> non-blob objects. So we'd notice then if we expected a
> commit and got a blob.
>
> The one exception is when we expect a blob, in which
> case we never actually read the object contents.
>
> So this is a slight weakening, but given that the whole
> point of --connectivity-only is to sacrifice some data
> integrity checks for speed, this seems like an
> acceptable tradeoff.
The only weakening is that a non-blob (or a corrupt blob) object
that sits where we expect a blob (because the containing tree or the
tag says so) would not be diagnosed as an error, then? I think that
is in line with the spirit of --connectivity-only and is a good
trade-off.
^ permalink raw reply
* Re: [PATCH 4/5] revision.c: refactor ref selection handler after --exclude
From: Junio C Hamano @ 2017-01-26 18:43 UTC (permalink / raw)
To: Duy Nguyen; +Cc: Jeff King, Git Mailing List, Jacob Keller
In-Reply-To: <CACsJy8ATM_kc5SPY0dqprUefRy3vtpKW-4QEyJFK54jw0QgeJA@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
> On Thu, Jan 26, 2017 at 3:57 AM, Jeff King <peff@peff.net> wrote:
>> I don't think it means either. It means to include remotes in the
>> selected revisions, but excluding the entries mentioned by --exclude.
>>
>> IOW:
>>
>> --exclude=foo --remotes
>> include all remotes except refs/remotes/foo
>>
>> --exclude=foo --unrelated --remotes
>> same
>>
>> --exclude=foo --decorate-reflog --remotes
>> decorate reflogs of all remotes except "foo". Do _not_ use them
>> as traversal tips.
>>
>> --decorate-reflog --exclude=foo --remotes
>> same
>>
>> IOW, the ref-selector options build up until a group option is given,
>> which acts on the built-up options (over that group) and then resets the
>> built-up options. Doing "--unrelated" as above is orthogonal (though I
>> think in practice nobody would do that, because it's hard to read).
>
> This is because it makes sense to combine --exclude and
> --decorate-reflog. But what about a new --something that conflicts
> with either --exclude or --decorate-reflog?
I would think that "--exclude=foo --something --remotes"
* should be diagnosed as an error if "--something" is not compatible
with "--exclude";
* should take effect at the concluding "--remotes" if "--something"
is similar to "--decorate-reflog" whose effect ends at a
concluding --remotes/--branches/etc.; and
* should work independently if "--something" is neither.
^ permalink raw reply
* Re: [PATCH] git-bisect: allow running in a working tree subdirectory
From: Stefan Beller @ 2017-01-26 18:46 UTC (permalink / raw)
To: marcandre.lureau, Duy Nguyen; +Cc: git@vger.kernel.org, Christian Couder
In-Reply-To: <20170126183030.28632-1-marcandre.lureau@redhat.com>
+ Duy, main author of the worktree feature.
On Thu, Jan 26, 2017 at 10:30 AM, <marcandre.lureau@redhat.com> wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> It looks like it can do it.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> git-bisect.sh | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/git-bisect.sh b/git-bisect.sh
> index ae3cb013e..b0bd604d4 100755
> --- a/git-bisect.sh
> +++ b/git-bisect.sh
> @@ -1,5 +1,6 @@
> #!/bin/sh
>
> +SUBDIRECTORY_OK=Yes
> USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'
> LONG_USAGE='git bisect help
> print this long help message.
> --
> 2.11.0.295.gd7dffce1c.dirty
>
^ permalink raw reply
* Re: PATCH 1/2] abspath: add absolute_pathdup()
From: Brandon Williams @ 2017-01-26 18:32 UTC (permalink / raw)
To: René Scharfe; +Cc: Git List, Junio C Hamano
In-Reply-To: <d94d742d-1247-ac35-c081-7db1f2178d34@web.de>
On 01/26, René Scharfe wrote:
> Add a function that returns a buffer containing the absolute path of its
> argument and a semantic patch for its intended use. It avoids an extra
> string copy to a static buffer.
>
> Signed-off-by: Rene Scharfe <l.s.r@web.de>
> ---
> abspath.c | 7 +++++++
> cache.h | 1 +
> contrib/coccinelle/xstrdup_or_null.cocci | 6 ++++++
> 3 files changed, 14 insertions(+)
>
> diff --git a/abspath.c b/abspath.c
> index fce40fddcc..2f0c26e0e2 100644
> --- a/abspath.c
> +++ b/abspath.c
> @@ -239,6 +239,13 @@ const char *absolute_path(const char *path)
> return sb.buf;
> }
>
> +char *absolute_pathdup(const char *path)
> +{
> + struct strbuf sb = STRBUF_INIT;
> + strbuf_add_absolute_path(&sb, path);
> + return strbuf_detach(&sb, NULL);
> +}
> +
> /*
> * Unlike prefix_path, this should be used if the named file does
> * not have to interact with index entry; i.e. name of a random file
> diff --git a/cache.h b/cache.h
> index 00a029af36..d7b7a8cd7a 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1069,6 +1069,7 @@ const char *real_path(const char *path);
> const char *real_path_if_valid(const char *path);
> char *real_pathdup(const char *path);
> const char *absolute_path(const char *path);
> +char *absolute_pathdup(const char *path);
> const char *remove_leading_path(const char *in, const char *prefix);
> const char *relative_path(const char *in, const char *prefix, struct strbuf *sb);
> int normalize_path_copy_len(char *dst, const char *src, int *prefix_len);
> diff --git a/contrib/coccinelle/xstrdup_or_null.cocci b/contrib/coccinelle/xstrdup_or_null.cocci
> index 3fceef132b..8e05d1ca4b 100644
> --- a/contrib/coccinelle/xstrdup_or_null.cocci
> +++ b/contrib/coccinelle/xstrdup_or_null.cocci
> @@ -5,3 +5,9 @@ expression V;
> - if (E)
> - V = xstrdup(E);
> + V = xstrdup_or_null(E);
> +
> +@@
> +expression E;
> +@@
> +- xstrdup(absolute_path(E))
> ++ absolute_pathdup(E)
> --
> 2.11.0
>
These two patches look good to me.
--
Brandon Williams
^ permalink raw reply
* [PATCH] git-bisect: allow running in a working tree subdirectory
From: marcandre.lureau @ 2017-01-26 18:30 UTC (permalink / raw)
To: git; +Cc: chriscool, Marc-André Lureau
From: Marc-André Lureau <marcandre.lureau@redhat.com>
It looks like it can do it.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
git-bisect.sh | 1 +
1 file changed, 1 insertion(+)
diff --git a/git-bisect.sh b/git-bisect.sh
index ae3cb013e..b0bd604d4 100755
--- a/git-bisect.sh
+++ b/git-bisect.sh
@@ -1,5 +1,6 @@
#!/bin/sh
+SUBDIRECTORY_OK=Yes
USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]'
LONG_USAGE='git bisect help
print this long help message.
--
2.11.0.295.gd7dffce1c.dirty
^ permalink raw reply related
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