* [RFC 05/14] fetch: refactor fetch_refs into two functions
From: Jonathan Tan @ 2017-01-25 22:02 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Refactor the fetch_refs function into a function that does the fetching
of refs and another function that stores them. This prepares for a
future patch where some processing may be done between those tasks.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/fetch.c | 19 +++++++++++++------
1 file changed, 13 insertions(+), 6 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index c71d5eb9b..43e35c494 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -918,10 +918,16 @@ static int fetch_refs(struct transport *transport, struct ref *ref_map)
int ret = quickfetch(ref_map);
if (ret)
ret = transport_fetch_refs(transport, ref_map);
- if (!ret)
- ret |= store_updated_refs(transport->url,
- transport->remote->name,
- ref_map);
+ if (ret)
+ transport_unlock_pack(transport);
+ return ret;
+}
+
+static int consume_refs(struct transport *transport, struct ref *ref_map)
+{
+ int ret = store_updated_refs(transport->url,
+ transport->remote->name,
+ ref_map);
transport_unlock_pack(transport);
return ret;
}
@@ -1062,7 +1068,8 @@ static void backfill_tags(struct transport *transport, struct ref *ref_map)
transport_set_option(transport, TRANS_OPT_FOLLOWTAGS, NULL);
transport_set_option(transport, TRANS_OPT_DEPTH, "0");
transport_set_option(transport, TRANS_OPT_DEEPEN_RELATIVE, NULL);
- fetch_refs(transport, ref_map);
+ if (!fetch_refs(transport, ref_map))
+ consume_refs(transport, ref_map);
if (gsecondary) {
transport_disconnect(gsecondary);
@@ -1115,7 +1122,7 @@ static int do_fetch(struct transport *transport,
transport->url);
}
}
- if (fetch_refs(transport, ref_map)) {
+ if (fetch_refs(transport, ref_map) || consume_refs(transport, ref_map)) {
free_refs(ref_map);
retcode = 1;
goto cleanup;
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 06/14] fetch: refactor to make function args narrower
From: Jonathan Tan @ 2017-01-25 22:02 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Refactor find_non_local_tags and get_ref_map to only take the
information they need instead of the entire transport struct. Besides
improving code clarity, this also improves their flexibility, which will
be needed in a future patch.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/fetch.c | 23 ++++++++++++-----------
1 file changed, 12 insertions(+), 11 deletions(-)
diff --git a/builtin/fetch.c b/builtin/fetch.c
index 43e35c494..ae7c6daa1 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -220,7 +220,7 @@ static int will_fetch(struct ref **head, const unsigned char *sha1)
return 0;
}
-static void find_non_local_tags(struct transport *transport,
+static void find_non_local_tags(const struct ref *refs,
struct ref **head,
struct ref ***tail)
{
@@ -230,7 +230,7 @@ static void find_non_local_tags(struct transport *transport,
struct string_list_item *item = NULL;
for_each_ref(add_existing, &existing_refs);
- for (ref = transport_get_remote_refs(transport); ref; ref = ref->next) {
+ for (ref = refs; ref; ref = ref->next) {
if (!starts_with(ref->name, "refs/tags/"))
continue;
@@ -302,7 +302,8 @@ static void find_non_local_tags(struct transport *transport,
string_list_clear(&remote_refs, 0);
}
-static struct ref *get_ref_map(struct transport *transport,
+static struct ref *get_ref_map(const struct remote *remote,
+ const struct ref *remote_refs,
struct refspec *refspecs, int refspec_count,
int tags, int *autotags)
{
@@ -314,8 +315,6 @@ static struct ref *get_ref_map(struct transport *transport,
/* opportunistically-updated references: */
struct ref *orefs = NULL, **oref_tail = &orefs;
- const struct ref *remote_refs = transport_get_remote_refs(transport);
-
struct string_list existing_refs = STRING_LIST_INIT_DUP;
if (refspec_count) {
@@ -355,8 +354,8 @@ static struct ref *get_ref_map(struct transport *transport,
fetch_refspec = parse_fetch_refspec(refmap_nr, refmap_array);
fetch_refspec_nr = refmap_nr;
} else {
- fetch_refspec = transport->remote->fetch;
- fetch_refspec_nr = transport->remote->fetch_refspec_nr;
+ fetch_refspec = remote->fetch;
+ fetch_refspec_nr = remote->fetch_refspec_nr;
}
for (i = 0; i < fetch_refspec_nr; i++)
@@ -365,7 +364,6 @@ static struct ref *get_ref_map(struct transport *transport,
die("--refmap option is only meaningful with command-line refspec(s).");
} else {
/* Use the defaults */
- struct remote *remote = transport->remote;
struct branch *branch = branch_get(NULL);
int has_merge = branch_has_merge_config(branch);
if (remote &&
@@ -404,7 +402,7 @@ static struct ref *get_ref_map(struct transport *transport,
/* also fetch all tags */
get_fetch_map(remote_refs, tag_refspec, &tail, 0);
else if (tags == TAGS_DEFAULT && *autotags)
- find_non_local_tags(transport, &ref_map, &tail);
+ find_non_local_tags(remote_refs, &ref_map, &tail);
/* Now append any refs to be updated opportunistically: */
*tail = orefs;
@@ -1083,6 +1081,7 @@ static int do_fetch(struct transport *transport,
struct ref *ref_map;
int autotags = (transport->remote->fetch_tags == 1);
int retcode = 0;
+ const struct ref *remote_refs;
if (tags == TAGS_DEFAULT) {
if (transport->remote->fetch_tags == 2)
@@ -1101,7 +1100,9 @@ static int do_fetch(struct transport *transport,
goto cleanup;
}
- ref_map = get_ref_map(transport, refs, ref_count, tags, &autotags);
+ remote_refs = transport_get_remote_refs(transport);
+ ref_map = get_ref_map(transport->remote, remote_refs, refs, ref_count,
+ tags, &autotags);
if (!update_head_ok)
check_not_current_branch(ref_map);
@@ -1134,7 +1135,7 @@ static int do_fetch(struct transport *transport,
if (tags == TAGS_DEFAULT && autotags) {
struct ref **tail = &ref_map;
ref_map = NULL;
- find_non_local_tags(transport, &ref_map, &tail);
+ find_non_local_tags(remote_refs, &ref_map, &tail);
if (ref_map)
backfill_tags(transport, ref_map);
free_refs(ref_map);
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 09/14] transport: put ref oid in out param
From: Jonathan Tan @ 2017-01-25 22:03 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Return new OID information obtained through fetching in new structs
instead of reusing the existing ones. With this change, the input
structs are no longer used for output, and are thus marked const.
This is the 3rd of 3 patches to eliminate using input refs to
communicate information obtained by the fetch mechanism.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/clone.c | 14 ++++++++++++--
builtin/fetch-pack.c | 4 ++--
fetch-pack.c | 26 +++++++++++++++-----------
fetch-pack.h | 2 +-
transport-helper.c | 34 +++++++++++++++++++++++-----------
transport.c | 6 +++---
transport.h | 13 +++++--------
7 files changed, 61 insertions(+), 38 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 0135c5f1c..3191da391 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -858,6 +858,8 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
struct refspec *refspec;
const char *fetch_pattern;
+ struct ref *new_remote_refs = NULL;
+
packet_trace_identity("clone");
argc = parse_options(argc, argv, prefix, builtin_clone_options,
builtin_clone_usage, 0);
@@ -1075,8 +1077,15 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
break;
}
- if (!is_local && !complete_refs_before_fetch)
- transport_fetch_refs(transport, mapped_refs, NULL);
+ if (!is_local && !complete_refs_before_fetch) {
+ transport_fetch_refs(transport, mapped_refs,
+ &new_remote_refs);
+ if (new_remote_refs) {
+ refs = new_remote_refs;
+ free_refs(mapped_refs);
+ mapped_refs = wanted_peer_refs(refs, refspec);
+ }
+ }
remote_head = find_ref_by_name(refs, "HEAD");
remote_head_points_at =
@@ -1148,5 +1157,6 @@ int cmd_clone(int argc, const char **argv, const char *prefix)
junk_mode = JUNK_LEAVE_ALL;
free(refspec);
+ free_refs(new_remote_refs);
return err;
}
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index f31f874a0..a18fd0c44 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -11,7 +11,7 @@ static const char fetch_pack_usage[] =
"[--include-tag] [--upload-pack=<git-upload-pack>] [--depth=<n>] "
"[--no-progress] [--diag-url] [-v] [<host>:]<directory> [<refs>...]";
-static void add_sought_entry(struct ref ***sought, int *nr, int *alloc,
+static void add_sought_entry(const struct ref ***sought, int *nr, int *alloc,
const char *name)
{
struct ref *ref;
@@ -44,7 +44,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
int i, ret;
struct ref *ref = NULL;
const char *dest = NULL;
- struct ref **sought = NULL;
+ const struct ref **sought = NULL;
int nr_sought = 0, alloc_sought = 0;
int fd[2];
char *pack_lockfile = NULL;
diff --git a/fetch-pack.c b/fetch-pack.c
index d4642b05c..8cc85c19f 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -52,6 +52,10 @@ static int non_common_revs, multi_ack, use_sideband;
#define ALLOW_REACHABLE_SHA1 02
static unsigned int allow_unadvertised_object_request;
+/* An arbitrary non-NULL non-const pointer to assign to the util field of
+ * string list items when we need one. */
+#define ARBITRARY (&transfer_unpack_limit)
+
__attribute__((format (printf, 2, 3)))
static inline void print_verbose(const struct fetch_pack_args *args,
const char *fmt, ...)
@@ -556,7 +560,7 @@ static void mark_recent_complete_commits(struct fetch_pack_args *args,
static void filter_refs(struct fetch_pack_args *args,
struct ref **refs,
- struct ref **sought, int nr_sought)
+ const struct ref **sought, int nr_sought)
{
struct ref *newlist = NULL;
struct ref **newtail = &newlist;
@@ -604,16 +608,16 @@ static void filter_refs(struct fetch_pack_args *args,
for (i = 0; i < nr_sought; i++) {
unsigned char sha1[20];
- ref = sought[i];
+ const struct ref *sref = sought[i];
if (matched[i])
continue;
- if (get_sha1_hex(ref->name, sha1) ||
- ref->name[40] != '\0' ||
- hashcmp(sha1, ref->old_oid.hash))
+ if (get_sha1_hex(sref->name, sha1) ||
+ sref->name[40] != '\0' ||
+ hashcmp(sha1, sref->old_oid.hash))
continue;
matched[i] = 1;
- *newtail = copy_ref(ref);
+ *newtail = copy_ref(sref);
newtail = &(*newtail)->next;
}
}
@@ -629,7 +633,7 @@ static void mark_alternate_complete(const struct ref *ref, void *unused)
static int everything_local(struct fetch_pack_args *args,
struct ref **refs,
- struct ref **sought, int nr_sought)
+ const struct ref **sought, int nr_sought)
{
struct ref *ref;
int retval;
@@ -831,7 +835,7 @@ static int cmp_ref_by_name(const void *a_, const void *b_)
static struct ref *do_fetch_pack(struct fetch_pack_args *args,
int fd[2],
const struct ref *orig_ref,
- struct ref **sought, int nr_sought,
+ const struct ref **sought, int nr_sought,
struct shallow_info *si,
char **pack_lockfile)
{
@@ -955,7 +959,7 @@ static void fetch_pack_setup(void)
did_setup = 1;
}
-static int remove_duplicates_in_refs(struct ref **ref, int nr)
+static int remove_duplicates_in_refs(const struct ref **ref, int nr)
{
struct string_list names = STRING_LIST_INIT_NODUP;
int src, dst;
@@ -965,7 +969,7 @@ static int remove_duplicates_in_refs(struct ref **ref, int nr)
item = string_list_insert(&names, ref[src]->name);
if (item->util)
continue; /* already have it */
- item->util = ref[src];
+ item->util = ARBITRARY;
if (src != dst)
ref[dst] = ref[src];
dst++;
@@ -1078,7 +1082,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
int fd[], struct child_process *conn,
const struct ref *ref,
const char *dest,
- struct ref **sought, int nr_sought,
+ const struct ref **sought, int nr_sought,
struct sha1_array *shallow,
char **pack_lockfile)
{
diff --git a/fetch-pack.h b/fetch-pack.h
index 76f7c719c..6e4fdbb68 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -38,7 +38,7 @@ struct ref *fetch_pack(struct fetch_pack_args *args,
int fd[], struct child_process *conn,
const struct ref *ref,
const char *dest,
- struct ref **sought,
+ const struct ref **sought,
int nr_sought,
struct sha1_array *shallow,
char **pack_lockfile);
diff --git a/transport-helper.c b/transport-helper.c
index f3d78bb9e..be0aa6d39 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -384,7 +384,7 @@ static int release_helper(struct transport *transport)
}
static int fetch_with_fetch(struct transport *transport,
- int nr_heads, struct ref **to_fetch)
+ int nr_heads, const struct ref **to_fetch)
{
struct helper_data *data = transport->data;
int i;
@@ -477,13 +477,14 @@ static int get_exporter(struct transport *transport,
}
static int fetch_with_import(struct transport *transport,
- int nr_heads, struct ref **to_fetch)
+ int nr_heads, const struct ref **to_fetch, struct ref **fetched_refs)
{
struct child_process fastimport;
struct helper_data *data = transport->data;
int i;
- struct ref *posn;
+ const struct ref *posn;
struct strbuf buf = STRBUF_INIT;
+ struct ref *fr_head = NULL, **fr_tail = &fr_head;
get_helper(transport);
@@ -522,28 +523,38 @@ static int fetch_with_import(struct transport *transport,
* (If no "refspec" capability was specified, for historical
* reasons we default to the equivalent of *:*.)
*
- * Store the result in to_fetch[i].old_sha1. Callers such
+ * Store the result in old_oid in fetched_refs. Callers such
* as "git fetch" can use the value to write feedback to the
* terminal, populate FETCH_HEAD, and determine what new value
* should be written to peer_ref if the update is a
* fast-forward or this is a forced update.
*/
+ if (!fetched_refs)
+ goto cleanup;
for (i = 0; i < nr_heads; i++) {
- char *private, *name;
- posn = to_fetch[i];
- if (posn->status & REF_STATUS_UPTODATE)
+ struct ref *ref;
+ char *private;
+ const char *name;
+
+ ref = copy_ref_peerless(to_fetch[i]);
+ *fr_tail = ref;
+ fr_tail = &ref->next;
+ if (ref->status & REF_STATUS_UPTODATE)
continue;
- name = posn->symref ? posn->symref : posn->name;
+ name = ref->symref ? ref->symref : ref->name;
if (data->refspecs)
private = apply_refspecs(data->refspecs, data->refspec_nr, name);
else
private = xstrdup(name);
if (private) {
- if (read_ref(private, posn->old_oid.hash) < 0)
+ if (read_ref(private, ref->old_oid.hash) < 0)
die("Could not read ref %s", private);
free(private);
}
}
+ *fetched_refs = fr_head;
+
+cleanup:
strbuf_release(&buf);
return 0;
}
@@ -646,7 +657,8 @@ static int connect_helper(struct transport *transport, const char *name,
}
static int fetch(struct transport *transport,
- int nr_heads, struct ref **to_fetch, struct ref **fetched_refs)
+ int nr_heads, const struct ref **to_fetch,
+ struct ref **fetched_refs)
{
struct helper_data *data = transport->data;
int i, count;
@@ -679,7 +691,7 @@ static int fetch(struct transport *transport,
return fetch_with_fetch(transport, nr_heads, to_fetch);
if (data->import)
- return fetch_with_import(transport, nr_heads, to_fetch);
+ return fetch_with_import(transport, nr_heads, to_fetch, fetched_refs);
return -1;
}
diff --git a/transport.c b/transport.c
index 80e007f2f..5ed3fc68e 100644
--- a/transport.c
+++ b/transport.c
@@ -95,7 +95,7 @@ static struct ref *get_refs_from_bundle(struct transport *transport, int for_pus
}
static int fetch_refs_from_bundle(struct transport *transport,
- int nr_heads, struct ref **to_fetch,
+ int nr_heads, const struct ref **to_fetch,
struct ref **fetched_refs)
{
struct bundle_transport_data *data = transport->data;
@@ -203,7 +203,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 nr_heads, const struct ref **to_fetch,
struct ref **fetched_refs)
{
struct git_transport_data *data = transport->data;
@@ -1101,7 +1101,7 @@ int transport_fetch_refs(struct transport *transport, struct ref *refs,
{
int rc;
int nr_heads = 0, nr_alloc = 0, nr_refs = 0;
- struct ref **heads = NULL;
+ const struct ref **heads = NULL;
struct ref *nop_head = NULL, **nop_tail = &nop_head;
struct ref *rm;
diff --git a/transport.h b/transport.h
index b9e7e4656..326ff9bd6 100644
--- a/transport.h
+++ b/transport.h
@@ -84,15 +84,12 @@ struct transport {
*
* The transport *may* provide, in fetched_refs, the list of refs that
* it fetched. If the transport knows anything about the fetched refs
- * that the caller does not know (for example, shallow status), it
- * should provide that list of refs and include that information in the
- * list.
- *
- * If the transport did not get hashes for refs in
- * get_refs_list(), it should set the old_sha1 fields in the
- * provided refs now.
+ * that the caller does not know (for example, shallow status or ref
+ * hashes), it should provide that list of refs and include that
+ * information in the list.
**/
- int (*fetch)(struct transport *transport, int refs_nr, struct ref **refs,
+ int (*fetch)(struct transport *transport,
+ int refs_nr, const struct ref **refs,
struct ref **fetched_refs);
/**
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 08/14] fetch-pack: check returned refs for matches
From: Jonathan Tan @ 2017-01-25 22:03 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Instead of setting "matched" on matched refs, detect matches by checking
the sought refs against the returned refs. Also, since the "matched"
field in struct ref is now no longer used, remove it.
This is the 2nd of 3 patches to eliminate using input refs to
communicate information obtained by the fetch mechanism.
(There are possibly ways more efficient than a nested for loop to
accomplish this. However, since a subsequent patch will compare the
user-provided refspecs against the fetched refs directly, and thus
necessitating the nested for loop anyway, I decided to use the nested
for loop in this patch.)
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/fetch-pack.c | 7 ++++++-
fetch-pack.c | 9 ++++++---
fetch-pack.h | 2 --
remote.h | 3 +--
4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index cfe9e447c..f31f874a0 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -4,6 +4,7 @@
#include "remote.h"
#include "connect.h"
#include "sha1-array.h"
+#include "refs.h"
static const char fetch_pack_usage[] =
"git fetch-pack [--all] [--stdin] [--quiet | -q] [--keep | -k] [--thin] "
@@ -220,7 +221,11 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
* an error.
*/
for (i = 0; i < nr_sought; i++) {
- if (!sought[i] || sought[i]->matched)
+ struct ref *r;
+ for (r = ref; r; r = r->next)
+ if (!sought[i] || refname_match(sought[i]->name, r->name))
+ break;
+ if (r)
continue;
error("no such remote ref %s", sought[i]->name);
ret = 1;
diff --git a/fetch-pack.c b/fetch-pack.c
index 9a87ddd3d..d4642b05c 100644
--- a/fetch-pack.c
+++ b/fetch-pack.c
@@ -562,6 +562,7 @@ static void filter_refs(struct fetch_pack_args *args,
struct ref **newtail = &newlist;
struct ref *ref, *next;
int i;
+ int *matched = xcalloc(nr_sought, sizeof(*matched));
i = 0;
for (ref = *refs; ref; ref = next) {
@@ -578,7 +579,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;
+ matched[i] = 1;
}
i++;
}
@@ -604,19 +605,21 @@ static void filter_refs(struct fetch_pack_args *args,
unsigned char sha1[20];
ref = sought[i];
- if (ref->matched)
+ if (matched[i])
continue;
if (get_sha1_hex(ref->name, sha1) ||
ref->name[40] != '\0' ||
hashcmp(sha1, ref->old_oid.hash))
continue;
- ref->matched = 1;
+ matched[i] = 1;
*newtail = copy_ref(ref);
newtail = &(*newtail)->next;
}
}
*refs = newlist;
+
+ free(matched);
}
static void mark_alternate_complete(const struct ref *ref, void *unused)
diff --git a/fetch-pack.h b/fetch-pack.h
index c912e3d32..76f7c719c 100644
--- a/fetch-pack.h
+++ b/fetch-pack.h
@@ -33,8 +33,6 @@ struct fetch_pack_args {
/*
* sought represents remote references that should be updated from.
- * On return, the names that were found on the remote will have been
- * marked as such.
*/
struct ref *fetch_pack(struct fetch_pack_args *args,
int fd[], struct child_process *conn,
diff --git a/remote.h b/remote.h
index 57d059431..2f7f23d47 100644
--- a/remote.h
+++ b/remote.h
@@ -89,8 +89,7 @@ struct ref {
force:1,
forced_update:1,
expect_old_sha1:1,
- deletion:1,
- matched:1;
+ deletion:1;
/*
* Order is important here, as we write to FETCH_HEAD
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* [RFC 10/14] fetch-pack: support partial names and globs
From: Jonathan Tan @ 2017-01-25 22:03 UTC (permalink / raw)
To: git; +Cc: Jonathan Tan
In-Reply-To: <cover.1485381677.git.jonathantanmy@google.com>
Teach fetch-pack to support partial ref names and ref patterns as input.
This does not use "want-ref" yet - support for that will be added in a
future patch.
Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
---
builtin/fetch-pack.c | 40 ++++++++++++-------------------------
remote.c | 55 +++++++++++++++++++++++++++++++++++++++++++++++++++
remote.h | 16 +++++++++++++++
t/t5500-fetch-pack.sh | 38 +++++++++++++++++++++++++++++++++++
4 files changed, 122 insertions(+), 27 deletions(-)
diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index a18fd0c44..5f14242ae 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -11,32 +11,12 @@ static const char fetch_pack_usage[] =
"[--include-tag] [--upload-pack=<git-upload-pack>] [--depth=<n>] "
"[--no-progress] [--diag-url] [-v] [<host>:]<directory> [<refs>...]";
-static void add_sought_entry(const struct ref ***sought, int *nr, int *alloc,
+static void add_sought_entry(struct refspec **sought, int *nr, int *alloc,
const char *name)
{
- struct ref *ref;
- struct object_id oid;
-
- if (!get_oid_hex(name, &oid)) {
- if (name[GIT_SHA1_HEXSZ] == ' ') {
- /* <sha1> <ref>, find refname */
- name += GIT_SHA1_HEXSZ + 1;
- } else if (name[GIT_SHA1_HEXSZ] == '\0') {
- ; /* <sha1>, leave sha1 as name */
- } else {
- /* <ref>, clear cruft from oid */
- oidclr(&oid);
- }
- } else {
- /* <ref>, clear cruft from get_oid_hex */
- oidclr(&oid);
- }
-
- ref = alloc_ref(name);
- oidcpy(&ref->old_oid, &oid);
(*nr)++;
ALLOC_GROW(*sought, *nr, *alloc);
- (*sought)[*nr - 1] = ref;
+ parse_ref_or_pattern(&(*sought)[*nr - 1], name);
}
int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
@@ -44,8 +24,10 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
int i, ret;
struct ref *ref = NULL;
const char *dest = NULL;
- const struct ref **sought = NULL;
+ struct refspec *sought = NULL;
int nr_sought = 0, alloc_sought = 0;
+ const struct ref **sought_refs;
+ int nr_sought_refs;
int fd[2];
char *pack_lockfile = NULL;
char **pack_lockfile_ptr = NULL;
@@ -195,8 +177,9 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
return args.diag_url ? 0 : 1;
}
get_remote_heads(fd[0], NULL, 0, &ref, 0, NULL, &shallow);
+ get_ref_array(&sought_refs, &nr_sought_refs, ref, sought, nr_sought);
- ref = fetch_pack(&args, fd, conn, ref, dest, sought, nr_sought,
+ ref = fetch_pack(&args, fd, conn, ref, dest, sought_refs, nr_sought_refs,
&shallow, pack_lockfile_ptr);
if (pack_lockfile) {
printf("lock %s\n", pack_lockfile);
@@ -222,12 +205,15 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix)
*/
for (i = 0; i < nr_sought; i++) {
struct ref *r;
- for (r = ref; r; r = r->next)
- if (!sought[i] || refname_match(sought[i]->name, r->name))
+ if (sought[i].pattern)
+ continue; /* patterns do not need to match anything */
+ for (r = ref; r; r = r->next) {
+ if (refname_match(sought[i].src, r->name))
break;
+ }
if (r)
continue;
- error("no such remote ref %s", sought[i]->name);
+ error("no such remote ref %s", sought[i].src);
ret = 1;
}
diff --git a/remote.c b/remote.c
index 725a2d39a..08f3c910e 100644
--- a/remote.c
+++ b/remote.c
@@ -612,6 +612,39 @@ static struct refspec *parse_refspec_internal(int nr_refspec, const char **refsp
die("Invalid refspec '%s'", refspec[i]);
}
+void parse_ref_or_pattern(struct refspec *refspec, const char *str)
+{
+ struct object_id oid;
+ memset(refspec, 0, sizeof(*refspec));
+
+ if (!get_oid_hex(str, &oid)) {
+ if (str[GIT_SHA1_HEXSZ] == ' ') {
+ struct object_id oid2;
+ /* <sha1> <ref>, find refname */
+ refspec->src = xstrdup(str + GIT_SHA1_HEXSZ + 1);
+ if (!get_oid_hex(refspec->src, &oid2)
+ && !oidcmp(&oid, &oid2))
+ /* The name is actually a SHA-1 */
+ refspec->exact_sha1 = 1;
+ } else if (str[GIT_SHA1_HEXSZ] == '\0') {
+ ; /* <sha1>, leave sha1 as name */
+ refspec->src = xstrdup(str);
+ refspec->exact_sha1 = 1;
+ } else {
+ /* <ref> */
+ refspec->src = xstrdup(str);
+ }
+ } else {
+ /* <ref> */
+ refspec->src = xstrdup(str);
+ }
+
+ if (has_glob_specials(refspec->src)) {
+ refspec->pattern = 1;
+ refspec->dst = refspec->src;
+ }
+}
+
int valid_fetch_refspec(const char *fetch_refspec_str)
{
struct refspec *refspec;
@@ -1924,6 +1957,28 @@ int get_fetch_map(const struct ref *remote_refs,
return 0;
}
+void get_ref_array(const struct ref ***refs, int *nr_ref,
+ const struct ref *remote_refs,
+ const struct refspec *refspecs, int nr_refspecs)
+{
+ struct ref *head = NULL, **tail = &head;
+ const struct ref **array = NULL;
+ int nr = 0, alloc = 0;
+
+ struct ref *r;
+ int i;
+
+ for (i = 0; i < nr_refspecs; i++)
+ get_fetch_map(remote_refs, &refspecs[i], &tail, 1);
+ for (r = head; r; r = r->next) {
+ nr++;
+ ALLOC_GROW(array, nr, alloc);
+ array[nr - 1] = r;
+ }
+ *refs = array;
+ *nr_ref = nr;
+}
+
int resolve_remote_symref(struct ref *ref, struct ref *list)
{
if (!ref->symref)
diff --git a/remote.h b/remote.h
index 2f7f23d47..daca1c65e 100644
--- a/remote.h
+++ b/remote.h
@@ -162,6 +162,13 @@ int ref_newer(const struct object_id *new_oid, const struct object_id *old_oid);
*/
struct ref *ref_remove_duplicates(struct ref *ref_map);
+/*
+ * Parse the given ref or ref pattern. If a ref, write a refspec with that ref
+ * as src, and with an empty dst. If a ref pattern, write a glob refspec with
+ * that pattern as src and dst.
+ */
+void parse_ref_or_pattern(struct refspec *refspec, const char *str);
+
int valid_fetch_refspec(const char *refspec);
struct refspec *parse_fetch_refspec(int nr_refspec, const char **refspec);
@@ -192,6 +199,15 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
int get_fetch_map(const struct ref *remote_refs, const struct refspec *refspec,
struct ref ***tail, int missing_ok);
+/*
+ * Convenience function to generate an array of refs corresponding to the given
+ * refspecs. This is equivalent to repeatedly calling get_fetch_map and
+ * rearranging the returned refs as an array.
+ */
+void get_ref_array(const struct ref ***refs, int *nr_ref,
+ const struct ref *remote_refs,
+ const struct refspec *refspecs, int nr_refspecs);
+
struct ref *get_remote_ref(const struct ref *remote_refs, const char *name);
/*
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 505e1b4a7..cb1b7d949 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -547,6 +547,44 @@ test_expect_success 'fetch-pack can fetch a raw sha1' '
git fetch-pack hidden $(git -C hidden rev-parse refs/hidden/one)
'
+test_expect_success 'fetch-pack can fetch refs using a partial name' '
+ git init server &&
+ (
+ cd server &&
+ test_commit 1 &&
+ test_commit 2 &&
+ git checkout -b one
+ ) &&
+ rm -f trace &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git fetch-pack server one >actual &&
+ grep " want " trace &&
+ ! grep " want-ref " trace &&
+
+ grep "$(printf "%s refs/heads/one" $(git -C server rev-parse --verify one))" actual
+'
+
+test_expect_success 'fetch-pack can fetch refs using a glob' '
+ rm -rf server &&
+ git init server &&
+ (
+ cd server &&
+ test_commit 1 &&
+ test_commit 2 &&
+ git checkout -b ona &&
+ git checkout -b onb &&
+ test_commit 3 &&
+ git checkout -b onc
+ ) &&
+ rm -f trace &&
+ GIT_TRACE_PACKET="$(pwd)/trace" git fetch-pack server "refs/heads/on*" >actual &&
+ grep " want " trace &&
+ ! grep " want-ref " trace &&
+
+ grep "$(printf "%s refs/heads/ona" $(git -C server rev-parse --verify ona))" actual &&
+ grep "$(printf "%s refs/heads/onb" $(git -C server rev-parse --verify onb))" actual &&
+ grep "$(printf "%s refs/heads/onc" $(git -C server rev-parse --verify onc))" actual
+'
+
check_prot_path () {
cat >expected <<-EOF &&
Diag: url=$1
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related
* Re: [PATCH] Add more proposals to SoC 2017 ideas page. (lots of submodule stuff)
From: Jeff King @ 2017-01-25 22:06 UTC (permalink / raw)
To: Stefan Beller; +Cc: git
In-Reply-To: <20170125215931.26339-1-sbeller@google.com>
On Wed, Jan 25, 2017 at 01:59:31PM -0800, Stefan Beller wrote:
> This applies to the repo at https://github.com/git/git.github.io
Thanks. I've applied and pushed, though I'll admit I didn't really read
it carefully for content. A few of the ideas look like they would need
to be aggregated to be big enough for a SoC project, but that can be
fleshed out in future patches (i.e., I don't think we care enough about
history to have people polish and re-roll what are essentially wiki
edits).
If you (or anybody interested in working on this) would prefer direct
push access to the git.github.io repo, I'm happy to set that up.
> If you're looking for a co-admin/mentors, I can help out.
I may take you up on the co-admin thing. I think it's good to have a
backup, just in case.
Anything you put on the Ideas page, you should probably be willing to
mentor. We definitely _don't_ want Ideas that don't have a mentor.
I think in general that each idea should have the possible mentors
listed below it.
-Peff
^ permalink raw reply
* Re: [PATCH] Add more proposals to SoC 2017 ideas page. (lots of submodule stuff)
From: Stefan Beller @ 2017-01-25 22:11 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org
In-Reply-To: <20170125220632.whjnpvrnhve2h6f6@sigill.intra.peff.net>
On Wed, Jan 25, 2017 at 2:06 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Jan 25, 2017 at 01:59:31PM -0800, Stefan Beller wrote:
>
>> This applies to the repo at https://github.com/git/git.github.io
>
> Thanks. I've applied and pushed, though I'll admit I didn't really read
> it carefully for content. A few of the ideas look like they would need
> to be aggregated to be big enough for a SoC project, but that can be
> fleshed out in future patches (i.e., I don't think we care enough about
> history to have people polish and re-roll what are essentially wiki
> edits).
Yeah, I wasn't sure if the ideas were meant to also contain microprojects
so I wrote down everything that came to mind, that I do not intend to work on
myself over the next couple month.
>
> If you (or anybody interested in working on this) would prefer direct
> push access to the git.github.io repo, I'm happy to set that up.
Yeah I wouldn't mind direct push access there, then I could fixup
what I just sent you, e.g. adding myself as a possible mentor or
re-shuffling these ideas. :)
>
>> If you're looking for a co-admin/mentors, I can help out.
>
> I may take you up on the co-admin thing. I think it's good to have a
> backup, just in case.
>
> Anything you put on the Ideas page, you should probably be willing to
> mentor. We definitely _don't_ want Ideas that don't have a mentor.
agreed.
> I think in general that each idea should have the possible mentors
> listed below it.
ok, I can make a patch for that; but pushing seems (slightly) easier than
mailing a patch.
Thanks,
Stefan
^ permalink raw reply
* Force Confirmation for Dropping Changed Lines
From: Hilco Wijbenga @ 2017-01-25 22:16 UTC (permalink / raw)
To: Git Users
Hi all,
Most of the time, when a later commit changes a line in an identical
fashion during, say, a rebase, you want Git to silently continue by
dropping the duplicate change from the later commit. I have a common
(for me) scenario where I want Git to specifically ask me to resolve
this "conflict" myself instead of simply assuming that the change has
already been applied.
Let's say I have a file my-code.x that starts with a line indicating
its version:
===== my-code.x =====
VERSION=1.2
line 1
line 2
line 3
=====
In my branch my-branch off of master, I make a change:
===== my-code.x =====
VERSION=1.3
line 1
line 2
line 2a
line 3
=====
Now someone else makes a different change on master and pushes it ([1]):
===== my-code.x =====
VERSION=1.3
line 1
line 2
line 3
line 4
=====
When I rebase my-branch onto origin/master, I get no conflicts and
everything seems fine ([2]):
===== my-code.x =====
VERSION=1.3
line 1
line 2
line 2a
line 3
line 4
=====
Except that I should have used VERSION=1.4, not VERSION=1.3 because I
made a change to my-code.x. Obviously, for a single file this is easy
to remember/check but when it's hidden among lots of files and commits
it becomes very hard to find these types of errors.
How can I force Git to not assume my change to the first line is "redundant"?
Cheers,
Hilco
[1] Note that this someone is (correctly) using the same, new version of 1.3.
[2] If my example is actually incorrect, then please just pretend
there are no conflicts.
^ permalink raw reply
* Re: [PATCH v1 1/3] blame: add --aggregate option
From: Jeff King @ 2017-01-25 22:22 UTC (permalink / raw)
To: Edmundo Carmona Antoranz; +Cc: git, apelisse, kevin, gitster
In-Reply-To: <20170125052734.17550-1-eantoranz@gmail.com>
On Tue, Jan 24, 2017 at 11:27:32PM -0600, Edmundo Carmona Antoranz wrote:
> To avoid taking up so much space on normal output printing duplicate
> information on consecutive lines that "belong" to the same revision,
> this option allows to print a single line with the information about
> the revision before the lines of content themselves.
I admit I have not followed the preceding discussion closely, so ignore
me if my suggestion is way off base.
But it really seems like the various outputs you are looking for are
really all about customizing git-blame's human-readable output.
Would it be more productive to move towards a "--format" option that
shows custom items for each line? It could build on the custom formats
in the pretty-print code (though I think you would want to add some
custom ones, like filename, line number, line content, etc).
I know that only does half of what you want, which is making some output
not just per-line, but per-block. But that can come easily on top, if we
allow separate per-line and per-block formats (which would default to
the current output and the empty string, respectively).
Then you could do something like:
git blame --format-block='%h %an <%ae>%n %s' \
--format-line='\t%(filename):%(linenumber) %(linecontent)'
and get something like:
1234abcd A U Thor <author@example.com>
initial commit
foo.c:1 #include <foo.h>
foo.c:2 #include <bar.h>
5678abcd A U Thor <author@example.com>
add third include
foo.c:3 #include <third.h>
and so on. But people can mix and match what they want to see on each
line, and what they'd rather push to other lines.
I don't have a huge interest in the feature myself. I switched to "tig
blame" years ago and never looked back. But it just seems like your
options touch no this kind of flexibility, but will limit somebody as
soon as they want to switch around which information goes where.
-Peff
^ permalink raw reply
* Re: Force Confirmation for Dropping Changed Lines
From: Jacob Keller @ 2017-01-25 22:24 UTC (permalink / raw)
To: Hilco Wijbenga; +Cc: Git Users
In-Reply-To: <CAE1pOi2YZayEfKxxh3gsTds1mQ9L1E9AW=wPnmW=Dg=-EMj=tw@mail.gmail.com>
On Wed, Jan 25, 2017 at 2:16 PM, Hilco Wijbenga
<hilco.wijbenga@gmail.com> wrote:
> How can I force Git to not assume my change to the first line is "redundant"?
>
My guess is that you probably want a custom merge driver for your file
types. That's where I would look initially.
Thanks,
Jake
> Cheers,
> Hilco
>
> [1] Note that this someone is (correctly) using the same, new version of 1.3.
> [2] If my example is actually incorrect, then please just pretend
> there are no conflicts.
^ permalink raw reply
* Re: [PATCH] Add more proposals to SoC 2017 ideas page. (lots of submodule stuff)
From: Jeff King @ 2017-01-25 22:26 UTC (permalink / raw)
To: Stefan Beller; +Cc: git@vger.kernel.org
In-Reply-To: <CAGZ79kb_g5Wq=Aeo1RH-iA5M-drU5Gm1LAJZuPZU7oe=xdHaOQ@mail.gmail.com>
On Wed, Jan 25, 2017 at 02:11:15PM -0800, Stefan Beller wrote:
> > Thanks. I've applied and pushed, though I'll admit I didn't really read
> > it carefully for content. A few of the ideas look like they would need
> > to be aggregated to be big enough for a SoC project, but that can be
> > fleshed out in future patches (i.e., I don't think we care enough about
> > history to have people polish and re-roll what are essentially wiki
> > edits).
>
> Yeah, I wasn't sure if the ideas were meant to also contain microprojects
> so I wrote down everything that came to mind, that I do not intend to work on
> myself over the next couple month.
Microprojects go on their own page. But make sure that they really are
"micro" sized for an applicant.
> > If you (or anybody interested in working on this) would prefer direct
> > push access to the git.github.io repo, I'm happy to set that up.
>
> Yeah I wouldn't mind direct push access there, then I could fixup
> what I just sent you, e.g. adding myself as a possible mentor or
> re-shuffling these ideas. :)
OK, done. For anybody else interested, I just need to know your GitHub
username.
-Peff
^ permalink raw reply
* Re: [PATCH] Add more proposals to SoC 2017 ideas page. (lots of submodule stuff)
From: Stefan Beller @ 2017-01-25 22:34 UTC (permalink / raw)
To: Jeff King; +Cc: git@vger.kernel.org
In-Reply-To: <20170125222627.jlswvwmzli62fnnt@sigill.intra.peff.net>
On Wed, Jan 25, 2017 at 2:26 PM, Jeff King <peff@peff.net> wrote:
> On Wed, Jan 25, 2017 at 02:11:15PM -0800, Stefan Beller wrote:
>
>> > Thanks. I've applied and pushed, though I'll admit I didn't really read
>> > it carefully for content. A few of the ideas look like they would need
>> > to be aggregated to be big enough for a SoC project, but that can be
>> > fleshed out in future patches (i.e., I don't think we care enough about
>> > history to have people polish and re-roll what are essentially wiki
>> > edits).
>>
>> Yeah, I wasn't sure if the ideas were meant to also contain microprojects
>> so I wrote down everything that came to mind, that I do not intend to work on
>> myself over the next couple month.
>
> Microprojects go on their own page. But make sure that they really are
> "micro" sized for an applicant.
Yeah micro as in real micro.
e.g. fix the SubmittingPatches doc, after confusion about "signing",
start reading here
https://public-inbox.org/git/923cd4e4-5c9c-4eaf-0fea-6deff6875b88@tngtech.com/
(I did not push it, as I'd need to copy over the micro projects page from 2016,
and then find out where to put links to have the web page not look broken)
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
From: Junio C Hamano @ 2017-01-25 22:35 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Segev Finer
In-Reply-To: <alpine.DEB.2.20.1701251327090.3469@virtualbox>
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> Now, with the patch in question (without the follow-up, which I would like
> to ask you to ignore, just like you did so far), Git would not figure out
> that your script calls PuTTY eventually. The work-around? Easy:
>
> DUMMY=/plink.exe /path/to/junio-is-a-superstar.sh
Think about how you would explain that to an end-user in our
document? You'll need to explain how exactly the auto-detection
works, so that the user can "exploit" the loophole to do that. And
what maintenance burden does it add when auto-detection is updated?
I think I know you well enough that you know well enough that it is
too ugly to live, and I suspect that the above is a tongue-in-cheek
"arguing for the sake of argument" and would not need a serious
response, but just in case...
> ...
> Of course, given our discussion I think all of this should be documented
> in the commit message as well as in the man page.
Yes. Here is what comes on an obvious clean-up patch (which will be
sent as a follow-up to this message).
-- >8 --
Subject: [PATCH] connect: core.sshvariant to correct misidentification
While the logic we have been using to guess which variant of SSH
implementation is in use by looking at the name of the program has
been robust enough for GIT_SSH (which does not go through the shell,
hence it can only spell the name the SSH program and nothing else),
extending it to GIT_SSH_COMMAND and core.sshcommand opens it for
larger chance of misidentification, because these can specify
arbitrary shell command, and a simple "the first word on the line
must be the command name" heuristic may not even look at the command
name at all.
As any effort to improve the heuristic will give us only diminishing
returns, and especially given that most of the users are expected to
have a command line for which the heuristic would work correctly,
give an explicit escape hatch to override a misidentification, just
in case one happens.
It is arguably bad to add this new variable to the core.* set, in
the sense that you'll never see this variable if you do not interact
with other repositories over ssh, but core.sshcommand is already
there for some reason, so let's match it.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
Documentation/config.txt | 13 +++++++++++++
connect.c | 26 ++++++++++++++++++++++++++
2 files changed, 39 insertions(+)
diff --git a/Documentation/config.txt b/Documentation/config.txt
index af2ae4cc02..8ea1db49c6 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -455,6 +455,19 @@ core.sshCommand::
the `GIT_SSH_COMMAND` environment variable and is overridden
when the environment variable is set.
+core.sshVariant::
+ SSH implementations used by Git when running `git fetch`,
+ `git clone`, and `git push` use slightly different command
+ line options (e.g. PuTTY and TortoisePlink use `-P <port>`
+ while OpenSSH uses `-p <port>` to specify the port number.
+ TortoisePlink in addition requires `-batch` option to be
+ passed). Git guesses which variant is in use correctly from
+ the name of the ssh command used (see `core.sshCommand`
+ configuration variable and also `GIT_SSH_COMMAND`
+ environment variable) most of the time. You can set this
+ variable to 'putty', 'tortoiseplink', or 'ssh' to correct it
+ when Git makes an incorrect guess.
+
core.ignoreStat::
If true, Git will avoid using lstat() calls to detect if files have
changed by setting the "assume-unchanged" bit for those tracked files
diff --git a/connect.c b/connect.c
index aa51b33bfc..358e9c06f0 100644
--- a/connect.c
+++ b/connect.c
@@ -691,6 +691,29 @@ static const char *get_ssh_command(void)
return NULL;
}
+static void override_ssh_variant(int *port_option, int *needs_batch)
+{
+ const char *variant;
+
+ if (git_config_get_string_const("core.sshvariant", &variant))
+ return;
+ if (!strcmp(variant, "tortoiseplink")) {
+ *port_option = 'P';
+ *needs_batch = 1;
+ } else if (!strcmp(variant, "putty")) {
+ *port_option = 'P';
+ *needs_batch = 0;
+ } else {
+ /* default */
+ if (strcmp(variant, "ssh")) {
+ warning(_("core.sshvariant: unknown value '%s'"), variant);
+ warning(_("using OpenSSH compatible behaviour"));
+ }
+ *port_option = 'p';
+ *needs_batch = 0;
+ }
+}
+
/*
* This returns a dummy child_process if the transport protocol does not
* need fork(2), or a struct child_process object if it does. Once done,
@@ -836,6 +859,9 @@ struct child_process *git_connect(int fd[2], const char *url,
argv_array_push(&conn->args, "-4");
else if (flags & CONNECT_IPV6)
argv_array_push(&conn->args, "-6");
+
+ override_ssh_variant(&port_option, &needs_batch);
+
if (needs_batch)
argv_array_push(&conn->args, "-batch");
if (port) {
--
2.11.0-699-ga1f1475476
^ permalink raw reply related
* Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
From: Junio C Hamano @ 2017-01-25 22:37 UTC (permalink / raw)
To: Johannes Schindelin; +Cc: git, Segev Finer
In-Reply-To: <xmqqinp2939j.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Yes. Here is what comes on an obvious clean-up patch (which will be
> sent as a follow-up to this message).
... and this is the "obvious clean-up patch" to apply directly on
top of Segev's GIT_SSH_COMMAND support, which comes before the
previous one.
-- >8 --
Subject: [PATCH] connect: rename tortoiseplink and putty variables
One of these two may have originally been named after "what exact
SSH implementation do we have" so that we can tweak the command line
options, but these days "putty=1" no longer means "We are using the
plink SSH implementation that comes with PuTTY". It is set when we
guess that either PuTTY plink or Tortoiseplink is in use.
Rename them after what effect is desired. The current "putty"
option is about using "-P <port>" when OpenSSH would use "-p <port>",
so rename it to port_option whose value is either 'p' or 'P". The
other one is about passing an extra command line option "-batch",
so rename it needs_batch.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
connect.c | 22 ++++++++++++----------
1 file changed, 12 insertions(+), 10 deletions(-)
diff --git a/connect.c b/connect.c
index c81f77001b..aa51b33bfc 100644
--- a/connect.c
+++ b/connect.c
@@ -769,7 +769,8 @@ struct child_process *git_connect(int fd[2], const char *url,
conn->in = conn->out = -1;
if (protocol == PROTO_SSH) {
const char *ssh;
- int putty = 0, tortoiseplink = 0;
+ int needs_batch = 0;
+ int port_option = 'p';
char *ssh_host = hostandport;
const char *port = NULL;
char *ssh_argv0 = NULL;
@@ -819,12 +820,14 @@ struct child_process *git_connect(int fd[2], const char *url,
if (ssh_argv0) {
const char *base = basename(ssh_argv0);
- tortoiseplink = !strcasecmp(base, "tortoiseplink") ||
- !strcasecmp(base, "tortoiseplink.exe");
- putty = tortoiseplink ||
- !strcasecmp(base, "plink") ||
- !strcasecmp(base, "plink.exe");
-
+ if (!strcasecmp(base, "tortoiseplink") ||
+ !strcasecmp(base, "tortoiseplink.exe")) {
+ port_option = 'P';
+ needs_batch = 1;
+ } else if (!strcasecmp(base, "plink") ||
+ !strcasecmp(base, "plink.exe")) {
+ port_option = 'P';
+ }
free(ssh_argv0);
}
@@ -833,11 +836,10 @@ struct child_process *git_connect(int fd[2], const char *url,
argv_array_push(&conn->args, "-4");
else if (flags & CONNECT_IPV6)
argv_array_push(&conn->args, "-6");
- if (tortoiseplink)
+ if (needs_batch)
argv_array_push(&conn->args, "-batch");
if (port) {
- /* P is for PuTTY, p is for OpenSSH */
- argv_array_push(&conn->args, putty ? "-P" : "-p");
+ argv_array_pushf(&conn->args, "-%c", port_option);
argv_array_push(&conn->args, port);
}
argv_array_push(&conn->args, ssh_host);
--
2.11.0-699-ga1f1475476
^ permalink raw reply related
* Re: SubmittingPatches: drop temporal reference for PGP signing
From: Stefan Beller @ 2017-01-25 22:38 UTC (permalink / raw)
To: Philip Oakley
Cc: Cornelius Weig, Johannes Sixt, bitte.keine.werbung.einwerfen,
git@vger.kernel.org, Junio C Hamano, thomas.braun, John Keeping
In-Reply-To: <33E354BCDB9A4192B69B9B399381659E@PhilipOakley>
On Tue, Jan 24, 2017 at 10:54 PM, Philip Oakley <philipoakley@iee.org> wrote:
>> -Do not PGP sign your patch, at least for now. Most likely, your
>> -maintainer or other people on the list would not have your PGP
>> -key and would not bother obtaining it anyway. Your patch is not
>> -judged by who you are; a good patch from an unknown origin has a
>> -far better chance of being accepted than a patch from a known,
>> -respected origin that is done poorly or does incorrect things.
>> +Do not PGP sign your patch. Most likely, your maintainer or other
>> +people on the list would not have your PGP key and would not bother
>> +obtaining it anyway. Your patch is not judged by who you are; a good
>> +patch from an unknown origin has a far better chance of being accepted
>> +than a patch from a known, respected origin that is done poorly or
>> +does incorrect things.
>
>
> Wouldn't this also benefit from a forward reference to the section 5 on the
> DOC signining? This would avoid Cornelius's case where he felt that section
> 5 no longer applied.
Yeah I agree. My patch was not the best shot by far.
^ permalink raw reply
* Re: [PATCH] connect: handle putty/plink also in GIT_SSH_COMMAND
From: Jeff King @ 2017-01-25 22:40 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Johannes Schindelin, git, Segev Finer
In-Reply-To: <xmqqinp2939j.fsf@gitster.mtv.corp.google.com>
On Wed, Jan 25, 2017 at 02:35:36PM -0800, Junio C Hamano wrote:
> -- >8 --
> Subject: [PATCH] connect: core.sshvariant to correct misidentification
I have been watching this discussion from the sidelines, and I agree
that this direction is a big improvement.
> +static void override_ssh_variant(int *port_option, int *needs_batch)
> +{
> + const char *variant;
> +
> + if (git_config_get_string_const("core.sshvariant", &variant))
> + return;
> + if (!strcmp(variant, "tortoiseplink")) {
> + *port_option = 'P';
> + *needs_batch = 1;
> + } else if (!strcmp(variant, "putty")) {
> + *port_option = 'P';
> + *needs_batch = 0;
> + } else {
> + /* default */
> + if (strcmp(variant, "ssh")) {
> + warning(_("core.sshvariant: unknown value '%s'"), variant);
> + warning(_("using OpenSSH compatible behaviour"));
> + }
> + *port_option = 'p';
> + *needs_batch = 0;
> + }
> +}
IIRC, the "const" in git_config_get_string_const is only about avoiding
an annoying cast. The result is still allocated and needs freed. Since
you are not keeping the value after the function returns, I think you
could just use git_config_get_value().
(Grepping around, I see a few other places that seem to make the same
mistake. I think this is a confusing interface that should probably be
fixed).
-Peff
^ permalink raw reply
* Re: What's cooking in git.git (Jan 2017, #04; Mon, 23)
From: Junio C Hamano @ 2017-01-25 22:51 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, Lars Schneider, git
In-Reply-To: <20170125183924.6yclcjl4ggcu42yp@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> I guess the way to dig would be to add a test that looks at the output
> of "type mv" or something, push it to a Travis-hooked branch, and then
> wait for the output
Sounds tempting ;-)
^ permalink raw reply
* Re: Force Confirmation for Dropping Changed Lines
From: Hilco Wijbenga @ 2017-01-25 22:51 UTC (permalink / raw)
To: Jacob Keller; +Cc: Git Users
In-Reply-To: <CA+P7+xrupLuYAj7tn_1EaUiN6eaCmtgX-_d4mnByDq95cuqiWQ@mail.gmail.com>
On 25 January 2017 at 14:24, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Wed, Jan 25, 2017 at 2:16 PM, Hilco Wijbenga
> <hilco.wijbenga@gmail.com> wrote:
>> How can I force Git to not assume my change to the first line is "redundant"?
>>
>
> My guess is that you probably want a custom merge driver for your file
> types. That's where I would look initially.
Mmm, that sounds complex. The "my-code.x" is made up so I could keep
my example as simple as possible. In reality, it's Maven's POM files
(pom.xml).
So there is no setting for any of this? There is no way to switch off
auto merging for certain files?
^ permalink raw reply
* Re: Force Confirmation for Dropping Changed Lines
From: Jacob Keller @ 2017-01-25 22:54 UTC (permalink / raw)
To: Hilco Wijbenga; +Cc: Git Users
In-Reply-To: <CAE1pOi3eh7ao3NocV=PRFDby8y5ttjFR=-_VB0FoJv4MpjExaA@mail.gmail.com>
On Wed, Jan 25, 2017 at 2:51 PM, Hilco Wijbenga
<hilco.wijbenga@gmail.com> wrote:
> On 25 January 2017 at 14:24, Jacob Keller <jacob.keller@gmail.com> wrote:
>> On Wed, Jan 25, 2017 at 2:16 PM, Hilco Wijbenga
>> <hilco.wijbenga@gmail.com> wrote:
>>> How can I force Git to not assume my change to the first line is "redundant"?
>>>
>>
>> My guess is that you probably want a custom merge driver for your file
>> types. That's where I would look initially.
>
> Mmm, that sounds complex. The "my-code.x" is made up so I could keep
> my example as simple as possible. In reality, it's Maven's POM files
> (pom.xml).
>
> So there is no setting for any of this? There is no way to switch off
> auto merging for certain files?
Not really sure, but a quick google search revealed
https://github.com/ralfth/pom-merge-driver
Maybe that will help you?
Thanks,
Jake
^ permalink raw reply
* Re: [PATCHv2 3/3] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves
From: Junio C Hamano @ 2017-01-25 22:54 UTC (permalink / raw)
To: Stefan Beller; +Cc: git, bmwill, peff
In-Reply-To: <20170124235651.18749-4-sbeller@google.com>
Stefan Beller <sbeller@google.com> writes:
> Consider having a submodule 'sub' and a nested submodule at 'sub/nested'.
> When nested is already absorbed into sub, but sub is not absorbed into
> its superproject, then we need to fixup the gitfile and core.worktree
> setting for 'nested' when absorbing 'sub', but we do not need to move
> its git dir around.
>
> Previously 'nested's gitfile contained "gitdir: ../.git/modules/nested";
> it has to be corrected to "gitdir: ../../.git/modules/sub1/modules/nested".
That sounds like a sensible way to make things consistent.
^ permalink raw reply
* Re: [PATCH] tag: add tag.createReflog option
From: Junio C Hamano @ 2017-01-25 22:56 UTC (permalink / raw)
To: Jeff King; +Cc: Cornelius Weig, git, novalis, pclouds
In-Reply-To: <xmqqpoja95o5.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
> Jeff King <peff@peff.net> writes:
>
>> +enum log_refs_config {
>> + LOG_REFS_UNSET = -1,
>> + LOG_REFS_NONE = 0,
>> + LOG_REFS_NORMAL, /* see should_create_reflog for rules */
>> + LOG_REFS_ALWAYS
>> +};
>> +extern enum log_refs_config log_all_ref_updates;
>> +...
>> +int should_create_reflog(const char *refname)
>> +{
>> + switch (log_all_ref_updates) {
>> + case LOG_REFS_ALWAYS:
>> + return 1;
>> + case LOG_REFS_NORMAL:
>> + return !prefixcmp(refname, "refs/heads/") ||
>> + !prefixcmp(refname, "refs/remotes/") ||
>> + !prefixcmp(refname, "refs/notes/") ||
>> + !strcmp(refname, "HEAD");
>> + default:
>> + return 0;
>> + }
>> +}
>
> Yup, this is how I expected for the feature to be done.
>
> Just a hint for Cornelius; prefixcmp() is an old name for what is
> called starts_with() these days.
It may have been obvious, but to be explicit for somebody new,
!prefixcmp() corresponds to starts_with(). IOW, we changed the
meaning of the return value when moving from cmp-lookalike (where 0
means "equal") to "does it start with this string?" bool (where 1
means "yes").
^ permalink raw reply
* Re: [PATCHv2 3/3] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves
From: Stefan Beller @ 2017-01-25 23:04 UTC (permalink / raw)
To: Brandon Williams; +Cc: Junio C Hamano, git@vger.kernel.org, Jeff King
In-Reply-To: <20170125004610.GC58021@google.com>
On Tue, Jan 24, 2017 at 4:46 PM, Brandon Williams <bmwill@google.com> wrote:
> On 01/24, Stefan Beller wrote:
>> + connect_work_tree_and_git_dir(path, real_new_git_dir);
>
> Memory leak with 'real_new_git_dir'
yup. :(
> The scope of 'real_sub_git_dir' and 'real_common_git_dir' variable can
> be narrowed. Move their declaration here and free it prior to exiting
> the else block.
ok.
> 'v' isn't ever used, just use starts_with() and get rid of 'v'
makes sense.
>
>> + relocate_single_git_dir_into_superproject(prefix, path);
>> + }
>>
>
> There's a label 'out' at the end of the function which can be removed as
> there is no 'goto' statement using it.
We do use it in
if (err_code == READ_GITFILE_ERR_STAT_FAILED)
goto out; /* unpopulated as expected */
I took your proposed SQUASH and removed the goto, see the followup email.
^ permalink raw reply
* Re: [PATCH] gpg-interface: Add some output from gpg when it errors out.
From: Junio C Hamano @ 2017-01-25 23:04 UTC (permalink / raw)
To: Mike Hommey; +Cc: git
In-Reply-To: <20170125030434.26448-1-mh@glandium.org>
Mike Hommey <mh@glandium.org> writes:
> For instance, after changing my laptop for a new one, I copied my
> configs, but had some environment differences that broke gpg.
> With this change applied, the output becomes, on this new machine:
> gpg: keyblock resource '/usr/share/keyrings/debian-keyring.gpg': No
> such file or directory
> error: gpg failed to sign the data
> error: unable to sign the tag
>
> which makes it clearer what's wrong.
Overall I think this is a good thing to do. Instead of eating the
status output, showing what we got, especially when we know the
command failed, would make the bug-hunting of user's environment
easier, like you showed above.
The only thing in the design that makes me wonder is the filtering
out based on "[GNUPG:]" prefix. Why do we need to filter them out?
Implementation-wise, I'd be happier if we do not add any new
callsites of strbuf_split(), which is a horrible interface. But
that is a minor detail.
Thanks.
^ permalink raw reply
* [PATCHv3 3/3] submodule absorbing: fix worktree/gitdir pointers recursively for non-moves
From: Stefan Beller @ 2017-01-25 23:04 UTC (permalink / raw)
To: gitster; +Cc: git, bmwill, peff, Stefan Beller
In-Reply-To: <xmqq37g692da.fsf@gitster.mtv.corp.google.com>
Consider having a submodule 'sub' and a nested submodule at 'sub/nested'.
When nested is already absorbed into sub, but sub is not absorbed into
its superproject, then we need to fixup the gitfile and core.worktree
setting for 'nested' when absorbing 'sub', but we do not need to move
its git dir around.
Previously 'nested's gitfile contained "gitdir: ../.git/modules/nested";
it has to be corrected to "gitdir: ../../.git/modules/sub1/modules/nested".
An alternative I considered to do this work lazily, i.e. when resolving
"../.git/modules/nested", we would notice the ".git" being a gitfile
linking to another path. That seemed to be robuster by design, but harder
to get the implementation right. Maybe we have to do that anyway once we
try to have submodules and worktrees working nicely together, but for now
just produce 'correct' (i.e. direct) pointers.
Signed-off-by: Stefan Beller <sbeller@google.com>
---
This replaces the last patch of the series, containing Brandons SQUASH proposal
as well as the removal of the goto.
Thanks,
Stefan
submodule.c | 62 ++++++++++++++++++++++++++++----------
t/t7412-submodule-absorbgitdirs.sh | 27 +++++++++++++++++
2 files changed, 73 insertions(+), 16 deletions(-)
diff --git a/submodule.c b/submodule.c
index 4c4f033e8a..3b98766a6b 100644
--- a/submodule.c
+++ b/submodule.c
@@ -1437,22 +1437,57 @@ void absorb_git_dir_into_superproject(const char *prefix,
const char *path,
unsigned flags)
{
- const char *sub_git_dir, *v;
- char *real_sub_git_dir = NULL, *real_common_git_dir = NULL;
+ int err_code;
+ const char *sub_git_dir;
struct strbuf gitdir = STRBUF_INIT;
-
strbuf_addf(&gitdir, "%s/.git", path);
- sub_git_dir = resolve_gitdir(gitdir.buf);
+ sub_git_dir = resolve_gitdir_gently(gitdir.buf, &err_code);
/* Not populated? */
- if (!sub_git_dir)
- goto out;
+ if (!sub_git_dir) {
+ char *real_new_git_dir;
+ const char *new_git_dir;
+ const struct submodule *sub;
+
+ if (err_code == READ_GITFILE_ERR_STAT_FAILED) {
+ /* unpopulated as expected */
+ strbuf_release(&gitdir);
+ return;
+ }
+
+ if (err_code != READ_GITFILE_ERR_NOT_A_REPO)
+ /* We don't know what broke here. */
+ read_gitfile_error_die(err_code, path, NULL);
+
+ /*
+ * Maybe populated, but no git directory was found?
+ * This can happen if the superproject is a submodule
+ * itself and was just absorbed. The absorption of the
+ * superproject did not rewrite the git file links yet,
+ * fix it now.
+ */
+ sub = submodule_from_path(null_sha1, path);
+ if (!sub)
+ die(_("could not lookup name for submodule '%s'"), path);
+ new_git_dir = git_path("modules/%s", sub->name);
+ if (safe_create_leading_directories_const(new_git_dir) < 0)
+ die(_("could not create directory '%s'"), new_git_dir);
+ real_new_git_dir = real_pathdup(new_git_dir);
+ connect_work_tree_and_git_dir(path, real_new_git_dir);
+
+ free(real_new_git_dir);
+ } else {
+ /* Is it already absorbed into the superprojects git dir? */
+ char *real_sub_git_dir = real_pathdup(sub_git_dir);
+ char *real_common_git_dir = real_pathdup(get_git_common_dir());
- /* Is it already absorbed into the superprojects git dir? */
- real_sub_git_dir = real_pathdup(sub_git_dir);
- real_common_git_dir = real_pathdup(get_git_common_dir());
- if (!skip_prefix(real_sub_git_dir, real_common_git_dir, &v))
- relocate_single_git_dir_into_superproject(prefix, path);
+ if (!starts_with(real_sub_git_dir, real_common_git_dir))
+ relocate_single_git_dir_into_superproject(prefix, path);
+
+ free(real_sub_git_dir);
+ free(real_common_git_dir);
+ }
+ strbuf_release(&gitdir);
if (flags & ABSORB_GITDIR_RECURSE_SUBMODULES) {
struct child_process cp = CHILD_PROCESS_INIT;
@@ -1478,9 +1513,4 @@ void absorb_git_dir_into_superproject(const char *prefix,
strbuf_release(&sb);
}
-
-out:
- strbuf_release(&gitdir);
- free(real_sub_git_dir);
- free(real_common_git_dir);
}
diff --git a/t/t7412-submodule-absorbgitdirs.sh b/t/t7412-submodule-absorbgitdirs.sh
index 1c47780e2b..e2bbb449b6 100755
--- a/t/t7412-submodule-absorbgitdirs.sh
+++ b/t/t7412-submodule-absorbgitdirs.sh
@@ -64,6 +64,33 @@ test_expect_success 'absorb the git dir in a nested submodule' '
test_cmp expect.2 actual.2
'
+test_expect_success 're-setup nested submodule' '
+ # un-absorb the direct submodule, to test if the nested submodule
+ # is still correct (needs a rewrite of the gitfile only)
+ rm -rf sub1/.git &&
+ mv .git/modules/sub1 sub1/.git &&
+ GIT_WORK_TREE=. git -C sub1 config --unset core.worktree &&
+ # fixup the nested submodule
+ echo "gitdir: ../.git/modules/nested" >sub1/nested/.git &&
+ GIT_WORK_TREE=../../../nested git -C sub1/.git/modules/nested config \
+ core.worktree "../../../nested" &&
+ # make sure this re-setup is correct
+ git status --ignore-submodules=none
+'
+
+test_expect_success 'absorb the git dir in a nested submodule' '
+ git status >expect.1 &&
+ git -C sub1/nested rev-parse HEAD >expect.2 &&
+ git submodule absorbgitdirs &&
+ test -f sub1/.git &&
+ test -f sub1/nested/.git &&
+ test -d .git/modules/sub1/modules/nested &&
+ git status >actual.1 &&
+ git -C sub1/nested rev-parse HEAD >actual.2 &&
+ test_cmp expect.1 actual.1 &&
+ test_cmp expect.2 actual.2
+'
+
test_expect_success 'setup a gitlink with missing .gitmodules entry' '
git init sub2 &&
test_commit -C sub2 first &&
--
2.11.0.495.g04f60290a0.dirty
^ permalink raw reply related
* Re: [PATCH v2 0/7] Macros for Asciidoctor support
From: brian m. carlson @ 2017-01-25 23:19 UTC (permalink / raw)
To: Jeff King; +Cc: Johannes Schindelin, git
In-Reply-To: <20170125213544.eelk4pjhrhshi6zh@sigill.intra.peff.net>
[-- Attachment #1: Type: text/plain, Size: 938 bytes --]
On Wed, Jan 25, 2017 at 04:35:44PM -0500, Jeff King wrote:
> On Wed, Jan 25, 2017 at 02:28:55PM +0100, Johannes Schindelin wrote:
>
> > > The need for the extensions could be replaced with a small amount of
> > > Ruby code, if that's considered desirable. Previous opinions on doing
> > > so were negative, however.
> >
> > Quite frankly, it is annoying to be forced to install the extensions. I
> > would much rather have the small amount of Ruby code in Git's repository.
>
> Me too. Dependencies can be a big annoyance. I'd reserve judgement until
> I saw the actual Ruby code, though. :)
I've sent the patch before, but I can send it again. It's relatively
small and self-contained. I'm also happy to be responsible for
maintaining it.
--
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]
^ permalink raw reply
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