* [PATCH 2/3] fetch_refs_via_pack: call report_unmatched_refs
From: Matt McCutchen @ 2017-02-22 16:02 UTC (permalink / raw)
To: git
In-Reply-To: <xmqqvas4gie9.fsf@gitster.mtv.corp.google.com>
"git fetch" currently doesn't bother to check that it got all refs it
sought, because the common case of requesting a nonexistent ref triggers
a die() in get_fetch_map. However, there's at least one case that
slipped through: "git fetch REMOTE SHA1" if the server doesn't allow
requests for unadvertised objects. Make fetch_refs_via_pack (which is
on the "git fetch" code path) call report_unmatched_refs so that we at
least get an error message in that case.
---
t/t5516-fetch-push.sh | 3 ++-
transport.c | 14 +++++++++-----
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 26b2caf..0d13a45 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1098,7 +1098,8 @@ test_expect_success 'fetch exact SHA1' '
test_must_fail git cat-file -t $the_commit &&
# fetching the hidden object should fail by default
- test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy &&
+ test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err &&
+ test_i18ngrep "no such remote ref" err &&
test_must_fail git rev-parse --verify refs/heads/copy &&
# the server side can allow it to succeed
diff --git a/transport.c b/transport.c
index 04e5d66..c377907 100644
--- a/transport.c
+++ b/transport.c
@@ -204,6 +204,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus
static int fetch_refs_via_pack(struct transport *transport,
int nr_heads, struct ref **to_fetch)
{
+ int ret = 0;
struct git_transport_data *data = transport->data;
struct ref *refs;
char *dest = xstrdup(transport->url);
@@ -241,19 +242,22 @@ static int fetch_refs_via_pack(struct transport *transport,
&transport->pack_lockfile);
close(data->fd[0]);
close(data->fd[1]);
- if (finish_connect(data->conn)) {
- free_refs(refs);
- refs = NULL;
- }
+ if (finish_connect(data->conn))
+ ret = -1;
data->conn = NULL;
data->got_remote_heads = 0;
data->options.self_contained_and_connected =
args.self_contained_and_connected;
+ if (refs == NULL)
+ ret = -1;
+ if (report_unmatched_refs(to_fetch, nr_heads))
+ ret = -1;
+
free_refs(refs_tmp);
free_refs(refs);
free(dest);
- return (refs ? 0 : -1);
+ return ret;
}
static int push_had_errors(struct ref *ref)
--
2.9.3
^ permalink raw reply related
* [PATCH 1/3] fetch-pack: move code to report unmatched refs to a function
From: Matt McCutchen @ 2017-02-22 16:01 UTC (permalink / raw)
To: git
In-Reply-To: <xmqqvas4gie9.fsf@gitster.mtv.corp.google.com>
We're preparing to reuse this code in transport.c for "git fetch".
While I'm here, internationalize the existing error message.
---
builtin/fetch-pack.c | 7 +------
fetch-pack.c | 13 +++++++++++++
fetch-pack.h | 9 +++++++++
t/t5500-fetch-pack.sh | 6 +++---
4 files changed, 26 insertions(+), 9 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index cfe9e44..2a1c1c2 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -219,12 +219,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
* remote no-such-ref' would silently succeed without issuing
* an error.
*/
- for (i = 0; i < nr_sought; i++) {
- if (!sought[i] || sought[i]->matched)
- continue;
- error("no such remote ref %s", sought[i]->name);
- ret = 1;
- }
+ ret |= report_unmatched_refs(sought, nr_sought);
while (ref) {
printf("%s %s\n",
diff --git a/fetch-pack.c b/fetch-pack.c
index 601f077..7c8d44c 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1094,3 +1094,16 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
clear_shallow_info(&si);
return ref_cpy;
}
+
+int report_unmatched_refs(struct ref **sought, int nr_sought)
+{
+ int i, ret = 0;
+
+ for (i = 0; i < nr_sought; i++) {
+ if (!sought[i] || sought[i]->matched)
+ continue;
+ error(_("no such remote ref %s"), sought[i]->name);
+ ret = 1;
+ }
+ return ret;
+}
diff --git a/fetch-pack.h b/fetch-pack.h
index c912e3d..fd4d80e 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -45,4 +45,13 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
struct sha1_array *shallow,
char **pack_lockfile);
+/*
+ * Print an appropriate error message for each sought ref that wasn't
+ * matched. Return 0 if all sought refs were matched, otherwise 1.
+ *
+ * The type of "sought" should be "const struct ref *const *" but for
+ * http://stackoverflow.com/questions/5055655/double-pointer-const-correctness-warnings-in-c .
+ */
+int report_unmatched_refs(struct ref **sought, int nr_sought);
+
#endif
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 505e1b4..b5865b3 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -484,7 +484,7 @@ test_expect_success 'test lonely missing ref' '
cd client &&
test_must_fail git fetch-pack --no-progress .. refs/heads/xyzzy
) >/dev/null 2>error-m &&
- test_cmp expect-error error-m
+ test_i18ncmp expect-error error-m
'
test_expect_success 'test missing ref after existing' '
@@ -492,7 +492,7 @@ test_expect_success 'test missing ref after existing' '
cd client &&
test_must_fail git fetch-pack --no-progress .. refs/heads/A refs/heads/xyzzy
) >/dev/null 2>error-em &&
- test_cmp expect-error error-em
+ test_i18ncmp expect-error error-em
'
test_expect_success 'test missing ref before existing' '
@@ -500,7 +500,7 @@ test_expect_success 'test missing ref before existing' '
cd client &&
test_must_fail git fetch-pack --no-progress .. refs/heads/xyzzy refs/heads/A
) >/dev/null 2>error-me &&
- test_cmp expect-error error-me
+ test_i18ncmp expect-error error-me
'
test_expect_success 'test --all, --depth, and explicit head' '
--
2.9.3
^ permalink raw reply related
* [PATCH 3/3] fetch-pack: add specific error for fetching an unadvertised object
From: Matt McCutchen @ 2017-02-22 16:05 UTC (permalink / raw)
To: git
In-Reply-To: <xmqqvas4gie9.fsf@gitster.mtv.corp.google.com>
Enhance filter_refs (which decides whether a request for an unadvertised
object should be sent to the server) to record a new match status on the
"struct ref" when a request is not allowed, and have
report_unmatched_refs check for this status and print a special error
message, "Server does not allow request for unadvertised object".
---
fetch-pack.c | 42 +++++++++++++++++++++++++++---------------
remote.h | 9 +++++++--
t/t5516-fetch-push.sh | 2 +-
3 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 7c8d44c..f12bfcd 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -578,7 +578,7 @@ static void filter_refs(struct fetch_pack_args *args,
break; /* definitely do not have it */
else if (cmp == 0) {
keep = 1; /* definitely have it */
- sought[i]->matched = 1;
+ sought[i]->match_status = REF_MATCHED;
}
i++;
}
@@ -598,22 +598,24 @@ static void filter_refs(struct fetch_pack_args *args,
}
/* Append unmatched requests to the list */
- if ((allow_unadvertised_object_request &
- (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {
- for (i = 0; i < nr_sought; i++) {
- unsigned char sha1[20];
+ for (i = 0; i < nr_sought; i++) {
+ unsigned char sha1[20];
- ref = sought[i];
- if (ref->matched)
- continue;
- if (get_sha1_hex(ref->name, sha1) ||
- ref->name[40] != '\0' ||
- hashcmp(sha1, ref->old_oid.hash))
- continue;
+ ref = sought[i];
+ if (ref->match_status != REF_NOT_MATCHED)
+ continue;
+ if (get_sha1_hex(ref->name, sha1) ||
+ ref->name[40] != '\0' ||
+ hashcmp(sha1, ref->old_oid.hash))
+ continue;
- ref->matched = 1;
+ if ((allow_unadvertised_object_request &
+ (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {
+ ref->match_status = REF_MATCHED;
*newtail = copy_ref(ref);
newtail = &(*newtail)->next;
+ } else {
+ ref->match_status = REF_UNADVERTISED_NOT_ALLOWED;
}
}
*refs = newlist;
@@ -1100,9 +1102,19 @@ int report_unmatched_refs(struct ref **sought, int nr_sought)
int i, ret = 0;
for (i = 0; i < nr_sought; i++) {
- if (!sought[i] || sought[i]->matched)
+ if (!sought[i])
continue;
- error(_("no such remote ref %s"), sought[i]->name);
+ switch (sought[i]->match_status) {
+ case REF_MATCHED:
+ continue;
+ case REF_NOT_MATCHED:
+ error(_("no such remote ref %s"), sought[i]->name);
+ break;
+ case REF_UNADVERTISED_NOT_ALLOWED:
+ error(_("Server does not allow request for unadvertised object %s"),
+ sought[i]->name);
+ break;
+ }
ret = 1;
}
return ret;
diff --git a/remote.h b/remote.h
index 9248811..0b9d8c4 100644
--- a/remote.h
+++ b/remote.h
@@ -89,8 +89,13 @@ struct ref {
force:1,
forced_update:1,
expect_old_sha1:1,
- deletion:1,
- matched:1;
+ deletion:1;
+
+ enum {
+ REF_NOT_MATCHED = 0, /* initial value */
+ REF_MATCHED,
+ REF_UNADVERTISED_NOT_ALLOWED
+ } match_status;
/*
* Order is important here, as we write to FETCH_HEAD
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 0d13a45..78f3b8e 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1099,7 +1099,7 @@ test_expect_success 'fetch exact SHA1' '
# fetching the hidden object should fail by default
test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err &&
- test_i18ngrep "no such remote ref" err &&
+ test_i18ngrep "Server does not allow request for unadvertised object" err &&
test_must_fail git rev-parse --verify refs/heads/copy &&
# the server side can allow it to succeed
--
2.9.3
^ permalink raw reply related
* Re: [PATCH] fetch: print an error when declining to request an unadvertised object
From: Junio C Hamano @ 2017-02-22 17:07 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git
In-Reply-To: <1487780254.3016.1.camel@mattmccutchen.net>
Matt McCutchen <matt@mattmccutchen.net> writes:
> Anyway, I made a split series and will send it in a moment. I don't
> know if all the commit messages include exactly the information you
> want; hopefully you're happy to edit them as desired. Compared to the
> previous patch, there is one fix in the net result: fixing t5500-fetch-
> pack.sh to deal with the internationalized "no such remote ref"
> message.
Thanks for going an extra mile. I think many developers in the
future who reads "git log" will thank you, too. The changes,
especially the one in the last one, are very much more
understandable compared to the original, even if the end result is
the same.
^ permalink raw reply
* Re: [PATCH 1/3] fetch-pack: move code to report unmatched refs to a function
From: Junio C Hamano @ 2017-02-22 17:11 UTC (permalink / raw)
To: Matt McCutchen; +Cc: git
In-Reply-To: <b9b52233e84a7f5bd0526f9625e4be06cbbd0ace.1487779910.git.matt@mattmccutchen.net>
Matt McCutchen <matt@mattmccutchen.net> writes:
> We're preparing to reuse this code in transport.c for "git fetch".
>
> While I'm here, internationalize the existing error message.
> ---
Sounds good. Please just say it is OK for me to forge your sign-off ;-)
> diff --git a/fetch-pack.h b/fetch-pack.h
> index c912e3d..fd4d80e 100644
> --- a/fetch-pack.h
> +++ b/fetch-pack.h
> @@ -45,4 +45,13 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
> struct sha1_array *shallow,
> char **pack_lockfile);
>
> +/*
> + * Print an appropriate error message for each sought ref that wasn't
> + * matched. Return 0 if all sought refs were matched, otherwise 1.
> + *
> + * The type of "sought" should be "const struct ref *const *" but for
> + * http://stackoverflow.com/questions/5055655/double-pointer-const-correctness-warnings-in-c .
> + */
This is an unfinished sentence, but I wonder if we even need to have
it here? I'd be surprised if this function was unique in the
codebase that takes an array pointer whose type is looser than
necessary because of well-known language rules.
^ permalink raw reply
* Re: `git show --oneline commit` does not do what's intuitively expected
From: Linus Torvalds @ 2017-02-22 17:18 UTC (permalink / raw)
To: Jeff King; +Cc: Luna Kid, git
In-Reply-To: <20170217040522.jugt4uuak74l3qjw@sigill.intra.peff.net>
On Thu, Feb 16, 2017 at 8:05 PM, Jeff King <peff@peff.net> wrote:
> On Fri, Feb 17, 2017 at 02:51:36AM +0100, Luna Kid wrote:
>
>> tl;dr; --> Please add --no-diff (or --msg-only) to git show. We'll
>> love you for that. :)
>
> I think it is already spelled "--no-patch", or "-s" for short.
I think people should also learn about "--no-walk" (or the numberic
walk limiters) to the revision walking logic, because it can often be
useful.
IOW, if you only want the commit info, you can certainly use "git show
-s", but you can also use
git log --no-walk .. list of commits ..
or
git log -1 <single-commit>
to show a commit without any other details.
Basically, "git show" for a commit does what is mostly equivalent to
git log --cc --no-walk
although "git show" then has other features too (ie it shows non-commits etc).
Linus
^ permalink raw reply
* Re: [RFC][Git GUI] Make Commit message field in git GUI re sizable.
From: Marc Branchaud @ 2017-02-22 17:15 UTC (permalink / raw)
To: Jessie Hernandez, Bert Wesarg; +Cc: Git Mailing List, Pat Thoyts
In-Reply-To: <dfcc54b406d9f46cf358b7ead128bf65.squirrel@mail.jessiehernandez.com>
On 2017-02-22 06:59 AM, Jessie Hernandez wrote:
>> HI,
>>
>> the reason why it is fixed, is because commit messages should be
>> wrapped at 76 characters to be used in mails. So it helps you with the
>> wrapping.
>>
>> Bert
>
> Right ok. I understand.
> Knowing this I think I might start writing my commit messages differently
> then.
You can configure gui.commitMsgWidth if you don't like the default
(which is 75, not 76).
M.
> Thank you for this.
>
> Regards
>
> -----------------
> Jessie Hernandez
>
>>
>>
>> On Wed, Feb 22, 2017 at 10:27 AM, Jessie Hernandez
>> <jessie@jessiehernandez.com> wrote:
>>> Hi all,
>>>
>>> I have been using git for a few years now and really like the software.
>>> I have a small annoyance and was wondering if I could get the
>>> communities
>>> view on this.
>>>
>>> When using git GUI I find it handy to be able to re-size the "Unstaged
>>> Changes" and the "Staged Changed" fields.
>>>
>>> I would like the same thing for the "Commit Message" field, or to have
>>> it
>>> re-size with the git GUI window.
>>>
>>> I can re-size the "Commit Message" vertically when making the "Modified"
>>> panel smaller.
>>>
>>> Does this make sense?
>>> I would be happy to get into more detail if that is necessary or if
>>> something is not clear.
>>>
>>> Thank you.
>>>
>>> -----------------
>>> Jessie Hernandez
>>>
>>>
>>
>
>
^ permalink raw reply
* Re: [PATCH v5 00/24] Remove submodule from files-backend.c
From: Junio C Hamano @ 2017-02-22 17:18 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git, Michael Haggerty, Johannes Schindelin, Ramsay Jones,
Stefan Beller, novalis
In-Reply-To: <20170222140450.30886-1-pclouds@gmail.com>
Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
> v5 goes a bit longer than v4, basically:
>
> - files_path() is broken down into three smaller functions,
> files_{packed_refs,reflog,refname}_path().
>
> - most of store-based api is added because..
>
> - test-ref-store.c is added with t1405 and t1406 for some basic tests
> I'm not aimimg for complete ref store coverage. But we can continue
> to improve from there.
>
> - refs_store_init() now takes a "permission" flag, like open().
> Operations are allowed or forbidden based on this flag. The
> submodule_allowed flag is killed. files_assert_main.. remains.
>
> - get_*_ref_store() remain public api because it's used by
> test-ref-store.c and pack-refs.c.
>
> - files-backend.c should now make no function calls that implicitly
> target the main store. But this will have to be tested more to be
> sure. I'm tempted to add a tracing backend just for this purpose.
OK.
> Junio, if you take this on 'pu', you'll have to kick my other two
> series out (they should not even compile). I'm not resending them
> until I get a "looks mostly ok" from Michael. No point in updating
> them when this series keeps moving.
Thanks for a note.
^ permalink raw reply
* [PATCH v2 1/3] fetch-pack: move code to report unmatched refs to a function
From: Matt McCutchen @ 2017-02-22 16:01 UTC (permalink / raw)
To: git
In-Reply-To: <xmqqpoiacfqw.fsf@gitster.mtv.corp.google.com>
We're preparing to reuse this code in transport.c for "git fetch".
While I'm here, internationalize the existing error message.
Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
---
builtin/fetch-pack.c | 7 +------
fetch-pack.c | 13 +++++++++++++
fetch-pack.h | 6 ++++++
t/t5500-fetch-pack.sh | 6 +++---
4 files changed, 23 insertions(+), 9 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index cfe9e44..2a1c1c2 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -219,12 +219,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
* remote no-such-ref' would silently succeed without issuing
* an error.
*/
- for (i = 0; i < nr_sought; i++) {
- if (!sought[i] || sought[i]->matched)
- continue;
- error("no such remote ref %s", sought[i]->name);
- ret = 1;
- }
+ ret |= report_unmatched_refs(sought, nr_sought);
while (ref) {
printf("%s %s\n",
diff --git a/fetch-pack.c b/fetch-pack.c
index 601f077..7c8d44c 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -1094,3 +1094,16 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
clear_shallow_info(&si);
return ref_cpy;
}
+
+int report_unmatched_refs(struct ref **sought, int nr_sought)
+{
+ int i, ret = 0;
+
+ for (i = 0; i < nr_sought; i++) {
+ if (!sought[i] || sought[i]->matched)
+ continue;
+ error(_("no such remote ref %s"), sought[i]->name);
+ ret = 1;
+ }
+ return ret;
+}
diff --git a/fetch-pack.h b/fetch-pack.h
index c912e3d..a2d46e6 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -45,4 +45,10 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
struct sha1_array *shallow,
char **pack_lockfile);
+/*
+ * Print an appropriate error message for each sought ref that wasn't
+ * matched. Return 0 if all sought refs were matched, otherwise 1.
+ */
+int report_unmatched_refs(struct ref **sought, int nr_sought);
+
#endif
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 505e1b4..b5865b3 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -484,7 +484,7 @@ test_expect_success 'test lonely missing ref' '
cd client &&
test_must_fail git fetch-pack --no-progress .. refs/heads/xyzzy
) >/dev/null 2>error-m &&
- test_cmp expect-error error-m
+ test_i18ncmp expect-error error-m
'
test_expect_success 'test missing ref after existing' '
@@ -492,7 +492,7 @@ test_expect_success 'test missing ref after existing' '
cd client &&
test_must_fail git fetch-pack --no-progress .. refs/heads/A refs/heads/xyzzy
) >/dev/null 2>error-em &&
- test_cmp expect-error error-em
+ test_i18ncmp expect-error error-em
'
test_expect_success 'test missing ref before existing' '
@@ -500,7 +500,7 @@ test_expect_success 'test missing ref before existing' '
cd client &&
test_must_fail git fetch-pack --no-progress .. refs/heads/xyzzy refs/heads/A
) >/dev/null 2>error-me &&
- test_cmp expect-error error-me
+ test_i18ncmp expect-error error-me
'
test_expect_success 'test --all, --depth, and explicit head' '
--
2.9.3
^ permalink raw reply related
* [PATCH v2 3/3] fetch-pack: add specific error for fetching an unadvertised object
From: Matt McCutchen @ 2017-02-22 16:05 UTC (permalink / raw)
To: git
In-Reply-To: <xmqqpoiacfqw.fsf@gitster.mtv.corp.google.com>
Enhance filter_refs (which decides whether a request for an unadvertised
object should be sent to the server) to record a new match status on the
"struct ref" when a request is not allowed, and have
report_unmatched_refs check for this status and print a special error
message, "Server does not allow request for unadvertised object".
Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
---
fetch-pack.c | 42 +++++++++++++++++++++++++++---------------
remote.h | 9 +++++++--
t/t5516-fetch-push.sh | 2 +-
3 files changed, 35 insertions(+), 18 deletions(-)
diff --git a/fetch-pack.c b/fetch-pack.c
index 7c8d44c..f12bfcd 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -578,7 +578,7 @@ static void filter_refs(struct fetch_pack_args *args,
break; /* definitely do not have it */
else if (cmp == 0) {
keep = 1; /* definitely have it */
- sought[i]->matched = 1;
+ sought[i]->match_status = REF_MATCHED;
}
i++;
}
@@ -598,22 +598,24 @@ static void filter_refs(struct fetch_pack_args *args,
}
/* Append unmatched requests to the list */
- if ((allow_unadvertised_object_request &
- (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {
- for (i = 0; i < nr_sought; i++) {
- unsigned char sha1[20];
+ for (i = 0; i < nr_sought; i++) {
+ unsigned char sha1[20];
- ref = sought[i];
- if (ref->matched)
- continue;
- if (get_sha1_hex(ref->name, sha1) ||
- ref->name[40] != '\0' ||
- hashcmp(sha1, ref->old_oid.hash))
- continue;
+ ref = sought[i];
+ if (ref->match_status != REF_NOT_MATCHED)
+ continue;
+ if (get_sha1_hex(ref->name, sha1) ||
+ ref->name[40] != '\0' ||
+ hashcmp(sha1, ref->old_oid.hash))
+ continue;
- ref->matched = 1;
+ if ((allow_unadvertised_object_request &
+ (ALLOW_TIP_SHA1 | ALLOW_REACHABLE_SHA1))) {
+ ref->match_status = REF_MATCHED;
*newtail = copy_ref(ref);
newtail = &(*newtail)->next;
+ } else {
+ ref->match_status = REF_UNADVERTISED_NOT_ALLOWED;
}
}
*refs = newlist;
@@ -1100,9 +1102,19 @@ int report_unmatched_refs(struct ref **sought, int nr_sought)
int i, ret = 0;
for (i = 0; i < nr_sought; i++) {
- if (!sought[i] || sought[i]->matched)
+ if (!sought[i])
continue;
- error(_("no such remote ref %s"), sought[i]->name);
+ switch (sought[i]->match_status) {
+ case REF_MATCHED:
+ continue;
+ case REF_NOT_MATCHED:
+ error(_("no such remote ref %s"), sought[i]->name);
+ break;
+ case REF_UNADVERTISED_NOT_ALLOWED:
+ error(_("Server does not allow request for unadvertised object %s"),
+ sought[i]->name);
+ break;
+ }
ret = 1;
}
return ret;
diff --git a/remote.h b/remote.h
index 9248811..0b9d8c4 100644
--- a/remote.h
+++ b/remote.h
@@ -89,8 +89,13 @@ struct ref {
force:1,
forced_update:1,
expect_old_sha1:1,
- deletion:1,
- matched:1;
+ deletion:1;
+
+ enum {
+ REF_NOT_MATCHED = 0, /* initial value */
+ REF_MATCHED,
+ REF_UNADVERTISED_NOT_ALLOWED
+ } match_status;
/*
* Order is important here, as we write to FETCH_HEAD
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 0d13a45..78f3b8e 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1099,7 +1099,7 @@ test_expect_success 'fetch exact SHA1' '
# fetching the hidden object should fail by default
test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err &&
- test_i18ngrep "no such remote ref" err &&
+ test_i18ngrep "Server does not allow request for unadvertised object" err &&
test_must_fail git rev-parse --verify refs/heads/copy &&
# the server side can allow it to succeed
--
2.9.3
^ permalink raw reply related
* [PATCH v2 2/3] fetch_refs_via_pack: call report_unmatched_refs
From: Matt McCutchen @ 2017-02-22 16:02 UTC (permalink / raw)
To: git
In-Reply-To: <xmqqpoiacfqw.fsf@gitster.mtv.corp.google.com>
"git fetch" currently doesn't bother to check that it got all refs it
sought, because the common case of requesting a nonexistent ref triggers
a die() in get_fetch_map. However, there's at least one case that
slipped through: "git fetch REMOTE SHA1" if the server doesn't allow
requests for unadvertised objects. Make fetch_refs_via_pack (which is
on the "git fetch" code path) call report_unmatched_refs so that we at
least get an error message in that case.
Signed-off-by: Matt McCutchen <matt@mattmccutchen.net>
---
t/t5516-fetch-push.sh | 3 ++-
transport.c | 14 +++++++++-----
2 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 26b2caf..0d13a45 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1098,7 +1098,8 @@ test_expect_success 'fetch exact SHA1' '
test_must_fail git cat-file -t $the_commit &&
# fetching the hidden object should fail by default
- test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy &&
+ test_must_fail git fetch -v ../testrepo $the_commit:refs/heads/copy 2>err &&
+ test_i18ngrep "no such remote ref" err &&
test_must_fail git rev-parse --verify refs/heads/copy &&
# the server side can allow it to succeed
diff --git a/transport.c b/transport.c
index 04e5d66..c377907 100644
--- a/transport.c
+++ b/transport.c
@@ -204,6 +204,7 @@ static struct ref *get_refs_via_connect(struct transport *transport, int for_pus
static int fetch_refs_via_pack(struct transport *transport,
int nr_heads, struct ref **to_fetch)
{
+ int ret = 0;
struct git_transport_data *data = transport->data;
struct ref *refs;
char *dest = xstrdup(transport->url);
@@ -241,19 +242,22 @@ static int fetch_refs_via_pack(struct transport *transport,
&transport->pack_lockfile);
close(data->fd[0]);
close(data->fd[1]);
- if (finish_connect(data->conn)) {
- free_refs(refs);
- refs = NULL;
- }
+ if (finish_connect(data->conn))
+ ret = -1;
data->conn = NULL;
data->got_remote_heads = 0;
data->options.self_contained_and_connected =
args.self_contained_and_connected;
+ if (refs == NULL)
+ ret = -1;
+ if (report_unmatched_refs(to_fetch, nr_heads))
+ ret = -1;
+
free_refs(refs_tmp);
free_refs(refs);
free(dest);
- return (refs ? 0 : -1);
+ return ret;
}
static int push_had_errors(struct ref *ref)
--
2.9.3
^ permalink raw reply related
* Re: [PATCH 1/3] fetch-pack: move code to report unmatched refs to a function
From: Matt McCutchen @ 2017-02-22 17:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: git
In-Reply-To: <xmqqpoiacfqw.fsf@gitster.mtv.corp.google.com>
On Wed, 2017-02-22 at 09:11 -0800, Junio C Hamano wrote:
> Matt McCutchen <matt@mattmccutchen.net> writes:
>
> > We're preparing to reuse this code in transport.c for "git fetch".
> >
> > While I'm here, internationalize the existing error message.
> > ---
>
> Sounds good. Please just say it is OK for me to forge your sign-off
> ;-)
Oops. Given the other issue below, I'll just regenerate the patch
series.
> > diff --git a/fetch-pack.h b/fetch-pack.h
> > index c912e3d..fd4d80e 100644
> > --- a/fetch-pack.h
> > +++ b/fetch-pack.h
> > @@ -45,4 +45,13 @@ struct ref *fetch_pack(struct fetch_pack_args
> > *args,
> > struct sha1_array *shallow,
> > char **pack_lockfile);
> >
> > +/*
> > + * Print an appropriate error message for each sought ref that
> > wasn't
> > + * matched. Return 0 if all sought refs were matched, otherwise
> > 1.
> > + *
> > + * The type of "sought" should be "const struct ref *const *" but
> > for
> > + * http://stackoverflow.com/questions/5055655/double-pointer-const
> > -correctness-warnings-in-c .
> > + */
>
> This is an unfinished sentence, but I wonder if we even need to have
> it here? I'd be surprised if this function was unique in the
> codebase that takes an array pointer whose type is looser than
> necessary because of well-known language rules.
You're probably right. I'm in the habit of documenting things that
were unknown to me, but I'll take your word for what's well-known to
the average git developer. I'll remove the remark.
Matt
^ permalink raw reply
* [PATCH] http(s): automatically try NTLM authentication first
From: David Turner @ 2017-02-22 17:39 UTC (permalink / raw)
To: git; +Cc: sandals, Johannes Schindelin, David Turner
From: Johannes Schindelin <johannes.schindelin@gmx.de>
It is common in corporate setups to have permissions managed via a
domain account. That means that the user does not really have to log in
when accessing a central repository via https://, but that the login
credentials are used to authenticate with that repository.
The common way to do that used to require empty credentials, i.e. hitting
Enter twice when being asked for user name and password, or by using the
very funny notation https://:@server/repository
A recent commit (5275c3081c (http: http.emptyauth should allow empty (not
just NULL) usernames, 2016-10-04)) broke that usage, though, all of a
sudden requiring users to set http.emptyAuth = true.
Which brings us to the bigger question why http.emptyAuth defaults to
false, to begin with.
It would be one thing if cURL would not let the user specify credentials
interactively after attempting NTLM authentication (i.e. login
credentials), but that is not the case.
It would be another thing if attempting NTLM authentication was not
usually what users need to do when trying to authenticate via https://.
But that is also not the case.
So let's just go ahead and change the default, and unbreak the NTLM
authentication. As a bonus, this also makes the "you need to hit Enter
twice" (which is hard to explain: why enter empty credentials when you
want to authenticate with your login credentials?) and the ":@" hack
(which is also pretty, pretty hard to explain to users) obsolete.
This fixes https://github.com/git-for-windows/git/issues/987
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: David Turner <dturner@twosigma.com>
---
This has been in git for Windows for a few months (without the
config.txt change). We've also been using it internally. So I think
it's time to merge back to upstream git.
Documentation/config.txt | 3 ++-
http.c | 2 +-
2 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index fc5a28a320..b0da64ed33 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -1742,7 +1742,8 @@ http.emptyAuth::
Attempt authentication without seeking a username or password. This
can be used to attempt GSS-Negotiate authentication without specifying
a username in the URL, as libcurl normally requires a username for
- authentication.
+ authentication. Default is true, since if this fails, git will fall
+ back to asking the user for their username/password.
http.delegation::
Control GSSAPI credential delegation. The delegation is disabled
diff --git a/http.c b/http.c
index 90a1c0f113..943e630ea6 100644
--- a/http.c
+++ b/http.c
@@ -109,7 +109,7 @@ static int curl_save_cookies;
struct credential http_auth = CREDENTIAL_INIT;
static int http_proactive_auth;
static const char *user_agent;
-static int curl_empty_auth;
+static int curl_empty_auth = 1;
enum http_follow_config http_follow_config = HTTP_FOLLOW_INITIAL;
--
2.11.GIT
^ permalink raw reply related
* Re: [PATCH] Documentation: Link git-ls-files to core.quotePath variable.
From: Junio C Hamano @ 2017-02-22 17:41 UTC (permalink / raw)
To: Andreas Heiduk; +Cc: git
In-Reply-To: <e55b3cb7-65bf-1609-2e8d-823b4336e07a@gmail.com>
Andreas Heiduk <asheiduk@gmail.com> writes:
> [PATCH] Documentation: Clarify core.quotePath, remove cruft in
> git-ls-files.
>
> Signed-off-by: Andreas Heiduk <asheiduk@gmail.com>
> ---
>
> I have merged the best parts about quoting into the core.quotePath
> description and cleaned up the text in git-ls-files.txt regarding the
> control characters.
>
>
> Documentation/config.txt | 22 ++++++++++++----------
> Documentation/git-ls-files.txt | 11 ++++++-----
> 2 files changed, 18 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index f4721a0..25e65ae 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -340,16 +340,18 @@ core.checkStat::
> all fields, including the sub-second part of mtime and ctime.
>
> core.quotePath::
> - The commands that output paths (e.g. 'ls-files',
> - 'diff'), when not given the `-z` option, will quote
> - "unusual" characters in the pathname by enclosing the
> - pathname in a double-quote pair and with backslashes the
> - same way strings in C source code are quoted. If this
> - variable is set to false, the bytes higher than 0x80 are
> - not quoted but output as verbatim. Note that double
> - quote, backslash and control characters are always
> - quoted without `-z` regardless of the setting of this
> - variable.
> + Commands that output paths (e.g. 'ls-files', 'diff'), will
> + quote "unusual" characters in the pathname by enclosing the
> + pathname in double-quotes and escaping those characters with
> + backslashes in the same way C escapes control characters (e.g.
> + `\t` for TAB, `\n` for LF, `\\` for backslash) or bytes with
> + values larger than 0x80 (e.g. octal `\265` for "micro"). If
> + this variable is set to false, bytes higher than 0x80 are not
> + considered "unusual" any more. Double-quotes, backslash and
> + control characters are always escaped regardless of the
> + setting of this variable. Many commands can output pathnames
> + completely verbatim using the `-z` option. The default value is
> + true.
Even though I am not sure "\265 is micro" is a good example these
days, as "high-bit set" is primarily meant to catch UTF-8
multi-bytes, I find the above much easier to read than the original.
> diff --git a/Documentation/git-ls-files.txt b/Documentation/git-ls-files.txt
> index d2b17f2..88df561 100644
> --- a/Documentation/git-ls-files.txt
> +++ b/Documentation/git-ls-files.txt
> @@ -76,7 +76,8 @@ OPTIONS
> succeed.
>
> -z::
> - \0 line termination on output.
> + \0 line termination on output and do not quote filenames.
> + See OUTPUT below for more information.
>
> -x <pattern>::
> --exclude=<pattern>::
> @@ -192,10 +193,10 @@ the index records up to three such pairs; one from
> tree O in stage
> the user (or the porcelain) to see what should eventually be recorded
> at the
> path. (see linkgit:git-read-tree[1] for more information on state)
>
> -When `-z` option is not used, TAB, LF, and backslash characters
> -in pathnames are represented as `\t`, `\n`, and `\\`,
> -respectively. The path is also quoted according to the
> -configuration variable `core.quotePath` (see linkgit:git-config[1]).
> +Without the `-z` option pathnamens with "unusual" characters are
> +quoted as explained for the configuration variable `core.quotePath`
> +(see linkgit:git-config[1]). Using `-z` the filename is output
> +verbatim and the line is terminated by a NUL byte.
Yup, this looks much nicer than the original.
Thanks.
^ permalink raw reply
* Re: url.<base>.insteadOf vs. submodules
From: Junio C Hamano @ 2017-02-22 17:36 UTC (permalink / raw)
To: Jon Loeliger; +Cc: Stefan Beller, Jeff King, Toolforger, git@vger.kernel.org
In-Reply-To: <E1cgXSe-0007jp-QI@mylo.jdl.com>
Jon Loeliger <jdl@jdl.com> writes:
> So, like, Junio C Hamano said:
>> Stefan Beller <sbeller@google.com> writes:
>>
>> > Do we want to invent a special value for url.*.insteadOf to mean
>> > "look up in superproject, so I don't have to keep
>> > a copy that may get stale" ?
>>
>> My gut feeling is that we should do the selective/filtered include
>> Peff mentioned when a repository is known to be used as a submodule
>> of somebody else.
>
> Does the management of these submodue-related config values
> become easier if, instead of placing them in .config, we
> place them in a git/.context file?
Do you mean that Git users that use submodules adopt a convention
where a separate file in $GIT_DIR of the toplevel superproject holds
pieces of configuration that are meant to be shared between the
superproject and across all its submodules, and the $GIT_DIR/config
file in submodules and the superproject all include that shared one
via include.path mechanism?
That may allow us to do without being responsible for sifting of
configuration variables into safe and unsafe bins.
I dunno.
^ permalink raw reply
* Re: [PATCH] Documentation: correctly spell git worktree --detach
From: Junio C Hamano @ 2017-02-22 17:50 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Casey Rodarmor
In-Reply-To: <20170222123442.923694-1-sandals@crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> The option is “--detach”, but we accidentally spelled it “--detached” at
> one point in the man page.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> Reported-by: Casey Rodarmor <casey@rodarmor.com>
> ---
Thanks, both.
> Documentation/git-worktree.txt | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt
> index e257c19ebe..553cf8413f 100644
> --- a/Documentation/git-worktree.txt
> +++ b/Documentation/git-worktree.txt
> @@ -52,7 +52,7 @@ is linked to the current repository, sharing everything except working
> directory specific files such as HEAD, index, etc. `-` may also be
> specified as `<branch>`; it is synonymous with `@{-1}`.
> +
> -If `<branch>` is omitted and neither `-b` nor `-B` nor `--detached` used,
> +If `<branch>` is omitted and neither `-b` nor `-B` nor `--detach` used,
> then, as a convenience, a new branch based at HEAD is created automatically,
> as if `-b $(basename <path>)` was specified.
>
^ permalink raw reply
* Re: [PATCH v5 01/24] refs.h: add forward declaration for structs used in this file
From: Stefan Beller @ 2017-02-22 18:18 UTC (permalink / raw)
To: Nguyễn Thái Ngọc Duy
Cc: git@vger.kernel.org, Junio C Hamano, Michael Haggerty,
Johannes Schindelin, Ramsay Jones, David Turner
In-Reply-To: <20170222140450.30886-2-pclouds@gmail.com>
On Wed, Feb 22, 2017 at 6:04 AM, Nguyễn Thái Ngọc Duy <pclouds@gmail.com> wrote:
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
> refs.h | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/refs.h b/refs.h
> index 9fbff90e7..c494b641a 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -1,6 +1,11 @@
> #ifndef REFS_H
> #define REFS_H
>
> +struct object_id;
> +struct ref_transaction;
> +struct strbuf;
> +struct string_list;
> +
> /*
> * Resolve a reference, recursively following symbolic refererences.
> *
> @@ -144,7 +149,6 @@ int dwim_log(const char *str, int len, unsigned char *sha1, char **ref);
> * `ref_transaction_commit` is called. So `ref_transaction_verify`
> * won't report a verification failure until the commit is attempted.
> */
> -struct ref_transaction;
Leaving the detailed comment about ref_transaction dangling?
I can understand if you don't want to move it with the declaration,
as you want all declarations terse in a few lines.
Maybe move the comment to be part of the first large comment
(The one that you can see in the first hunk, starting with
" * Resolve a reference, recursively following")
Maybe Michael has a better idea how to make this comment
more accessible to the casual refs-reader.
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH] Documentation: use brackets for optional arguments
From: Junio C Hamano @ 2017-02-22 18:21 UTC (permalink / raw)
To: brian m. carlson; +Cc: git
In-Reply-To: <20170222122546.922199-1-sandals@crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> The documentation for git blame used vertical bars for optional
> arguments to -M and -C, which is unusual and potentially confusing.
> Since most man pages use brackets for optional items, and that's
> consistent with how we document the same options for git diff and
> friends, use brackets here, too.
This seems to date back to April 2007 X-<.
Thanks.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
> Documentation/blame-options.txt | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/blame-options.txt b/Documentation/blame-options.txt
> index 2669b87c9d..dc41957afa 100644
> --- a/Documentation/blame-options.txt
> +++ b/Documentation/blame-options.txt
> @@ -77,7 +77,7 @@ include::line-range-format.txt[]
> terminal. Can't use `--progress` together with `--porcelain`
> or `--incremental`.
>
> --M|<num>|::
> +-M[<num>]::
> Detect moved or copied lines within a file. When a commit
> moves or copies a block of lines (e.g. the original file
> has A and then B, and the commit changes it to B and then
> @@ -93,7 +93,7 @@ alphanumeric characters that Git must detect as moving/copying
> within a file for it to associate those lines with the parent
> commit. The default value is 20.
>
> --C|<num>|::
> +-C[<num>]::
> In addition to `-M`, detect lines moved or copied from other
> files that were modified in the same commit. This is
> useful when you reorganize your program and move code
^ permalink raw reply
* Re: [PATCH 03/15] lib-submodule-update.sh: define tests for recursing into submodules
From: Stefan Beller @ 2017-02-22 18:43 UTC (permalink / raw)
To: Junio C Hamano
Cc: git@vger.kernel.org, brian m. carlson, Jonathan Nieder,
Brandon Williams
In-Reply-To: <xmqqlgt5x430.fsf@gitster.mtv.corp.google.com>
On Thu, Feb 16, 2017 at 12:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Stefan Beller <sbeller@google.com> writes:
>
>> Currently lib-submodule-update.sh provides 2 functions
>> test_submodule_switch and test_submodule_forced_switch that are used by a
>> variety of tests to ensure that submodules behave as expected. The current
>> expected behavior is that submodules are not touched at all (see
>> 42639d2317a for the exact setup).
>>
>> In the future we want to teach all these commands to properly recurse
>> into submodules. To do that, we'll add two testing functions to
>> submodule-update-lib.sh test_submodule_switch_recursing and
>> test_submodule_forced_switch_recursing.
>
> I'd remove "properly" and insert a colon after "add two ... to
> submodule-update-lib.sh" before the names of two functions.
ok
>
>> +reset_work_tree_to_interested () {
>> + reset_work_tree_to $1 &&
>> + # indicate we are interested in the submodule:
>> + git -C submodule_update config submodule.sub1.url "bogus" &&
>> + # also have it available:
>> + if ! test -d submodule_update/.git/modules/sub1
>> + then
>> + mkdir submodule_update/.git/modules &&
>
> Would we want "mkdir -p" here to be safe?
Yes I cannot think of a downside of being overly cautious here.
>
>> + cp -r submodule_update_repo/.git/modules/sub1 submodule_update/.git/modules/sub1
>
> ... ahh, wouldn't matter that much, we checked that modules/sub1
> does not exist, and as long as nobody creates modules/ or modules/somethingelse
> we are OK.
Well, I'll add the -p
>> +# Test that submodule contents are correctly updated when switching
>> +# between commits that change a submodule.
>> +# Test that the following transitions are correctly handled:
>> +# (These tests are also above in the case where we expect no change
>> +# in the submodule)
>> +# - Updated submodule
>> +# - New submodule
>> +# - Removed submodule
>> +# - Directory containing tracked files replaced by submodule
>> +# - Submodule replaced by tracked files in directory
>> +# - Submodule replaced by tracked file with the same name
>> +# - tracked file replaced by submodule
>
> These should work without trouble only when the paths involved in
> the operation in the working tree are clean, right? Just double
> checking. If they are dirty we should expect a failure, instead of
> silent loss of information.
yes, I'll go over the tests again and add those cases if missing.
>> + command="$1"
>
> The dq-pair is not strictly needed on the RHS of the assignment, but
> it is a good way to signal that we considered that we might receive
> an argument with $IFS in it...
>
>> + $command add_sub1 &&
>
> ... and after doing so, not quoting $command here signals that we
> expect command line splitting to happen. Am I reading it correctly?
> Without an actual caller appearing in this step, it is rather hard
> to judge.
>
I followed the existing code without thinking about these points, but they are
valid and exactly how we'd expect the code to behave.
$1 / $command will be e.g. "git checkout --recurse-submodules" in
this patch series; but later on we could also have functions.
C.f. t4137 which defines a function
apply_3way () {
git diff --ignore-submodules=dirty "..$1" | git apply --3way -
}
test_submodule_switch "apply_3way"
We'd want to have a similar thing for the recursing part, e.g.
apply_3way_recursing () {
git diff --submodule=diff "..$1" | git apply
--recurse-submodules --3way -
}
test_submodule_switch_recursing "apply_3way_recursing"
>> + echo sub1 > .git/info/exclude
>
> ">.git/info/exclude"
ok
^ permalink raw reply
* [PATCH] git add -i: replace \t with blanks in the help message
From: Ralf Thielow @ 2017-02-22 18:46 UTC (permalink / raw)
To: git; +Cc: Junio C Hamano, Jiang Xin, Ralf Thielow
Within the help message of 'git add -i', the 'diff' command uses one
tab character and blanks to create the space between the name and the
description while the others use blanks only. So if the tab size is
not at 4 characters, this description will not be in range.
Replace the tab character with blanks.
Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com>
---
git-add--interactive.perl | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/git-add--interactive.perl b/git-add--interactive.perl
index cf6fc926a..982593c89 100755
--- a/git-add--interactive.perl
+++ b/git-add--interactive.perl
@@ -1678,7 +1678,7 @@ status - show paths with changes
update - add working tree state to the staged set of changes
revert - revert staged set of changes back to the HEAD version
patch - pick hunks and update selectively
-diff - view diff between HEAD and index
+diff - view diff between HEAD and index
add untracked - add contents of untracked files to the staged set of changes
EOF
}
--
2.12.0.rc2.424.g63d3652c7
^ permalink raw reply related
* Re: [PATCH v5 03/19] builtin/diff-tree: convert to struct object_id
From: Junio C Hamano @ 2017-02-22 18:50 UTC (permalink / raw)
To: brian m. carlson; +Cc: git, Jeff King, Michael Haggerty, Ramsay Jones
In-Reply-To: <20170221234737.894681-4-sandals@crustytoothpaste.net>
"brian m. carlson" <sandals@crustytoothpaste.net> writes:
> Convert most leaf functions to struct object_id. Change several
> hardcoded numbers to uses of parse_oid_hex. In doing so, verify that we
> when we want two trees, we have exactly two trees.
>
> Finally, in stdin_diff_commit, avoid accessing the byte after the NUL.
> This will be a NUL as well, since the first NUL was a newline we
> overwrote. However, with parse_oid_hex, we no longer need to increment
> the pointer directly, and can simply increment it as part of our check
> for the space character.
After reading the pre- and post-image twice, I think I convinced
myself that this is a faithful conersion and they do the same thing.
What the function does appears somewhat iffy in the modern world.
We are relying on the fact that while Git is operating in this mode
of reading a tuple of commits per line and doing log-tree, that Git
itself will not do the history traversal (instead, another instance
of Git that feeds us via our standard input is walking the history)
for this piece of code to work correctly.
Of course, the "diff-tree --stdin" command was meant to sit on the
downstream of "rev-list --parents", so the assumption the code makes
(i.e. the parents field of the in-core commit objects do not have to
be usable for history traversal) may be reasonable, but still...
^ permalink raw reply
* Re: url.<base>.insteadOf vs. submodules
From: Jeff King @ 2017-02-22 18:57 UTC (permalink / raw)
To: Junio C Hamano
Cc: Jon Loeliger, Stefan Beller, Toolforger, git@vger.kernel.org
In-Reply-To: <xmqqh93mcelv.fsf@gitster.mtv.corp.google.com>
On Wed, Feb 22, 2017 at 09:36:12AM -0800, Junio C Hamano wrote:
> >> My gut feeling is that we should do the selective/filtered include
> >> Peff mentioned when a repository is known to be used as a submodule
> >> of somebody else.
> >
> > Does the management of these submodue-related config values
> > become easier if, instead of placing them in .config, we
> > place them in a git/.context file?
>
> Do you mean that Git users that use submodules adopt a convention
> where a separate file in $GIT_DIR of the toplevel superproject holds
> pieces of configuration that are meant to be shared between the
> superproject and across all its submodules, and the $GIT_DIR/config
> file in submodules and the superproject all include that shared one
> via include.path mechanism?
>
> That may allow us to do without being responsible for sifting of
> configuration variables into safe and unsafe bins.
>
> I dunno.
Hmm. I certainly like that we punt on having to decide on the "should
this be shared with submodules" decision. That makes the end result more
flexible, and we don't have to get into a never-ending stream of
"whitelist this config option" patches.
My only concern is that it's not as discoverable. In the situation that
kicked off this thread, somebody put url.X.insteadOf into their
super-project .git/config, expecting it to work in the submodules. That
_still_ wouldn't work with this proposal. They'd have to:
1. Put it in .git/context (or whatever we call it)
2. Maybe add include.path=context in .git/config if they want the
config shared with the super-project (or this could be automatic?)
I guess it gives _a_ solution, which is more than we have now, but it
doesn't feel very ergonomic.
-Peff
^ permalink raw reply
* Re: url.<base>.insteadOf vs. submodules
From: Stefan Beller @ 2017-02-22 19:11 UTC (permalink / raw)
To: Jeff King; +Cc: Junio C Hamano, Jon Loeliger, Toolforger, git@vger.kernel.org
In-Reply-To: <20170222185711.2kpzeypptg6deytc@sigill.intra.peff.net>
On Wed, Feb 22, 2017 at 10:57 AM, Jeff King <peff@peff.net> wrote:
> On Wed, Feb 22, 2017 at 09:36:12AM -0800, Junio C Hamano wrote:
>
>> >> My gut feeling is that we should do the selective/filtered include
>> >> Peff mentioned when a repository is known to be used as a submodule
>> >> of somebody else.
>> >
>> > Does the management of these submodue-related config values
>> > become easier if, instead of placing them in .config, we
>> > place them in a git/.context file?
>>
>> Do you mean that Git users that use submodules adopt a convention
>> where a separate file in $GIT_DIR of the toplevel superproject holds
>> pieces of configuration that are meant to be shared between the
>> superproject and across all its submodules, and the $GIT_DIR/config
>> file in submodules and the superproject all include that shared one
>> via include.path mechanism?
>>
>> That may allow us to do without being responsible for sifting of
>> configuration variables into safe and unsafe bins.
>>
>> I dunno.
>
> Hmm. I certainly like that we punt on having to decide on the "should
> this be shared with submodules" decision. That makes the end result more
> flexible, and we don't have to get into a never-ending stream of
> "whitelist this config option" patches.
>
> My only concern is that it's not as discoverable. In the situation that
> kicked off this thread, somebody put url.X.insteadOf into their
> super-project .git/config, expecting it to work in the submodules. That
> _still_ wouldn't work with this proposal. They'd have to:
>
> 1. Put it in .git/context (or whatever we call it)
>
> 2. Maybe add include.path=context in .git/config if they want the
> config shared with the super-project (or this could be automatic?)
>
> I guess it gives _a_ solution, which is more than we have now, but it
> doesn't feel very ergonomic.
Well, currently ".git/config" is the one and only blessed way to configure
a single repo and our documentation and user expectations reflect that.
Once git-worktree takes off (which has per working tree configuration files)
it doesn't feel as obscure anymore to have multiple config files.
The working trees will share the $GIT_COMMON_DIR/config file for
all working trees and have its own config file at $GIT_DIR/config.worktree
in its respective git directories. C.f.
https://public-inbox.org/git/20170110112524.12870-2-pclouds@gmail.com/
So I could imagine that we just introduce another config file
config.submodules which is source'd by the submodules.
Then the hard part becomes to decide which config value to put
in which config file. (We'd still be left to guess where to put some initial
new configuration value. config or config.submodules. Any update of a
value can just stay in its respective file. And I don't think we'd want
to invent a config option that tells us which policy we use where to
put config options. That sounds just scary.)
^ permalink raw reply
* Re: [PATCH v5 03/19] builtin/diff-tree: convert to struct object_id
From: Jeff King @ 2017-02-22 19:16 UTC (permalink / raw)
To: Junio C Hamano; +Cc: brian m. carlson, git, Michael Haggerty, Ramsay Jones
In-Reply-To: <xmqqy3wyawlu.fsf@gitster.mtv.corp.google.com>
On Wed, Feb 22, 2017 at 10:50:21AM -0800, Junio C Hamano wrote:
> "brian m. carlson" <sandals@crustytoothpaste.net> writes:
>
> > Convert most leaf functions to struct object_id. Change several
> > hardcoded numbers to uses of parse_oid_hex. In doing so, verify that we
> > when we want two trees, we have exactly two trees.
> >
> > Finally, in stdin_diff_commit, avoid accessing the byte after the NUL.
> > This will be a NUL as well, since the first NUL was a newline we
> > overwrote. However, with parse_oid_hex, we no longer need to increment
> > the pointer directly, and can simply increment it as part of our check
> > for the space character.
>
> After reading the pre- and post-image twice, I think I convinced
> myself that this is a faithful conersion and they do the same thing.
I think this is correct, too (but then, I think it largely comes from
the patch I wrote the other night. So I did look at it carefully, but
it's not exactly an independent review).
> What the function does appears somewhat iffy in the modern world.
> We are relying on the fact that while Git is operating in this mode
> of reading a tuple of commits per line and doing log-tree, that Git
> itself will not do the history traversal (instead, another instance
> of Git that feeds us via our standard input is walking the history)
> for this piece of code to work correctly.
>
> Of course, the "diff-tree --stdin" command was meant to sit on the
> downstream of "rev-list --parents", so the assumption the code makes
> (i.e. the parents field of the in-core commit objects do not have to
> be usable for history traversal) may be reasonable, but still...
I'm not sure it's that weird. "diff-tree" is about diffing, not
traversal. The only reason it touches commit->parents at all (and
doesn't just kick off a diff between the arguments it gets) is that it's
been stuck with pretty-printing the commits, which might ask to show the
parents.
-Peff
^ permalink raw reply
* Re: [PATCH v5 03/19] builtin/diff-tree: convert to struct object_id
From: Junio C Hamano @ 2017-02-22 20:04 UTC (permalink / raw)
To: Jeff King; +Cc: brian m. carlson, git, Michael Haggerty, Ramsay Jones
In-Reply-To: <20170222191641.o2rtt2ymtb4h2yqe@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> On Wed, Feb 22, 2017 at 10:50:21AM -0800, Junio C Hamano wrote:
>
>> What the function does appears somewhat iffy in the modern world.
>> We are relying on the fact that while Git is operating in this mode
>> of reading a tuple of commits per line and doing log-tree, that Git
>> itself will not do the history traversal (instead, another instance
>> of Git that feeds us via our standard input is walking the history)
>> for this piece of code to work correctly.
>>
>> Of course, the "diff-tree --stdin" command was meant to sit on the
>> downstream of "rev-list --parents", so the assumption the code makes
>> (i.e. the parents field of the in-core commit objects do not have to
>> be usable for history traversal) may be reasonable, but still...
>
> I'm not sure it's that weird. "diff-tree" is about diffing, not
> traversal. The only reason it touches commit->parents at all (and
> doesn't just kick off a diff between the arguments it gets) is that it's
> been stuck with pretty-printing the commits, which might ask to show the
> parents.
Yeah, I understand all that as 45392a648d ("git-diff-tree --stdin:
show all parents.", 2006-02-05) was mostly mine. It's just I sense
that the recent trend is to take whatever existing code and see if
they are reusable in other contexts, and this is one of the things
that people might want to libify, but cannot be as-is.
^ 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