* [PATCH v5 06/19] builtin/fmt-merge-message: convert to struct object_id
From: brian m. carlson @ 2017-02-21 23:47 UTC (permalink / raw)
To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano, Ramsay Jones
In-Reply-To: <20170221234737.894681-1-sandals@crustytoothpaste.net>
Convert most of the code to use struct object_id, including struct
origin_data and struct merge_parents. Convert several instances of
hardcoded numbers into references to GIT_SHA1_HEXSZ.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
builtin/fmt-merge-msg.c | 70 ++++++++++++++++++++++++-------------------------
1 file changed, 35 insertions(+), 35 deletions(-)
diff --git a/builtin/fmt-merge-msg.c b/builtin/fmt-merge-msg.c
index efab62fd85..6faa3c0d24 100644
--- a/builtin/fmt-merge-msg.c
+++ b/builtin/fmt-merge-msg.c
@@ -41,7 +41,7 @@ struct src_data {
};
struct origin_data {
- unsigned char sha1[20];
+ struct object_id oid;
unsigned is_local_branch:1;
};
@@ -59,8 +59,8 @@ static struct string_list origins = STRING_LIST_INIT_DUP;
struct merge_parents {
int alloc, nr;
struct merge_parent {
- unsigned char given[20];
- unsigned char commit[20];
+ struct object_id given;
+ struct object_id commit;
unsigned char used;
} *item;
};
@@ -70,14 +70,14 @@ struct merge_parents {
* hundreds of heads at a time anyway.
*/
static struct merge_parent *find_merge_parent(struct merge_parents *table,
- unsigned char *given,
- unsigned char *commit)
+ struct object_id *given,
+ struct object_id *commit)
{
int i;
for (i = 0; i < table->nr; i++) {
- if (given && hashcmp(table->item[i].given, given))
+ if (given && oidcmp(&table->item[i].given, given))
continue;
- if (commit && hashcmp(table->item[i].commit, commit))
+ if (commit && oidcmp(&table->item[i].commit, commit))
continue;
return &table->item[i];
}
@@ -85,14 +85,14 @@ static struct merge_parent *find_merge_parent(struct merge_parents *table,
}
static void add_merge_parent(struct merge_parents *table,
- unsigned char *given,
- unsigned char *commit)
+ struct object_id *given,
+ struct object_id *commit)
{
if (table->nr && find_merge_parent(table, given, commit))
return;
ALLOC_GROW(table->item, table->nr + 1, table->alloc);
- hashcpy(table->item[table->nr].given, given);
- hashcpy(table->item[table->nr].commit, commit);
+ oidcpy(&table->item[table->nr].given, given);
+ oidcpy(&table->item[table->nr].commit, commit);
table->item[table->nr].used = 0;
table->nr++;
}
@@ -106,30 +106,30 @@ static int handle_line(char *line, struct merge_parents *merge_parents)
struct src_data *src_data;
struct string_list_item *item;
int pulling_head = 0;
- unsigned char sha1[20];
+ struct object_id oid;
- if (len < 43 || line[40] != '\t')
+ if (len < GIT_SHA1_HEXSZ + 3 || line[GIT_SHA1_HEXSZ] != '\t')
return 1;
- if (starts_with(line + 41, "not-for-merge"))
+ if (starts_with(line + GIT_SHA1_HEXSZ + 1, "not-for-merge"))
return 0;
- if (line[41] != '\t')
+ if (line[GIT_SHA1_HEXSZ + 1] != '\t')
return 2;
- i = get_sha1_hex(line, sha1);
+ i = get_oid_hex(line, &oid);
if (i)
return 3;
- if (!find_merge_parent(merge_parents, sha1, NULL))
+ if (!find_merge_parent(merge_parents, &oid, NULL))
return 0; /* subsumed by other parents */
origin_data = xcalloc(1, sizeof(struct origin_data));
- hashcpy(origin_data->sha1, sha1);
+ oidcpy(&origin_data->oid, &oid);
if (line[len - 1] == '\n')
line[len - 1] = 0;
- line += 42;
+ line += GIT_SHA1_HEXSZ + 2;
/*
* At this point, line points at the beginning of comment e.g.
@@ -338,10 +338,10 @@ static void shortlog(const char *name,
struct string_list committers = STRING_LIST_INIT_DUP;
int flags = UNINTERESTING | TREESAME | SEEN | SHOWN | ADDED;
struct strbuf sb = STRBUF_INIT;
- const unsigned char *sha1 = origin_data->sha1;
+ const struct object_id *oid = &origin_data->oid;
int limit = opts->shortlog_len;
- branch = deref_tag(parse_object(sha1), sha1_to_hex(sha1), 40);
+ branch = deref_tag(parse_object(oid->hash), oid_to_hex(oid), GIT_SHA1_HEXSZ);
if (!branch || branch->type != OBJ_COMMIT)
return;
@@ -531,7 +531,7 @@ static void fmt_merge_msg_sigs(struct strbuf *out)
}
static void find_merge_parents(struct merge_parents *result,
- struct strbuf *in, unsigned char *head)
+ struct strbuf *in, struct object_id *head)
{
struct commit_list *parents;
struct commit *head_commit;
@@ -542,31 +542,31 @@ static void find_merge_parents(struct merge_parents *result,
int len;
char *p = in->buf + pos;
char *newline = strchr(p, '\n');
- unsigned char sha1[20];
+ struct object_id oid;
struct commit *parent;
struct object *obj;
len = newline ? newline - p : strlen(p);
pos += len + !!newline;
- if (len < 43 ||
- get_sha1_hex(p, sha1) ||
- p[40] != '\t' ||
- p[41] != '\t')
+ if (len < GIT_SHA1_HEXSZ + 3 ||
+ get_oid_hex(p, &oid) ||
+ p[GIT_SHA1_HEXSZ] != '\t' ||
+ p[GIT_SHA1_HEXSZ + 1] != '\t')
continue; /* skip not-for-merge */
/*
* Do not use get_merge_parent() here; we do not have
* "name" here and we do not want to contaminate its
* util field yet.
*/
- obj = parse_object(sha1);
+ obj = parse_object(oid.hash);
parent = (struct commit *)peel_to_type(NULL, 0, obj, OBJ_COMMIT);
if (!parent)
continue;
commit_list_insert(parent, &parents);
- add_merge_parent(result, obj->oid.hash, parent->object.oid.hash);
+ add_merge_parent(result, &obj->oid, &parent->object.oid);
}
- head_commit = lookup_commit(head);
+ head_commit = lookup_commit(head->hash);
if (head_commit)
commit_list_insert(head_commit, &parents);
parents = reduce_heads(parents);
@@ -574,7 +574,7 @@ static void find_merge_parents(struct merge_parents *result,
while (parents) {
struct commit *cmit = pop_commit(&parents);
for (i = 0; i < result->nr; i++)
- if (!hashcmp(result->item[i].commit, cmit->object.oid.hash))
+ if (!oidcmp(&result->item[i].commit, &cmit->object.oid))
result->item[i].used = 1;
}
@@ -592,7 +592,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
struct fmt_merge_msg_opts *opts)
{
int i = 0, pos = 0;
- unsigned char head_sha1[20];
+ struct object_id head_oid;
const char *current_branch;
void *current_branch_to_free;
struct merge_parents merge_parents;
@@ -601,13 +601,13 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
/* get current branch */
current_branch = current_branch_to_free =
- resolve_refdup("HEAD", RESOLVE_REF_READING, head_sha1, NULL);
+ resolve_refdup("HEAD", RESOLVE_REF_READING, head_oid.hash, NULL);
if (!current_branch)
die("No current branch");
if (starts_with(current_branch, "refs/heads/"))
current_branch += 11;
- find_merge_parents(&merge_parents, in, head_sha1);
+ find_merge_parents(&merge_parents, in, &head_oid);
/* get a line */
while (pos < in->len) {
@@ -633,7 +633,7 @@ int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
struct commit *head;
struct rev_info rev;
- head = lookup_commit_or_die(head_sha1, "HEAD");
+ head = lookup_commit_or_die(head_oid.hash, "HEAD");
init_revisions(&rev, NULL);
rev.commit_format = CMIT_FMT_ONELINE;
rev.ignore_merges = 1;
--
2.11.0
^ permalink raw reply related
* [PATCH v5 09/19] builtin/clone: convert to struct object_id
From: brian m. carlson @ 2017-02-21 23:47 UTC (permalink / raw)
To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano, Ramsay Jones
In-Reply-To: <20170221234737.894681-1-sandals@crustytoothpaste.net>
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
builtin/clone.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c
index 3f63edbbf9..b4c929bb8a 100644
--- a/builtin/clone.c
+++ b/builtin/clone.c
@@ -681,7 +681,7 @@ static void update_head(const struct ref *our, const struct ref *remote,
static int checkout(int submodule_progress)
{
- unsigned char sha1[20];
+ struct object_id oid;
char *head;
struct lock_file *lock_file;
struct unpack_trees_options opts;
@@ -692,7 +692,7 @@ static int checkout(int submodule_progress)
if (option_no_checkout)
return 0;
- head = resolve_refdup("HEAD", RESOLVE_REF_READING, sha1, NULL);
+ head = resolve_refdup("HEAD", RESOLVE_REF_READING, oid.hash, NULL);
if (!head) {
warning(_("remote HEAD refers to nonexistent ref, "
"unable to checkout.\n"));
@@ -700,7 +700,7 @@ static int checkout(int submodule_progress)
}
if (!strcmp(head, "HEAD")) {
if (advice_detached_head)
- detach_advice(sha1_to_hex(sha1));
+ detach_advice(oid_to_hex(&oid));
} else {
if (!starts_with(head, "refs/heads/"))
die(_("HEAD not found below refs/heads!"));
@@ -721,7 +721,7 @@ static int checkout(int submodule_progress)
opts.src_index = &the_index;
opts.dst_index = &the_index;
- tree = parse_tree_indirect(sha1);
+ tree = parse_tree_indirect(oid.hash);
parse_tree(tree);
init_tree_desc(&t, tree->buffer, tree->size);
if (unpack_trees(1, &t, &opts) < 0)
@@ -731,7 +731,7 @@ static int checkout(int submodule_progress)
die(_("unable to write new index file"));
err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
- sha1_to_hex(sha1), "1", NULL);
+ oid_to_hex(&oid), "1", NULL);
if (!err && option_recursive) {
struct argv_array args = ARGV_ARRAY_INIT;
--
2.11.0
^ permalink raw reply related
* [PATCH v5 04/19] builtin/describe: convert to struct object_id
From: brian m. carlson @ 2017-02-21 23:47 UTC (permalink / raw)
To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano, Ramsay Jones
In-Reply-To: <20170221234737.894681-1-sandals@crustytoothpaste.net>
Convert the functions in this file and struct commit_name to struct
object_id.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
builtin/describe.c | 50 +++++++++++++++++++++++++-------------------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/builtin/describe.c b/builtin/describe.c
index 01490a157e..738e68f95b 100644
--- a/builtin/describe.c
+++ b/builtin/describe.c
@@ -39,11 +39,11 @@ static const char *diff_index_args[] = {
struct commit_name {
struct hashmap_entry entry;
- unsigned char peeled[20];
+ struct object_id peeled;
struct tag *tag;
unsigned prio:2; /* annotated tag = 2, tag = 1, head = 0 */
unsigned name_checked:1;
- unsigned char sha1[20];
+ struct object_id oid;
char *path;
};
@@ -54,17 +54,17 @@ static const char *prio_names[] = {
static int commit_name_cmp(const struct commit_name *cn1,
const struct commit_name *cn2, const void *peeled)
{
- return hashcmp(cn1->peeled, peeled ? peeled : cn2->peeled);
+ return oidcmp(&cn1->peeled, peeled ? peeled : &cn2->peeled);
}
-static inline struct commit_name *find_commit_name(const unsigned char *peeled)
+static inline struct commit_name *find_commit_name(const struct object_id *peeled)
{
- return hashmap_get_from_hash(&names, sha1hash(peeled), peeled);
+ return hashmap_get_from_hash(&names, sha1hash(peeled->hash), peeled->hash);
}
static int replace_name(struct commit_name *e,
int prio,
- const unsigned char *sha1,
+ const struct object_id *oid,
struct tag **tag)
{
if (!e || e->prio < prio)
@@ -77,13 +77,13 @@ static int replace_name(struct commit_name *e,
struct tag *t;
if (!e->tag) {
- t = lookup_tag(e->sha1);
+ t = lookup_tag(e->oid.hash);
if (!t || parse_tag(t))
return 1;
e->tag = t;
}
- t = lookup_tag(sha1);
+ t = lookup_tag(oid->hash);
if (!t || parse_tag(t))
return 0;
*tag = t;
@@ -96,24 +96,24 @@ static int replace_name(struct commit_name *e,
}
static void add_to_known_names(const char *path,
- const unsigned char *peeled,
+ const struct object_id *peeled,
int prio,
- const unsigned char *sha1)
+ const struct object_id *oid)
{
struct commit_name *e = find_commit_name(peeled);
struct tag *tag = NULL;
- if (replace_name(e, prio, sha1, &tag)) {
+ if (replace_name(e, prio, oid, &tag)) {
if (!e) {
e = xmalloc(sizeof(struct commit_name));
- hashcpy(e->peeled, peeled);
- hashmap_entry_init(e, sha1hash(peeled));
+ oidcpy(&e->peeled, peeled);
+ hashmap_entry_init(e, sha1hash(peeled->hash));
hashmap_add(&names, e);
e->path = NULL;
}
e->tag = tag;
e->prio = prio;
e->name_checked = 0;
- hashcpy(e->sha1, sha1);
+ oidcpy(&e->oid, oid);
free(e->path);
e->path = xstrdup(path);
}
@@ -154,7 +154,7 @@ static int get_name(const char *path, const struct object_id *oid, int flag, voi
else
prio = 0;
- add_to_known_names(all ? path + 5 : path + 10, peeled.hash, prio, oid->hash);
+ add_to_known_names(all ? path + 5 : path + 10, &peeled, prio, oid);
return 0;
}
@@ -212,7 +212,7 @@ static unsigned long finish_depth_computation(
static void display_name(struct commit_name *n)
{
if (n->prio == 2 && !n->tag) {
- n->tag = lookup_tag(n->sha1);
+ n->tag = lookup_tag(n->oid.hash);
if (!n->tag || parse_tag(n->tag))
die(_("annotated tag %s not available"), n->path);
}
@@ -230,14 +230,14 @@ static void display_name(struct commit_name *n)
printf("%s", n->path);
}
-static void show_suffix(int depth, const unsigned char *sha1)
+static void show_suffix(int depth, const struct object_id *oid)
{
- printf("-%d-g%s", depth, find_unique_abbrev(sha1, abbrev));
+ printf("-%d-g%s", depth, find_unique_abbrev(oid->hash, abbrev));
}
static void describe(const char *arg, int last_one)
{
- unsigned char sha1[20];
+ struct object_id oid;
struct commit *cmit, *gave_up_on = NULL;
struct commit_list *list;
struct commit_name *n;
@@ -246,20 +246,20 @@ static void describe(const char *arg, int last_one)
unsigned long seen_commits = 0;
unsigned int unannotated_cnt = 0;
- if (get_sha1(arg, sha1))
+ if (get_oid(arg, &oid))
die(_("Not a valid object name %s"), arg);
- cmit = lookup_commit_reference(sha1);
+ cmit = lookup_commit_reference(oid.hash);
if (!cmit)
die(_("%s is not a valid '%s' object"), arg, commit_type);
- n = find_commit_name(cmit->object.oid.hash);
+ n = find_commit_name(&cmit->object.oid);
if (n && (tags || all || n->prio == 2)) {
/*
* Exact match to an existing ref.
*/
display_name(n);
if (longformat)
- show_suffix(0, n->tag ? n->tag->tagged->oid.hash : sha1);
+ show_suffix(0, n->tag ? &n->tag->tagged->oid : &oid);
if (dirty)
printf("%s", dirty);
printf("\n");
@@ -276,7 +276,7 @@ static void describe(const char *arg, int last_one)
struct commit *c;
struct commit_name *n = hashmap_iter_first(&names, &iter);
for (; n; n = hashmap_iter_next(&iter)) {
- c = lookup_commit_reference_gently(n->peeled, 1);
+ c = lookup_commit_reference_gently(n->peeled.hash, 1);
if (c)
c->util = n;
}
@@ -380,7 +380,7 @@ static void describe(const char *arg, int last_one)
display_name(all_matches[0].name);
if (abbrev)
- show_suffix(all_matches[0].depth, cmit->object.oid.hash);
+ show_suffix(all_matches[0].depth, &cmit->object.oid);
if (dirty)
printf("%s", dirty);
printf("\n");
--
2.11.0
^ permalink raw reply related
* [PATCH v5 07/19] builtin/grep: convert to struct object_id
From: brian m. carlson @ 2017-02-21 23:47 UTC (permalink / raw)
To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano, Ramsay Jones
In-Reply-To: <20170221234737.894681-1-sandals@crustytoothpaste.net>
Convert several functions to use struct object_id, and rename them so
that they no longer refer to SHA-1.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
builtin/grep.c | 24 ++++++++++++------------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/builtin/grep.c b/builtin/grep.c
index 2c727ef499..0393b0fdc4 100644
--- a/builtin/grep.c
+++ b/builtin/grep.c
@@ -294,17 +294,17 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)
return st;
}
-static void *lock_and_read_sha1_file(const unsigned char *sha1, enum object_type *type, unsigned long *size)
+static void *lock_and_read_oid_file(const struct object_id *oid, enum object_type *type, unsigned long *size)
{
void *data;
grep_read_lock();
- data = read_sha1_file(sha1, type, size);
+ data = read_sha1_file(oid->hash, type, size);
grep_read_unlock();
return data;
}
-static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
+static int grep_oid(struct grep_opt *opt, const struct object_id *oid,
const char *filename, int tree_name_len,
const char *path)
{
@@ -323,7 +323,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
#ifndef NO_PTHREADS
if (num_threads) {
- add_work(opt, GREP_SOURCE_SHA1, pathbuf.buf, path, sha1);
+ add_work(opt, GREP_SOURCE_SHA1, pathbuf.buf, path, oid);
strbuf_release(&pathbuf);
return 0;
} else
@@ -332,7 +332,7 @@ static int grep_sha1(struct grep_opt *opt, const unsigned char *sha1,
struct grep_source gs;
int hit;
- grep_source_init(&gs, GREP_SOURCE_SHA1, pathbuf.buf, path, sha1);
+ grep_source_init(&gs, GREP_SOURCE_SHA1, pathbuf.buf, path, oid);
strbuf_release(&pathbuf);
hit = grep_source(opt, &gs);
@@ -690,7 +690,7 @@ static int grep_cache(struct grep_opt *opt, const struct pathspec *pathspec,
ce_skip_worktree(ce)) {
if (ce_stage(ce) || ce_intent_to_add(ce))
continue;
- hit |= grep_sha1(opt, ce->oid.hash, ce->name,
+ hit |= grep_oid(opt, &ce->oid, ce->name,
0, ce->name);
} else {
hit |= grep_file(opt, ce->name);
@@ -750,7 +750,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
strbuf_add(base, entry.path, te_len);
if (S_ISREG(entry.mode)) {
- hit |= grep_sha1(opt, entry.oid->hash, base->buf, tn_len,
+ hit |= grep_oid(opt, entry.oid, base->buf, tn_len,
check_attr ? base->buf + tn_len : NULL);
} else if (S_ISDIR(entry.mode)) {
enum object_type type;
@@ -758,7 +758,7 @@ static int grep_tree(struct grep_opt *opt, const struct pathspec *pathspec,
void *data;
unsigned long size;
- data = lock_and_read_sha1_file(entry.oid->hash, &type, &size);
+ data = lock_and_read_oid_file(entry.oid, &type, &size);
if (!data)
die(_("unable to read tree (%s)"),
oid_to_hex(entry.oid));
@@ -787,7 +787,7 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,
struct object *obj, const char *name, const char *path)
{
if (obj->type == OBJ_BLOB)
- return grep_sha1(opt, obj->oid.hash, name, 0, path);
+ return grep_oid(opt, &obj->oid, name, 0, path);
if (obj->type == OBJ_COMMIT || obj->type == OBJ_TREE) {
struct tree_desc tree;
void *data;
@@ -1152,11 +1152,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
/* Check revs and then paths */
for (i = 0; i < argc; i++) {
const char *arg = argv[i];
- unsigned char sha1[20];
+ struct object_id oid;
struct object_context oc;
/* Is it a rev? */
- if (!get_sha1_with_context(arg, 0, sha1, &oc)) {
- struct object *object = parse_object_or_die(sha1, arg);
+ if (!get_sha1_with_context(arg, 0, oid.hash, &oc)) {
+ struct object *object = parse_object_or_die(oid.hash, arg);
if (!seen_dashdash)
verify_non_filename(prefix, arg);
add_object_array_with_path(object, arg, &list, oc.mode, oc.path);
--
2.11.0
^ permalink raw reply related
* [PATCH v5 00/19] object_id part 6
From: brian m. carlson @ 2017-02-21 23:47 UTC (permalink / raw)
To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano, Ramsay Jones
This is another series in the continuing conversion to struct object_id.
This series converts more of the builtin directory and some of the refs
code to use struct object_id. Additionally, it implements an
nth_packed_object_oid function which provides a struct object_id version
of the nth_packed_object function, and a parse_oid_hex function that
makes parsing easier.
I'll be submitting the test I wrote as a separate patch, but I did build
and test with it, so I feel confident that it works properly.
Changes from v4:
* Fix breakage in builtin/diff-tree pointed out by Peff.
Changes from v3:
* Move the parse_oid_hex patch earlier in the series.
* Use parse_oid_hex in builtin/diff-tree.c.
* Fix several warts with parse_oid_hex pointed out by Peff.
Changes from v2:
* Fix misnamed function in commit message.
* Improve parameter name of parse_oid_hex.
* Improve docstring of parse_oid_hex.
* Remove needless variable.
* Rebase on master.
Changes from v1:
* Implement parse_oid_hex and use it.
* Make nth_packed_object_oid take a variable into which to store the
object ID. This avoids concerns about unsafe casts.
* Rebase on master.
brian m. carlson (19):
hex: introduce parse_oid_hex
builtin/commit: convert to struct object_id
builtin/diff-tree: convert to struct object_id
builtin/describe: convert to struct object_id
builtin/fast-export: convert to struct object_id
builtin/fmt-merge-message: convert to struct object_id
builtin/grep: convert to struct object_id
builtin/branch: convert to struct object_id
builtin/clone: convert to struct object_id
builtin/merge: convert to struct object_id
Convert remaining callers of resolve_refdup to object_id
builtin/replace: convert to struct object_id
reflog-walk: convert struct reflog_info to struct object_id
refs: convert each_reflog_ent_fn to struct object_id
refs: simplify parsing of reflog entries
sha1_file: introduce an nth_packed_object_oid function
Convert object iteration callbacks to struct object_id
builtin/merge-base: convert to struct object_id
wt-status: convert to struct object_id
builtin/branch.c | 26 +++++-----
builtin/cat-file.c | 8 +--
builtin/clone.c | 10 ++--
builtin/commit.c | 46 ++++++++---------
builtin/count-objects.c | 4 +-
builtin/describe.c | 50 +++++++++---------
builtin/diff-tree.c | 61 +++++++++++-----------
builtin/fast-export.c | 58 ++++++++++-----------
builtin/fmt-merge-msg.c | 70 ++++++++++++-------------
builtin/fsck.c | 40 +++++++--------
builtin/grep.c | 24 ++++-----
builtin/merge-base.c | 30 +++++------
builtin/merge.c | 134 ++++++++++++++++++++++++------------------------
builtin/notes.c | 18 +++----
builtin/pack-objects.c | 6 +--
builtin/prune-packed.c | 4 +-
builtin/prune.c | 8 +--
builtin/receive-pack.c | 4 +-
builtin/reflog.c | 2 +-
builtin/replace.c | 112 ++++++++++++++++++++--------------------
cache.h | 19 ++++++-
hex.c | 8 +++
reachable.c | 30 +++++------
ref-filter.c | 4 +-
reflog-walk.c | 26 +++++-----
refs.c | 24 ++++-----
refs.h | 2 +-
refs/files-backend.c | 29 ++++++-----
revision.c | 12 ++---
sha1_file.c | 27 +++++++---
sha1_name.c | 2 +-
transport.c | 4 +-
wt-status.c | 52 +++++++++----------
33 files changed, 493 insertions(+), 461 deletions(-)
--
2.11.0
^ permalink raw reply
* [PATCH v5 02/19] builtin/commit: convert to struct object_id
From: brian m. carlson @ 2017-02-21 23:47 UTC (permalink / raw)
To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano, Ramsay Jones
In-Reply-To: <20170221234737.894681-1-sandals@crustytoothpaste.net>
Convert most leaf functions to use struct object_id.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
builtin/commit.c | 46 +++++++++++++++++++++++-----------------------
1 file changed, 23 insertions(+), 23 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 2de5f6cc64..4e288bc513 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -496,7 +496,7 @@ static const char *prepare_index(int argc, const char **argv, const char *prefix
static int run_status(FILE *fp, const char *index_file, const char *prefix, int nowarn,
struct wt_status *s)
{
- unsigned char sha1[20];
+ struct object_id oid;
if (s->relative_paths)
s->prefix = prefix;
@@ -509,9 +509,9 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int
s->index_file = index_file;
s->fp = fp;
s->nowarn = nowarn;
- s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0;
+ s->is_initial = get_sha1(s->reference, oid.hash) ? 1 : 0;
if (!s->is_initial)
- hashcpy(s->sha1_commit, sha1);
+ hashcpy(s->sha1_commit, oid.hash);
s->status_format = status_format;
s->ignore_submodule_arg = ignore_submodule_arg;
@@ -885,7 +885,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
commitable = run_status(s->fp, index_file, prefix, 1, s);
s->use_color = saved_color_setting;
} else {
- unsigned char sha1[20];
+ struct object_id oid;
const char *parent = "HEAD";
if (!active_nr && read_cache() < 0)
@@ -894,7 +894,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,
if (amend)
parent = "HEAD^1";
- if (get_sha1(parent, sha1)) {
+ if (get_sha1(parent, oid.hash)) {
int i, ita_nr = 0;
for (i = 0; i < active_nr; i++)
@@ -1332,7 +1332,7 @@ int cmd_status(int argc, const char **argv, const char *prefix)
{
static struct wt_status s;
int fd;
- unsigned char sha1[20];
+ struct object_id oid;
static struct option builtin_status_options[] = {
OPT__VERBOSE(&verbose, N_("be verbose")),
OPT_SET_INT('s', "short", &status_format,
@@ -1382,9 +1382,9 @@ int cmd_status(int argc, const char **argv, const char *prefix)
fd = hold_locked_index(&index_lock, 0);
- s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0;
+ s.is_initial = get_sha1(s.reference, oid.hash) ? 1 : 0;
if (!s.is_initial)
- hashcpy(s.sha1_commit, sha1);
+ hashcpy(s.sha1_commit, oid.hash);
s.ignore_submodule_arg = ignore_submodule_arg;
s.status_format = status_format;
@@ -1418,19 +1418,19 @@ static const char *implicit_ident_advice(void)
}
-static void print_summary(const char *prefix, const unsigned char *sha1,
+static void print_summary(const char *prefix, const struct object_id *oid,
int initial_commit)
{
struct rev_info rev;
struct commit *commit;
struct strbuf format = STRBUF_INIT;
- unsigned char junk_sha1[20];
+ struct object_id junk_oid;
const char *head;
struct pretty_print_context pctx = {0};
struct strbuf author_ident = STRBUF_INIT;
struct strbuf committer_ident = STRBUF_INIT;
- commit = lookup_commit(sha1);
+ commit = lookup_commit(oid->hash);
if (!commit)
die(_("couldn't look up newly created commit"));
if (parse_commit(commit))
@@ -1477,7 +1477,7 @@ static void print_summary(const char *prefix, const unsigned char *sha1,
rev.diffopt.break_opt = 0;
diff_setup_done(&rev.diffopt);
- head = resolve_ref_unsafe("HEAD", 0, junk_sha1, NULL);
+ head = resolve_ref_unsafe("HEAD", 0, junk_oid.hash, NULL);
if (!strcmp(head, "HEAD"))
head = _("detached HEAD");
else
@@ -1522,8 +1522,8 @@ static int git_commit_config(const char *k, const char *v, void *cb)
return git_status_config(k, v, s);
}
-static int run_rewrite_hook(const unsigned char *oldsha1,
- const unsigned char *newsha1)
+static int run_rewrite_hook(const struct object_id *oldoid,
+ const struct object_id *newoid)
{
struct child_process proc = CHILD_PROCESS_INIT;
const char *argv[3];
@@ -1544,7 +1544,7 @@ static int run_rewrite_hook(const unsigned char *oldsha1,
code = start_command(&proc);
if (code)
return code;
- strbuf_addf(&sb, "%s %s\n", sha1_to_hex(oldsha1), sha1_to_hex(newsha1));
+ strbuf_addf(&sb, "%s %s\n", oid_to_hex(oldoid), oid_to_hex(newoid));
sigchain_push(SIGPIPE, SIG_IGN);
write_in_full(proc.in, sb.buf, sb.len);
close(proc.in);
@@ -1636,7 +1636,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
struct strbuf author_ident = STRBUF_INIT;
const char *index_file, *reflog_msg;
char *nl;
- unsigned char sha1[20];
+ struct object_id oid;
struct commit_list *parents = NULL;
struct stat statbuf;
struct commit *current_head = NULL;
@@ -1651,10 +1651,10 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
status_format = STATUS_FORMAT_NONE; /* Ignore status.short */
s.colopts = 0;
- if (get_sha1("HEAD", sha1))
+ if (get_sha1("HEAD", oid.hash))
current_head = NULL;
else {
- current_head = lookup_commit_or_die(sha1, "HEAD");
+ current_head = lookup_commit_or_die(oid.hash, "HEAD");
if (parse_commit(current_head))
die(_("could not parse HEAD commit"));
}
@@ -1759,7 +1759,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
}
if (commit_tree_extended(sb.buf, sb.len, active_cache_tree->sha1,
- parents, sha1, author_ident.buf, sign_commit, extra)) {
+ parents, oid.hash, author_ident.buf, sign_commit, extra)) {
rollback_index_files();
die(_("failed to write commit object"));
}
@@ -1776,7 +1776,7 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
transaction = ref_transaction_begin(&err);
if (!transaction ||
- ref_transaction_update(transaction, "HEAD", sha1,
+ ref_transaction_update(transaction, "HEAD", oid.hash,
current_head
? current_head->object.oid.hash : null_sha1,
0, sb.buf, &err) ||
@@ -1805,13 +1805,13 @@ int cmd_commit(int argc, const char **argv, const char *prefix)
cfg = init_copy_notes_for_rewrite("amend");
if (cfg) {
/* we are amending, so current_head is not NULL */
- copy_note_for_rewrite(cfg, current_head->object.oid.hash, sha1);
+ copy_note_for_rewrite(cfg, current_head->object.oid.hash, oid.hash);
finish_copy_notes_for_rewrite(cfg, "Notes added by 'git commit --amend'");
}
- run_rewrite_hook(current_head->object.oid.hash, sha1);
+ run_rewrite_hook(¤t_head->object.oid, &oid);
}
if (!quiet)
- print_summary(prefix, sha1, !current_head);
+ print_summary(prefix, &oid, !current_head);
strbuf_release(&err);
return 0;
--
2.11.0
^ permalink raw reply related
* [PATCH v5 01/19] hex: introduce parse_oid_hex
From: brian m. carlson @ 2017-02-21 23:47 UTC (permalink / raw)
To: git; +Cc: Jeff King, Michael Haggerty, Junio C Hamano, Ramsay Jones
In-Reply-To: <20170221234737.894681-1-sandals@crustytoothpaste.net>
Introduce a function, parse_oid_hex, which parses a hexadecimal object
ID and if successful, sets a pointer to just beyond the last character.
This allows for simpler, more robust parsing without needing to
hard-code integer values throughout the codebase.
Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
---
cache.h | 9 +++++++++
hex.c | 8 ++++++++
2 files changed, 17 insertions(+)
diff --git a/cache.h b/cache.h
index 61fc86e6d7..e03a672d15 100644
--- a/cache.h
+++ b/cache.h
@@ -1319,6 +1319,15 @@ extern char *oid_to_hex_r(char *out, const struct object_id *oid);
extern char *sha1_to_hex(const unsigned char *sha1); /* static buffer result! */
extern char *oid_to_hex(const struct object_id *oid); /* same static buffer as sha1_to_hex */
+/*
+ * Parse a 40-character hexadecimal object ID starting from hex, updating the
+ * pointer specified by end when parsing stops. The resulting object ID is
+ * stored in oid. Returns 0 on success. Parsing will stop on the first NUL or
+ * other invalid character. end is only updated on success; otherwise, it is
+ * unmodified.
+ */
+extern int parse_oid_hex(const char *hex, struct object_id *oid, const char **end);
+
extern int interpret_branch_name(const char *str, int len, struct strbuf *);
extern int get_oid_mb(const char *str, struct object_id *oid);
diff --git a/hex.c b/hex.c
index 845b01a874..eab7b626ee 100644
--- a/hex.c
+++ b/hex.c
@@ -53,6 +53,14 @@ int get_oid_hex(const char *hex, struct object_id *oid)
return get_sha1_hex(hex, oid->hash);
}
+int parse_oid_hex(const char *hex, struct object_id *oid, const char **end)
+{
+ int ret = get_oid_hex(hex, oid);
+ if (!ret)
+ *end = hex + GIT_SHA1_HEXSZ;
+ return ret;
+}
+
char *sha1_to_hex_r(char *buffer, const unsigned char *sha1)
{
static const char hex[] = "0123456789abcdef";
--
2.11.0
^ permalink raw reply related
* Re: [PATCH 12/15] unpack-trees: check if we can perform the operation for submodules
From: Stefan Beller @ 2017-02-21 23:44 UTC (permalink / raw)
To: Jacob Keller
Cc: Git mailing list, brian m. carlson, Jonathan Nieder,
Brandon Williams, Junio C Hamano
In-Reply-To: <CA+P7+xq3XF2=erRLiNjOUq9H66Sa5VCxNBzTF=RAycPYrPRFJg@mail.gmail.com>
On Tue, Feb 21, 2017 at 3:35 PM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Tue, Feb 21, 2017 at 2:16 PM, Stefan Beller <sbeller@google.com> wrote:
>> On Fri, Feb 17, 2017 at 10:42 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>>> On Wed, Feb 15, 2017 at 4:38 PM, Stefan Beller <sbeller@google.com> wrote:
>>>> + if (is_active_submodule_with_strategy(ce, SM_UPDATE_UNSPECIFIED))
>>>
>>> Here, and in other cases where we use
>>> is_active_submodule_with_strategy(), why do we only ever check
>>> SM_UPDATE_UNSPECIFIED? It seems really weird that we're only going to
>>> check submodules who's strategy is unspecified, when that defaults to
>>> checkout if I recall correctly? Shouldn't we check both? This applies
>>> to pretty much everywhere that you call this function that I noticed,
>>> which is why I removed the context.
>>
>> I am torn between this.
>>
>> submodule.<name>.update = {rebase, merge, checkout, none !command}
>> is currently documented in GIT-CONFIG(1) as
>>
>> submodule.<name>.update
>> The default update procedure for a submodule. This variable is
>> populated by git submodule init from the gitmodules(5) file. See
>> description of update command in git-submodule(1).
>>
>> and in GIT-SUBMODULE(1) as
>>
>> update
>> [...] can be done in several ways
>> depending on command line options and the value of
>> submodule.<name>.update configuration variable. Supported update
>> procedures are:
>>
>> checkout
>> [...] or no option is given, and
>> submodule.<name>.update is unset, or if it is set to checkout.
>>
>> So the "update" config clearly only applies to the "submodule update"
>> command, right?
>>
>> Well no, "checkout --recurse-submodules" is very similar
>> to running "submodule update", except with a bit more checks, so you could
>> think that such an option applies to checkout as well. (and eventually
>> rebase/merge etc. are supported as well.)
>>
>> So initially I assumed both "unspecified" as well as "checkout"
>> are good matches to support in the first round.
>>
>> Then I flip flopped to think that we should not interfere with these
>> settings at all (The checkout command does checkout and checkout only;
>> no implicit rebase/merge ever in the future, because that would be
>> confusing). So ignoring that option seemed like the way to go.
>
> Hmm. So it's a bit complicated.
>
>>
>> But ignoring that option is also not the right approach.
>> What if you have set it to "none" and really *expect* Git to not touch
>> that submodule?
>
> Or set it to "rebase" and suddenly git-checkout is ignoring you and
> just checking things out anyways.
>
>>
>> So I dunno. Maybe it is a documentation issue, we need to spell out
>> in the man page for checkout that --recurse-submodules is
>> following one of these models. Now which is the best default model here?
>
> Personally, I would go with that the config option sets the general
> strategy used by the submodule whenever its updated, regardless of
> how.
>
> So, for example, setting it to none, means that recurse-submoduls will
> ignore it when checking out. Setting it to rebase, or merge, and the
> checkout will try to do those things?
That is generally a sound idea when it comes to git-checkout.
What about other future things like git-revert?
(Ok I already brought up this example too many times; it should have
a revert-submodules as well switch, which is neither of the current strategies,
so we'd have to invent a new strategy and make that the default for
revert. That strategy would make no sense in any other command though)
What about "git-rebase --recurse-submodules"?
Should git-rebase merge the submodules when it is configured to "merge"
Or just "checkout" (the possibly non-fast-forward-y old sha1) ?
The only sane option IMO is "rebase" as well in the submodules, rewriting
the submodule pointers in the rebased commits in the superproject.
>
> Or, if that's not really feasible, have the checkout go "hey.. you
> asked me to recurse, but uhhh these submodules don't allow me to do
> checkout, so I'm gonna fail"? I think that's the best approach for
> now.
So you'd propose to generally use the submodule.<name>.update
strategies with aggressive error-out but also keeping in mind
that the strategies might grow by a lot in the future (well only revert
comes to mind here).
ok, let's do that then.
Thanks,
Stefan
^ permalink raw reply
* Re: url.<base>.insteadOf vs. submodules
From: Jeff King @ 2017-02-21 23:40 UTC (permalink / raw)
To: Stefan Beller; +Cc: Toolforger, git@vger.kernel.org
In-Reply-To: <CAGZ79kby-UhUqci9Mgdhw+wvS5Y39=Q7AmCrWaTMWbcZPNT6Dw@mail.gmail.com>
On Tue, Feb 21, 2017 at 03:16:27PM -0800, Stefan Beller wrote:
> > I guess one answer is that this is the wrong approach entirely, and the
> > right one is something like: submodules should understand that they are
> > part of a superproject, and respect some whitelisted set of config from
> > the superproject .git/config file.
>
> This would break one of the core assumptions that submodules
> are "independent" repos.
Yeah, that was the "first half" that I said was hard. :)
You could rationalize it under the fact that they _are_ independent
repos; we're just adding a new config source. Arguably it could be a
feature for any repository embedded inside the working tree of another,
submodule or not, to consider the outer repository as a (limited) source
of config.
But there are probably a lot of irritating corner cases with the whole
concept unless we apply a strict whitelist of keys (e.g., you probably
don't want remote.* to be propagated). And as the recent
GIT_CONFIG_PARAMETERS whitelist showed, that approach ended up confusing
and annoying.
So maybe the whole thing is insane, and the right answer is that config
values should go into ~/.gitconfig. And we may need better tools there
for limiting that global config to certain parts of the tree (like Duy's
conditional include thing).
> Though I do not know if this is actually a good assumption.
> e.g. "[PATCH v2] git-prompt.sh: add submodule indicator"
> https://public-inbox.org/git/1486075892-20676-2-git-send-email-email@benjaminfuchs.de/
> really had trouble in the first version to nail down how to tell you are in
> a submodule, but people want to know that.
Right, I think it's an interesting thing to know, but I agree there are
probably a lot of corner cases.
> Maybe we need to change that fundamental assumption.
> So a more sophisticated way (thinking long term here) would be
> to include the superprojects config file (with exceptions), and that
> config file has more priority than e.g. the ~/.gitconfig file, but less
> than the submodules own $GIT_DIR/config file.
Yeah, that priority matches what I had been thinking.
> > One other caveat: I'm not sure if we do insteadOf recursively, but it
> > may be surprising to the child "git clone" that we've already applied
> > the insteadOf rewriting (especially if the rules are coming from
> > ~/.gitconfig and may be applied twice).
>
> When a rule is having effect twice the rule sounds broken. (the outcome
> ought to be sufficiently different from the original?)
If you have:
url.bar.insteadOf=foo
url.baz.insteadOf=bar
do we convert "foo" to "baz"? If so, then I think applying the rules
again shouldn't matter. But if we don't, and only do a single level,
then having the caller rewrite the URL before it hands it to "git clone"
means we may end up unexpectedly doing two levels of rewriting.
-Peff
^ permalink raw reply
* Re: url.<base>.insteadOf vs. submodules
From: Junio C Hamano @ 2017-02-21 23:37 UTC (permalink / raw)
To: Stefan Beller; +Cc: Jeff King, Toolforger, git@vger.kernel.org
In-Reply-To: <CAGZ79kby-UhUqci9Mgdhw+wvS5Y39=Q7AmCrWaTMWbcZPNT6Dw@mail.gmail.com>
Stefan Beller <sbeller@google.com> writes:
> On Tue, Feb 21, 2017 at 3:00 PM, Jeff King <peff@peff.net> wrote:
> ...
>> I guess one answer is that this is the wrong approach entirely, and the
>> right one is something like: submodules should understand that they are
>> part of a superproject, and respect some whitelisted set of config from
>> the superproject .git/config file.
>
> This would break one of the core assumptions that submodules
> are "independent" repos.
>
> The way of action is a one way street:
> * The superproject is aware of the submodule and when you invoke a
> command on the superproject, you may mess around with the submodule,
> e.g. update/remove it; absorb its git directory.
> * The submodule is "just" a repository with weird .git link file and a
> respective core.worktree setup. Currently it doesn't know if it is
> guided by a superproject.
While that is a good discipline to follow, I think you need to
differenciate the project that is bound as a submodule to a
superproject, and a specific instance of a submodule repository,
i.e. a clone of such a project.
It is true that the Linux kernel project should *NEVER* know your
appliance project only because you happen to use it as a component
of your appliance that happens to use the kernel as one of its
submodules. But that does not mean your copy of the kernel that
sits in your recursive checkout of your appliance project should
not know anything about your superproject.
This is true even without any submodules. The Git project itself
does not even care you are Stefan, but you still can and do add
[user] name = "Stefan Beller" to .git/config of your clone of the
Git project. A clone of the project may want to know more than the
data project itself keeps track of to describe the context in which
the particular clone is being used. And .git/config is a good place
to keep such pieces of information.
So I would think it is entirely reasonable if "git submodule init
sub" that is run in the superproject to initialize "sub" writes
something in "sub/.git" to tell that "sub" is used in the context of
that particular toplevel superproject and customize its behavour
accordingly. Perhaps it may want to add the url.*.insteadOf that is
useful for updating the submodule repository when it does "submodule
init", for example.
^ permalink raw reply
* Re: [PATCH 12/15] unpack-trees: check if we can perform the operation for submodules
From: Jacob Keller @ 2017-02-21 23:35 UTC (permalink / raw)
To: Stefan Beller
Cc: Git mailing list, brian m. carlson, Jonathan Nieder,
Brandon Williams, Junio C Hamano
In-Reply-To: <CAGZ79kahCN2dd9=CzqXJWSJKrkOfvd3HeQ-NORhyP=6B=KCrqg@mail.gmail.com>
On Tue, Feb 21, 2017 at 2:16 PM, Stefan Beller <sbeller@google.com> wrote:
> On Fri, Feb 17, 2017 at 10:42 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>> On Wed, Feb 15, 2017 at 4:38 PM, Stefan Beller <sbeller@google.com> wrote:
>>> + if (is_active_submodule_with_strategy(ce, SM_UPDATE_UNSPECIFIED))
>>
>> Here, and in other cases where we use
>> is_active_submodule_with_strategy(), why do we only ever check
>> SM_UPDATE_UNSPECIFIED? It seems really weird that we're only going to
>> check submodules who's strategy is unspecified, when that defaults to
>> checkout if I recall correctly? Shouldn't we check both? This applies
>> to pretty much everywhere that you call this function that I noticed,
>> which is why I removed the context.
>
> I am torn between this.
>
> submodule.<name>.update = {rebase, merge, checkout, none !command}
> is currently documented in GIT-CONFIG(1) as
>
> submodule.<name>.update
> The default update procedure for a submodule. This variable is
> populated by git submodule init from the gitmodules(5) file. See
> description of update command in git-submodule(1).
>
> and in GIT-SUBMODULE(1) as
>
> update
> [...] can be done in several ways
> depending on command line options and the value of
> submodule.<name>.update configuration variable. Supported update
> procedures are:
>
> checkout
> [...] or no option is given, and
> submodule.<name>.update is unset, or if it is set to checkout.
>
> So the "update" config clearly only applies to the "submodule update"
> command, right?
>
> Well no, "checkout --recurse-submodules" is very similar
> to running "submodule update", except with a bit more checks, so you could
> think that such an option applies to checkout as well. (and eventually
> rebase/merge etc. are supported as well.)
>
> So initially I assumed both "unspecified" as well as "checkout"
> are good matches to support in the first round.
>
> Then I flip flopped to think that we should not interfere with these
> settings at all (The checkout command does checkout and checkout only;
> no implicit rebase/merge ever in the future, because that would be
> confusing). So ignoring that option seemed like the way to go.
Hmm. So it's a bit complicated.
>
> But ignoring that option is also not the right approach.
> What if you have set it to "none" and really *expect* Git to not touch
> that submodule?
Or set it to "rebase" and suddenly git-checkout is ignoring you and
just checking things out anyways.
>
> So I dunno. Maybe it is a documentation issue, we need to spell out
> in the man page for checkout that --recurse-submodules is
> following one of these models. Now which is the best default model here?
Personally, I would go with that the config option sets the general
strategy used by the submodule whenever its updated, regardless of
how.
So, for example, setting it to none, means that recurse-submoduls will
ignore it when checking out. Setting it to rebase, or merge, and the
checkout will try to do those things?
Or, if that's not really feasible, have the checkout go "hey.. you
asked me to recurse, but uhhh these submodules don't allow me to do
checkout, so I'm gonna fail"? I think that's the best approach for
now.
>
> Thanks,
> Stefan
^ permalink raw reply
* [PATCH] submodule init: warn about falling back to a local path
From: Stefan Beller @ 2017-02-21 23:18 UTC (permalink / raw)
To: jrnieder; +Cc: git, gitster, sop, Stefan Beller
When a submodule is initialized, the config variable 'submodule.<name>.url'
is set depending on the value of the same variable in the .gitmodules
file. When the URL indicates to be relative, then the url is computed
relative to its default remote. The default remote cannot be determined
accurately in all cases, such that it falls back to 'origin'.
The 'origin' remote may not exist, though. In that case we give up looking
for a suitable remote and we'll just assume it to be a local relative path.
This can be confusing to users as there is a lot of guessing involved,
which is not obvious to the user.
So in the corner case of assuming a local autoritative truth, warn the
user to lessen the confusion.
This behavior was introduced in 4d6893200 (submodule add: allow relative
repository path even when no url is set, 2011-06-06), which shared the
code with submodule-init and then ported to C in 3604242f080a (submodule:
port init from shell to C, 2016-04-15).
In case of submodule-add, this behavior makes sense in some use cases[1],
however for submodule-init there does not seem to be an immediate obvious
use case to fall back to a local submodule. However there might be, so
warn instead of die here.
[1] e.g. http://stackoverflow.com/questions/8721984/git-ignore-files-for-public-repository-but-not-for-private
"store a secret locally in a submodule, with no intention to publish it"
Reported-by: Shawn Pearce <spearce@spearce.org>
Signed-off-by: Stefan Beller <sbeller@google.com>
---
builtin/submodule--helper.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 899dc334e3..44c11dd91e 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -356,12 +356,10 @@ static void init_submodule(const char *path, const char *prefix, int quiet)
strbuf_addf(&remotesb, "remote.%s.url", remote);
free(remote);
- if (git_config_get_string(remotesb.buf, &remoteurl))
- /*
- * The repository is its own
- * authoritative upstream
- */
+ if (git_config_get_string(remotesb.buf, &remoteurl)) {
remoteurl = xgetcwd();
+ warning(_("could not lookup configuration '%s'. Assuming this repository is its own authoritative upstream."), remotesb.buf);
+ }
relurl = relative_url(remoteurl, url, NULL);
strbuf_release(&remotesb);
free(remoteurl);
--
2.12.0.rc1.16.ge4278d41a0.dirty
^ permalink raw reply related
* Re: url.<base>.insteadOf vs. submodules
From: Stefan Beller @ 2017-02-21 23:16 UTC (permalink / raw)
To: Jeff King; +Cc: Toolforger, git@vger.kernel.org
In-Reply-To: <20170221230029.cs36tjwpsw2opuwp@sigill.intra.peff.net>
On Tue, Feb 21, 2017 at 3:00 PM, Jeff King <peff@peff.net> wrote:
> On Tue, Feb 21, 2017 at 10:19:38AM -0800, Stefan Beller wrote:
>
>> On Mon, Feb 20, 2017 at 11:06 PM, Jeff King <peff@peff.net> wrote:
>> >
>> > We'll see if the submodule folks have any ideas on how to implement
>> > that.
>> >
>>
>> So from reading your discussion, the user expectation is to have
>> `git submodule {init, update --init, sync}`
>> to pay attention to url.<base>.insteadOf when setting up the
>> submodule.<name>.URL, such that the modified URL is used for the
>> initial clone of the submodule (and hence any subsequent usage within
>> the submodule).
>
> Yeah, that was what I was envisioning.
>
>> Two caveates:
>>
>> * After running `git submodule init`, you change url.<base>.insteadOf
>> in the superproject. How do we need to word the documentation to
>> have users expecting this change doesn't affect submodules?
>> (See above Any vs. "Any except (initialized) submodules")
>
> Good question.
>
> I guess one answer is that this is the wrong approach entirely, and the
> right one is something like: submodules should understand that they are
> part of a superproject, and respect some whitelisted set of config from
> the superproject .git/config file.
This would break one of the core assumptions that submodules
are "independent" repos.
The way of action is a one way street:
* The superproject is aware of the submodule and when you invoke a
command on the superproject, you may mess around with the submodule,
e.g. update/remove it; absorb its git directory.
* The submodule is "just" a repository with weird .git link file and a
respective core.worktree setup. Currently it doesn't know if it is
guided by a superproject.
Though I do not know if this is actually a good assumption.
e.g. "[PATCH v2] git-prompt.sh: add submodule indicator"
https://public-inbox.org/git/1486075892-20676-2-git-send-email-email@benjaminfuchs.de/
really had trouble in the first version to nail down how to tell you are in
a submodule, but people want to know that.
>
> The second half is pretty easy to do (use git_config_from_file on the
> super-project's $GIT_DIR
There goes the "pretty easy"; currently there is no concept to find out
the existence of a super-project.
> /config, and pass a callback which filters the
> keys before passing them along to the real callback).
>
> I'm not sure about the first half (submodules know about their
> superproject), though.
Maybe we need to change that fundamental assumption.
So a more sophisticated way (thinking long term here) would be
to include the superprojects config file (with exceptions), and that
config file has more priority than e.g. the ~/.gitconfig file, but less
than the submodules own $GIT_DIR/config file.
Then a setting like the url rewriting would be "inherited" by the
submodule, with the option to overwrite the default as given by the
superproject.
>
>> * So with the point above the insteadOf config only applies to the
>> init/sync process, (i.e. once in time, ideally).
>> Is that confusing or actually simplifying the submodule workflow?
>
> Not sure. That's why I asked you. :)
I think that would be ok. With the idea of inheriting the superprojects
config, we allow for not storing the rewritten url, so the submodule
handling is less of a corner case here, and as another advantage the
rewriting rule is applied in real time, e.g. you can change the superprojects
rule after the fact and the submodule would automagically make use of it.
>
> One other caveat: I'm not sure if we do insteadOf recursively, but it
> may be surprising to the child "git clone" that we've already applied
> the insteadOf rewriting (especially if the rules are coming from
> ~/.gitconfig and may be applied twice).
When a rule is having effect twice the rule sounds broken. (the outcome
ought to be sufficiently different from the original?)
>
> -Peff
Thanks,
Stefan
^ permalink raw reply
* Re: url.<base>.insteadOf vs. submodules
From: Jeff King @ 2017-02-21 23:00 UTC (permalink / raw)
To: Stefan Beller; +Cc: Toolforger, git@vger.kernel.org
In-Reply-To: <CAGZ79kZgMbEZy7hoA+VxsKdKBavt59SmC1c6FpDdgrW2GKMHvQ@mail.gmail.com>
On Tue, Feb 21, 2017 at 10:19:38AM -0800, Stefan Beller wrote:
> On Mon, Feb 20, 2017 at 11:06 PM, Jeff King <peff@peff.net> wrote:
> >
> > We'll see if the submodule folks have any ideas on how to implement
> > that.
> >
>
> So from reading your discussion, the user expectation is to have
> `git submodule {init, update --init, sync}`
> to pay attention to url.<base>.insteadOf when setting up the
> submodule.<name>.URL, such that the modified URL is used for the
> initial clone of the submodule (and hence any subsequent usage within
> the submodule).
Yeah, that was what I was envisioning.
> Two caveates:
>
> * After running `git submodule init`, you change url.<base>.insteadOf
> in the superproject. How do we need to word the documentation to
> have users expecting this change doesn't affect submodules?
> (See above Any vs. "Any except (initialized) submodules")
Good question.
I guess one answer is that this is the wrong approach entirely, and the
right one is something like: submodules should understand that they are
part of a superproject, and respect some whitelisted set of config from
the superproject .git/config file.
The second half is pretty easy to do (use git_config_from_file on the
super-project's $GIT_DIR/config, and pass a callback which filters the
keys before passing them along to the real callback).
I'm not sure about the first half (submodules know about their
superproject), though.
> * So with the point above the insteadOf config only applies to the
> init/sync process, (i.e. once in time, ideally).
> Is that confusing or actually simplifying the submodule workflow?
Not sure. That's why I asked you. :)
One other caveat: I'm not sure if we do insteadOf recursively, but it
may be surprising to the child "git clone" that we've already applied
the insteadOf rewriting (especially if the rules are coming from
~/.gitconfig and may be applied twice).
-Peff
^ permalink raw reply
* Re: Git bisect does not find commit introducing the bug
From: Philip Oakley @ 2017-02-21 22:39 UTC (permalink / raw)
To: Alex Hoffman
Cc: Oleg Taranenko, Jakub Narębski, Jacob Keller, Johannes Sixt,
Christian Couder, Stephan Beyer, git
In-Reply-To: <CAMX8fZUR4h0T3hBmh6Z_0=f5LHFJetOi-vO_JJgnYPB7KjU_bg@mail.gmail.com>
From: "Alex Hoffman" <spec@gal.ro>
>> isn't that spelt `--ancestry-path` ?
>> (--ancestry-path has it's own issues such as needing
>> an --first-parent-show
>> option, but that's possibly a by the by)
>
> Indeed it is spelled `--ancestry-path`. And interestingly enough you
> may use it multiple times with the wanted effect in our case (e.g when
> the user has multiple good commit and a single bad commit before
> running the bisect itself).
> Also it is `--first-parent` (not `--first-parent-show`),
My spelling was deliberate ;-)
If you use the currently coded --first-parent with a properly relevant
commit for --ancestry-path then you get nothing. The purpose of
ancestry-path is to find the ancestry chain that deviates from being a
first-parent traversal [1], but the first-parent want to hold the walk to
just the first parent chain - a contradiction.
Adding the -show at the end is my attempt to indicate that it is for the
second aspect, that of selecting which commits to show/use.
I had an initial discussion back at [2], but failed then too.
> but I do not understand why do we need this
> option? What kind of issues does `--ancestry-path` have?
>
> Best regards,
> VG
>
--
Philip
[1] \git\Documentation\technical\api-revision-walking.txt ["two
diff_options, one for path limiting, another for output"]
[2]
https://public-inbox.org/git/2FA1998250474E76A386B82AD635E56A@PhilipOakley/
^ permalink raw reply
* Re: Git trademark status and policy
From: Jeff King @ 2017-02-21 22:31 UTC (permalink / raw)
To: G. Sylvie Davies; +Cc: Git Users
In-Reply-To: <CAAj3zPzrD+R6kDdqR3C7aYTDjaE+Y5zN+MfoXe5EuH4ZPxroHA@mail.gmail.com>
On Tue, Feb 21, 2017 at 07:55:15AM -0800, G. Sylvie Davies wrote:
> Is "Gitter" allowed? (https://gitter.im/).
>
> More info here:
>
> https://en.wikipedia.org/wiki/Gitter
>
> Also, their twitter handle is @gitchat.
>
> Not sure I'd even classify "gitter" as a portmanteau.
I don't think the Git committee has discussed that one. I'll mention it
there.
I wouldn't get hung up on the "is it a strict portmanteau" question. I
think the more important question is whether it creates confusion about
endorsement or interoperability. The portmanteau thing is more of a rule
of thumb there. (That's all IMHO, of course, and not an official
statement of the committee).
-Peff
^ permalink raw reply
* Re: [RFC PATCH] show decorations at the end of the line
From: Jeff King @ 2017-02-21 22:24 UTC (permalink / raw)
To: Junio C Hamano; +Cc: Linus Torvalds, Jacob Keller, Git Mailing List
In-Reply-To: <xmqq8tozfc7a.fsf@gitster.mtv.corp.google.com>
On Tue, Feb 21, 2017 at 01:47:37PM -0800, Junio C Hamano wrote:
> > So the best I could come up with is:
> >
> > git config pretty.twoline '%C(auto)%h %s%C(auto)%+d'
> > git log --format=twoline
> > [...]
>
> Yeah, I had a similar thought to use something around "%n%-d", but
>
> $ git log --format='%h%n%-d%C(auto) %s %C(auto)'
>
> is not it.
>
> I guess we could pile on another hack to make the sign between % and
> the format specifier cumulative and then "%n%-+d" may do what we
> want, but we need a true %(if)...%(then)...%(else)...%(end) support
> if we really want to do this thing properly.
Yeah, I'd rather not pile up more hacks. The for-each-ref placeholders
are more verbose, but I think the end result is a lot easier to read and
maintain, and the terseness doesn't matter if you're sticking it behind
an alias or config option.
(Don't get me wrong; I think the %(if) ones are pretty ugly, too, but
the next step beyond that is embedding some kind of templating or
scripting language, and that just seems like overkill).
-Peff
^ permalink raw reply
* What's cooking in git.git (Feb 2017, #06; Tue, 21)
From: Junio C Hamano @ 2017-02-21 22:23 UTC (permalink / raw)
To: git
Here are the topics that have been cooking. Commits prefixed with
'-' are only in 'pu' (proposed updates) while commits prefixed with
'+' are in 'next'. The ones marked with '.' do not appear in any of
the integration branches, but I am still holding onto them.
The tip of the 'master' is still at -rc2 but with a git-svn update
from Eric. I'd like to get pull requests for gitk and git-gui
updates soonish, if we are to have one during this cycle.
You can find the changes described here in the integration branches
of the repositories listed at
http://git-blame.blogspot.com/p/git-public-repositories.html
--------------------------------------------------
[New Topics]
* jk/tempfile-ferror-fclose-confusion (2017-02-17) 1 commit
(merged to 'next' on 2017-02-21 at 479ba0131f)
+ tempfile: set errno to a known value before calling ferror()
A caller of tempfile API that uses stdio interface to write to
files may ignore errors while writing, which is detected when
tempfile is closed (with a call to ferror()). By that time, the
original errno that may have told us what went wrong is likely to
be long gone and was overwritten by an irrelevant value.
close_tempfile() now resets errno to EIO to make errno at least
predictable.
Will cook in 'next'.
* ah/doc-ls-files-quotepath (2017-02-20) 1 commit
- Documentation: link git-ls-files to core.quotePath variable
Documentation for "git ls-files" did not refer to core.quotePath
Needs review.
* dr/doc-check-ref-format-normalize (2017-02-21) 1 commit
(merged to 'next' on 2017-02-21 at 5e88b7a93d)
+ git-check-ref-format: clarify documentation for --normalize
Doc update.
Will cook in 'next'.
* gp/document-dotfiles-in-templates-are-not-copied (2017-02-17) 1 commit
(merged to 'next' on 2017-02-21 at bbfa2bb7d4)
+ init: document dotfiles exclusion on template copy
Doc update.
Will cook in 'next'.
* jc/config-case-cmdline (2017-02-21) 2 commits
(merged to 'next' on 2017-02-21 at 354f023a3a)
+ config: reject invalid VAR in 'git -c VAR=VAL command'
+ config: preserve <subsection> case for one-shot config on the command line
The code to parse "git -c VAR=VAL cmd" and set configuration
variable for the duration of cmd had two small bugs, which have
been fixed.
Will cook in 'next'.
* jh/memihash-opt (2017-02-17) 5 commits
- name-hash: remember previous dir_entry during lazy_init_name_hash
- name-hash: specify initial size for istate.dir_hash table
- name-hash: precompute hash values during preload-index
- hashmap: allow memihash computation to be continued
- name-hash: eliminate duplicate memihash call
Expecting an update for perf?
* km/delete-ref-reflog-message (2017-02-20) 4 commits
(merged to 'next' on 2017-02-21 at 4ee4ce3f64)
+ branch: record creation of renamed branch in HEAD's log
+ rename_ref: replace empty message in HEAD's log
+ update-ref: pass reflog message to delete_ref()
+ delete_ref: accept a reflog message argument
"git update-ref -d" and other operations to delete references did
not leave any entry in HEAD's reflog when the reference being
deleted was the current branch. This is not a problem in practice
because you do not want to delete the branch you are currently on,
but caused renaming of the current branch to something else not to
be logged in a useful way.
Will cook in 'next'.
* nd/prune-in-worktree (2017-02-19) 15 commits
- rev-list: expose and document --single-worktree
- revision.c: --reflog add HEAD reflog from all worktrees
- files-backend: make reflog iterator go through per-worktree reflog
- refs: add refs_for_each_reflog[_ent]()
- revision.c: --all adds HEAD from all worktrees
- refs: remove dead for_each_*_submodule()
- revision.c: use refs_for_each*() instead of for_each_*_submodule()
- refs: add a refs_for_each_in() and friends
- refs: add refs_for_each_ref()
- refs: add refs_head_ref()
- refs: add refs_read_ref[_full]()
- refs: move submodule slash stripping code to get_submodule_ref_store
- revision.c: --indexed-objects add objects from all worktrees
- revision.c: refactor add_index_objects_to_pending()
- revision.h: new flag in struct rev_info wrt. worktree-related refs
(this branch uses mh/ref-remove-empty-directory, mh/submodule-hash, nd/files-backend-git-dir and nd/worktree-kill-parse-ref.)
"git gc" and friends when multiple worktrees are used off of a
single repository did not consider the index and per-worktree refs
of other worktrees as the root for reachability traversal, making
objects that are in use only in other worktrees to be subject to
garbage collection.
* mm/fetch-show-error-message-on-unadvertised-object (2017-02-21) 1 commit
- fetch: print an error when declining to request an unadvertised object
"git fetch" that requests a commit by object name, when the other
side does not allow such an request, failed without much
explanation.
Expecting a split series?
* rl/remote-allow-missing-branch-name-merge (2017-02-21) 1 commit
- remote: ignore failure to remove missing branch.<name>.merge
"git remote rm X", when a branch has remote X configured as the
value of its branch.*.remote, tried to remove branch.*.remote and
branch.*.merge and failed if either is unset.
Will merge to and then cook in 'next'.
--------------------------------------------------
[Stalled]
* nd/worktree-move (2017-01-27) 7 commits
. fixup! worktree move: new command
. worktree remove: new command
. worktree move: refuse to move worktrees with submodules
. worktree move: accept destination as directory
. worktree move: new command
. worktree.c: add update_worktree_location()
. worktree.c: add validate_worktree()
"git worktree" learned move and remove subcommands.
Tentatively ejected as it seems to break 'pu' when merged.
* cc/split-index-config (2016-12-26) 21 commits
- Documentation/git-update-index: explain splitIndex.*
- Documentation/config: add splitIndex.sharedIndexExpire
- read-cache: use freshen_shared_index() in read_index_from()
- read-cache: refactor read_index_from()
- t1700: test shared index file expiration
- read-cache: unlink old sharedindex files
- config: add git_config_get_expiry() from gc.c
- read-cache: touch shared index files when used
- sha1_file: make check_and_freshen_file() non static
- Documentation/config: add splitIndex.maxPercentChange
- t1700: add tests for splitIndex.maxPercentChange
- read-cache: regenerate shared index if necessary
- config: add git_config_get_max_percent_split_change()
- Documentation/git-update-index: talk about core.splitIndex config var
- Documentation/config: add information for core.splitIndex
- t1700: add tests for core.splitIndex
- update-index: warn in case of split-index incoherency
- read-cache: add and then use tweak_split_index()
- split-index: add {add,remove}_split_index() functions
- config: add git_config_get_split_index()
- config: mark an error message up for translation
The experimental "split index" feature has gained a few
configuration variables to make it easier to use.
Expecting a reroll.
cf. <20161226102222.17150-1-chriscool@tuxfamily.org>
cf. <a1a44640-ff6c-2294-72ac-46322eff8505@ramsayjones.plus.com>
cf. <CAP8UFD3_1EN=0EsD12Cew1MuW8yhtPAZw0M_g3wmvKFk-uGXxw@mail.gmail.com>
cf. <CAP8UFD1wmbR_rHyqn0q=0hw6-hHYFTzr=3yxS2XS9qTdY1kWFA@mail.gmail.com>
cf. <xmqqbmunq6mg.fsf@gitster.mtv.corp.google.com>
cf. <CAP8UFD0bgxVrc=RGHs1GrZ_5PF4cdfhqXLMiCSJTNw9axrr=_w@mail.gmail.com>
* pb/bisect (2017-02-18) 28 commits
- fixup! bisect--helper: `bisect_next_check` & bisect_voc shell function in C
- bisect--helper: remove the dequote in bisect_start()
- bisect--helper: retire `--bisect-auto-next` subcommand
- bisect--helper: retire `--bisect-autostart` subcommand
- bisect--helper: retire `--bisect-write` subcommand
- bisect--helper: `bisect_replay` shell function in C
- bisect--helper: `bisect_log` shell function in C
- bisect--helper: retire `--write-terms` subcommand
- bisect--helper: retire `--check-expected-revs` subcommand
- bisect--helper: `bisect_state` & `bisect_head` shell function in C
- bisect--helper: `bisect_autostart` shell function in C
- bisect--helper: retire `--next-all` subcommand
- bisect--helper: retire `--bisect-clean-state` subcommand
- bisect--helper: `bisect_next` and `bisect_auto_next` shell function in C
- t6030: no cleanup with bad merge base
- bisect--helper: `bisect_start` shell function partially in C
- bisect--helper: `get_terms` & `bisect_terms` shell function in C
- bisect--helper: `bisect_next_check` & bisect_voc shell function in C
- bisect--helper: `check_and_set_terms` shell function in C
- bisect--helper: `bisect_write` shell function in C
- bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C
- bisect--helper: `bisect_reset` shell function in C
- wrapper: move is_empty_file() and rename it as is_empty_or_missing_file()
- t6030: explicitly test for bisection cleanup
- bisect--helper: `bisect_clean_state` shell function in C
- bisect--helper: `write_terms` shell function in C
- bisect: rewrite `check_term_format` shell function in C
- bisect--helper: use OPT_CMDMODE instead of OPT_BOOL
Move more parts of "git bisect" to C.
Expecting a reroll.
cf. <CAFZEwPPXPPHi8KiEGS9ggzNHDCGhuqMgH9Z8-Pf9GLshg8+LPA@mail.gmail.com>
cf. <CAFZEwPM9RSTGN54dzaw9gO9iZmsYjJ_d1SjUD4EzSDDbmh-XuA@mail.gmail.com>
cf. <CAFZEwPNUXcNY9Qdz=_B7q2kQuaecPzJtTMGdv8YMUPEz2vnp8A@mail.gmail.com>
* ls/filter-process-delayed (2017-01-08) 1 commit
. convert: add "status=delayed" to filter process protocol
Ejected, as does not build when merged to 'pu'.
* sh/grep-tree-obj-tweak-output (2017-01-20) 2 commits
- grep: use '/' delimiter for paths
- grep: only add delimiter if there isn't one already
"git grep", when fed a tree-ish as an input, shows each hit
prefixed with "<tree-ish>:<path>:<lineno>:". As <tree-ish> is
almost always either a commit or a tag that points at a commit, the
early part of the output "<tree-ish>:<path>" can be used as the
name of the blob and given to "git show". When <tree-ish> is a
tree given in the extended SHA-1 syntax (e.g. "<commit>:", or
"<commit>:<dir>"), however, this results in a string that does not
name a blob (e.g. "<commit>::<path>" or "<commit>:<dir>:<path>").
"git grep" has been taught to be a bit more intelligent about these
cases and omit a colon (in the former case) or use slash (in the
latter case) to produce "<commit>:<path>" and
"<commit>:<dir>/<path>" that can be used as the name of a blob.
Expecting a reroll? Is this good enough with known limitations?
* jc/diff-b-m (2015-02-23) 5 commits
. WIPWIP
. WIP: diff-b-m
- diffcore-rename: allow easier debugging
- diffcore-rename.c: add locate_rename_src()
- diffcore-break: allow debugging
"git diff -B -M" produced incorrect patch when the postimage of a
completely rewritten file is similar to the preimage of a removed
file; such a resulting file must not be expressed as a rename from
other place.
The fix in this patch is broken, unfortunately.
Will discard.
--------------------------------------------------
[Cooking]
* vn/xdiff-func-context (2017-01-15) 1 commit
(merged to 'next' on 2017-02-21 at 838eab8d93)
+ xdiff -W: relax end-of-file function detection
"git diff -W" has been taught to handle the case where a new
function is added at the end of the file better.
Will cook in 'next'.
* nd/worktree-kill-parse-ref (2017-02-19) 5 commits
- refs: kill set_worktree_head_symref()
- refs: add refs_create_symref()
- worktree.c: kill parse_ref() in favor of refs_resolve_ref_unsafe()
- refs.c: add refs_resolve_ref_unsafe()
- refs: introduce get_worktree_ref_store()
(this branch is used by nd/prune-in-worktree; uses mh/ref-remove-empty-directory, mh/submodule-hash and nd/files-backend-git-dir.)
(hopefully) a beginning of safer "git worktree" that is resistant
to "gc".
Needs review.
* nd/files-backend-git-dir (2017-02-19) 17 commits
- refs: rename get_ref_store() to get_submodule_ref_store() and make it public
- files-backend: remove submodule_allowed from files_downcast()
- refs: move submodule code out of files-backend.c
- path.c: move some code out of strbuf_git_path_submodule()
- refs.c: make get_main_ref_store() public and use it
- refs.c: kill register_ref_store(), add register_submodule_ref_store()
- refs.c: flatten get_ref_store() a bit
- refs: rename lookup_ref_store() to lookup_submodule_ref_store()
- refs.c: introduce get_main_ref_store()
- files-backend: remove the use of git_path()
- refs.c: share is_per_worktree_ref() to files-backend.c
- files-backend: replace *git_path*() with files_path()
- files-backend: add files_path()
- files-backend: convert git_path() to strbuf_git_path()
- refs-internal.c: make files_log_ref_write() static
- Merge branch 'mh/ref-remove-empty-directory' into nd/files-backend-git-dir
- Merge branch 'mh/submodule-hash' into nd/files-backend-git-dir
(this branch is used by nd/prune-in-worktree and nd/worktree-kill-parse-ref; uses mh/ref-remove-empty-directory and mh/submodule-hash.)
The "submodule" specific field in the ref_store structure is
replaced with a more generic "gitdir" that can later be used also
when dealing with ref_store that represents the set of refs visible
from the other worktrees.
Needs review.
* nd/clean-preserve-errno-in-warning (2017-02-16) 1 commit
(merged to 'next' on 2017-02-16 at c0802f7627)
+ clean: use warning_errno() when appropriate
Some warning() messages from "git clean" were updated to show the
errno from failed system calls.
Will cook in 'next'.
* mm/two-more-xstrfmt (2017-02-16) 2 commits
(merged to 'next' on 2017-02-17 at 2454ee9847)
+ bisect_next_all: convert xsnprintf to xstrfmt
+ stop_progress_msg: convert xsnprintf to xstrfmt
Code clean-up and a string truncation fix.
Will cook in 'next'.
* sb/checkout-recurse-submodules (2017-02-16) 15 commits
- builtin/checkout: add --recurse-submodules switch
- entry.c: update submodules when interesting
- read-cache: remove_marked_cache_entries to wipe selected submodules.
- unpack-trees: check if we can perform the operation for submodules
- unpack-trees: pass old oid to verify_clean_submodule
- update submodules: add submodule_go_from_to
- update submodules: move up prepare_submodule_repo_env
- submodules: introduce check to see whether to touch a submodule
- update submodules: add a config option to determine if submodules are updated
- update submodules: add submodule config parsing
- connect_work_tree_and_git_dir: safely create leading directories
- make is_submodule_populated gently
- lib-submodule-update.sh: define tests for recursing into submodules
- lib-submodule-update.sh: do not use ./. as submodule remote
- lib-submodule-update.sh: reorder create_lib_submodule_repo
"git checkout" is taught --recurse-submodules option.
Needs review.
* jh/preload-index-skip-skip (2017-02-10) 1 commit
(merged to 'next' on 2017-02-16 at 39077062f9)
+ preload-index: avoid lstat for skip-worktree items
The preload-index code has been taught not to bother with the index
entries that are paths that are not checked out by "sparse checkout".
Will cook in 'next'.
* tg/stash-push (2017-02-19) 6 commits
- stash: allow pathspecs in the no verb form
- stash: use stash_push for no verb form
- stash: teach 'push' (and 'create_stash') to honor pathspec
- stash: refactor stash_create
- stash: add test for the create command line arguments
- stash: introduce push verb
Allow "git stash" to take pathspec so that the local changes can be
stashed away only partially.
Needs review.
cf. <xmqqmvdfh4az.fsf@gitster.mtv.corp.google.com>
* bc/object-id (2017-02-20) 19 commits
- wt-status: convert to struct object_id
- builtin/merge-base: convert to struct object_id
- Convert object iteration callbacks to struct object_id
- sha1_file: introduce an nth_packed_object_oid function
- refs: simplify parsing of reflog entries
- refs: convert each_reflog_ent_fn to struct object_id
- reflog-walk: convert struct reflog_info to struct object_id
- builtin/replace: convert to struct object_id
- Convert remaining callers of resolve_refdup to object_id
- builtin/merge: convert to struct object_id
- builtin/clone: convert to struct object_id
- builtin/branch: convert to struct object_id
- builtin/grep: convert to struct object_id
- builtin/fmt-merge-message: convert to struct object_id
- builtin/fast-export: convert to struct object_id
- builtin/describe: convert to struct object_id
- builtin/diff-tree: convert to struct object_id
- builtin/commit: convert to struct object_id
- hex: introduce parse_oid_hex
"uchar [40]" to "struct object_id" conversion continues.
Expecting a reroll.
cf. <20170221011058.cpx7uio6ibkvrtbv@genre.crustytoothpaste.net>
* jk/grep-no-index-fix (2017-02-14) 7 commits
(merged to 'next' on 2017-02-16 at c84c927fa8)
+ grep: treat revs the same for --untracked as for --no-index
+ grep: do not diagnose misspelt revs with --no-index
+ grep: avoid resolving revision names in --no-index case
+ grep: fix "--" rev/pathspec disambiguation
+ grep: re-order rev-parsing loop
+ grep: do not unnecessarily query repo for "--"
+ grep: move thread initialization a little lower
The code to parse the command line "git grep <patterns>... <rev>
[[--] <pathspec>...]" has been cleaned up, and a handful of bugs
have been fixed (e.g. we used to check "--" if it is a rev).
Will cook in 'next'.
* jk/show-branch-lift-name-len-limit (2017-02-15) 3 commits
(merged to 'next' on 2017-02-16 at 40d22f5f34)
+ show-branch: use skip_prefix to drop magic numbers
+ show-branch: store resolved head in heap buffer
+ show-branch: drop head_len variable
"git show-branch" expected there were only very short branch names
in the repository and used a fixed-length buffer to hold them
without checking for overflow.
Will cook in 'next'.
* lt/oneline-decoration-at-end (2017-02-21) 2 commits
(merged to 'next' on 2017-02-21 at 35c7731ee3)
+ log: fix regression to "--source" when "--decorate" was updated
(merged to 'next' on 2017-02-16 at 5854e58811)
+ show decorations at the end of the line
The output from "git log --oneline --decorate" has been updated to
show the extra information at the end of the line, not near the
front.
Will cook in 'next'.
Perhaps this is not such a cool idea?
cf. <CA+55aFwT2HUBzZO8Gpt9tHoJtdRxv9oe3TDoSH5jcEOixRNBXg@mail.gmail.com>
* jn/remote-helpers-with-git-dir (2017-02-14) 2 commits
(merged to 'next' on 2017-02-16 at c093c543c4)
+ remote helpers: avoid blind fall-back to ".git" when setting GIT_DIR
+ remote: avoid reading $GIT_DIR config in non-repo
"git ls-remote" and "git archive --remote" are designed to work
without being in a directory under Git's control. However, recent
updates revealed that we randomly look into a directory called
.git/ without actually doing necessary set-up when working in a
repository. Stop doing so.
Will cook in 'next'.
* jk/alternate-ref-optim (2017-02-08) 11 commits
(merged to 'next' on 2017-02-10 at f26f32cff6)
+ receive-pack: avoid duplicates between our refs and alternates
+ receive-pack: treat namespace .have lines like alternates
+ receive-pack: fix misleading namespace/.have comment
+ receive-pack: use oidset to de-duplicate .have lines
+ add oidset API
+ fetch-pack: cache results of for_each_alternate_ref
+ for_each_alternate_ref: replace transport code with for-each-ref
+ for_each_alternate_ref: pass name/oid instead of ref struct
+ for_each_alternate_ref: use strbuf for path allocation
+ for_each_alternate_ref: stop trimming trailing slashes
+ for_each_alternate_ref: handle failure from real_pathdup()
Optimizes resource usage while enumerating refs from alternate
object store, to help receiving end of "push" that hosts a
repository with many "forks".
Will cook in 'next'.
* lt/pathspec-negative (2017-02-10) 2 commits
(merged to 'next' on 2017-02-10 at 8ea7874076)
+ pathspec: don't error out on all-exclusionary pathspec patterns
+ pathspec magic: add '^' as alias for '!'
The "negative" pathspec feature was somewhat more cumbersome to use
than necessary in that its short-hand used "!" which needed to be
escaped from shells, and it required "exclude from what?" specified.
Will cook in 'next'.
* js/rebase-helper (2017-02-09) 2 commits
(merged to 'next' on 2017-02-14 at ae2474048e)
+ rebase -i: use the rebase--helper builtin
+ rebase--helper: add a builtin helper for interactive rebases
"git rebase -i" starts using the recently updated "sequencer" code.
Will cook in 'next'.
The change itself is small, but what it enables is rather a large
body of new code. We are getting there ;-)
* mh/submodule-hash (2017-02-13) 9 commits
(merged to 'next' on 2017-02-14 at 43f2dcbe29)
+ read_loose_refs(): read refs using resolve_ref_recursively()
+ files_ref_store::submodule: use NULL for the main repository
+ base_ref_store_init(): remove submodule argument
+ refs: push the submodule attribute down
+ refs: store submodule ref stores in a hashmap
+ register_ref_store(): new function
+ refs: remove some unnecessary handling of submodule == ""
+ refs: make some ref_store lookup functions private
+ refs: reorder some function definitions
(this branch is used by nd/files-backend-git-dir, nd/prune-in-worktree and nd/worktree-kill-parse-ref.)
Code and design clean-up for the refs API.
Will cook in 'next'.
The tip one is newer than the one posted to the list but was sent
privately by the author via his GitHub repository.
* jh/mingw-openssl-sha1 (2017-02-09) 1 commit
(merged to 'next' on 2017-02-10 at 084b3d8503)
+ mingw: use OpenSSL's SHA-1 routines
Windows port wants to use OpenSSL's implementation of SHA-1
routines, so let them.
Will cook in 'next'.
cf. <31bb0b9f-d498-24b3-57d5-9f34cb8e3914@kdbg.org>
* dt/gc-ignore-old-gc-logs (2017-02-13) 1 commit
(merged to 'next' on 2017-02-16 at 8f48e1b405)
+ gc: ignore old gc.log files
A "gc.log" file left by a backgrounded "gc --auto" disables further
automatic gc; it has been taught to run at least once a day (by
default) by ignoring a stale "gc.log" file that is too old.
Will cook in 'next'.
This is v6 posted on Feb 10th.
* js/git-path-in-subdir (2017-02-17) 2 commits
(merged to 'next' on 2017-02-17 at b3c3b2dce6)
+ rev-parse: fix several options when running in a subdirectory
+ rev-parse tests: add tests executed from a subdirectory
The "--git-path", "--git-common-dir", and "--shared-index-path"
options of "git rev-parse" did not produce usable output. They are
now updated to show the path to the correct file, relative to where
the caller is.
Will cook in 'next'.
* mh/ref-remove-empty-directory (2017-01-07) 23 commits
(merged to 'next' on 2017-02-10 at bcfd359e95)
+ files_transaction_commit(): clean up empty directories
+ try_remove_empty_parents(): teach to remove parents of reflogs, too
+ try_remove_empty_parents(): don't trash argument contents
+ try_remove_empty_parents(): rename parameter "name" -> "refname"
+ delete_ref_loose(): inline function
+ delete_ref_loose(): derive loose reference path from lock
+ log_ref_write_1(): inline function
+ log_ref_setup(): manage the name of the reflog file internally
+ log_ref_write_1(): don't depend on logfile argument
+ log_ref_setup(): pass the open file descriptor back to the caller
+ log_ref_setup(): improve robustness against races
+ log_ref_setup(): separate code for create vs non-create
+ log_ref_write(): inline function
+ rename_tmp_log(): improve error reporting
+ rename_tmp_log(): use raceproof_create_file()
+ lock_ref_sha1_basic(): use raceproof_create_file()
+ lock_ref_sha1_basic(): inline constant
+ raceproof_create_file(): new function
+ safe_create_leading_directories(): set errno on SCLD_EXISTS
+ safe_create_leading_directories_const(): preserve errno
+ t5505: use "for-each-ref" to test for the non-existence of references
+ refname_is_safe(): correct docstring
+ files_rename_ref(): tidy up whitespace
(this branch is used by nd/files-backend-git-dir, nd/prune-in-worktree and nd/worktree-kill-parse-ref.)
Deletion of a branch "foo/bar" could remove .git/refs/heads/foo
once there no longer is any other branch whose name begins with
"foo/", but we didn't do so so far. Now we do.
Will cook in 'next'.
* cw/tag-reflog-message (2017-02-08) 1 commit
(merged to 'next' on 2017-02-10 at 3968b3a58b)
+ tag: generate useful reflog message
"git tag", because refs/tags/* doesn't keep reflog by default, did
not leave useful message when adding a new entry to reflog.
Will cook in 'next'.
* sg/completion (2017-02-13) 22 commits
(merged to 'next' on 2017-02-13 at 118c192874)
+ completion: restore removed line continuating backslash
(merged to 'next' on 2017-02-10 at 55b2785d89)
+ completion: cache the path to the repository
+ completion: extract repository discovery from __gitdir()
+ completion: don't guard git executions with __gitdir()
+ completion: consolidate silencing errors from git commands
+ completion: don't use __gitdir() for git commands
+ completion: respect 'git -C <path>'
+ rev-parse: add '--absolute-git-dir' option
+ completion: fix completion after 'git -C <path>'
+ completion: don't offer commands when 'git --opt' needs an argument
+ completion: list short refs from a remote given as a URL
+ completion: don't list 'HEAD' when trying refs completion outside of a repo
+ completion: list refs from remote when remote's name matches a directory
+ completion: respect 'git --git-dir=<path>' when listing remote refs
+ completion: fix most spots not respecting 'git --git-dir=<path>'
+ completion: ensure that the repository path given on the command line exists
+ completion tests: add tests for the __git_refs() helper function
+ completion tests: check __gitdir()'s output in the error cases
+ completion tests: consolidate getting path of current working directory
+ completion tests: make the $cur variable local to the test helper functions
+ completion tests: don't add test cruft to the test repository
+ completion: improve __git_refs()'s in-code documentation
(this branch is used by sg/completion-refs-speedup.)
Clean-up and updates to command line completion (in contrib/).
Will cook in 'next'.
* sg/completion-refs-speedup (2017-02-13) 13 commits
- squash! completion: fill COMPREPLY directly when completing refs
- completion: fill COMPREPLY directly when completing refs
- completion: list only matching symbolic and pseudorefs when completing refs
- completion: let 'for-each-ref' sort remote branches for 'checkout' DWIMery
- completion: let 'for-each-ref' filter remote branches for 'checkout' DWIMery
- completion: let 'for-each-ref' strip the remote name from remote branches
- completion: let 'for-each-ref' and 'ls-remote' filter matching refs
- completion: don't disambiguate short refs
- completion: don't disambiguate tags and branches
- completion: support excluding full refs
- completion: support completing full refs after '--option=refs/<TAB>'
- completion: wrap __git_refs() for better option parsing
- completion: remove redundant __gitcomp_nl() options from _git_commit()
(this branch uses sg/completion.)
The refs completion for large number of refs has been sped up,
partly by giving up disambiguating ambiguous refs and partly by
eliminating most of the shell processing between 'git for-each-ref'
and 'ls-remote' and Bash's completion facility.
Will hold.
* sk/parse-remote-cleanup (2017-02-21) 2 commits
(merged to 'next' on 2017-02-21 at 302250072e)
+ Revert "parse-remote: remove reference to unused op_prep"
(merged to 'next' on 2017-02-06 at 6ec89f72d5)
+ parse-remote: remove reference to unused op_prep
Code clean-up.
Will discard.
There may be third-party scripts that are dot-sourcing this one.
* jk/delta-chain-limit (2017-01-27) 2 commits
(merged to 'next' on 2017-02-06 at 9ff36ae9b2)
+ pack-objects: convert recursion to iteration in break_delta_chain()
+ pack-objects: enforce --depth limit in reused deltas
"git repack --depth=<n>" for a long time busted the specified depth
when reusing delta from existing packs. This has been corrected.
Will cook in 'next'.
* mm/merge-rename-delete-message (2017-01-30) 1 commit
(merged to 'next' on 2017-02-10 at 8bf8146029)
+ merge-recursive: make "CONFLICT (rename/delete)" message show both paths
When "git merge" detects a path that is renamed in one history
while the other history deleted (or modified) it, it now reports
both paths to help the user understand what is going on in the two
histories being merged.
Will cook in 'next'.
* ps/urlmatch-wildcard (2017-02-01) 5 commits
(merged to 'next' on 2017-02-10 at 2ed9ea48ee)
+ urlmatch: allow globbing for the URL host part
+ urlmatch: include host in urlmatch ranking
+ urlmatch: split host and port fields in `struct url_info`
+ urlmatch: enable normalization of URLs with globs
+ mailmap: add Patrick Steinhardt's work address
The <url> part in "http.<url>.<variable>" configuration variable
can now be spelled with '*' that serves as wildcard.
E.g. "http.https://*.example.com.proxy" can be used to specify the
proxy used for https://a.example.com, https://b.example.com, etc.,
i.e. any host in the example.com domain.
Will cook in 'next'.
* sf/putty-w-args (2017-02-10) 5 commits
(merged to 'next' on 2017-02-14 at 7f157e7020)
+ connect.c: stop conflating ssh command names and overrides
+ connect: Add the envvar GIT_SSH_VARIANT and ssh.variant config
+ git_connect(): factor out SSH variant handling
+ connect: rename tortoiseplink and putty variables
+ connect: handle putty/plink also in GIT_SSH_COMMAND
The command line options for ssh invocation needs to be tweaked for
some implementations of SSH (e.g. PuTTY plink wants "-P <port>"
while OpenSSH wants "-p <port>" to specify port to connect to), and
the variant was guessed when GIT_SSH environment variable is used
to specify it. The logic to guess now applies to the command
specified by the newer GIT_SSH_COMMAND and also core.sshcommand
configuration variable, and comes with an escape hatch for users to
deal with misdetected cases.
Will cook in 'next'.
* jk/describe-omit-some-refs (2017-01-23) 5 commits
(merged to 'next' on 2017-01-23 at f8a14b4996)
+ describe: teach describe negative pattern matches
+ describe: teach --match to accept multiple patterns
+ name-rev: add support to exclude refs by pattern match
+ name-rev: extend --refs to accept multiple patterns
+ doc: add documentation for OPT_STRING_LIST
"git describe" and "git name-rev" have been taught to take more
than one refname patterns to restrict the set of refs to base their
naming output on, and also learned to take negative patterns to
name refs not to be used for naming via their "--exclude" option.
Will cook in 'next'.
* bw/attr (2017-02-01) 27 commits
(merged to 'next' on 2017-02-14 at d35c1d7e4a)
+ attr: reformat git_attr_set_direction() function
+ attr: push the bare repo check into read_attr()
+ attr: store attribute stack in attr_check structure
+ attr: tighten const correctness with git_attr and match_attr
+ attr: remove maybe-real, maybe-macro from git_attr
+ attr: eliminate global check_all_attr array
+ attr: use hashmap for attribute dictionary
+ attr: change validity check for attribute names to use positive logic
+ attr: pass struct attr_check to collect_some_attrs
+ attr: retire git_check_attrs() API
+ attr: convert git_check_attrs() callers to use the new API
+ attr: convert git_all_attrs() to use "struct attr_check"
+ attr: (re)introduce git_check_attr() and struct attr_check
+ attr: rename function and struct related to checking attributes
+ attr.c: outline the future plans by heavily commenting
+ Documentation: fix a typo
+ attr.c: add push_stack() helper
+ attr: support quoting pathname patterns in C style
+ attr.c: plug small leak in parse_attr_line()
+ attr.c: tighten constness around "git_attr" structure
+ attr.c: simplify macroexpand_one()
+ attr.c: mark where #if DEBUG ends more clearly
+ attr.c: complete a sentence in a comment
+ attr.c: explain the lack of attr-name syntax check in parse_attr()
+ attr.c: update a stale comment on "struct match_attr"
+ attr.c: use strchrnul() to scan for one line
+ commit.c: use strchrnul() to scan for one line
The gitattributes machinery is being taught to work better in a
multi-threaded environment.
Will cook in 'next'.
* kn/ref-filter-branch-list (2017-02-07) 21 commits
(merged to 'next' on 2017-02-10 at 794bb8284d)
+ ref-filter: resurrect "strip" as a synonym to "lstrip"
(merged to 'next' on 2017-01-31 at e7592a5461)
+ branch: implement '--format' option
+ branch: use ref-filter printing APIs
+ branch, tag: use porcelain output
+ ref-filter: allow porcelain to translate messages in the output
+ ref-filter: add an 'rstrip=<N>' option to atoms which deal with refnames
+ ref-filter: modify the 'lstrip=<N>' option to work with negative '<N>'
+ ref-filter: Do not abruptly die when using the 'lstrip=<N>' option
+ ref-filter: rename the 'strip' option to 'lstrip'
+ ref-filter: make remote_ref_atom_parser() use refname_atom_parser_internal()
+ ref-filter: introduce refname_atom_parser()
+ ref-filter: introduce refname_atom_parser_internal()
+ ref-filter: make "%(symref)" atom work with the ':short' modifier
+ ref-filter: add support for %(upstream:track,nobracket)
+ ref-filter: make %(upstream:track) prints "[gone]" for invalid upstreams
+ ref-filter: introduce format_ref_array_item()
+ ref-filter: move get_head_description() from branch.c
+ ref-filter: modify "%(objectname:short)" to take length
+ ref-filter: implement %(if:equals=<string>) and %(if:notequals=<string>)
+ ref-filter: include reference to 'used_atom' within 'atom_value'
+ ref-filter: implement %(if), %(then), and %(else) atoms
The code to list branches in "git branch" has been consolidated
with the more generic ref-filter API.
Will cook in 'next'.
* jk/no-looking-at-dotgit-outside-repo-final (2016-10-26) 1 commit
(merged to 'next' on 2016-12-05 at 0c77e39cd5)
+ setup_git_env: avoid blind fall-back to ".git"
Originally merged to 'next' on 2016-10-26
This is the endgame of the topic to avoid blindly falling back to
".git" when the setup sequence said we are _not_ in Git repository.
A corner case that happens to work right now may be broken by a
call to die("BUG").
Will cook in 'next'.
* jc/merge-drop-old-syntax (2015-04-29) 1 commit
(merged to 'next' on 2016-12-05 at 041946dae0)
+ merge: drop 'git merge <message> HEAD <commit>' syntax
Originally merged to 'next' on 2016-10-11
Stop supporting "git merge <message> HEAD <commit>" syntax that has
been deprecated since October 2007, and issues a deprecation
warning message since v2.5.0.
Will cook in 'next'.
* jc/bundle (2016-03-03) 6 commits
- index-pack: --clone-bundle option
- Merge branch 'jc/index-pack' into jc/bundle
- bundle v3: the beginning
- bundle: keep a copy of bundle file name in the in-core bundle header
- bundle: plug resource leak
- bundle doc: 'verify' is not about verifying the bundle
The beginning of "split bundle", which could be one of the
ingredients to allow "git clone" traffic off of the core server
network to CDN.
--------------------------------------------------
[Discarded]
* sb/push-make-submodule-check-the-default (2017-01-26) 2 commits
(merged to 'next' on 2017-01-26 at 5f4715cea6)
+ Revert "push: change submodule default to check when submodules exist"
(merged to 'next' on 2016-12-12 at 1863e05af5)
+ push: change submodule default to check when submodules exist
Turn the default of "push.recurseSubmodules" to "check" when
submodules seem to be in use.
Retracted.
* ls/submodule-config-ucase (2017-02-15) 2 commits
. submodule config does not apply to upper case submodules?
. t7400: cleanup "submodule add clone shallow submodule" test
Demonstrate a breakage in handling submodule.UPPERCASENAME.update
configuration variables.
Superseded by jc/config-case-cmdline.
^ permalink raw reply
* Re: [PATCH 12/15] unpack-trees: check if we can perform the operation for submodules
From: Stefan Beller @ 2017-02-21 22:16 UTC (permalink / raw)
To: Jacob Keller
Cc: Git mailing list, brian m. carlson, Jonathan Nieder,
Brandon Williams, Junio C Hamano
In-Reply-To: <CA+P7+xrwxb9G3QVOascSZqnBa_vGEx95nv0jN9USmBqB5Q_Mzw@mail.gmail.com>
On Fri, Feb 17, 2017 at 10:42 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Wed, Feb 15, 2017 at 4:38 PM, Stefan Beller <sbeller@google.com> wrote:
>> + if (is_active_submodule_with_strategy(ce, SM_UPDATE_UNSPECIFIED))
>
> Here, and in other cases where we use
> is_active_submodule_with_strategy(), why do we only ever check
> SM_UPDATE_UNSPECIFIED? It seems really weird that we're only going to
> check submodules who's strategy is unspecified, when that defaults to
> checkout if I recall correctly? Shouldn't we check both? This applies
> to pretty much everywhere that you call this function that I noticed,
> which is why I removed the context.
I am torn between this.
submodule.<name>.update = {rebase, merge, checkout, none !command}
is currently documented in GIT-CONFIG(1) as
submodule.<name>.update
The default update procedure for a submodule. This variable is
populated by git submodule init from the gitmodules(5) file. See
description of update command in git-submodule(1).
and in GIT-SUBMODULE(1) as
update
[...] can be done in several ways
depending on command line options and the value of
submodule.<name>.update configuration variable. Supported update
procedures are:
checkout
[...] or no option is given, and
submodule.<name>.update is unset, or if it is set to checkout.
So the "update" config clearly only applies to the "submodule update"
command, right?
Well no, "checkout --recurse-submodules" is very similar
to running "submodule update", except with a bit more checks, so you could
think that such an option applies to checkout as well. (and eventually
rebase/merge etc. are supported as well.)
So initially I assumed both "unspecified" as well as "checkout"
are good matches to support in the first round.
Then I flip flopped to think that we should not interfere with these
settings at all (The checkout command does checkout and checkout only;
no implicit rebase/merge ever in the future, because that would be
confusing). So ignoring that option seemed like the way to go.
But ignoring that option is also not the right approach.
What if you have set it to "none" and really *expect* Git to not touch
that submodule?
So I dunno. Maybe it is a documentation issue, we need to spell out
in the man page for checkout that --recurse-submodules is
following one of these models. Now which is the best default model here?
Thanks,
Stefan
^ permalink raw reply
* Re: [PATCH] remote: Ignore failure to remove missing branch.<name>.merge
From: Junio C Hamano @ 2017-02-21 22:03 UTC (permalink / raw)
To: Ross Lagerwall; +Cc: git
In-Reply-To: <20170221203836.GA11736@hobo.lan>
Ross Lagerwall <rosslagerwall@gmail.com> writes:
> On Tue, Feb 21, 2017 at 11:32:54AM -0800, Junio C Hamano wrote:
>
>> I was waiting for others to comment on this patch but nobody seems
>> to be interested. Which is a bit sad, because except for minor
>> nits, this patch is very well done.
>>
>> The explanation of the motivation and solution in the proposed log
>> message is excellent. It would have been perfect if you described
>> HOW you get into a state where branch.<name>.remote is pointing at
>> the remote being removed, without having branch.<name>.merge in the
>> first place, but even if such a state is invalid or unplausible,
>> removing the remote should be a usable way to recover from such a
>> situation.
>
> I got into this situation by setting branch.<name>.remote directly. I
> was using push.default=current, and wanted a bare "git push" on the
> branch to push to a different remote from origin (which it defaults to).
> Configuring branch.<name>.remote made git do the right thing.
Ah, OK. As you may have seen from the test I sent, I thought the
user started with
git checkout -b <new> -t <remote>/<branch>
in which case both are always set, and removed only one of them,
and that is what I called "deliberate sabotage".
What you did does sound like a very valid use case. Let's update
the test to use that pattern and document the intended use case to
help with this fix in the updated log message.
Here is what I tentatively queued.
Thanks.
-- >8 --
From: Ross Lagerwall <rosslagerwall@gmail.com>
Date: Sat, 18 Feb 2017 00:23:41 +0000
Subject: [PATCH] remote: ignore failure to remove missing branch.<name>.merge
It is not all too unusual for a branch to use "branch.<name>.remote"
without "branch.<name>.merge". You may be using the 'push.default'
configuration set to 'current', for example, and do
$ git checkout -b side colleague/side
$ git config branch.side.remote colleague
However, "git remote rm" to remove the remote used in such a manner
fails with
"fatal: could not unset 'branch.<name>.merge'"
because it assumes that a branch that has .remote defined must also
have .merge defined. Detect the "cannot unset because it is not set
to begin with" case and ignore it.
Signed-off-by: Ross Lagerwall <rosslagerwall@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
builtin/remote.c | 4 +++-
t/t5505-remote.sh | 19 +++++++++++++++++++
2 files changed, 22 insertions(+), 1 deletion(-)
diff --git a/builtin/remote.c b/builtin/remote.c
index e52cf3925b..01055b7272 100644
--- a/builtin/remote.c
+++ b/builtin/remote.c
@@ -769,7 +769,9 @@ static int rm(int argc, const char **argv)
strbuf_reset(&buf);
strbuf_addf(&buf, "branch.%s.%s",
item->string, *k);
- git_config_set(buf.buf, NULL);
+ result = git_config_set_gently(buf.buf, NULL);
+ if (result && result != CONFIG_NOTHING_SET)
+ die(_("could not unset '%s'"), buf.buf);
}
}
}
diff --git a/t/t5505-remote.sh b/t/t5505-remote.sh
index 8198d8eb05..f558ad0b39 100755
--- a/t/t5505-remote.sh
+++ b/t/t5505-remote.sh
@@ -153,6 +153,25 @@ test_expect_success 'remove errors out early when deleting non-existent branch'
)
'
+test_expect_success 'remove remote with a branch without configured merge' '
+ test_when_finished "(
+ git -C test checkout master;
+ git -C test branch -D two;
+ git -C test config --remove-section remote.two;
+ git -C test config --remove-section branch.second;
+ true
+ )" &&
+ (
+ cd test &&
+ git remote add two ../two &&
+ git fetch two &&
+ git checkout -b second two/master^0 &&
+ git config branch.second.remote two &&
+ git checkout master &&
+ git remote rm two
+ )
+'
+
test_expect_success 'rename errors out early when deleting non-existent branch' '
(
cd test &&
--
2.12.0-rc2-231-g83a1c8597c
^ permalink raw reply related
* Re: [RFC PATCH] show decorations at the end of the line
From: Junio C Hamano @ 2017-02-21 21:47 UTC (permalink / raw)
To: Jeff King; +Cc: Linus Torvalds, Jacob Keller, Git Mailing List
In-Reply-To: <20170221210808.3ryri33ve7w7csdp@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
> The for-each-ref formatting code has %(if), but it's not unified with
> the commit-format ones.
>
> So the best I could come up with is:
>
> git config pretty.twoline '%C(auto)%h %s%C(auto)%+d'
> git log --format=twoline
>
> which looks like:
>
> 80ba04ed9 Merge branch 'svn-escape-backslash' of git://bogomips.org/git-svn
> (origin/master, origin/HEAD)
> 20769079d Git 2.12-rc2
> (tag: v2.12.0-rc2)
> 076c05393 Hopefully the final batch of mini-topics before the final
> c5b22b819 Merge branch 'jk/tempfile-ferror-fclose-confusion'
> 62fef5c56 Merge branch 'dp/submodule-doc-markup-fix'
> 1f73ff080 Merge branch 'jk/reset-to-break-a-commit-doc-updated'
Yeah, I had a similar thought to use something around "%n%-d", but
$ git log --format='%h%n%-d%C(auto) %s %C(auto)'
is not it.
I guess we could pile on another hack to make the sign between % and
the format specifier cumulative and then "%n%-+d" may do what we
want, but we need a true %(if)...%(then)...%(else)...%(end) support
if we really want to do this thing properly.
^ permalink raw reply
* [PATCH v2] config: reject invalid VAR in 'git -c VAR=VAL command'
From: Junio C Hamano @ 2017-02-21 21:24 UTC (permalink / raw)
To: git@vger.kernel.org; +Cc: Stefan Beller
In-Reply-To: <xmqqlgszffm0.fsf@gitster.mtv.corp.google.com>
The parsing of one-shot assignments of configuration variables that
come from the command line historically was quite loose and allowed
anything to pass.
The configuration variable names that come from files are validated
in git_config_parse_source(), which uses get_base_var() that grabs
the <section> (and subsection) while making sure that <section>
consists of iskeychar() letters, the function itself that makes sure
that the first letter in <variable> is isalpha(), and get_value()
that grabs the remainder of the <variable> name while making sure
that it consists of iskeychar() letters.
Perform an equivalent check in canonicalize_config_variable_name()
to catch invalid configuration variable names that come from the
command line.
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
* Now with an updated test; while writing it it uncovered a bug in
the original test that expected to fail---they failed alright but
sometimes failed for a wrong reason.
config.c | 48 +++++++++++++++++++++++++++++++++++++-----------
t/t1300-repo-config.sh | 16 ++++++++++++++++
2 files changed, 53 insertions(+), 11 deletions(-)
diff --git a/config.c b/config.c
index 4128debc71..e7f7ff1938 100644
--- a/config.c
+++ b/config.c
@@ -199,32 +199,62 @@ void git_config_push_parameter(const char *text)
strbuf_release(&env);
}
+static inline int iskeychar(int c)
+{
+ return isalnum(c) || c == '-';
+}
+
/*
* downcase the <section> and <variable> in <section>.<variable> or
* <section>.<subsection>.<variable> and do so in place. <subsection>
* is left intact.
+ *
+ * The configuration variable names that come from files are validated
+ * in git_config_parse_source(), which uses get_base_var() that grabs
+ * the <section> (and subsection) while making sure that <section>
+ * consists of iskeychar() letters, the function itself that makes
+ * sure that the first letter in <variable> is isalpha(), and
+ * get_value() that grabs the remainder of the <variable> name while
+ * making sure that it consists of iskeychar() letters. Perform a
+ * matching validation for configuration variables that come from
+ * the command line.
*/
-static void canonicalize_config_variable_name(char *varname)
+static int canonicalize_config_variable_name(char *varname)
{
- char *cp, *last_dot;
+ char *cp, *first_dot, *last_dot;
/* downcase the first segment */
for (cp = varname; *cp; cp++) {
if (*cp == '.')
break;
+ if (!iskeychar(*cp))
+ return -1;
*cp = tolower(*cp);
}
if (!*cp)
- return;
+ return -1; /* no dot anywhere? */
+
+ first_dot = cp;
+ if (first_dot == varname)
+ return -1; /* no section? */
/* find the last dot (we start from the first dot we just found) */
- for (last_dot = cp; *cp; cp++)
+ for (; *cp; cp++)
if (*cp == '.')
last_dot = cp;
+ if (!last_dot[1])
+ return -1; /* no variable? */
+
/* downcase the last segment */
- for (cp = last_dot; *cp; cp++)
+ for (cp = last_dot + 1; *cp; cp++) {
+ if (cp == last_dot + 1 && !isalpha(*cp))
+ return -1;
+ else if (!iskeychar(*cp))
+ return -1;
*cp = tolower(*cp);
+ }
+ return 0;
}
int git_config_parse_parameter(const char *text,
@@ -249,7 +279,8 @@ int git_config_parse_parameter(const char *text,
strbuf_list_free(pair);
return error("bogus config parameter: %s", text);
}
- canonicalize_config_variable_name(pair[0]->buf);
+ if (canonicalize_config_variable_name(pair[0]->buf))
+ return error("bogus config parameter: %s", text);
if (fn(pair[0]->buf, value, data) < 0) {
strbuf_list_free(pair);
return -1;
@@ -382,11 +413,6 @@ static char *parse_value(void)
}
}
-static inline int iskeychar(int c)
-{
- return isalnum(c) || c == '-';
-}
-
static int get_value(config_fn_t fn, void *data, struct strbuf *name)
{
int c;
diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
index 7a16f66a9d..ea371020fa 100755
--- a/t/t1300-repo-config.sh
+++ b/t/t1300-repo-config.sh
@@ -1143,6 +1143,22 @@ test_expect_success 'last one wins: three level vars' '
test_cmp expect actual
'
+for VAR in a .a a. a.0b a."b c". a."b c".0d
+do
+ test_expect_success "git -c $VAR=VAL rejects invalid '$VAR'" '
+ test_must_fail git -c "$VAR=VAL" config -l
+ '
+done
+
+for VAR in a.b a."b c".d
+do
+ test_expect_success "git -c $VAR=VAL works with valid '$VAR'" '
+ echo VAL >expect &&
+ git -c "$VAR=VAL" config --get "$VAR" >actual &&
+ test_cmp expect actual
+ '
+done
+
test_expect_success 'git -c is not confused by empty environment' '
GIT_CONFIG_PARAMETERS="" git -c x.one=1 config --list
'
--
2.12.0-rc2-222-gff02733afe
^ permalink raw reply related
* Re: [RFC PATCH] show decorations at the end of the line
From: Jeff King @ 2017-02-21 21:08 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Junio C Hamano, Jacob Keller, Git Mailing List
In-Reply-To: <CA+55aFwT2HUBzZO8Gpt9tHoJtdRxv9oe3TDoSH5jcEOixRNBXg@mail.gmail.com>
On Tue, Feb 21, 2017 at 12:40:11PM -0800, Linus Torvalds wrote:
> In fact, I played around with some formats, and the one I lines the
> most was actually one that split the line for decorations, but that
> one was admittedly pretty funky. It gives output like
>
> b9df16a4c (HEAD -> master)
> pathspec: don't error out on all-exclusionary pathspec patterns
> 91a491f05 pathspec magic: add '^' as alias for '!'
> c8e05fd6d ls-remote: add "--diff" option to show only refs that differ
> 20769079d (tag: v2.12.0-rc2, origin/master, origin/HEAD)
> Git 2.12-rc2
> 076c05393 Hopefully the final batch of mini-topics before the final
> c5b22b819 Merge branch 'jk/tempfile-ferror-fclose-confusion'
> 62fef5c56 Merge branch 'dp/submodule-doc-markup-fix'
> 1f73ff080 Merge branch 'jk/reset-to-break-a-commit-doc-updated'
> bf5f11918 Merge branch 'jk/reset-to-break-a-commit-doc'
> e048a257b Merge branch 'js/mingw-isatty'
>
> (which looks better with colorization than it looks in the email).
>
> But I'm not even going to send out that patch, because it was such an
> atrocious hack to line things up.
I was going to suggest a custom format string that does the same, but
what we have is not _quite_ flexible enough.
You can use "%+d" to insert a newline only when "%d" is not empty. But
it always inserts _before_ the decoration, not after. Likewise, you
cannot say "if it's not empty, then insert %d and a leading tab".
The for-each-ref formatting code has %(if), but it's not unified with
the commit-format ones.
So the best I could come up with is:
git config pretty.twoline '%C(auto)%h %s%C(auto)%+d'
git log --format=twoline
which looks like:
80ba04ed9 Merge branch 'svn-escape-backslash' of git://bogomips.org/git-svn
(origin/master, origin/HEAD)
20769079d Git 2.12-rc2
(tag: v2.12.0-rc2)
076c05393 Hopefully the final batch of mini-topics before the final
c5b22b819 Merge branch 'jk/tempfile-ferror-fclose-confusion'
62fef5c56 Merge branch 'dp/submodule-doc-markup-fix'
1f73ff080 Merge branch 'jk/reset-to-break-a-commit-doc-updated'
-Peff
^ permalink raw reply
* Re: [PATCH 08/15] submodules: introduce check to see whether to touch a submodule
From: Stefan Beller @ 2017-02-21 20:56 UTC (permalink / raw)
To: Jacob Keller
Cc: Git mailing list, brian m. carlson, Jonathan Nieder,
Brandon Williams, Junio C Hamano
In-Reply-To: <CA+P7+xqpxTt7KibOrVr2ekjLy6Hva4KJ6nCaN22j-Qpspky3aQ@mail.gmail.com>
On Fri, Feb 17, 2017 at 10:36 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
> On Wed, Feb 15, 2017 at 4:38 PM, Stefan Beller <sbeller@google.com> wrote:
>> In later patches we introduce the --recurse-submodule flag for commands
>> that modify the working directory, e.g. git-checkout.
>>
>> It is potentially expensive to check if a submodule needs an update,
>> because a common theme to interact with submodules is to spawn a child
>> process for each interaction.
>>
>> So let's introduce a function that checks if a submodule needs
>> to be checked for an update before attempting the update.
>>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>> submodule.c | 27 +++++++++++++++++++++++++++
>> submodule.h | 13 +++++++++++++
>> 2 files changed, 40 insertions(+)
>>
>> diff --git a/submodule.c b/submodule.c
>> index 591f4a694e..2a37e03420 100644
>> --- a/submodule.c
>> +++ b/submodule.c
>> @@ -548,6 +548,33 @@ void set_config_update_recurse_submodules(int value)
>> config_update_recurse_submodules = value;
>> }
>>
>> +int touch_submodules_in_worktree(void)
>> +{
>> + /*
>> + * Update can't be "none", "merge" or "rebase",
>> + * treat any value as OFF, except an explicit ON.
>> + */
>> + return config_update_recurse_submodules == RECURSE_SUBMODULES_ON;
>> +}
>
> Ok, so here, we're just checking whether the value is
> RECURSE_SUBMODULES_ON. The comment doesn't make sense to me at all.
Yes the comment was not updated in the last round of patches and is
now out of context.
> What is "update" and why "can't" it be those values? How is this code
> treating thise values as OFF exept for an explicit ON?
Please disregard the comment; I'll remove it in the next reroll.
The submodule API is in such a way that
config_update_recurse_submodules
>
> At first I thought this comment was related to check in the previous
> patch. I think I see the patch is "recurse submodules = true" or
> "recurse submodules = checkout" as a specific strategy? Ie:
> recurse-submodules=checkout" means "recurse into submodules and update
> them using checkout strategy?
Yes that is what I had in mind. See previous comment, in a later series
we could extend that to other strategies such as "revert-in-submodules"
for git-revert or "rebase", "merge" as we curreently have for
"git submodule update".
> Maybe this should be called something like
> recurse_into_submodules_in_worktree() though that is pretty verbose.
I like that. (It's less than double the number of characters, so it's
fine, isn't it?)
Maybe we can abbreviate worktree by "wt" ans "submodules by subs:
/* recurse into submodules in the worktree? */
int rec_subs_wt;
That looks short enough to qualify as non-Java.
> I'm not sure I have a good name really. But I wonder why we need this
> in the first place. Basically, we set RECURSE_SUBMODULES_* value which
> could be OFF, ON, or even future extensions of CHECKOUT, REBASE,
> MERGE, etc?
>
> But shouldn't we just return the mode, and let the later code decide
> "oh. actually I don't support this mode". For now, obviously we
> wouldn't set any of the new modes yet.
Mh, makes sense. Maybe I tricked myself into premature optimization,
because I'd expect most of the users not caring about submodules, such
that we want to have a *really* cheap way of saying "no, not interesting in
submodules", which is what this method mainly offers.
Junio also remarked this and the following
"is_active_submodule_with_strategy" to be bad design.
I'll redo those, such that the caller decides what to do with each strategy.
>
>> +
>> +int is_active_submodule_with_strategy(const struct cache_entry *ce,
>> + enum submodule_update_type strategy)
>> +{
>> + const struct submodule *sub;
>> +
>> + if (!S_ISGITLINK(ce->ce_mode))
>> + return 0;
>> +
>> + if (!touch_submodules_in_worktree())
>> + return 0;
>> +
>> + sub = submodule_from_path(null_sha1, ce->name);
>> + if (!sub)
>> + return 0;
>> +
>> + return sub->update_strategy.type == strategy;
>> +}
>> +
>
> I liked Junio's suggestion where this just returns the strategy that
> it can use, or 0 if it shouldn't be updated. Then, other code just
> decides: Yes, I can use this strategy or no I cannot.
>
> Should this be tied in with the strategy used by the
> recurse_submodules above? ie: when/if we support recursing using other
> strategies, shouldn't we make these match here, so that if the recurse
> mode is "checkout" we don't checkout a submodule that was configured
> to be rebased? Or do you want to blindly apply checkout to all
> submodules even if they don't have strategy?
>
> I assume you do not, since you check here with passing a strategy
> value, and then see if it matches.
>
> this could be named something like:
>
> get_active_submodule_strategy() and return the strategy directly. It
> would then return 0 in any case where it shouldn't be updated. Later
> code can then check the strategy.
0 is already taken as SM_UPDATE_UNSPECIFIED,
so maybe we'd introduce a new update command
SM_UPDATE_IGNORE = -1 or rather use
SM_UPDATE_NONE instead.
>
>> static int has_remote(const char *refname, const struct object_id *oid,
>> int flags, void *cb_data)
>> {
>> diff --git a/submodule.h b/submodule.h
>> index b4e60c08d2..46d9f0f293 100644
>> --- a/submodule.h
>> +++ b/submodule.h
>> @@ -65,6 +65,19 @@ extern void show_submodule_inline_diff(FILE *f, const char *path,
>> const struct diff_options *opt);
>> extern void set_config_fetch_recurse_submodules(int value);
>> extern void set_config_update_recurse_submodules(int value);
>> +
>> +/*
>> + * Traditionally Git ignored changes made for submodules.
>> + * This function checks if we are interested in the given submodule
>> + * for any kind of operation.
>
> This comment seems a bit weird.
correct, I'll reword that.
>
>> + */
>> +extern int touch_submodules_in_worktree(void);
>> +/*
>> + * Check if the given ce entry is a submodule with the given update
>> + * strategy configured.
>
> I like Junio's suggestion of this "getting the current configured
> strategy for a submodule. When we aren't set to recurse into
> submodules we (obviously) return that we have no strategy since we're
> not going to update it at all.
>
>> + */
>> +extern int is_active_submodule_with_strategy(const struct cache_entry *ce,
>> + enum submodule_update_type strategy);
>> extern void check_for_new_submodule_commits(unsigned char new_sha1[20]);
>> extern int fetch_populated_submodules(const struct argv_array *options,
>> const char *prefix, int command_line_option,
>> --
>> 2.12.0.rc1.16.ge4278d41a0.dirty
>>
^ permalink raw reply
* Re: [PATCH 06/15] update submodules: add submodule config parsing
From: Jacob Keller @ 2017-02-21 20:48 UTC (permalink / raw)
To: Stefan Beller
Cc: Git mailing list, brian m. carlson, Jonathan Nieder,
Brandon Williams, Junio C Hamano
In-Reply-To: <CAGZ79kZyFfC9Xx-p8dpoAFFpz48BqmftpMonuxeiKg1sV68iuQ@mail.gmail.com>
On Tue, Feb 21, 2017 at 11:42 AM, Stefan Beller <sbeller@google.com> wrote:
> On Fri, Feb 17, 2017 at 10:24 AM, Jacob Keller <jacob.keller@gmail.com> wrote:
>>
>> Ok so this function here reads a recurse submodules parameter which is
>> a boolean or it can be set to the word "checkout"? Why does checkout
>> need its own value separate from true? Just so that we have a synonym?
>> or so that we can expand on it in the future?
>
> I think eventually we want all the commands that touch the worktree to
> be able to cope with submodules.
>
> Now what should e.g. git-revert --recurse-submodules do?
> yes == "checkout" means we'd revert the superproject commit and
> if that commit changed any submodule pointers we'd just "checkout"
> those states in the submodule.
>
> For revert you could also imagine to have
> git-revert --recurse-submodules=revert-in-subs
> that would not repoint the submodule pointer to the old state, but
> would try to revert $OLD..$NEW in the submodule and take the newly
> reverted state as the new submodule pointer.
>
> As I want to focus on checkout first, I went with "yes == checkout"
> here (or rather the other way round).
Ok I understand, but this seems like the variable could eventually
start to included more and more complex things? For now, "checkout"
means "when changing submodules prefer to check out contents" right?
I guess that sort of makes some sense.
Thanks,
Jake
^ 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