* Re: [PATCH] fetch/push: document that private data can be leaked
From: Junio C Hamano @ 2016-11-14 19:00 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git, Jeff King
In-Reply-To: <1479148088.2406.27.camel@mattmccutchen.net>
Matt McCutchen <matt@mattmccutchen.net> writes:
>> Yup, and then "do not push to untrustworthy place without checking
>> what you are pushing", too?
>
> If there is no private data in the repository, then there is no need
> for the user to check what they are pushing. As I've indicated before,
> IMO manually checking each push would not be a workable security
> measure in the long term anyway.
Then what is? Don't answer; this is a rhetorical question.
The answer is "do not push to untrustworthy place", if you are
unable to check what you are pushing.
^ permalink raw reply
* Re: [PATCH v3 5/6] grep: enable recurse-submodules to work on <tree> objects
From: Junio C Hamano @ 2016-11-14 18:56 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, sbeller, Brandon Williams
In-Reply-To: <c83066bc-5b2b-998c-7e22-c4fccbaba5de@google.com>
Jonathan Tan <jonathantanmy@google.com> writes:
>>> to:
>>> HEAD:file
>>> HEAD:sub/file
>>>
>>> Signed-off-by: Brandon Williams <bmwill@google.com>
>>> ---
>>
>> Unrelated tangent, but this makes readers wonder what the updated
>> trailer code would do to the last paragraph ;-). Does it behave
>> sensibly (with some sane definition of sensibleness)?
>>
>> I am guessing that it would, because neither To: or HEAD: is what we
>> normally recognize as a known trailer block element.
>
> Yes, it behaves sensibly :-) because "Signed-off-by:" is preceded by a
> blank line, so the trailer block consists only of that line.
Oh, that was not what I was wondering. Imagine Brandon writing his
message that ends in these three questionable lines and then running
"commit -s --amend" to add his sign-off---that was the case I was
wondering.
^ permalink raw reply
* Re: [PATCH v3 5/6] grep: enable recurse-submodules to work on <tree> objects
From: Jonathan Tan @ 2016-11-14 18:44 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, sbeller, Brandon Williams
In-Reply-To: <xmqqk2c6x79c.fsf@gitster.mtv.corp.google.com>
On 11/14/2016 10:10 AM, Junio C Hamano wrote:
> Brandon Williams <bmwill@google.com> writes:
>
>> Teach grep to recursively search in submodules when provided with a
>> <tree> object. This allows grep to search a submodule based on the state
>> of the submodule that is present in a commit of the super project.
>>
>> When grep is provided with a <tree> object, the name of the object is
>> prefixed to all output. In order to provide uniformity of output
>> between the parent and child processes the option `--parent-basename`
>> has been added so that the child can preface all of it's output with the
>> name of the parent's object instead of the name of the commit SHA1 of
>> the submodule. This changes output from the command
>> `git grep -e. -l --recurse-submodules HEAD` from:
>> HEAD:file
>> <commit sha1 of submodule>:sub/file
>>
>> to:
>> HEAD:file
>> HEAD:sub/file
>>
>> Signed-off-by: Brandon Williams <bmwill@google.com>
>> ---
>
> Unrelated tangent, but this makes readers wonder what the updated
> trailer code would do to the last paragraph ;-). Does it behave
> sensibly (with some sane definition of sensibleness)?
>
> I am guessing that it would, because neither To: or HEAD: is what we
> normally recognize as a known trailer block element.
Yes, it behaves sensibly :-) because "Signed-off-by:" is preceded by a
blank line, so the trailer block consists only of that line.
Having said that, it is probably better to indent those examples in the
commit message (by at least one space or one tab) - then they will never
be confused with trailers (once my patch set is in).
^ permalink raw reply
* [PATCH] doc: mention transfer data leaks in more places
From: Matt McCutchen @ 2016-11-14 18:20 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jeff King
In-Reply-To: <1479148088.2406.27.camel@mattmccutchen.net>
The "SECURITY" section of the gitnamespaces(7) man page described two
ways for a client to steal data from a server that wasn't intended to be
shared. Similar attacks can be performed by a server on a client, so
adapt the section to cover both directions and add it to the
git-fetch(1), git-pull(1), and git-push(1) man pages. Also add
references to this section from the documentation of server
configuration options that attempt to control data leakage but may not
be fully effective.
Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
---
Documentation/config.txt | 17 ++++++++++++++---
Documentation/git-fetch.txt | 2 ++
Documentation/git-pull.txt | 2 ++
Documentation/git-push.txt | 2 ++
Documentation/gitnamespaces.txt | 20 +-------------------
Documentation/transfer-data-leaks.txt | 30 ++++++++++++++++++++++++++++++
6 files changed, 51 insertions(+), 22 deletions(-)
create mode 100644 Documentation/transfer-data-leaks.txt
diff --git a/Documentation/config.txt b/Documentation/config.txt
index 21fdddf..fc2cf83 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -2898,6 +2898,11 @@ is omitted from the advertisements but `refs/heads/master` and
`refs/namespaces/bar/refs/heads/master` are still advertised as so-called
"have" lines. In order to match refs before stripping, add a `^` in front of
the ref name. If you combine `!` and `^`, `!` must be specified first.
++
+Even if you hide refs, a client may still be able to steal the target
+objects via the techniques described in the "SECURITY" section of the
+linkgit:gitnamespaces[7] man page; it's best to keep private data in a
+separate repository.
transfer.unpackLimit::
When `fetch.unpackLimit` or `receive.unpackLimit` are
@@ -2907,7 +2912,7 @@ transfer.unpackLimit::
uploadarchive.allowUnreachable::
If true, allow clients to use `git archive --remote` to request
any tree, whether reachable from the ref tips or not. See the
- discussion in the `SECURITY` section of
+ discussion in the "SECURITY" section of
linkgit:git-upload-archive[1] for more details. Defaults to
`false`.
@@ -2921,13 +2926,19 @@ uploadpack.allowTipSHA1InWant::
When `uploadpack.hideRefs` is in effect, allow `upload-pack`
to accept a fetch request that asks for an object at the tip
of a hidden ref (by default, such a request is rejected).
- see also `uploadpack.hideRefs`.
+ See also `uploadpack.hideRefs`. Even if this is false, a client
+ may be able to steal objects via the techniques described in the
+ "SECURITY" section of the linkgit:gitnamespaces[7] man page; it's
+ best to keep private data in a separate repository.
uploadpack.allowReachableSHA1InWant::
Allow `upload-pack` to accept a fetch request that asks for an
object that is reachable from any ref tip. However, note that
calculating object reachability is computationally expensive.
- Defaults to `false`.
+ Defaults to `false`. Even if this is false, a client may be able
+ to steal objects via the techniques described in the "SECURITY"
+ section of the linkgit:gitnamespaces[7] man page; it's best to
+ keep private data in a separate repository.
uploadpack.keepAlive::
When `upload-pack` has started `pack-objects`, there may be a
diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
index 9e42169..b153aef 100644
--- a/Documentation/git-fetch.txt
+++ b/Documentation/git-fetch.txt
@@ -192,6 +192,8 @@ The first command fetches the `maint` branch from the repository at
objects will eventually be removed by git's built-in housekeeping (see
linkgit:git-gc[1]).
+include::transfer-data-leaks.txt[]
+
BUGS
----
Using --recurse-submodules can only fetch new commits in already checked
diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index d033b25..4470e4b 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -237,6 +237,8 @@ If you tried a pull which resulted in complex conflicts and
would want to start over, you can recover with 'git reset'.
+include::transfer-data-leaks.txt[]
+
BUGS
----
Using --recurse-submodules can only fetch new commits in already checked
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index 47b77e6..8eefabd 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -559,6 +559,8 @@ Commits A and B would no longer belong to a branch with a symbolic name,
and so would be unreachable. As such, these commits would be removed by
a `git gc` command on the origin repository.
+include::transfer-data-leaks.txt[]
+
GIT
---
Part of the linkgit:git[1] suite
diff --git a/Documentation/gitnamespaces.txt b/Documentation/gitnamespaces.txt
index 7685e36..b614969 100644
--- a/Documentation/gitnamespaces.txt
+++ b/Documentation/gitnamespaces.txt
@@ -61,22 +61,4 @@ For a simple local test, you can use linkgit:git-remote-ext[1]:
git clone ext::'git --namespace=foo %s /tmp/prefixed.git'
----------
-SECURITY
---------
-
-Anyone with access to any namespace within a repository can potentially
-access objects from any other namespace stored in the same repository.
-You can't directly say "give me object ABCD" if you don't have a ref to
-it, but you can do some other sneaky things like:
-
-. Claiming to push ABCD, at which point the server will optimize out the
- need for you to actually send it. Now you have a ref to ABCD and can
- fetch it (claiming not to have it, of course).
-
-. Requesting other refs, claiming that you have ABCD, at which point the
- server may generate deltas against ABCD.
-
-None of this causes a problem if you only host public repositories, or
-if everyone who may read one namespace may also read everything in every
-other namespace (for instance, if everyone in an organization has read
-permission to every repository).
+include::transfer-data-leaks.txt[]
diff --git a/Documentation/transfer-data-leaks.txt b/Documentation/transfer-data-leaks.txt
new file mode 100644
index 0000000..914bacc
--- /dev/null
+++ b/Documentation/transfer-data-leaks.txt
@@ -0,0 +1,30 @@
+SECURITY
+--------
+The fetch and push protocols are not designed to prevent one side from
+stealing data from the other repository that was not intended to be
+shared. If you have private data that you need to protect from a malicious
+peer, your best option is to store it in another repository. This applies
+to both clients and servers. In particular, namespaces on a server are not
+effective for read access control; you should only grant read access to a
+namespace to clients that you would trust with read access to the entire
+repository.
+
+The known attack vectors are as follows:
+
+. The victim sends "have" lines advertising the IDs of objects it has that
+ are not explicitly intended to be shared but can be used to optimize the
+ transfer if the peer also has them. The attacker chooses an object ID X
+ to steal and sends a ref to X, but isn't required to send the content of
+ X because the victim already has it. Now the victim believes that the
+ attacker has X, and it sends the content of X back to the attacker
+ later. (This attack is most straightforward for a client to perform on a
+ server, by creating a ref to X in the namespace the client has access
+ to and then fetching it. The most likely way for a server to perform it
+ on a client is to "merge" X into a public branch and hope that the user
+ does additional work on this branch and pushes it back to the server
+ without noticing the merge.)
+
+. As in #1, the attacker chooses an object ID X to steal. The victim sends
+ an object Y that the attacker already has, and the attacker falsely
+ claims to have X and not Y, so the victim sends Y as a delta against X.
+ The delta reveals regions of X that are similar to Y to the attacker.
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] fetch/push: document that private data can be leaked
From: Matt McCutchen @ 2016-11-14 18:28 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git, Jeff King
In-Reply-To: <xmqq1syezs3g.fsf@gitster.mtv.corp.google.com>
On Sun, 2016-11-13 at 18:57 -0800, Junio C Hamano wrote:
> Matt McCutchen <matt@mattmccutchen.net> writes:
>
> >
> > Documentation/fetch-push-security.txt | 9 +++++++++
>
> A new (consolidated) piece like this that can be included in
> multiple places is a good idea. I wonder if the original
> description in "namespaces" thing can be moved here and then
> "namespaces" page can be made to also borrow from this?
I gave this a try. New patch coming.
> > --- /dev/null
> > +++ b/Documentation/fetch-push-security.txt
> > @@ -0,0 +1,9 @@
> > +SECURITY
> > +--------
> > +The fetch and push protocols are not designed to prevent a
> > malicious
> > +server from stealing data from your repository that you did not
> > intend to
> > +share. The possible attacks are similar to the ones described in
> > the
> > +"SECURITY" section of linkgit:gitnamespaces[7]. If you have
> > private data
> > +that you need to protect from the server, keep it in a separate
> > +repository.
>
> Yup, and then "do not push to untrustworthy place without checking
> what you are pushing", too?
If there is no private data in the repository, then there is no need
for the user to check what they are pushing. As I've indicated before,
IMO manually checking each push would not be a workable security
measure in the long term anyway.
Matt
^ permalink raw reply
* Re: [PATCH] remote-curl: don't hang when a server dies before any output
From: Jeff King @ 2016-11-14 18:24 UTC (permalink / raw)
To: David Turner; +Cc: git, spearce
In-Reply-To: <1478729910-26232-1-git-send-email-dturner@twosigma.com>
On Wed, Nov 09, 2016 at 05:18:30PM -0500, David Turner wrote:
> In the event that a HTTP server closes the connection after giving a
> 200 but before giving any packets, we don't want to hang forever
> waiting for a response that will never come. Instead, we should die
> immediately.
I agree we don't want to hang forever, but this leaves open the
question: what is hanging?
My guess is that fetch-pack is waiting for more data from the server,
and remote-curl is waiting for fetch-pack to tell us what to send for
the next request. Neither will make forward progress because they are
effectively waiting on each other.
Which means this is likely a special case of malformed input from the
server. A server which likewise sends a partial response could end up in
the same deadlock, I would think (e.g., a half-finished pktline, or a
pktline but no trailing flush).
That doesn't make it wrong to fix this specific case (especially if it's
a common one), but I wonder if we could do better.
The root of the issue is that only fetch-pack understands the protocol,
and remote-curl is blindly proxying the data. But only remote-curl knows
that the HTTP request has ended, and it doesn't relay that information
to fetch-pack. So I can think of two solutions:
1. Some way of remote-curl communicating the EOF to fetch-pack. It
can't just close the descriptor, since we need to pass more data
over it for the followup requests. You'd need something
out-of-band, or to frame the HTTP data inside another layer of
pktlines, both of which are kind of gross.
2. Have remote-curl understand enough of the protocol that it can
abort rather than hang.
I think that's effectively the approach of your patch, but for one
specific case. But could we, for example, make sure that everything
we proxy is a complete set of pktlines and ends with a flush? And
if not, then we hang up on fetch-pack.
I _think_ that would work, because even the pack is always encased
in pktlines for smart-http.
> @@ -659,6 +662,8 @@ static int post_rpc(struct rpc_state *rpc)
> curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, rpc_in);
> curl_easy_setopt(slot->curl, CURLOPT_FILE, rpc);
>
> +
> + rpc->any_written = 0;
Extra blank line here?
> @@ -667,6 +672,9 @@ static int post_rpc(struct rpc_state *rpc)
> if (err != HTTP_OK)
> err = -1;
>
> + if (!rpc->any_written)
> + err = -1;
> +
I wondered if there were any cases where it was normal for the server to
return zero bytes. Possibly the ref advertisement is one, but this is
_just_ handling post_rpc(), so that's OK. And I think by definition
every response has to at least return a flush packet, or we would make
no forward progress (i.e., the exact case you are dealing with here).
-Peff
^ permalink raw reply
* Re: [PATCH v3 5/6] grep: enable recurse-submodules to work on <tree> objects
From: Junio C Hamano @ 2016-11-14 18:10 UTC (permalink / raw)
To: jonathantanmy; +Cc: git, sbeller, Brandon Williams
In-Reply-To: <1478908273-190166-6-git-send-email-bmwill@google.com>
Brandon Williams <bmwill@google.com> writes:
> Teach grep to recursively search in submodules when provided with a
> <tree> object. This allows grep to search a submodule based on the state
> of the submodule that is present in a commit of the super project.
>
> When grep is provided with a <tree> object, the name of the object is
> prefixed to all output. In order to provide uniformity of output
> between the parent and child processes the option `--parent-basename`
> has been added so that the child can preface all of it's output with the
> name of the parent's object instead of the name of the commit SHA1 of
> the submodule. This changes output from the command
> `git grep -e. -l --recurse-submodules HEAD` from:
> HEAD:file
> <commit sha1 of submodule>:sub/file
>
> to:
> HEAD:file
> HEAD:sub/file
>
> Signed-off-by: Brandon Williams <bmwill@google.com>
> ---
Unrelated tangent, but this makes readers wonder what the updated
trailer code would do to the last paragraph ;-). Does it behave
sensibly (with some sane definition of sensibleness)?
I am guessing that it would, because neither To: or HEAD: is what we
normally recognize as a known trailer block element.
^ permalink raw reply
* Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)
From: Junio C Hamano @ 2016-11-14 18:01 UTC (permalink / raw)
To: Michael J Gruber
Cc: Johannes Schindelin, Jacob Keller, Dennis Kaarsemaker,
Git mailing list
In-Reply-To: <0c39be16-76f8-0800-41a2-b7b1dccdd652@drmicha.warpmail.net>
Michael J Gruber <git@drmicha.warpmail.net> writes:
> *My* idea of --no-index was for it to behave as similar to the
> --index-version as possible, regarding formatting etc., and to be a good
> substitute for ordinary diff. The proposed patch achieves exactly that -
Does it? It looks to me that it does a lot more.
> why should a *file* argument (which is not a pathspec in --no-index
> mode) not be treated in the same way in which every other command treats
> a file argument? The patch un-breaks the most natural expectation.
I think a filename given as a command line argument, e.g. <(cmd), is
now treated more sensibly with [2/2]. Something that is not a
directory to be descended into and is not a regular file needs to be
made into a form that we can use as a blob, and reading it into an
in-core buffer is a workable way to do so.
However, when taken together with [1/2], doesn't the proposed patch
"achieves" a lot more than "exactly that", namely, by not treating
symbolic links discovered during traversals of directories given
from the command line as such and dereferencing?
^ permalink raw reply
* Re: [PATCH v3 6/6] grep: search history of moved submodules
From: Brandon Williams @ 2016-11-14 17:43 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org, Jonathan Tan, Junio C Hamano
In-Reply-To: <CAGZ79kbf2i5s8Y84i2Wehbffsw1dUDUY6LYPEMME3vC6zo8-aw@mail.gmail.com>
On 11/11, Stefan Beller wrote:
> On Fri, Nov 11, 2016 at 3:51 PM, Brandon Williams <bmwill@google.com> wrote:
>
> > +
> > + rm -rf parent sub
>
> This line sounds like a perfect candidate for "test_when_finished"
> at the beginning of the test
K will do.
--
Brandon Williams
^ permalink raw reply
* Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables
From: Junio C Hamano @ 2016-11-14 17:21 UTC (permalink / raw)
To: Lars Schneider; +Cc: Johannes Schindelin, Johannes Sixt, git, Jeff King
In-Reply-To: <0BEC2674-20B5-4AD1-851A-97CA34C0CE7F@gmail.com>
Lars Schneider <larsxschneider@gmail.com> writes:
>> On 13 Nov 2016, at 02:13, Junio C Hamano <gitster@pobox.com> wrote:
>> ...
>> Earlier you said 'pu' did even not build, but do we know where this
>> "still times out" comes from? As long as I don't merge anything
>> prematurely, which I need to be careful about, it shouldn't impact
>> the upcoming release, but we'd need to figure it out before moving
>> things forward post release.
>
> What is the goal for 'pu'?
>
> (1) Builds clean on all platforms + passes all tests
> (2) Builds clean on all platforms
> (3) Builds clean on Linux
> (4) Just makes new topics easily available to a broader audience
>
> My understanding was always (4) but the discussion above sounds
> more like (1) or (2)?
The purpose of 'pu' is none of the above, but its intended effect
for people other than me is (4). It is primarily meant as a way for
me to avoid having to go back to the mailing list archive to find
topics that I felt that were potentially interesting but not yet
ready and may want to get commented on later. When queued on 'pu',
it is not unusual that I haven't really looked at the patches yet,
and it is not surprising if it does not build on any platform.
When queued to 'pu', the reason of the initial "not yet ready"
assessment may not have anything to do with the quality of the
patches but based on the phase of the development (e.g. during a
feature-freeze, it is less efficient use of our time to take new
topics to 'next'), so what was queued on 'pu' may get merged to
'next' without any update, after getting looked at by me or by
other people and deemed to be worth trying out.
Dscho's mention of 'still times out' may be an indiciation that
something unspecified on 'pu' is not ready to be merged to 'next',
but blocking all of 'pu' with a blanket statement is not useful,
and that was where my response comes from. We need to know more
to say "this particular topic is not ready", so that we can unblock
other innocent topics.
^ permalink raw reply
* Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables
From: Jeff King @ 2016-11-14 17:01 UTC (permalink / raw)
To: Torsten Bögershausen
Cc: Lars Schneider, Junio C Hamano, Johannes Schindelin,
Johannes Sixt, git
In-Reply-To: <35d3a07d-5d2f-aedd-94bc-4d92e5aa4661@web.de>
On Mon, Nov 14, 2016 at 05:35:56PM +0100, Torsten Bögershausen wrote:
> > What is the goal for 'pu'?
>
> > (1) Builds clean on all platforms + passes all tests
> Yes
> > (2) Builds clean on all platforms
> Yes
> > (3) Builds clean on Linux
> Yes
> > (4) Just makes new topics easily available to a broader audience
> Yes
I'd have answered differently, though I think in the end we agree on the
outcome.
I think the only thing that matters is (4). It _usually_ builds and
passes all tests, but not always. But the point is that nobody should
care in particular about "pu". What we care about is whether the
individual topics will build and pass before they are merged to master
(or even "next").
So "pu" is a tool, because you can test all of the topics at once and
find out early if there are any problems. And it's good to investigate
problems there before topics hit next (though they are also often caught
in review, or by people trying the broken topic on their various
platforms, or sometimes Junio even pushes out a known-broken state in pu
and mentions it in "What's cooking").
So yes, it should do all of those things, but we don't necessarily
expect that it will never be broken. That's expected to happen from time
to time, and the purpose of the branch. With respect to Lars' original
point:
> > Git 'pu' does not compile on macOS right now:
> > builtin/bisect--helper.c:299:6: error: variable 'good_syn' is used uninitialized
> > whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
The next step is to make sure that the topic author is aware (in this
case, one assumes it's pb/bisect).
Better still is to make a patch that can either be applied on top, or
squashed as appropriate. I know that Ramsay Jones does this, for
example, with some of his sparse-related checks, and I'm pretty sure
from the turnaround-time that he runs it against "pu".
-Peff
^ permalink raw reply
* Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)
From: Johannes Schindelin @ 2016-11-14 16:40 UTC (permalink / raw)
To: Michael J Gruber
Cc: Jacob Keller, Junio C Hamano, Dennis Kaarsemaker,
Git mailing list
In-Reply-To: <0c39be16-76f8-0800-41a2-b7b1dccdd652@drmicha.warpmail.net>
Hi Michael,
On Mon, 14 Nov 2016, Michael J Gruber wrote:
> why should a *file* argument (which is not a pathspec in --no-index
> mode) not be treated in the same way in which every other command treats
> a file argument?
We are talking about `<(command)` here, which is most distinctly not a
file argument at all.
Ciao,
Johannes
^ permalink raw reply
* Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables
From: Torsten Bögershausen @ 2016-11-14 16:35 UTC (permalink / raw)
To: Lars Schneider, Junio C Hamano
Cc: Johannes Schindelin, Johannes Sixt, git, Jeff King
In-Reply-To: <0BEC2674-20B5-4AD1-851A-97CA34C0CE7F@gmail.com>
On 14.11.16 10:11, Lars Schneider wrote:
>
>> On 13 Nov 2016, at 02:13, Junio C Hamano <gitster@pobox.com> wrote:
>>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>>
>>>> Thanks. Dscho, does this fix both of these issues to you?
>>>
>>> Apparently it does because the CI jobs for `master` and for `next` pass.
>>
>> OK, thanks for a quick confirmation.
>>
>>> The one for `pu` still times out, of course.
>>
>> Earlier you said 'pu' did even not build, but do we know where this
>> "still times out" comes from? As long as I don't merge anything
>> prematurely, which I need to be careful about, it shouldn't impact
>> the upcoming release, but we'd need to figure it out before moving
>> things forward post release.
>
> What is the goal for 'pu'?
>
> (1) Builds clean on all platforms + passes all tests
Yes
> (2) Builds clean on all platforms
Yes
> (3) Builds clean on Linux
Yes
> (4) Just makes new topics easily available to a broader audience
Yes
>
> My understanding was always (4) but the discussion above sounds
> more like (1) or (2)?
All commits should work on all platforms - in the ideal world there is no problem.
From time to time things sneak in, which are not portable.
(in the sense that not all "supported" compile/run tests without breakages)
And if everybody reports breakages and problems found on the pu
branch, there is a good chance that they don't reach next or master.
Does this make sense ?
>
> --
>
> Git 'pu' does not compile on macOS right now:
> builtin/bisect--helper.c:299:6: error: variable 'good_syn' is used uninitialized
> whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
> ...
>
> More info here:
> https://api.travis-ci.org/jobs/175417712/log.txt?deansi=true
>
> --
>
> Cheers,
> Lars
>
^ permalink raw reply
* Re: GIT Problem/ISSUE
From: Konstantin Khomoutov @ 2016-11-14 16:22 UTC (permalink / raw)
To: Robert Fellendorf; +Cc: git
In-Reply-To: <a3751faa-91b5-2ce8-767a-a25e25f23433@googlemail.com>
On Mon, 14 Nov 2016 16:59:41 +0100
Robert Fellendorf <robert.fellendorf@googlemail.com> wrote:
[...]
> Couldn't resolve host 'gitlab.lrz.de'
[...]
So, what happens when you open a console prompt (Click the Start menu
icon, type "cmd" without the double quotes and activate the application
found), type
ping gitlab.lrz.de
there and hit the Enter key?
Does it successfully sends three network packets or you do get the
same/similar error message about the hostname "gitlab.lrz.de" being not
resolvable?
As it stands, this issue looks completely unrelated to Git -- please
read the Wikipedia page on the Domain Name System (DNS) for a start.
^ permalink raw reply
* RE: [PATCH v2] upload-pack: Optionally allow fetching any sha1
From: David Turner @ 2016-11-14 16:02 UTC (permalink / raw)
To: 'Junio C Hamano'; +Cc: git@vger.kernel.org, spearce@spearce.org
In-Reply-To: <xmqqpom0yxyo.fsf@gitster.mtv.corp.google.com>
Sorry about that -- the first version of this patch noted:
"""This one is on top of yesterday's patch, "remote-curl: don't hang when
a server dies before any output".
That's because I want my test to show that allowanysha1inhead allows a
fetch to succeed where allowreachablesha1inhead would fail. Prior to
the previous patch, the first fetch's failure would instead be a hang.""""
But I didn't carry over this message to v2.
> -----Original Message-----
> From: Junio C Hamano [mailto:gitster@pobox.com]
> Sent: Saturday, November 12, 2016 8:23 PM
> To: David Turner
> Cc: git@vger.kernel.org; spearce@spearce.org
> Subject: Re: [PATCH v2] upload-pack: Optionally allow fetching any sha1
>
> David Turner <dturner@twosigma.com> writes:
>
> > diff --git a/t/t5551-http-fetch-smart.sh b/t/t5551-http-fetch-smart.sh
> > index 43665ab..8d3db40 100755
>
> It seems that I haven't heard of 43665ab.
>
> > --- a/t/t5551-http-fetch-smart.sh
> > +++ b/t/t5551-http-fetch-smart.sh
> > @@ -306,6 +306,28 @@ test_expect_success 'test allowreachablesha1inwant
> with unreachable' '
> > test_must_fail git -C test_reachable.git fetch origin "$(git rev-
> parse HEAD)"
> > '
>
> Specifically, the above seems to be missing in my tree.
>
> Perhaps you noticed the lack of test for allowReachableSHA1InWant and
> added one, but forgot to send it out, while building this patch on top?
>
> > +uploadpack.allowAnySHA1InWant::
> > + Allow `upload-pack` to accept a fetch request that asks for any
> > + object at all.
> > + Defaults to `false`.
^ permalink raw reply
* GIT Problem/ISSUE
From: Robert Fellendorf @ 2016-11-14 15:59 UTC (permalink / raw)
To: git
Dear Git Team,
I'm having some trouble with my git software. I just would like to
'pull' a project out of a repository.
At the beginning git worked just fine but since a few days ago I'm
constantly getting the error:
Couldn't resolve host 'gitlab.lrz.de'
git did not exit cleanly (exit code 128) (391 ms @ 14.11.2016 16:53:37)
To my architecture: I'm having a Windows 10 Pro Version with the Intel
U6600 processor.
I have Kaspersky running (But, indeed, I also tried it to shut this
software down while testing git - additionally I killed all tasks which
could considered to be a firewall)
Furthermore, I tried to use the ssh protocoll which did'nt worked out,
as well.
I added a ssh key changed the environment variable. Didn't worked as well.
I have a direct link to the internet (in particular I'm not accessing
the internet via a proxy)
A reinstallation of the software did't helped neither.
For your help I would like to thank you already in advance.
Kind Regards
Robert Fellendorf
^ permalink raw reply
* Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)
From: Michael J Gruber @ 2016-11-14 15:31 UTC (permalink / raw)
To: Johannes Schindelin, Jacob Keller
Cc: Junio C Hamano, Dennis Kaarsemaker, Git mailing list
In-Reply-To: <alpine.DEB.2.20.1611121106110.3746@virtualbox>
Johannes Schindelin venit, vidit, dixit 12.11.2016 11:08:
> Hi,
>
> On Fri, 11 Nov 2016, Jacob Keller wrote:
>
>> On Fri, Nov 11, 2016 at 1:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
>>>
>>>> No tests or documentation updates yet, and I'm not sure whether
>>>> --follow-symlinks in other modes than --no-index should be supported, ignored
>>>> (as it is now) or cause an error, but I'm leaning towards the third option.
>>>
>>> My knee-jerk reaction is:
>>>
>>> * The --no-index mode should default to your --follow-symlinks
>>> behaviour, without any option to turn it on or off.
>>>
>>
>> I agree. We shouldn't have to specify this for no-index.
>
> Ummm. *My* idea of --no-index was for it to behave as similar to the
> --index version as possible. For example when comparing directories
> containing symlinks. You seem intent on breaking this scenario.
*My* idea of --no-index was for it to behave as similar to the
--index-version as possible, regarding formatting etc., and to be a good
substitute for ordinary diff. The proposed patch achieves exactly that -
why should a *file* argument (which is not a pathspec in --no-index
mode) not be treated in the same way in which every other command treats
a file argument? The patch un-breaks the most natural expectation.
Michael
^ permalink raw reply
* Re: [PATCH v2] diffcore-delta: remove unused parameter to diffcore_count_changes()
From: Jeff King @ 2016-11-14 14:02 UTC (permalink / raw)
To: Tobias Klauser; +Cc: git, gitster
In-Reply-To: <20161114133905.6632-1-tklauser@distanz.ch>
On Mon, Nov 14, 2016 at 02:39:05PM +0100, Tobias Klauser wrote:
> The delta_limit parameter to diffcore_count_changes() has been unused
> since commit ba23bbc8e ("diffcore-delta: make change counter to byte
> oriented again.", 2006-03-04).
>
> Remove the parameter and adjust all callers.
>
> Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
> ---
> v2: In the commit message, reference the correct commit where the parameter
> usage was removed. Spotted by Jeff King.
Looks good. Thanks for following up.
-Peff
^ permalink raw reply
* [PATCH v2] diffcore-delta: remove unused parameter to diffcore_count_changes()
From: Tobias Klauser @ 2016-11-14 13:39 UTC (permalink / raw)
To: git, gitster; +Cc: Jeff King
The delta_limit parameter to diffcore_count_changes() has been unused
since commit ba23bbc8e ("diffcore-delta: make change counter to byte
oriented again.", 2006-03-04).
Remove the parameter and adjust all callers.
Signed-off-by: Tobias Klauser <tklauser@distanz.ch>
---
v2: In the commit message, reference the correct commit where the parameter
usage was removed. Spotted by Jeff King.
diff.c | 2 +-
diffcore-break.c | 1 -
diffcore-delta.c | 1 -
diffcore-rename.c | 4 ----
diffcore.h | 1 -
5 files changed, 1 insertion(+), 8 deletions(-)
diff --git a/diff.c b/diff.c
index 8981477c436d..ec8728362dae 100644
--- a/diff.c
+++ b/diff.c
@@ -2023,7 +2023,7 @@ static void show_dirstat(struct diff_options *options)
if (DIFF_FILE_VALID(p->one) && DIFF_FILE_VALID(p->two)) {
diff_populate_filespec(p->one, 0);
diff_populate_filespec(p->two, 0);
- diffcore_count_changes(p->one, p->two, NULL, NULL, 0,
+ diffcore_count_changes(p->one, p->two, NULL, NULL,
&copied, &added);
diff_free_filespec_data(p->one);
diff_free_filespec_data(p->two);
diff --git a/diffcore-break.c b/diffcore-break.c
index 881a74f29e4f..c64359f489c8 100644
--- a/diffcore-break.c
+++ b/diffcore-break.c
@@ -73,7 +73,6 @@ static int should_break(struct diff_filespec *src,
if (diffcore_count_changes(src, dst,
&src->cnt_data, &dst->cnt_data,
- 0,
&src_copied, &literal_added))
return 0;
diff --git a/diffcore-delta.c b/diffcore-delta.c
index 2ebedb32d18a..ebe70fb06851 100644
--- a/diffcore-delta.c
+++ b/diffcore-delta.c
@@ -166,7 +166,6 @@ int diffcore_count_changes(struct diff_filespec *src,
struct diff_filespec *dst,
void **src_count_p,
void **dst_count_p,
- unsigned long delta_limit,
unsigned long *src_copied,
unsigned long *literal_added)
{
diff --git a/diffcore-rename.c b/diffcore-rename.c
index 54a2396653df..f7444c86bde3 100644
--- a/diffcore-rename.c
+++ b/diffcore-rename.c
@@ -145,7 +145,6 @@ static int estimate_similarity(struct diff_filespec *src,
* call into this function in that case.
*/
unsigned long max_size, delta_size, base_size, src_copied, literal_added;
- unsigned long delta_limit;
int score;
/* We deal only with regular files. Symlink renames are handled
@@ -191,11 +190,8 @@ static int estimate_similarity(struct diff_filespec *src,
if (!dst->cnt_data && diff_populate_filespec(dst, 0))
return 0;
- delta_limit = (unsigned long)
- (base_size * (MAX_SCORE-minimum_score) / MAX_SCORE);
if (diffcore_count_changes(src, dst,
&src->cnt_data, &dst->cnt_data,
- delta_limit,
&src_copied, &literal_added))
return 0;
diff --git a/diffcore.h b/diffcore.h
index c11b8465fc8e..623024135478 100644
--- a/diffcore.h
+++ b/diffcore.h
@@ -142,7 +142,6 @@ extern int diffcore_count_changes(struct diff_filespec *src,
struct diff_filespec *dst,
void **src_count_p,
void **dst_count_p,
- unsigned long delta_limit,
unsigned long *src_copied,
unsigned long *literal_added);
--
2.9.0
^ permalink raw reply related
* Re: [PATCH] t0021, t5615: use $PWD instead of $(pwd) in PATH-like shell variables
From: Lars Schneider @ 2016-11-14 9:11 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, Johannes Sixt, git, Jeff King
In-Reply-To: <xmqqtwbcyyfe.fsf@gitster.mtv.corp.google.com>
> On 13 Nov 2016, at 02:13, Junio C Hamano <gitster@pobox.com> wrote:
>
> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>
>>> Thanks. Dscho, does this fix both of these issues to you?
>>
>> Apparently it does because the CI jobs for `master` and for `next` pass.
>
> OK, thanks for a quick confirmation.
>
>> The one for `pu` still times out, of course.
>
> Earlier you said 'pu' did even not build, but do we know where this
> "still times out" comes from? As long as I don't merge anything
> prematurely, which I need to be careful about, it shouldn't impact
> the upcoming release, but we'd need to figure it out before moving
> things forward post release.
What is the goal for 'pu'?
(1) Builds clean on all platforms + passes all tests
(2) Builds clean on all platforms
(3) Builds clean on Linux
(4) Just makes new topics easily available to a broader audience
My understanding was always (4) but the discussion above sounds
more like (1) or (2)?
--
Git 'pu' does not compile on macOS right now:
builtin/bisect--helper.c:299:6: error: variable 'good_syn' is used uninitialized
whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
...
More info here:
https://api.travis-ci.org/jobs/175417712/log.txt?deansi=true
--
Cheers,
Lars
^ permalink raw reply
* Re: [ANNOUNCE] Prerelease: Git for Windows v2.11.0-rc0
From: stefan.naewe @ 2016-11-14 8:04 UTC (permalink / raw)
To: Johannes.Schindelin; +Cc: git-for-windows, git
In-Reply-To: <alpine.DEB.2.20.1611111933001.9967@virtualbox>
Am 11.11.2016 um 19:33 schrieb Johannes Schindelin:
> Hi Stefan,
>
> On Fri, 11 Nov 2016, Johannes Schindelin wrote:
>
>> Will keep you posted,
>
> I published the prerelease:
>
> https://github.com/git-for-windows/git/releases/tag/v2.11.0-rc0.windows.2
That version brings all my PATH entries back!
Tanks a lot!
Stefan
--
----------------------------------------------------------------
/dev/random says: Catholic (n.) A cat with a drinking problem.
python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')"
GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9 9666 829B 49C5 9221 27AF
^ permalink raw reply
* Re: [RFC/PATCH 0/2] git diff <(command1) <(command2)
From: Junio C Hamano @ 2016-11-14 3:14 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: Jacob Keller, Dennis Kaarsemaker, Git mailing list
In-Reply-To: <alpine.DEB.2.20.1611121106110.3746@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> On Fri, 11 Nov 2016, Jacob Keller wrote:
>
>> On Fri, Nov 11, 2016 at 1:27 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> > Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
>> >
>> >> No tests or documentation updates yet, and I'm not sure whether
>> >> --follow-symlinks in other modes than --no-index should be supported, ignored
>> >> (as it is now) or cause an error, but I'm leaning towards the third option.
>> >
>> > My knee-jerk reaction is:
>> >
>> > * The --no-index mode should default to your --follow-symlinks
>> > behaviour, without any option to turn it on or off.
>> >
>>
>> I agree. We shouldn't have to specify this for no-index.
>
> Ummm. *My* idea of --no-index was for it to behave as similar to the
> --index version as possible. For example when comparing directories
> containing symlinks. You seem intent on breaking this scenario.
Perhaps a viable compromise between the two is to only always
dereference at the top-level (i.e. the trees to be compared) under
"--no-index" mode and not changing anything else?
The original use case by Dennis is not even about doing a recursive
two-directories-in-a-filesystem comparison and encountering a
symbolic link (it was to compare two BLOBs, which happen to be
output from two commands).
^ permalink raw reply
* Re: [PATCH] fetch/push: document that private data can be leaked
From: Junio C Hamano @ 2016-11-14 2:57 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git, Jeff King
In-Reply-To: <1479001205.3471.1.camel@mattmccutchen.net>
Matt McCutchen <matt@mattmccutchen.net> writes:
> Documentation/fetch-push-security.txt | 9 +++++++++
A new (consolidated) piece like this that can be included in
multiple places is a good idea. I wonder if the original
description in "namespaces" thing can be moved here and then
"namespaces" page can be made to also borrow from this?
> Documentation/git-fetch.txt | 2 ++
> Documentation/git-pull.txt | 2 ++
> Documentation/git-push.txt | 2 ++
> 4 files changed, 15 insertions(+)
> create mode 100644 Documentation/fetch-push-security.txt
>
> diff --git a/Documentation/fetch-push-security.txt b/Documentation/fetch-push-security.txt
> new file mode 100644
> index 0000000..00944ed
> --- /dev/null
> +++ b/Documentation/fetch-push-security.txt
> @@ -0,0 +1,9 @@
> +SECURITY
> +--------
> +The fetch and push protocols are not designed to prevent a malicious
> +server from stealing data from your repository that you did not intend to
> +share. The possible attacks are similar to the ones described in the
> +"SECURITY" section of linkgit:gitnamespaces[7]. If you have private data
> +that you need to protect from the server, keep it in a separate
> +repository.
Yup, and then "do not push to untrustworthy place without checking
what you are pushing", too?
> diff --git a/Documentation/git-fetch.txt b/Documentation/git-fetch.txt
> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
> diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
These three look sensible.
^ permalink raw reply
* Re: [PATCH v7 13/17] ref-filter: add `:dir` and `:base` options for ref printing atoms
From: Junio C Hamano @ 2016-11-14 1:55 UTC (permalink / raw)
To: Karthik Nayak; +Cc: Jacob Keller, Git mailing list
In-Reply-To: <CAOLa=ZTWFuzWBjGUX_nV4rVVDRpaabmj0-M6S7aJkX3w+dK2Jw@mail.gmail.com>
Karthik Nayak <karthik.188@gmail.com> writes:
>>> diff --git a/Documentation/git-for-each-ref.txt b/Documentation/git-for-each-ref.txt
>>> index 600b703..f4ad297 100644
>>> --- a/Documentation/git-for-each-ref.txt
>>> +++ b/Documentation/git-for-each-ref.txt
>>> @@ -96,7 +96,9 @@ refname::
>>> slash-separated path components from the front of the refname
>>> (e.g., `%(refname:strip=2)` turns `refs/tags/foo` into `foo`.
>>> `<N>` must be a positive integer. If a displayed ref has fewer
>>> - components than `<N>`, the command aborts with an error.
>>> + components than `<N>`, the command aborts with an error. For the base
>>> + directory of the ref (i.e. foo in refs/foo/bar/boz) append
>>> + `:base`. For the entire directory path append `:dir`.
Sorry that I missed this so far and I do not know how many recent
rerolls had them like this, but I am not sure about these :base and
:dir suffixes. From their names I think readers would expect that
they do rough equivalents to basename() and dirname() applied to the
refname, but the example contradicts with that intuition. The
result of applying basename() to 'refs/boo/bar/boz' would be 'boz'
and not 'foo' as the example says.
So assuming that :base and :dir are unrelated to basename() and
dirname():
- I think calling these :base and :dir may be misleading
- More importantly, what do these do? I do not think of a good
description that generalizes "base of refs/foo/bar/boz is foo" to
explain your :base.
- A :dir that corresponds to the :base that picks 'foo' from
'refs/foo/bar/boz' needs an example, too.
Or is the above example simply a typo? Is refs/foo/bar/boz:base
'boz', not 'foo'?
^ permalink raw reply
* Draft of Git Rev News edition 21
From: Christian Couder @ 2016-11-13 20:55 UTC (permalink / raw)
To: git
Cc: Thomas Ferris Nicolaisen, Jakub Narebski, Junio C Hamano,
Jeff King, Stefan Beller, Jacob Keller, Aaron Pelly,
Nguyen Thai Ngoc Duy, Johannes Schindelin, Alexei Lozovsky,
Martin Braun, Brendan Forster, Parker Moore, Shawn Pearce
Hi,
A draft of a new Git Rev News edition is available here:
https://github.com/git/git.github.io/blob/master/rev_news/drafts/edition-21.md
Everyone is welcome to contribute in any section either by editing the
above page on GitHub and sending a pull request, or by commenting on
this GitHub issue:
https://github.com/git/git.github.io/issues/199
You can also reply to this email.
I tried to cc everyone who appears in this edition but maybe I missed
some people, sorry about that.
Thomas, Jakub Narębski (who is now part of the team, welcome Jakub!)
and myself plan to publish this edition on Wednesday November 16.
Thanks,
Christian.
^ 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