Re: [PATCH v2 15/16] fsck: reduce word legos to help i18n
On Tue, Nov 6, 2018 at 4:41 AM Junio C Hamano wrote:
> > static int fsck_error_func(struct fsck_options *o,
> > struct object *obj, int type, const char *message)
> > {
> > - objreport(obj, (type == FSCK_WARN) ? "warning" : "error", message);
> > - return (type == FSCK_WARN) ? 0 : 1;
> > + if (type == FSCK_WARN) {
> > + fprintf_ln(stderr, "warning in %s %s: %s",
> > +printable_type(obj), describe_object(obj),
> > message);
> > + return 0;
> > + }
> > +
> > + fprintf_ln(stderr, "error in %s %s: %s",
> > +printable_type(obj), describe_object(obj), message);
> > + return 1;
>
> Make it look more symmetrical like the original, perhaps by
>
> if (type == FSCK_WARN) {
> ...
> return 0;
> } else { /* FSCK_ERROR */
> ...
> return 1;
> }
>
> Actually, wouldn't it be clearer to see what is going on, if we did
> it like this instead?
>
> const char *fmt = (type == FSCK_WARN)
> ? N_("warning in %s %s: %s")
> : N_("error in %s %s: %s");
> fprintf_ln(stderr, _(fmt),
>printable_type(obj), describe_object(obj), message);
> return (type == FSCK_WARN) ? 0 : 1;
>
> It would show that in either case we show these three things in the
> message. I dunno.
Specifying "type == FSCK_WARN" twice triggers me (what if we add a
third fsck type?) so I just turn this to a switch/case block instead
(and get to know the third fsck type FSCK_IGNORE).
--
Duy
[PATCH v2 02/22] wt-status.c: remove implicit dependency the_repository
Signed-off-by: Nguyễn Thái Ngọc Duy
---
ref-filter.c | 2 +-
wt-status.c | 18 ++
wt-status.h | 4 +++-
3 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 0c45ed9d94..c4eaf30313 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1413,7 +1413,7 @@ char *get_head_description(void)
struct strbuf desc = STRBUF_INIT;
struct wt_status_state state;
memset(, 0, sizeof(state));
- wt_status_get_state(, 1);
+ wt_status_get_state(the_repository, , 1);
if (state.rebase_in_progress ||
state.rebase_interactive_in_progress) {
if (state.branch)
diff --git a/wt-status.c b/wt-status.c
index 6d401b2c24..e582c54238 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -755,7 +755,7 @@ void wt_status_collect(struct wt_status *s)
wt_status_collect_changes_index(s);
wt_status_collect_untracked(s);
- wt_status_get_state(>state, s->branch && !strcmp(s->branch, "HEAD"));
+ wt_status_get_state(s->repo, >state, s->branch && !strcmp(s->branch,
"HEAD"));
if (s->state.merge_in_progress && !has_unmerged(s))
s->committable = 1;
}
@@ -1482,7 +1482,8 @@ static int grab_1st_switch(struct object_id *ooid, struct
object_id *noid,
return 1;
}
-static void wt_status_get_detached_from(struct wt_status_state *state)
+static void wt_status_get_detached_from(struct repository *r,
+ struct wt_status_state *state)
{
struct grab_1st_switch_cbdata cb;
struct commit *commit;
@@ -1499,7 +1500,7 @@ static void wt_status_get_detached_from(struct
wt_status_state *state)
/* sha1 is a commit? match without further lookup */
(oideq(, ) ||
/* perhaps sha1 is a tag, try to dereference to a commit */
-((commit = lookup_commit_reference_gently(the_repository, ,
1)) != NULL &&
+((commit = lookup_commit_reference_gently(r, , 1)) != NULL &&
oideq(, >object.oid {
const char *from = ref;
if (!skip_prefix(from, "refs/tags/", ))
@@ -1556,30 +1557,31 @@ int wt_status_check_bisect(const struct worktree *wt,
return 0;
}
-void wt_status_get_state(struct wt_status_state *state,
+void wt_status_get_state(struct repository *r,
+struct wt_status_state *state,
int get_detached_from)
{
struct stat st;
struct object_id oid;
- if (!stat(git_path_merge_head(the_repository), )) {
+ if (!stat(git_path_merge_head(r), )) {
state->merge_in_progress = 1;
} else if (wt_status_check_rebase(NULL, state)) {
; /* all set */
- } else if (!stat(git_path_cherry_pick_head(the_repository), ) &&
+ } else if (!stat(git_path_cherry_pick_head(r), ) &&
!get_oid("CHERRY_PICK_HEAD", )) {
state->cherry_pick_in_progress = 1;
oidcpy(>cherry_pick_head_oid, );
}
wt_status_check_bisect(NULL, state);
- if (!stat(git_path_revert_head(the_repository), ) &&
+ if (!stat(git_path_revert_head(r), ) &&
!get_oid("REVERT_HEAD", )) {
state->revert_in_progress = 1;
oidcpy(>revert_head_oid, );
}
if (get_detached_from)
- wt_status_get_detached_from(state);
+ wt_status_get_detached_from(r, state);
}
static void wt_longstatus_print_state(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 8375e816fb..3a95975032 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -134,7 +134,9 @@ void wt_status_prepare(struct repository *r, struct
wt_status *s);
void wt_status_print(struct wt_status *s);
void wt_status_collect(struct wt_status *s);
void wt_status_collect_free_buffers(struct wt_status *s);
-void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
+void wt_status_get_state(struct repository *repo,
+struct wt_status_state *state,
+int get_detached_from);
int wt_status_check_rebase(const struct worktree *wt,
struct wt_status_state *state);
int wt_status_check_bisect(const struct worktree *wt,
--
2.19.1.1231.g84aef82467
[PATCH v2 03/22] list-objects-filter.c: remove implicit dependency on the_index
While at there, since we have access to struct repository now,
eliminate the only the_repository reference in this file.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
list-objects-filter.c | 10 +++---
list-objects-filter.h | 2 ++
list-objects.c| 9 ++---
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/list-objects-filter.c b/list-objects-filter.c
index 765f3df3b0..a62624a1ce 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -34,6 +34,7 @@ struct filter_blobs_none_data {
};
static enum list_objects_filter_result filter_blobs_none(
+ struct repository *r,
enum list_objects_filter_situation filter_situation,
struct object *obj,
const char *pathname,
@@ -88,6 +89,7 @@ struct filter_trees_none_data {
};
static enum list_objects_filter_result filter_trees_none(
+ struct repository *r,
enum list_objects_filter_situation filter_situation,
struct object *obj,
const char *pathname,
@@ -144,6 +146,7 @@ struct filter_blobs_limit_data {
};
static enum list_objects_filter_result filter_blobs_limit(
+ struct repository *r,
enum list_objects_filter_situation filter_situation,
struct object *obj,
const char *pathname,
@@ -171,7 +174,7 @@ static enum list_objects_filter_result filter_blobs_limit(
assert(obj->type == OBJ_BLOB);
assert((obj->flags & SEEN) == 0);
- t = oid_object_info(the_repository, >oid, _length);
+ t = oid_object_info(r, >oid, _length);
if (t != OBJ_BLOB) { /* probably OBJ_NONE */
/*
* We DO NOT have the blob locally, so we cannot
@@ -249,6 +252,7 @@ struct filter_sparse_data {
};
static enum list_objects_filter_result filter_sparse(
+ struct repository *r,
enum list_objects_filter_situation filter_situation,
struct object *obj,
const char *pathname,
@@ -268,7 +272,7 @@ static enum list_objects_filter_result filter_sparse(
dtype = DT_DIR;
val = is_excluded_from_list(pathname, strlen(pathname),
filename, , _data->el,
- _index);
+ r->index);
if (val < 0)
val = filter_data->array_frame[filter_data->nr].defval;
@@ -331,7 +335,7 @@ static enum list_objects_filter_result filter_sparse(
dtype = DT_REG;
val = is_excluded_from_list(pathname, strlen(pathname),
filename, , _data->el,
- _index);
+ r->index);
if (val < 0)
val = frame->defval;
if (val > 0) {
diff --git a/list-objects-filter.h b/list-objects-filter.h
index 52b4a84da9..1d45a4ad57 100644
--- a/list-objects-filter.h
+++ b/list-objects-filter.h
@@ -4,6 +4,7 @@
struct list_objects_filter_options;
struct object;
struct oidset;
+struct repository;
/*
* During list-object traversal we allow certain objects to be
@@ -60,6 +61,7 @@ enum list_objects_filter_situation {
};
typedef enum list_objects_filter_result (*filter_object_fn)(
+ struct repository *r,
enum list_objects_filter_situation filter_situation,
struct object *obj,
const char *pathname,
diff --git a/list-objects.c b/list-objects.c
index c41cc80db5..0cfd646026 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -55,7 +55,8 @@ static void process_blob(struct traversal_context *ctx,
pathlen = path->len;
strbuf_addstr(path, name);
if ((obj->flags & NOT_USER_GIVEN) && ctx->filter_fn)
- r = ctx->filter_fn(LOFS_BLOB, obj,
+ r = ctx->filter_fn(ctx->revs->repo,
+ LOFS_BLOB, obj,
path->buf, >buf[pathlen],
ctx->filter_data);
if (r & LOFR_MARK_SEEN)
@@ -175,7 +176,8 @@ static void process_tree(struct traversal_context *ctx,
strbuf_addstr(base, name);
if ((obj->flags & NOT_USER_GIVEN) && ctx->filter_fn)
- r = ctx->filter_fn(LOFS_BEGIN_TREE, obj,
+ r = ctx->filter_fn(ctx->revs->repo,
+ LOFS_BEGIN_TREE, obj,
base->buf, >buf[baselen],
ctx->filter_data);
if (r & LOFR_MARK_SEEN)
@@ -191,7 +193,8 @@ static void process_tree(struct traversal_context *ctx,
process_tree_contents(ctx, tree, base);
if ((obj->flags & NOT_USER_GIVEN) && ctx->filter_fn) {
- r = ctx->filter_fn(LOFS_END_TREE, obj,
+ r = ctx->filter_fn(ctx->revs->repo,
+ LOFS_END_TREE, obj,
[PATCH v2 10/22] blame.c: remove implicit dependency the_repository
Signed-off-by: Nguyễn Thái Ngọc Duy
---
blame.c | 39 ++-
1 file changed, 22 insertions(+), 17 deletions(-)
diff --git a/blame.c b/blame.c
index d84c937780..43861437f7 100644
--- a/blame.c
+++ b/blame.c
@@ -116,35 +116,38 @@ static void verify_working_tree_path(struct repository *r,
die("no such path '%s' in HEAD", path);
}
-static struct commit_list **append_parent(struct commit_list **tail, const
struct object_id *oid)
+static struct commit_list **append_parent(struct repository *r,
+ struct commit_list **tail,
+ const struct object_id *oid)
{
struct commit *parent;
- parent = lookup_commit_reference(the_repository, oid);
+ parent = lookup_commit_reference(r, oid);
if (!parent)
die("no such commit %s", oid_to_hex(oid));
return _list_insert(parent, tail)->next;
}
-static void append_merge_parents(struct commit_list **tail)
+static void append_merge_parents(struct repository *r,
+struct commit_list **tail)
{
int merge_head;
struct strbuf line = STRBUF_INIT;
- merge_head = open(git_path_merge_head(the_repository), O_RDONLY);
+ merge_head = open(git_path_merge_head(r), O_RDONLY);
if (merge_head < 0) {
if (errno == ENOENT)
return;
die("cannot open '%s' for reading",
- git_path_merge_head(the_repository));
+ git_path_merge_head(r));
}
while (!strbuf_getwholeline_fd(, merge_head, '\n')) {
struct object_id oid;
if (line.len < GIT_SHA1_HEXSZ || get_oid_hex(line.buf, ))
die("unknown line in '%s': %s",
- git_path_merge_head(the_repository), line.buf);
- tail = append_parent(tail, );
+ git_path_merge_head(r), line.buf);
+ tail = append_parent(r, tail, );
}
close(merge_head);
strbuf_release();
@@ -155,11 +158,13 @@ static void append_merge_parents(struct commit_list
**tail)
* want to transfer ownership of the buffer to the commit (so we
* must use detach).
*/
-static void set_commit_buffer_from_strbuf(struct commit *c, struct strbuf *sb)
+static void set_commit_buffer_from_strbuf(struct repository *r,
+ struct commit *c,
+ struct strbuf *sb)
{
size_t len;
void *buf = strbuf_detach(sb, );
- set_commit_buffer(the_repository, c, buf, len);
+ set_commit_buffer(r, c, buf, len);
}
/*
@@ -185,7 +190,7 @@ static struct commit *fake_working_tree_commit(struct
repository *r,
read_index(r->index);
time();
- commit = alloc_commit_node(the_repository);
+ commit = alloc_commit_node(r);
commit->object.parsed = 1;
commit->date = now;
parent_tail = >parents;
@@ -193,8 +198,8 @@ static struct commit *fake_working_tree_commit(struct
repository *r,
if (!resolve_ref_unsafe("HEAD", RESOLVE_REF_READING, _oid, NULL))
die("no such ref: HEAD");
- parent_tail = append_parent(parent_tail, _oid);
- append_merge_parents(parent_tail);
+ parent_tail = append_parent(r, parent_tail, _oid);
+ append_merge_parents(r, parent_tail);
verify_working_tree_path(r, commit, path);
origin = make_origin(commit, path);
@@ -211,7 +216,7 @@ static struct commit *fake_working_tree_commit(struct
repository *r,
ident, ident, path,
(!contents_from ? path :
(!strcmp(contents_from, "-") ? "standard input" :
contents_from)));
- set_commit_buffer_from_strbuf(commit, );
+ set_commit_buffer_from_strbuf(r, commit, );
if (!contents_from || strcmp("-", contents_from)) {
struct stat st;
@@ -1678,7 +1683,7 @@ static struct commit *find_single_final(struct rev_info
*revs,
struct object *obj = revs->pending.objects[i].item;
if (obj->flags & UNINTERESTING)
continue;
- obj = deref_tag(the_repository, obj, NULL, 0);
+ obj = deref_tag(revs->repo, obj, NULL, 0);
if (obj->type != OBJ_COMMIT)
die("Non commit %s?", revs->pending.objects[i].name);
if (found)
@@ -1709,14 +1714,14 @@ static struct commit *dwim_reverse_initial(struct
rev_info *revs,
/* Is that sole rev a committish? */
obj = revs->pending.objects[0].item;
- obj = deref_tag(the_repository, obj, NULL, 0);
+ obj = deref_tag(revs->repo, obj, NULL, 0);
if (obj->type != OBJ_COMMIT)
return NULL;
/* Do we have HEAD? */
if (!resolve_ref_unsafe("HEAD",
[PATCH v2 18/22] notes-cache.c: remove the_repository references
Signed-off-by: Nguyễn Thái Ngọc Duy
---
combine-diff.c | 2 +-
diff.c | 12 ++--
diff.h | 2 +-
diffcore-pickaxe.c | 4 ++--
grep.c | 2 +-
notes-cache.c | 12 +++-
notes-cache.h | 6 --
userdiff.c | 5 +++--
userdiff.h | 4 +++-
9 files changed, 28 insertions(+), 21 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 10155e0ec8..3d796af3ca 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -991,7 +991,7 @@ static void show_patch_diff(struct combine_diff_path *elem,
int num_parent,
if (!userdiff)
userdiff = userdiff_find_by_name("default");
if (opt->flags.allow_textconv)
- textconv = userdiff_get_textconv(userdiff);
+ textconv = userdiff_get_textconv(opt->repo, userdiff);
/* Read the result of merge first */
if (!working_tree_file)
diff --git a/diff.c b/diff.c
index 8647db3d30..1135377a7f 100644
--- a/diff.c
+++ b/diff.c
@@ -3312,14 +3312,14 @@ void diff_set_mnemonic_prefix(struct diff_options
*options, const char *a, const
options->b_prefix = b;
}
-struct userdiff_driver *get_textconv(struct index_state *istate,
+struct userdiff_driver *get_textconv(struct repository *r,
struct diff_filespec *one)
{
if (!DIFF_FILE_VALID(one))
return NULL;
- diff_filespec_load_driver(one, istate);
- return userdiff_get_textconv(one->driver);
+ diff_filespec_load_driver(one, r->index);
+ return userdiff_get_textconv(r, one->driver);
}
static void builtin_diff(const char *name_a,
@@ -3368,8 +3368,8 @@ static void builtin_diff(const char *name_a,
}
if (o->flags.allow_textconv) {
- textconv_one = get_textconv(o->repo->index, one);
- textconv_two = get_textconv(o->repo->index, two);
+ textconv_one = get_textconv(o->repo, one);
+ textconv_two = get_textconv(o->repo, two);
}
/* Never use a non-valid filename anywhere if at all possible */
@@ -6436,7 +6436,7 @@ int textconv_object(struct repository *r,
df = alloc_filespec(path);
fill_filespec(df, oid, oid_valid, mode);
- textconv = get_textconv(r->index, df);
+ textconv = get_textconv(r, df);
if (!textconv) {
free_filespec(df);
return 0;
diff --git a/diff.h b/diff.h
index 3197a976a4..412138ba08 100644
--- a/diff.h
+++ b/diff.h
@@ -461,7 +461,7 @@ size_t fill_textconv(struct repository *r,
* and only if it has textconv enabled (otherwise return NULL). The result
* can be passed to fill_textconv().
*/
-struct userdiff_driver *get_textconv(struct index_state *istate,
+struct userdiff_driver *get_textconv(struct repository *r,
struct diff_filespec *one);
/*
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index d2361e06a1..b815f1c449 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -139,8 +139,8 @@ static int pickaxe_match(struct diff_filepair *p, struct
diff_options *o,
return 0;
if (o->flags.allow_textconv) {
- textconv_one = get_textconv(o->repo->index, p->one);
- textconv_two = get_textconv(o->repo->index, p->two);
+ textconv_one = get_textconv(o->repo, p->one);
+ textconv_two = get_textconv(o->repo, p->two);
}
/*
diff --git a/grep.c b/grep.c
index f6bd89e40b..b05b2e61bb 100644
--- a/grep.c
+++ b/grep.c
@@ -1811,7 +1811,7 @@ static int grep_source_1(struct grep_opt *opt, struct
grep_source *gs, int colle
* is not thread-safe.
*/
grep_attr_lock();
- textconv = userdiff_get_textconv(gs->driver);
+ textconv = userdiff_get_textconv(opt->repo, gs->driver);
grep_attr_unlock();
}
diff --git a/notes-cache.c b/notes-cache.c
index d87e7ca91c..2473314d68 100644
--- a/notes-cache.c
+++ b/notes-cache.c
@@ -5,7 +5,9 @@
#include "commit.h"
#include "refs.h"
-static int notes_cache_match_validity(const char *ref, const char *validity)
+static int notes_cache_match_validity(struct repository *r,
+ const char *ref,
+ const char *validity)
{
struct object_id oid;
struct commit *commit;
@@ -16,7 +18,7 @@ static int notes_cache_match_validity(const char *ref, const
char *validity)
if (read_ref(ref, ) < 0)
return 0;
- commit = lookup_commit_reference_gently(the_repository, , 1);
+ commit = lookup_commit_reference_gently(r, , 1);
if (!commit)
return 0;
@@ -30,8 +32,8 @@ static int notes_cache_match_validity(const char *ref, const
char *validity)
return ret;
}
-void notes_cache_init(struct notes_cache *c, const char
[PATCH v2 16/22] diff-lib.c: remove the_repository references
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/commit.c | 3 ++-
builtin/merge-ours.c | 2 +-
diff-lib.c | 7 ---
diff.h | 3 ++-
sequencer.c | 4 ++--
5 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index e89bf35634..91b1920255 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -908,7 +908,8 @@ static int prepare_to_commit(const char *index_file, const
char *prefix,
if (ignore_submodule_arg &&
!strcmp(ignore_submodule_arg, "all"))
flags.ignore_submodules = 1;
- committable = index_differs_from(parent, , 1);
+ committable = index_differs_from(the_repository,
+parent, , 1);
}
}
strbuf_release(_ident);
diff --git a/builtin/merge-ours.c b/builtin/merge-ours.c
index c84c6e05e9..0b07263415 100644
--- a/builtin/merge-ours.c
+++ b/builtin/merge-ours.c
@@ -26,7 +26,7 @@ int cmd_merge_ours(int argc, const char **argv, const char
*prefix)
*/
if (read_cache() < 0)
die_errno("read_cache failed");
- if (index_differs_from("HEAD", NULL, 0))
+ if (index_differs_from(the_repository, "HEAD", NULL, 0))
exit(2);
exit(0);
}
diff --git a/diff-lib.c b/diff-lib.c
index 83fce51518..23c8d351b3 100644
--- a/diff-lib.c
+++ b/diff-lib.c
@@ -542,7 +542,7 @@ int do_diff_cache(const struct object_id *tree_oid, struct
diff_options *opt)
{
struct rev_info revs;
- repo_init_revisions(the_repository, , NULL);
+ repo_init_revisions(opt->repo, , NULL);
copy_pathspec(_data, >pathspec);
revs.diffopt = *opt;
@@ -551,13 +551,14 @@ int do_diff_cache(const struct object_id *tree_oid,
struct diff_options *opt)
return 0;
}
-int index_differs_from(const char *def, const struct diff_flags *flags,
+int index_differs_from(struct repository *r,
+ const char *def, const struct diff_flags *flags,
int ita_invisible_in_index)
{
struct rev_info rev;
struct setup_revision_opt opt;
- repo_init_revisions(the_repository, , NULL);
+ repo_init_revisions(r, , NULL);
memset(, 0, sizeof(opt));
opt.def = def;
setup_revisions(0, NULL, , );
diff --git a/diff.h b/diff.h
index ce5e8a8183..3197a976a4 100644
--- a/diff.h
+++ b/diff.h
@@ -436,7 +436,8 @@ int diff_result_code(struct diff_options *, int);
void diff_no_index(struct repository *, struct rev_info *, int, const char **);
-int index_differs_from(const char *def, const struct diff_flags *flags,
+int index_differs_from(struct repository *r, const char *def,
+ const struct diff_flags *flags,
int ita_invisible_in_index);
/*
diff --git a/sequencer.c b/sequencer.c
index 726f727644..d726f77e11 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1713,7 +1713,7 @@ static int do_pick_commit(struct repository *r,
unborn = 1;
} else if (unborn)
oidcpy(, the_hash_algo->empty_tree);
- if (index_differs_from(unborn ? empty_tree_oid_hex() : "HEAD",
+ if (index_differs_from(r, unborn ? empty_tree_oid_hex() :
"HEAD",
NULL, 0))
return error_dirty_index(r->index, opts);
}
@@ -3915,7 +3915,7 @@ int sequencer_continue(struct repository *r, struct
replay_opts *opts)
if (res)
goto release_todo_list;
}
- if (index_differs_from("HEAD", NULL, 0)) {
+ if (index_differs_from(r, "HEAD", NULL, 0)) {
res = error_dirty_index(r->index, opts);
goto release_todo_list;
}
--
2.19.1.1231.g84aef82467
[PATCH v2 07/22] transport.c: remove implicit dependency on the_index
note, there's still another hidden dependency related to this: even
though we pass a repo to transport_push() we still use
is_bare_repository() which pretty much assumes the_repository (and
some other global state).
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/push.c | 3 ++-
transport.c| 7 ---
transport.h| 3 ++-
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/builtin/push.c b/builtin/push.c
index d09a42062c..efb3e38a8d 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -355,7 +355,8 @@ static int push_with_options(struct transport *transport,
struct refspec *rs,
if (verbosity > 0)
fprintf(stderr, _("Pushing to %s\n"), transport->url);
- err = transport_push(transport, rs, flags, _reasons);
+ err = transport_push(the_repository, transport,
+rs, flags, _reasons);
if (err != 0) {
fprintf(stderr, "%s", push_get_color(PUSH_COLOR_ERROR));
error(_("failed to push some refs to '%s'"), transport->url);
diff --git a/transport.c b/transport.c
index 5a74b609ff..71f663743f 100644
--- a/transport.c
+++ b/transport.c
@@ -1105,7 +1105,8 @@ static int run_pre_push_hook(struct transport *transport,
return ret;
}
-int transport_push(struct transport *transport,
+int transport_push(struct repository *r,
+ struct transport *transport,
struct refspec *rs, int flags,
unsigned int *reject_reasons)
{
@@ -1172,7 +1173,7 @@ int transport_push(struct transport *transport,
oid_array_append(,
>new_oid);
- if (!push_unpushed_submodules(the_repository,
+ if (!push_unpushed_submodules(r,
,
transport->remote,
rs,
@@ -1197,7 +1198,7 @@ int transport_push(struct transport *transport,
oid_array_append(,
>new_oid);
- if (find_unpushed_submodules(the_repository,
+ if (find_unpushed_submodules(r,
,
transport->remote->name,
_pushing)) {
diff --git a/transport.h b/transport.h
index 9baeca2d7a..f2ee7c4f49 100644
--- a/transport.h
+++ b/transport.h
@@ -223,7 +223,8 @@ void transport_set_verbosity(struct transport *transport,
int verbosity,
#define REJECT_FETCH_FIRST 0x08
#define REJECT_NEEDS_FORCE 0x10
-int transport_push(struct transport *connection,
+int transport_push(struct repository *repo,
+ struct transport *connection,
struct refspec *rs, int flags,
unsigned int * reject_reasons);
--
2.19.1.1231.g84aef82467
[PATCH v2 13/22] bundle.c: remove the_repository references
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/bundle.c | 7 ---
bundle.c | 26 ++
bundle.h | 9 +
transport.c | 2 +-
4 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/builtin/bundle.c b/builtin/bundle.c
index d0de59b94f..9e9c65d9c6 100644
--- a/builtin/bundle.c
+++ b/builtin/bundle.c
@@ -40,7 +40,7 @@ int cmd_bundle(int argc, const char **argv, const char
*prefix)
usage(builtin_bundle_usage);
return 1;
}
- if (verify_bundle(, 1))
+ if (verify_bundle(the_repository, , 1))
return 1;
fprintf(stderr, _("%s is okay\n"), bundle_file);
return 0;
@@ -56,11 +56,12 @@ int cmd_bundle(int argc, const char **argv, const char
*prefix)
}
if (!startup_info->have_repository)
die(_("Need a repository to create a bundle."));
- return !!create_bundle(, bundle_file, argc, argv);
+ return !!create_bundle(the_repository, ,
+ bundle_file, argc, argv);
} else if (!strcmp(cmd, "unbundle")) {
if (!startup_info->have_repository)
die(_("Need a repository to unbundle."));
- return !!unbundle(, bundle_fd, 0) ||
+ return !!unbundle(the_repository, , bundle_fd, 0) ||
list_bundle_refs(, argc, argv);
} else
usage(builtin_bundle_usage);
diff --git a/bundle.c b/bundle.c
index 1ef584b93b..b94992675a 100644
--- a/bundle.c
+++ b/bundle.c
@@ -127,7 +127,9 @@ static int list_refs(struct ref_list *r, int argc, const
char **argv)
/* Remember to update object flag allocation in object.h */
#define PREREQ_MARK (1u<<16)
-int verify_bundle(struct bundle_header *header, int verbose)
+int verify_bundle(struct repository *r,
+ struct bundle_header *header,
+ int verbose)
{
/*
* Do fast check, then if any prereqs are missing then go line by line
@@ -140,10 +142,10 @@ int verify_bundle(struct bundle_header *header, int
verbose)
int i, ret = 0, req_nr;
const char *message = _("Repository lacks these prerequisite commits:");
- repo_init_revisions(the_repository, , NULL);
+ repo_init_revisions(r, , NULL);
for (i = 0; i < p->nr; i++) {
struct ref_list_entry *e = p->list + i;
- struct object *o = parse_object(the_repository, >oid);
+ struct object *o = parse_object(r, >oid);
if (o) {
o->flags |= PREREQ_MARK;
add_pending_object(, o, e->name);
@@ -168,7 +170,7 @@ int verify_bundle(struct bundle_header *header, int verbose)
for (i = 0; i < p->nr; i++) {
struct ref_list_entry *e = p->list + i;
- struct object *o = parse_object(the_repository, >oid);
+ struct object *o = parse_object(r, >oid);
assert(o); /* otherwise we'd have returned early */
if (o->flags & SHOWN)
continue;
@@ -180,7 +182,7 @@ int verify_bundle(struct bundle_header *header, int verbose)
/* Clean up objects used, as they will be reused. */
for (i = 0; i < p->nr; i++) {
struct ref_list_entry *e = p->list + i;
- commit = lookup_commit_reference_gently(the_repository,
>oid, 1);
+ commit = lookup_commit_reference_gently(r, >oid, 1);
if (commit)
clear_commit_marks(commit, ALL_REV_FLAGS);
}
@@ -375,8 +377,7 @@ static int write_bundle_refs(int bundle_fd, struct rev_info
*revs)
* in terms of a tag (e.g. v2.0 from the range
* "v1.0..v2.0")?
*/
- struct commit *one =
lookup_commit_reference(the_repository,
-);
+ struct commit *one =
lookup_commit_reference(revs->repo, );
struct object *obj;
if (e->item == &(one->object)) {
@@ -409,8 +410,8 @@ static int write_bundle_refs(int bundle_fd, struct rev_info
*revs)
return ref_count;
}
-int create_bundle(struct bundle_header *header, const char *path,
- int argc, const char **argv)
+int create_bundle(struct repository *r, struct bundle_header *header,
+ const char *path, int argc, const char **argv)
{
struct lock_file lock = LOCK_INIT;
int bundle_fd = -1;
@@ -441,7 +442,7 @@ int create_bundle(struct bundle_header *header, const char
*path,
/* init revs to list objects for pack-objects later */
save_commit_buffer = 0;
- repo_init_revisions(the_repository, , NULL);
+
[PATCH v2 11/22] bisect.c: remove the_repository reference
Signed-off-by: Nguyễn Thái Ngọc Duy
---
bisect.c | 48 +++-
bisect.h | 5 -
builtin/bisect--helper.c | 2 +-
3 files changed, 33 insertions(+), 22 deletions(-)
diff --git a/bisect.c b/bisect.c
index 487675c672..4c1b80bff6 100644
--- a/bisect.c
+++ b/bisect.c
@@ -626,14 +626,15 @@ static struct commit_list *managed_skipped(struct
commit_list *list,
return skip_away(list, count);
}
-static void bisect_rev_setup(struct rev_info *revs, const char *prefix,
+static void bisect_rev_setup(struct repository *r, struct rev_info *revs,
+const char *prefix,
const char *bad_format, const char *good_format,
int read_paths)
{
struct argv_array rev_argv = ARGV_ARRAY_INIT;
int i;
- repo_init_revisions(the_repository, revs, prefix);
+ repo_init_revisions(r, revs, prefix);
revs->abbrev = 0;
revs->commit_format = CMIT_FMT_UNSPECIFIED;
@@ -723,23 +724,25 @@ static int bisect_checkout(const struct object_id
*bisect_rev, int no_checkout)
return run_command_v_opt(argv_show_branch, RUN_GIT_CMD);
}
-static struct commit *get_commit_reference(const struct object_id *oid)
+static struct commit *get_commit_reference(struct repository *r,
+ const struct object_id *oid)
{
- struct commit *r = lookup_commit_reference(the_repository, oid);
- if (!r)
+ struct commit *c = lookup_commit_reference(r, oid);
+ if (!c)
die(_("Not a valid commit name %s"), oid_to_hex(oid));
- return r;
+ return c;
}
-static struct commit **get_bad_and_good_commits(int *rev_nr)
+static struct commit **get_bad_and_good_commits(struct repository *r,
+ int *rev_nr)
{
struct commit **rev;
int i, n = 0;
ALLOC_ARRAY(rev, 1 + good_revs.nr);
- rev[n++] = get_commit_reference(current_bad_oid);
+ rev[n++] = get_commit_reference(r, current_bad_oid);
for (i = 0; i < good_revs.nr; i++)
- rev[n++] = get_commit_reference(good_revs.oid + i);
+ rev[n++] = get_commit_reference(r, good_revs.oid + i);
*rev_nr = n;
return rev;
@@ -823,12 +826,13 @@ static void check_merge_bases(int rev_nr, struct commit
**rev, int no_checkout)
free_commit_list(result);
}
-static int check_ancestors(int rev_nr, struct commit **rev, const char *prefix)
+static int check_ancestors(struct repository *r, int rev_nr,
+ struct commit **rev, const char *prefix)
{
struct rev_info revs;
int res;
- bisect_rev_setup(, prefix, "^%s", "%s", 0);
+ bisect_rev_setup(r, , prefix, "^%s", "%s", 0);
bisect_common();
res = (revs.commits != NULL);
@@ -847,7 +851,9 @@ static int check_ancestors(int rev_nr, struct commit **rev,
const char *prefix)
* If a merge base must be tested by the user, its source code will be
* checked out to be tested by the user and we will exit.
*/
-static void check_good_are_ancestors_of_bad(const char *prefix, int
no_checkout)
+static void check_good_are_ancestors_of_bad(struct repository *r,
+ const char *prefix,
+ int no_checkout)
{
char *filename = git_pathdup("BISECT_ANCESTORS_OK");
struct stat st;
@@ -866,8 +872,8 @@ static void check_good_are_ancestors_of_bad(const char
*prefix, int no_checkout)
goto done;
/* Check if all good revs are ancestor of the bad rev. */
- rev = get_bad_and_good_commits(_nr);
- if (check_ancestors(rev_nr, rev, prefix))
+ rev = get_bad_and_good_commits(r, _nr);
+ if (check_ancestors(r, rev_nr, rev, prefix))
check_merge_bases(rev_nr, rev, no_checkout);
free(rev);
@@ -885,12 +891,14 @@ static void check_good_are_ancestors_of_bad(const char
*prefix, int no_checkout)
/*
* This does "git diff-tree --pretty COMMIT" without one fork+exec.
*/
-static void show_diff_tree(const char *prefix, struct commit *commit)
+static void show_diff_tree(struct repository *r,
+ const char *prefix,
+ struct commit *commit)
{
struct rev_info opt;
/* diff-tree init */
- repo_init_revisions(the_repository, , prefix);
+ repo_init_revisions(r, , prefix);
git_config(git_diff_basic_config, NULL); /* no "diff" UI options */
opt.abbrev = 0;
opt.diff = 1;
@@ -945,7 +953,7 @@ void read_bisect_terms(const char **read_bad, const char
**read_good)
* If no_checkout is non-zero, the bisection process does not
* checkout the trial commit but instead simply updates BISECT_HEAD.
*/
-int bisect_next_all(const char *prefix, int no_checkout)
+int
[PATCH v2 01/22] wt-status.c: remove implicit dependency on the_index
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/commit.c | 2 +-
builtin/pull.c | 3 +-
builtin/rebase.c | 7 +++--
sequencer.c | 8 ++---
wt-status.c | 76 +++-
wt-status.h | 17 +++
6 files changed, 66 insertions(+), 47 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 074bd9a551..6637a928a7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -185,7 +185,7 @@ static void determine_whence(struct wt_status *s)
static void status_init_config(struct wt_status *s, config_fn_t fn)
{
- wt_status_prepare(s);
+ wt_status_prepare(the_repository, s);
init_diff_ui_defaults();
git_config(fn, s);
determine_whence(s);
diff --git a/builtin/pull.c b/builtin/pull.c
index c21aa276f1..6026ce1a69 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -888,7 +888,8 @@ int cmd_pull(int argc, const char **argv, const char
*prefix)
die(_("Updating an unborn branch with changes added to
the index."));
if (!autostash)
- require_clean_work_tree(N_("pull with rebase"),
+ require_clean_work_tree(the_repository,
+ N_("pull with rebase"),
_("please commit or stash them."), 1, 0);
if (get_rebase_fork_point(_fork_point, repo, *refspecs))
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0ee06aa363..b9eb958454 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -983,7 +983,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
_file);
rollback_lock_file(_file);
- if (has_unstaged_changes(1)) {
+ if (has_unstaged_changes(the_repository, 1)) {
puts(_("You must edit all merge conflicts and then\n"
"mark them as resolved using git add"));
exit(1);
@@ -1351,7 +1351,8 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
update_index_if_able(_index, _file);
rollback_lock_file(_file);
- if (has_unstaged_changes(1) || has_uncommitted_changes(1)) {
+ if (has_unstaged_changes(the_repository, 1) ||
+ has_uncommitted_changes(the_repository, 1)) {
const char *autostash =
state_dir_path("autostash", );
struct child_process stash = CHILD_PROCESS_INIT;
@@ -1397,7 +1398,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
}
}
- if (require_clean_work_tree("rebase",
+ if (require_clean_work_tree(the_repository, "rebase",
_("Please commit or stash them."), 1, 1)) {
ret = 1;
goto cleanup;
diff --git a/sequencer.c b/sequencer.c
index 9e1ab3a2a7..0b8f18fd36 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2773,7 +2773,7 @@ static int do_exec(const char *command_line)
if (discard_cache() < 0 || read_cache() < 0)
return error(_("could not read index"));
- dirty = require_clean_work_tree("rebase", NULL, 1, 1);
+ dirty = require_clean_work_tree(the_repository, "rebase", NULL, 1, 1);
if (status) {
warning(_("execution failed: %s\n%s"
@@ -3714,10 +3714,10 @@ static int commit_staged_changes(struct replay_opts
*opts,
unsigned int flags = ALLOW_EMPTY | EDIT_MSG;
unsigned int final_fixup = 0, is_clean;
- if (has_unstaged_changes(1))
+ if (has_unstaged_changes(the_repository, 1))
return error(_("cannot rebase: You have unstaged changes."));
- is_clean = !has_uncommitted_changes(0);
+ is_clean = !has_uncommitted_changes(the_repository, 0);
if (file_exists(rebase_path_amend())) {
struct strbuf rev = STRBUF_INIT;
@@ -4847,7 +4847,7 @@ int complete_action(struct replay_opts *opts, unsigned
flags,
if (checkout_onto(opts, onto_name, oid_to_hex(), orig_head))
return -1;
;
- if (require_clean_work_tree("rebase", "", 1, 1))
+ if (require_clean_work_tree(the_repository, "rebase", "", 1, 1))
return -1;
return sequencer_continue(opts);
diff --git a/wt-status.c b/wt-status.c
index 187568a112..6d401b2c24 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -119,9 +119,10 @@ static void status_printf_more(struct wt_status *s, const
char *color,
va_end(ap);
}
-void wt_status_prepare(struct wt_status *s)
+void wt_status_prepare(struct repository *r, struct wt_status *s)
{
memset(s, 0, sizeof(*s));
+ s->repo = r;
memcpy(s->color_palette, default_wt_status_colors,
sizeof(default_wt_status_colors));
s->show_untracked_files =
[PATCH v2 20/22] pack-*.c: remove the_repository references
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/pack-objects.c | 2 +-
pack-bitmap-write.c| 6 +++---
pack-bitmap.c | 13 +++--
pack-bitmap.h | 3 ++-
pack-objects.c | 6 --
pack-objects.h | 5 -
6 files changed, 21 insertions(+), 14 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 7812c2b1f3..8abfb69e2d 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -3481,7 +3481,7 @@ int cmd_pack_objects(int argc, const char **argv, const
char *prefix)
}
}
- prepare_packing_data(_pack);
+ prepare_packing_data(the_repository, _pack);
if (progress)
progress_state = start_progress(_("Enumerating objects"), 0);
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index 9d1b951697..5566e94abe 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -77,7 +77,7 @@ void bitmap_writer_build_type_index(struct packing_data
*to_pack,
break;
default:
- real_type = oid_object_info(the_repository,
+ real_type = oid_object_info(to_pack->repo,
>idx.oid, NULL);
break;
}
@@ -262,7 +262,7 @@ void bitmap_writer_build(struct packing_data *to_pack)
if (writer.show_progress)
writer.progress = start_progress("Building bitmaps",
writer.selected_nr);
- repo_init_revisions(the_repository, , NULL);
+ repo_init_revisions(to_pack->repo, , NULL);
revs.tag_objects = 1;
revs.tree_objects = 1;
revs.blob_objects = 1;
@@ -363,7 +363,7 @@ static int date_compare(const void *_a, const void *_b)
void bitmap_writer_reuse_bitmaps(struct packing_data *to_pack)
{
struct bitmap_index *bitmap_git;
- if (!(bitmap_git = prepare_bitmap_git()))
+ if (!(bitmap_git = prepare_bitmap_git(to_pack->repo)))
return;
writer.reused = kh_init_sha1();
diff --git a/pack-bitmap.c b/pack-bitmap.c
index 5848cc93aa..4695aaf6b4 100644
--- a/pack-bitmap.c
+++ b/pack-bitmap.c
@@ -328,14 +328,15 @@ static int load_pack_bitmap(struct bitmap_index
*bitmap_git)
return -1;
}
-static int open_pack_bitmap(struct bitmap_index *bitmap_git)
+static int open_pack_bitmap(struct repository *r,
+ struct bitmap_index *bitmap_git)
{
struct packed_git *p;
int ret = -1;
assert(!bitmap_git->map);
- for (p = get_all_packs(the_repository); p; p = p->next) {
+ for (p = get_all_packs(r); p; p = p->next) {
if (open_pack_bitmap_1(bitmap_git, p) == 0)
ret = 0;
}
@@ -343,11 +344,11 @@ static int open_pack_bitmap(struct bitmap_index
*bitmap_git)
return ret;
}
-struct bitmap_index *prepare_bitmap_git(void)
+struct bitmap_index *prepare_bitmap_git(struct repository *r)
{
struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
- if (!open_pack_bitmap(bitmap_git) && !load_pack_bitmap(bitmap_git))
+ if (!open_pack_bitmap(r, bitmap_git) && !load_pack_bitmap(bitmap_git))
return bitmap_git;
free_bitmap_index(bitmap_git);
@@ -690,7 +691,7 @@ struct bitmap_index *prepare_bitmap_walk(struct rev_info
*revs)
struct bitmap_index *bitmap_git = xcalloc(1, sizeof(*bitmap_git));
/* try to open a bitmapped pack, but don't parse it yet
* because we may not need to use it */
- if (open_pack_bitmap(bitmap_git) < 0)
+ if (open_pack_bitmap(revs->repo, bitmap_git) < 0)
goto cleanup;
for (i = 0; i < revs->pending.nr; ++i) {
@@ -955,7 +956,7 @@ void test_bitmap_walk(struct rev_info *revs)
struct bitmap_test_data tdata;
struct bitmap_index *bitmap_git;
- if (!(bitmap_git = prepare_bitmap_git()))
+ if (!(bitmap_git = prepare_bitmap_git(revs->repo)))
die("failed to load bitmap indexes");
if (revs->pending.nr != 1)
diff --git a/pack-bitmap.h b/pack-bitmap.h
index 189dd68ad3..8418ba8c79 100644
--- a/pack-bitmap.h
+++ b/pack-bitmap.h
@@ -6,6 +6,7 @@
#include "pack-objects.h"
struct commit;
+struct repository;
struct rev_info;
struct bitmap_disk_header {
@@ -39,7 +40,7 @@ typedef int (*show_reachable_fn)(
struct bitmap_index;
-struct bitmap_index *prepare_bitmap_git(void);
+struct bitmap_index *prepare_bitmap_git(struct repository *r);
void count_bitmap_commit_list(struct bitmap_index *, uint32_t *commits,
uint32_t *trees, uint32_t *blobs, uint32_t *tags);
void traverse_bitmap_commit_list(struct bitmap_index *,
diff --git a/pack-objects.c b/pack-objects.c
index b6cdbb0166..9c45842df3 100644
--- a/pack-objects.c
+++ b/pack-objects.c
@@ -99,7 +99,7 @@ static void prepare_in_pack_by_idx(struct packing_data *pdata)
*
[PATCH v2 19/22] pack-check.c: remove the_repository references
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/fsck.c | 3 ++-
pack-check.c | 9 +
pack.h | 4 +++-
3 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 06eb421720..8ea2823864 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -752,7 +752,8 @@ int cmd_fsck(int argc, const char **argv, const char
*prefix)
for (p = get_all_packs(the_repository); p;
p = p->next) {
/* verify gives error messages itself */
- if (verify_pack(p, fsck_obj_buffer,
+ if (verify_pack(the_repository,
+ p, fsck_obj_buffer,
progress, count))
errors_found |= ERROR_PACK;
count += p->num_objects;
diff --git a/pack-check.c b/pack-check.c
index fa5f0ff8fa..2cc3603189 100644
--- a/pack-check.c
+++ b/pack-check.c
@@ -48,7 +48,8 @@ int check_pack_crc(struct packed_git *p, struct pack_window
**w_curs,
return data_crc != ntohl(*index_crc);
}
-static int verify_packfile(struct packed_git *p,
+static int verify_packfile(struct repository *r,
+ struct packed_git *p,
struct pack_window **w_curs,
verify_fn fn,
struct progress *progress, uint32_t base_count)
@@ -135,7 +136,7 @@ static int verify_packfile(struct packed_git *p,
data = NULL;
data_valid = 0;
} else {
- data = unpack_entry(the_repository, p,
entries[i].offset, , );
+ data = unpack_entry(r, p, entries[i].offset, ,
);
data_valid = 1;
}
@@ -186,7 +187,7 @@ int verify_pack_index(struct packed_git *p)
return err;
}
-int verify_pack(struct packed_git *p, verify_fn fn,
+int verify_pack(struct repository *r, struct packed_git *p, verify_fn fn,
struct progress *progress, uint32_t base_count)
{
int err = 0;
@@ -196,7 +197,7 @@ int verify_pack(struct packed_git *p, verify_fn fn,
if (!p->index_data)
return -1;
- err |= verify_packfile(p, _curs, fn, progress, base_count);
+ err |= verify_packfile(r, p, _curs, fn, progress, base_count);
unuse_pack(_curs);
return err;
diff --git a/pack.h b/pack.h
index 34a9d458b4..da99fdd1d2 100644
--- a/pack.h
+++ b/pack.h
@@ -4,6 +4,8 @@
#include "object.h"
#include "csum-file.h"
+struct repository;
+
/*
* Packed object header
*/
@@ -80,7 +82,7 @@ typedef int (*verify_fn)(const struct object_id *, enum
object_type, unsigned lo
extern const char *write_idx_file(const char *index_name, struct
pack_idx_entry **objects, int nr_objects, const struct pack_idx_option *, const
unsigned char *sha1);
extern int check_pack_crc(struct packed_git *p, struct pack_window **w_curs,
off_t offset, off_t len, unsigned int nr);
extern int verify_pack_index(struct packed_git *);
-extern int verify_pack(struct packed_git *, verify_fn fn, struct progress *,
uint32_t);
+extern int verify_pack(struct repository *, struct packed_git *, verify_fn fn,
struct progress *, uint32_t);
extern off_t write_pack_header(struct hashfile *f, uint32_t);
extern void fixup_pack_header_footer(int, unsigned char *, const char *,
uint32_t, unsigned char *, off_t);
extern char *index_pack_lockfile(int fd);
--
2.19.1.1231.g84aef82467
[PATCH v2 09/22] sequencer.c: remove implicit dependency on the_repository
Note that the_hash_algo stays, even if we can easily replace it with
repo->hash_algo. My reason is I still believe tying hash_algo to a
struct repository is a wrong move. But if I'm wrong, we can always go
for another round of conversion.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/rebase--interactive.c | 11 +--
builtin/revert.c | 2 +-
rebase-interactive.c | 4 +-
sequencer.c | 134 ++
sequencer.h | 12 +--
5 files changed, 86 insertions(+), 77 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index b7ce48a1ec..d867050fd2 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -114,7 +114,8 @@ static int do_interactive_rebase(struct replay_opts *opts,
unsigned flags,
error(_("could not generate todo list"));
else {
discard_cache();
- ret = complete_action(opts, flags, shortrevisions, onto_name,
onto,
+ ret = complete_action(the_repository, opts, flags,
+ shortrevisions, onto_name, onto,
head_hash, cmd, autosquash);
}
@@ -252,16 +253,16 @@ int cmd_rebase__interactive(int argc, const char **argv,
const char *prefix)
}
case SHORTEN_OIDS:
case EXPAND_OIDS:
- ret = transform_todos(flags);
+ ret = transform_todos(the_repository, flags);
break;
case CHECK_TODO_LIST:
- ret = check_todo_list();
+ ret = check_todo_list(the_repository);
break;
case REARRANGE_SQUASH:
- ret = rearrange_squash();
+ ret = rearrange_squash(the_repository);
break;
case ADD_EXEC:
- ret = sequencer_add_exec_commands(cmd);
+ ret = sequencer_add_exec_commands(the_repository, cmd);
break;
default:
BUG("invalid command '%d'", command);
diff --git a/builtin/revert.c b/builtin/revert.c
index cd9f068195..1fa75b2773 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -201,7 +201,7 @@ static int run_sequencer(int argc, const char **argv,
struct replay_opts *opts)
if (cmd == 'c')
return sequencer_continue(the_repository, opts);
if (cmd == 'a')
- return sequencer_rollback(opts);
+ return sequencer_rollback(the_repository, opts);
return sequencer_pick_revisions(the_repository, opts);
}
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 78f3263fc1..3cc19283ff 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -69,7 +69,7 @@ int edit_todo_list(unsigned flags)
strbuf_release();
- transform_todos(flags | TODO_LIST_SHORTEN_IDS);
+ transform_todos(the_repository, flags | TODO_LIST_SHORTEN_IDS);
if (strbuf_read_file(, todo_file, 0) < 0)
return error_errno(_("could not read '%s'."), todo_file);
@@ -85,7 +85,7 @@ int edit_todo_list(unsigned flags)
if (launch_sequence_editor(todo_file, NULL, NULL))
return -1;
- transform_todos(flags & ~(TODO_LIST_SHORTEN_IDS));
+ transform_todos(the_repository, flags & ~(TODO_LIST_SHORTEN_IDS));
return 0;
}
diff --git a/sequencer.c b/sequencer.c
index 6cf3f65b0c..111cb747d6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -356,7 +356,8 @@ static void free_message(struct commit *commit, struct
commit_message *msg)
unuse_commit_buffer(commit, msg->message);
}
-static void print_advice(int show_hint, struct replay_opts *opts)
+static void print_advice(struct repository *r, int show_hint,
+struct replay_opts *opts)
{
char *msg = getenv("GIT_CHERRY_PICK_HELP");
@@ -367,7 +368,7 @@ static void print_advice(int show_hint, struct replay_opts
*opts)
* (typically rebase --interactive) wants to take care
* of the commit itself so remove CHERRY_PICK_HEAD
*/
- unlink(git_path_cherry_pick_head(the_repository));
+ unlink(git_path_cherry_pick_head(r));
return;
}
@@ -440,9 +441,9 @@ static int read_oneliner(struct strbuf *buf,
return 1;
}
-static struct tree *empty_tree(void)
+static struct tree *empty_tree(struct repository *r)
{
- return lookup_tree(the_repository,
the_repository->hash_algo->empty_tree);
+ return lookup_tree(r, the_hash_algo->empty_tree);
}
static int error_dirty_index(struct index_state *istate, struct replay_opts
*opts)
@@ -553,8 +554,8 @@ static int do_recursive_merge(struct repository *r,
o.show_rename_progress = 1;
head_tree = parse_tree_indirect(head);
- next_tree = next ? get_commit_tree(next) : empty_tree();
- base_tree = base ? get_commit_tree(base) :
[PATCH v2 05/22] notes-merge.c: remove implicit dependency on the_index
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/notes.c | 2 +-
notes-merge.c | 12 +++-
notes-merge.h | 5 -
3 files changed, 12 insertions(+), 7 deletions(-)
diff --git a/builtin/notes.c b/builtin/notes.c
index c05cd004ab..15a6c78855 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c
@@ -808,7 +808,7 @@ static int merge(int argc, const char **argv, const char
*prefix)
usage_with_options(git_notes_merge_usage, options);
}
- init_notes_merge_options();
+ init_notes_merge_options(the_repository, );
o.verbosity = verbosity + NOTES_MERGE_VERBOSITY_DEFAULT;
if (do_abort)
diff --git a/notes-merge.c b/notes-merge.c
index bd05d50b05..0c22f09b28 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -18,11 +18,13 @@ struct notes_merge_pair {
struct object_id obj, base, local, remote;
};
-void init_notes_merge_options(struct notes_merge_options *o)
+void init_notes_merge_options(struct repository *r,
+ struct notes_merge_options *o)
{
memset(o, 0, sizeof(struct notes_merge_options));
strbuf_init(&(o->commit_msg), 0);
o->verbosity = NOTES_MERGE_VERBOSITY_DEFAULT;
+ o->repo = r;
}
static int path_to_oid(const char *path, struct object_id *oid)
@@ -127,7 +129,7 @@ static struct notes_merge_pair *diff_tree_remote(struct
notes_merge_options *o,
trace_printf("\tdiff_tree_remote(base = %.7s, remote = %.7s)\n",
oid_to_hex(base), oid_to_hex(remote));
- repo_diff_setup(the_repository, );
+ repo_diff_setup(o->repo, );
opt.flags.recursive = 1;
opt.output_format = DIFF_FORMAT_NO_OUTPUT;
diff_setup_done();
@@ -190,7 +192,7 @@ static void diff_tree_local(struct notes_merge_options *o,
trace_printf("\tdiff_tree_local(len = %i, base = %.7s, local = %.7s)\n",
len, oid_to_hex(base), oid_to_hex(local));
- repo_diff_setup(the_repository, );
+ repo_diff_setup(o->repo, );
opt.flags.recursive = 1;
opt.output_format = DIFF_FORMAT_NO_OUTPUT;
diff_setup_done();
@@ -350,7 +352,7 @@ static int ll_merge_in_worktree(struct notes_merge_options
*o,
status = ll_merge(_buf, oid_to_hex(>obj), , NULL,
, o->local_ref, , o->remote_ref,
- _index, NULL);
+ o->repo->index, NULL);
free(base.ptr);
free(local.ptr);
@@ -711,7 +713,7 @@ int notes_merge_commit(struct notes_merge_options *o,
/* write file as blob, and add to partial_tree */
if (stat(path.buf, ))
die_errno("Failed to stat '%s'", path.buf);
- if (index_path(_index, _oid, path.buf, ,
HASH_WRITE_OBJECT))
+ if (index_path(o->repo->index, _oid, path.buf, ,
HASH_WRITE_OBJECT))
die("Failed to write blob object from '%s'", path.buf);
if (add_note(partial_tree, _oid, _oid, NULL))
die("Failed to add resolved note '%s' to notes tree",
diff --git a/notes-merge.h b/notes-merge.h
index 6c74e9385b..99f9c709c5 100644
--- a/notes-merge.h
+++ b/notes-merge.h
@@ -6,6 +6,7 @@
struct commit;
struct object_id;
+struct repository;
#define NOTES_MERGE_WORKTREE "NOTES_MERGE_WORKTREE"
@@ -15,6 +16,7 @@ enum notes_merge_verbosity {
};
struct notes_merge_options {
+ struct repository *repo;
const char *local_ref;
const char *remote_ref;
struct strbuf commit_msg;
@@ -23,7 +25,8 @@ struct notes_merge_options {
unsigned has_worktree:1;
};
-void init_notes_merge_options(struct notes_merge_options *o);
+void init_notes_merge_options(struct repository *r,
+ struct notes_merge_options *o);
/*
* Merge notes from o->remote_ref into o->local_ref
--
2.19.1.1231.g84aef82467
[PATCH v2 21/22] rerere.c: remove the_repository references
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/am.c | 2 +-
builtin/rebase--interactive.c | 2 +-
builtin/rebase.c | 4 ++--
builtin/rerere.c | 10 ++
rerere.c | 26 +-
rerere.h | 6 +++---
6 files changed, 26 insertions(+), 24 deletions(-)
diff --git a/builtin/am.c b/builtin/am.c
index 232f3962d7..f0eeec82a9 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2033,7 +2033,7 @@ static int clean_index(const struct object_id *head,
const struct object_id *rem
static void am_rerere_clear(void)
{
struct string_list merge_rr = STRING_LIST_INIT_DUP;
- rerere_clear(_rr);
+ rerere_clear(the_repository, _rr);
string_list_clear(_rr, 1);
}
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index d867050fd2..bd7d39e2af 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -233,7 +233,7 @@ int cmd_rebase__interactive(int argc, const char **argv,
const char *prefix)
case SKIP: {
struct string_list merge_rr = STRING_LIST_INIT_DUP;
- rerere_clear(_rr);
+ rerere_clear(the_repository, _rr);
/* fallthrough */
case CONTINUE:
ret = sequencer_continue(the_repository, );
diff --git a/builtin/rebase.c b/builtin/rebase.c
index d9bedecf86..bc07d932a5 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -997,7 +997,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
options.action = "skip";
- rerere_clear(_rr);
+ rerere_clear(the_repository, _rr);
string_list_clear(_rr, 1);
if (reset_head(NULL, "reset", NULL, 0, NULL, NULL) < 0)
@@ -1010,7 +1010,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
struct string_list merge_rr = STRING_LIST_INIT_DUP;
options.action = "abort";
- rerere_clear(_rr);
+ rerere_clear(the_repository, _rr);
string_list_clear(_rr, 1);
if (read_basic_state())
diff --git a/builtin/rerere.c b/builtin/rerere.c
index e89ccbc524..6f28c19b20 100644
--- a/builtin/rerere.c
+++ b/builtin/rerere.c
@@ -82,11 +82,12 @@ int cmd_rerere(int argc, const char **argv, const char
*prefix)
}
if (!strcmp(argv[0], "clear")) {
- rerere_clear(_rr);
+ rerere_clear(the_repository, _rr);
} else if (!strcmp(argv[0], "gc"))
- rerere_gc(_rr);
+ rerere_gc(the_repository, _rr);
else if (!strcmp(argv[0], "status")) {
- if (setup_rerere(_rr, flags | RERERE_READONLY) < 0)
+ if (setup_rerere(the_repository, _rr,
+flags | RERERE_READONLY) < 0)
return 0;
for (i = 0; i < merge_rr.nr; i++)
printf("%s\n", merge_rr.items[i].string);
@@ -101,7 +102,8 @@ int cmd_rerere(int argc, const char **argv, const char
*prefix)
merge_rr.items[i].util = NULL;
}
} else if (!strcmp(argv[0], "diff")) {
- if (setup_rerere(_rr, flags | RERERE_READONLY) < 0)
+ if (setup_rerere(the_repository, _rr,
+flags | RERERE_READONLY) < 0)
return 0;
for (i = 0; i < merge_rr.nr; i++) {
const char *path = merge_rr.items[i].string;
diff --git a/rerere.c b/rerere.c
index b5b2357411..13624038e6 100644
--- a/rerere.c
+++ b/rerere.c
@@ -198,10 +198,10 @@ static struct rerere_id *new_rerere_id(unsigned char
*sha1)
* work on (i.e. what is left by the previous invocation of "git
* rerere" during the current conflict resolution session).
*/
-static void read_rr(struct string_list *rr)
+static void read_rr(struct repository *r, struct string_list *rr)
{
struct strbuf buf = STRBUF_INIT;
- FILE *in = fopen_or_warn(git_path_merge_rr(the_repository), "r");
+ FILE *in = fopen_or_warn(git_path_merge_rr(r), "r");
if (!in)
return;
@@ -593,7 +593,7 @@ int rerere_remaining(struct repository *r, struct
string_list *merge_rr)
{
int i;
- if (setup_rerere(merge_rr, RERERE_READONLY))
+ if (setup_rerere(r, merge_rr, RERERE_READONLY))
return 0;
if (read_index(r->index) < 0)
return error(_("index file corrupt"));
@@ -882,7 +882,7 @@ static int is_rerere_enabled(void)
return 1;
}
-int setup_rerere(struct string_list *merge_rr, int flags)
+int setup_rerere(struct repository *r, struct string_list *merge_rr, int flags)
{
int fd;
@@ -896,9 +896,9 @@ int setup_rerere(struct string_list *merge_rr, int flags)
fd = 0;
else
fd =
[PATCH v2 12/22] branch.c: remove the_repository reference
Signed-off-by: Nguyễn Thái Ngọc Duy
---
branch.c | 21 +++--
branch.h | 8 ++--
builtin/am.c | 2 +-
builtin/branch.c | 6 --
builtin/checkout.c | 5 +++--
builtin/reset.c| 2 +-
builtin/revert.c | 2 +-
7 files changed, 27 insertions(+), 19 deletions(-)
diff --git a/branch.c b/branch.c
index 776f55fc66..28b81a7e02 100644
--- a/branch.c
+++ b/branch.c
@@ -242,7 +242,8 @@ N_("\n"
"will track its remote counterpart, you may want to use\n"
"\"git push -u\" to set the upstream config as you push.");
-void create_branch(const char *name, const char *start_name,
+void create_branch(struct repository *r,
+ const char *name, const char *start_name,
int force, int clobber_head_ok, int reflog,
int quiet, enum branch_track track)
{
@@ -300,7 +301,7 @@ void create_branch(const char *name, const char *start_name,
break;
}
- if ((commit = lookup_commit_reference(the_repository, )) == NULL)
+ if ((commit = lookup_commit_reference(r, )) == NULL)
die(_("Not a valid branch point: '%s'."), start_name);
oidcpy(, >object.oid);
@@ -336,15 +337,15 @@ void create_branch(const char *name, const char
*start_name,
free(real_ref);
}
-void remove_branch_state(void)
+void remove_branch_state(struct repository *r)
{
- unlink(git_path_cherry_pick_head(the_repository));
- unlink(git_path_revert_head(the_repository));
- unlink(git_path_merge_head(the_repository));
- unlink(git_path_merge_rr(the_repository));
- unlink(git_path_merge_msg(the_repository));
- unlink(git_path_merge_mode(the_repository));
- unlink(git_path_squash_msg(the_repository));
+ unlink(git_path_cherry_pick_head(r));
+ unlink(git_path_revert_head(r));
+ unlink(git_path_merge_head(r));
+ unlink(git_path_merge_rr(r));
+ unlink(git_path_merge_msg(r));
+ unlink(git_path_merge_mode(r));
+ unlink(git_path_squash_msg(r));
}
void die_if_checked_out(const char *branch, int ignore_current_worktree)
diff --git a/branch.h b/branch.h
index 5cace4581f..29c1afa4d0 100644
--- a/branch.h
+++ b/branch.h
@@ -1,6 +1,7 @@
#ifndef BRANCH_H
#define BRANCH_H
+struct repository;
struct strbuf;
enum branch_track {
@@ -19,6 +20,8 @@ extern enum branch_track git_branch_track;
/*
* Creates a new branch, where:
*
+ * - r is the repository to add a branch to
+ *
* - name is the new branch name
*
* - start_name is the name of the existing branch that the new branch should
@@ -37,7 +40,8 @@ extern enum branch_track git_branch_track;
* that start_name is a tracking branch for (if any).
*
*/
-void create_branch(const char *name, const char *start_name,
+void create_branch(struct repository *r,
+ const char *name, const char *start_name,
int force, int clobber_head_ok,
int reflog, int quiet, enum branch_track track);
@@ -60,7 +64,7 @@ extern int validate_new_branchname(const char *name, struct
strbuf *ref, int for
* Remove information about the state of working on the current
* branch. (E.g., MERGE_HEAD)
*/
-void remove_branch_state(void);
+void remove_branch_state(struct repository *r);
/*
* Configure local branch "local" as downstream to branch "remote"
diff --git a/builtin/am.c b/builtin/am.c
index 3ee9a9d2a9..232f3962d7 100644
--- a/builtin/am.c
+++ b/builtin/am.c
@@ -2022,7 +2022,7 @@ static int clean_index(const struct object_id *head,
const struct object_id *rem
if (merge_tree(remote_tree))
return -1;
- remove_branch_state();
+ remove_branch_state(the_repository);
return 0;
}
diff --git a/builtin/branch.c b/builtin/branch.c
index 0c55f7f065..1be727209b 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -783,7 +783,8 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
* create_branch takes care of setting up the tracking
* info and making sure new_upstream is correct
*/
- create_branch(branch->name, new_upstream, 0, 0, 0, quiet,
BRANCH_TRACK_OVERRIDE);
+ create_branch(the_repository, branch->name, new_upstream,
+ 0, 0, 0, quiet, BRANCH_TRACK_OVERRIDE);
} else if (unset_upstream) {
struct branch *branch = branch_get(argv[0]);
struct strbuf buf = STRBUF_INIT;
@@ -814,7 +815,8 @@ int cmd_branch(int argc, const char **argv, const char
*prefix)
if (track == BRANCH_TRACK_OVERRIDE)
die(_("the '--set-upstream' option is no longer
supported. Please use '--track' or '--set-upstream-to' instead."));
- create_branch(argv[0], (argc == 2) ? argv[1] : head,
+ create_branch(the_repository,
+ argv[0], (argc == 2) ?
[PATCH v2 04/22] list-objects.c: reduce the_repository references
Signed-off-by: Nguyễn Thái Ngọc Duy
---
list-objects.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/list-objects.c b/list-objects.c
index 0cfd646026..cf7f25bed3 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -123,7 +123,7 @@ static void process_tree_contents(struct traversal_context
*ctx,
}
if (S_ISDIR(entry.mode)) {
- struct tree *t = lookup_tree(the_repository, entry.oid);
+ struct tree *t = lookup_tree(ctx->revs->repo,
entry.oid);
t->object.flags |= NOT_USER_GIVEN;
process_tree(ctx, t, base, entry.path);
}
@@ -131,7 +131,7 @@ static void process_tree_contents(struct traversal_context
*ctx,
process_gitlink(ctx, entry.oid->hash,
base, entry.path);
else {
- struct blob *b = lookup_blob(the_repository, entry.oid);
+ struct blob *b = lookup_blob(ctx->revs->repo,
entry.oid);
b->object.flags |= NOT_USER_GIVEN;
process_blob(ctx, b, base, entry.path);
}
--
2.19.1.1231.g84aef82467
[PATCH v2 00/22] Kill the_index part 5
(Please ignore v1 which contains unrelated patches) I lied about part 5 being final. This series contains most of my WIP series [2] except sha1-name.c, read-cache.c and merge-recursive.c because there will be conflicts on 'pu' so they will be in part 6. At least this only results in two small conflicts in sequencer.c. I did start pushing the_repository out of library code [1] in the bottom half of this series. There aren't big surprises except that cache-tree API now take _both_ struct repository and struct index_state. The reason is cache-tree can work on temporary indexes and repo->index is about $GIT_DIR/index. [1] Stefan and I make a good team. He keeps adding the_repository. I keep removing. We both hold hands leading commit counts (with Brian just a bit behind frantically replacing sha1 with oid to keep up). Meanwhile Junio is crying in the corner because of too many conflicts. :-D [2] https://public-inbox.org/git/20181019145237.16079-1-pclo...@gmail.com/ Nguyễn Thái Ngọc Duy (22): wt-status.c: remove implicit dependency on the_index wt-status.c: remove implicit dependency the_repository list-objects-filter.c: remove implicit dependency on the_index list-objects.c: reduce the_repository references notes-merge.c: remove implicit dependency on the_index notes-merge.c: remove implicit dependency the_repository transport.c: remove implicit dependency on the_index sequencer.c: remove implicit dependency on the_index sequencer.c: remove implicit dependency on the_repository blame.c: remove implicit dependency the_repository bisect.c: remove the_repository reference branch.c: remove the_repository reference bundle.c: remove the_repository references cache-tree.c: remove the_repository references delta-islands.c: remove the_repository references diff-lib.c: remove the_repository references line-log.c: remove the_repository reference notes-cache.c: remove the_repository references pack-check.c: remove the_repository references pack-*.c: remove the_repository references rerere.c: remove the_repository references rebase-interactive.c: remove the_repository references bisect.c | 48 ++-- bisect.h | 5 +- blame.c | 39 +-- branch.c | 21 +- branch.h | 8 +- builtin/am.c | 4 +- builtin/bisect--helper.c | 2 +- builtin/branch.c | 6 +- builtin/bundle.c | 7 +- builtin/checkout.c| 5 +- builtin/commit.c | 8 +- builtin/fsck.c| 3 +- builtin/merge-ours.c | 2 +- builtin/merge.c | 2 +- builtin/notes.c | 2 +- builtin/pack-objects.c| 6 +- builtin/pull.c| 3 +- builtin/push.c| 3 +- builtin/read-tree.c | 4 +- builtin/rebase--interactive.c | 19 +- builtin/rebase.c | 13 +- builtin/rerere.c | 10 +- builtin/reset.c | 4 +- builtin/revert.c | 8 +- bundle.c | 26 +- bundle.h | 9 +- cache-tree.c | 26 +- cache-tree.h | 4 +- combine-diff.c| 2 +- delta-islands.c | 24 +- delta-islands.h | 9 +- diff-lib.c| 7 +- diff.c| 12 +- diff.h| 5 +- diffcore-pickaxe.c| 4 +- grep.c| 2 +- line-log.c| 2 +- list-objects-filter.c | 10 +- list-objects-filter.h | 2 + list-objects.c| 13 +- notes-cache.c | 12 +- notes-cache.h | 6 +- notes-merge.c | 16 +- notes-merge.h | 5 +- pack-bitmap-write.c | 6 +- pack-bitmap.c | 13 +- pack-bitmap.h | 3 +- pack-check.c | 9 +- pack-objects.c| 6 +- pack-objects.h| 5 +- pack.h| 4 +- read-cache.c | 2 +- rebase-interactive.c | 6 +- rebase-interactive.h | 5 +- ref-filter.c | 2 +- rerere.c | 26 +- rerere.h | 6 +- sequencer.c | 453 +++--- sequencer.h | 27 +- transport.c | 9 +- transport.h | 3 +- unpack-trees.c| 2 +- userdiff.c| 5 +- userdiff.h| 4 +- wt-status.c | 94 --- wt-status.h | 21 +- 66 files changed, 650 insertions(+), 489 deletions(-) -- 2.19.1.1231.g84aef82467
[PATCH v2 06/22] notes-merge.c: remove implicit dependency the_repository
Signed-off-by: Nguyễn Thái Ngọc Duy
---
notes-merge.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/notes-merge.c b/notes-merge.c
index 0c22f09b28..72688d301b 100644
--- a/notes-merge.c
+++ b/notes-merge.c
@@ -558,7 +558,7 @@ int notes_merge(struct notes_merge_options *o,
else if (!check_refname_format(o->local_ref, 0) &&
is_null_oid(_oid))
local = NULL; /* local_oid == null_oid indicates unborn ref */
- else if (!(local = lookup_commit_reference(the_repository, _oid)))
+ else if (!(local = lookup_commit_reference(o->repo, _oid)))
die("Could not parse local commit %s (%s)",
oid_to_hex(_oid), o->local_ref);
trace_printf("\tlocal commit: %.7s\n", oid_to_hex(_oid));
@@ -576,7 +576,7 @@ int notes_merge(struct notes_merge_options *o,
die("Failed to resolve remote notes ref '%s'",
o->remote_ref);
}
- } else if (!(remote = lookup_commit_reference(the_repository,
_oid))) {
+ } else if (!(remote = lookup_commit_reference(o->repo, _oid))) {
die("Could not parse remote commit %s (%s)",
oid_to_hex(_oid), o->remote_ref);
}
--
2.19.1.1231.g84aef82467
[PATCH v2 15/22] delta-islands.c: remove the_repository references
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/pack-objects.c | 4 ++--
delta-islands.c| 24 ++--
delta-islands.h| 9 ++---
3 files changed, 22 insertions(+), 15 deletions(-)
diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index c99ee79c31..7812c2b1f3 100644
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -2628,7 +2628,7 @@ static void prepare_pack(int window, int depth)
unsigned n;
if (use_delta_islands)
- resolve_tree_islands(progress, _pack);
+ resolve_tree_islands(the_repository, progress, _pack);
get_object_details();
@@ -3143,7 +3143,7 @@ static void get_object_list(int ac, const char **av)
return;
if (use_delta_islands)
- load_delta_islands();
+ load_delta_islands(the_repository);
if (prepare_revision_walk())
die(_("revision walk setup failed"));
diff --git a/delta-islands.c b/delta-islands.c
index 8e5018e406..191a930705 100644
--- a/delta-islands.c
+++ b/delta-islands.c
@@ -190,13 +190,15 @@ static void set_island_marks(struct object *obj, struct
island_bitmap *marks)
island_bitmap_or(b, marks);
}
-static void mark_remote_island_1(struct remote_island *rl, int is_core_island)
+static void mark_remote_island_1(struct repository *r,
+struct remote_island *rl,
+int is_core_island)
{
uint32_t i;
for (i = 0; i < rl->oids.nr; ++i) {
struct island_bitmap *marks;
- struct object *obj = parse_object(the_repository,
>oids.oid[i]);
+ struct object *obj = parse_object(r, >oids.oid[i]);
if (!obj)
continue;
@@ -211,7 +213,7 @@ static void mark_remote_island_1(struct remote_island *rl,
int is_core_island)
while (obj && obj->type == OBJ_TAG) {
obj = ((struct tag *)obj)->tagged;
if (obj) {
- parse_object(the_repository, >oid);
+ parse_object(r, >oid);
marks = create_or_get_island_marks(obj);
island_bitmap_set(marks, island_counter);
}
@@ -237,7 +239,9 @@ static int tree_depth_compare(const void *a, const void *b)
return todo_a->depth - todo_b->depth;
}
-void resolve_tree_islands(int progress, struct packing_data *to_pack)
+void resolve_tree_islands(struct repository *r,
+ int progress,
+ struct packing_data *to_pack)
{
struct progress *progress_state = NULL;
struct tree_islands_todo *todo;
@@ -281,7 +285,7 @@ void resolve_tree_islands(int progress, struct packing_data
*to_pack)
root_marks = kh_value(island_marks, pos);
- tree = lookup_tree(the_repository, >idx.oid);
+ tree = lookup_tree(r, >idx.oid);
if (!tree || parse_tree(tree) < 0)
die(_("bad tree object %s"), oid_to_hex(>idx.oid));
@@ -292,7 +296,7 @@ void resolve_tree_islands(int progress, struct packing_data
*to_pack)
if (S_ISGITLINK(entry.mode))
continue;
- obj = lookup_object(the_repository, entry.oid->hash);
+ obj = lookup_object(r, entry.oid->hash);
if (!obj)
continue;
@@ -415,7 +419,7 @@ static struct remote_island *get_core_island(void)
return NULL;
}
-static void deduplicate_islands(void)
+static void deduplicate_islands(struct repository *r)
{
struct remote_island *island, *core = NULL, **list;
unsigned int island_count, dst, src, ref, i = 0;
@@ -444,20 +448,20 @@ static void deduplicate_islands(void)
core = get_core_island();
for (i = 0; i < island_count; ++i) {
- mark_remote_island_1(list[i], core && list[i]->hash ==
core->hash);
+ mark_remote_island_1(r, list[i], core && list[i]->hash ==
core->hash);
}
free(list);
}
-void load_delta_islands(void)
+void load_delta_islands(struct repository *r)
{
island_marks = kh_init_sha1();
remote_islands = kh_init_str();
git_config(island_config_callback, NULL);
for_each_ref(find_island_for_ref, NULL);
- deduplicate_islands();
+ deduplicate_islands(r);
fprintf(stderr, _("Marked %d islands, done.\n"), island_counter);
}
diff --git a/delta-islands.h b/delta-islands.h
index b635cd07d8..3ac8045d8c 100644
--- a/delta-islands.h
+++ b/delta-islands.h
@@ -1,14 +1,17 @@
#ifndef DELTA_ISLANDS_H
#define DELTA_ISLANDS_H
+struct commit;
struct object_id;
struct packing_data;
-struct commit;
+struct repository;
int island_delta_cmp(const struct
[PATCH v2 17/22] line-log.c: remove the_repository reference
Signed-off-by: Nguyễn Thái Ngọc Duy
---
line-log.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/line-log.c b/line-log.c
index d1d429d738..24e21731c4 100644
--- a/line-log.c
+++ b/line-log.c
@@ -479,7 +479,7 @@ static struct commit *check_single_commit(struct rev_info
*revs)
struct object *obj = revs->pending.objects[i].item;
if (obj->flags & UNINTERESTING)
continue;
- obj = deref_tag(the_repository, obj, NULL, 0);
+ obj = deref_tag(revs->repo, obj, NULL, 0);
if (obj->type != OBJ_COMMIT)
die("Non commit %s?", revs->pending.objects[i].name);
if (commit)
--
2.19.1.1231.g84aef82467
[PATCH v2 14/22] cache-tree.c: remove the_repository references
This case is more interesting than other boring "remove the_repo"
commits because while we need access to the object database, we cannot
simply use r->index because unpack-trees.c can operate on a temporary
index, not $GIT_DIR/index. Ideally we should be able to pass an object
database to lookup_tree() but that ship has sailed.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/read-tree.c | 4 +++-
builtin/rebase.c| 2 +-
builtin/reset.c | 2 +-
cache-tree.c| 26 +++---
cache-tree.h| 4 ++--
read-cache.c| 2 +-
sequencer.c | 2 +-
unpack-trees.c | 2 +-
8 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/builtin/read-tree.c b/builtin/read-tree.c
index fbbc98e516..c05c12d034 100644
--- a/builtin/read-tree.c
+++ b/builtin/read-tree.c
@@ -255,7 +255,9 @@ int cmd_read_tree(int argc, const char **argv, const char
*unused_prefix)
* what came from the tree.
*/
if (nr_trees == 1 && !opts.prefix)
- prime_cache_tree(_index, trees[0]);
+ prime_cache_tree(the_repository,
+the_repository->index,
+trees[0]);
if (write_locked_index(_index, _file, COMMIT_LOCK))
die("unable to write new index file");
diff --git a/builtin/rebase.c b/builtin/rebase.c
index b9eb958454..d9bedecf86 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -582,7 +582,7 @@ static int reset_head(struct object_id *oid, const char
*action,
}
tree = parse_tree_indirect(oid);
- prime_cache_tree(the_repository->index, tree);
+ prime_cache_tree(the_repository, the_repository->index, tree);
if (write_locked_index(the_repository->index, , COMMIT_LOCK) < 0)
ret = error(_("could not write index"));
diff --git a/builtin/reset.c b/builtin/reset.c
index 5b4bbb0fb5..161b5e0cae 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -93,7 +93,7 @@ static int reset_index(const struct object_id *oid, int
reset_type, int quiet)
if (reset_type == MIXED || reset_type == HARD) {
tree = parse_tree_indirect(oid);
- prime_cache_tree(_index, tree);
+ prime_cache_tree(the_repository, the_repository->index, tree);
}
ret = 0;
diff --git a/cache-tree.c b/cache-tree.c
index 9d454d24bc..190c6e5aa6 100644
--- a/cache-tree.c
+++ b/cache-tree.c
@@ -659,7 +659,9 @@ int write_index_as_tree(struct object_id *oid, struct
index_state *index_state,
return ret;
}
-static void prime_cache_tree_rec(struct cache_tree *it, struct tree *tree)
+static void prime_cache_tree_rec(struct repository *r,
+struct cache_tree *it,
+struct tree *tree)
{
struct tree_desc desc;
struct name_entry entry;
@@ -673,24 +675,25 @@ static void prime_cache_tree_rec(struct cache_tree *it,
struct tree *tree)
cnt++;
else {
struct cache_tree_sub *sub;
- struct tree *subtree = lookup_tree(the_repository,
- entry.oid);
+ struct tree *subtree = lookup_tree(r, entry.oid);
if (!subtree->object.parsed)
parse_tree(subtree);
sub = cache_tree_sub(it, entry.path);
sub->cache_tree = cache_tree();
- prime_cache_tree_rec(sub->cache_tree, subtree);
+ prime_cache_tree_rec(r, sub->cache_tree, subtree);
cnt += sub->cache_tree->entry_count;
}
}
it->entry_count = cnt;
}
-void prime_cache_tree(struct index_state *istate, struct tree *tree)
+void prime_cache_tree(struct repository *r,
+ struct index_state *istate,
+ struct tree *tree)
{
cache_tree_free(>cache_tree);
istate->cache_tree = cache_tree();
- prime_cache_tree_rec(istate->cache_tree, tree);
+ prime_cache_tree_rec(r, istate->cache_tree, tree);
istate->cache_changed |= CACHE_TREE_CHANGED;
}
@@ -726,7 +729,8 @@ int cache_tree_matches_traversal(struct cache_tree *root,
return 0;
}
-static void verify_one(struct index_state *istate,
+static void verify_one(struct repository *r,
+ struct index_state *istate,
struct cache_tree *it,
struct strbuf *path)
{
@@ -736,13 +740,13 @@ static void verify_one(struct index_state *istate,
for (i = 0; i < it->subtree_nr; i++) {
strbuf_addf(path, "%s/", it->down[i]->name);
- verify_one(istate, it->down[i]->cache_tree, path);
+ verify_one(r, istate, it->down[i]->cache_tree, path);
strbuf_setlen(path, len);
}
[PATCH v2 08/22] sequencer.c: remove implicit dependency on the_index
Since we're going to pass 'struct repository *' around most of the
time instead of 'struct index_state *' because most sequencer.c
operations need more than just the index, the_repository is replaced
as well in the functions that now take 'struct repository
*'. the_repository is still present in this file, but total clean up
will be done later. It's not the main focus of this patch.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/commit.c | 3 +-
builtin/merge.c | 2 +-
builtin/rebase--interactive.c | 4 +-
builtin/revert.c | 4 +-
sequencer.c | 321 +++---
sequencer.h | 15 +-
6 files changed, 197 insertions(+), 152 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 6637a928a7..e89bf35634 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -1679,7 +1679,8 @@ int cmd_commit(int argc, const char **argv, const char
*prefix)
flags |= SUMMARY_INITIAL_COMMIT;
if (author_date_is_interesting())
flags |= SUMMARY_SHOW_AUTHOR_DATE;
- print_commit_summary(prefix, , flags);
+ print_commit_summary(the_repository, prefix,
+, flags);
}
UNLEAK(err);
diff --git a/builtin/merge.c b/builtin/merge.c
index 4aa6071598..db22119c93 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -896,7 +896,7 @@ static int suggest_conflicts(void)
filename = git_path_merge_msg(the_repository);
fp = xfopen(filename, "a");
- append_conflicts_hint();
+ append_conflicts_hint(_index, );
fputs(msgbuf.buf, fp);
strbuf_release();
fclose(fp);
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index a2ab68ed06..b7ce48a1ec 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -105,7 +105,7 @@ static int do_interactive_rebase(struct replay_opts *opts,
unsigned flags,
if (restrict_revision)
argv_array_push(_script_args, restrict_revision);
- ret = sequencer_make_script(todo_list,
+ ret = sequencer_make_script(the_repository, todo_list,
make_script_args.argc,
make_script_args.argv,
flags);
fclose(todo_list);
@@ -235,7 +235,7 @@ int cmd_rebase__interactive(int argc, const char **argv,
const char *prefix)
rerere_clear(_rr);
/* fallthrough */
case CONTINUE:
- ret = sequencer_continue();
+ ret = sequencer_continue(the_repository, );
break;
}
case EDIT_TODO:
diff --git a/builtin/revert.c b/builtin/revert.c
index c93393c89b..cd9f068195 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -199,10 +199,10 @@ static int run_sequencer(int argc, const char **argv,
struct replay_opts *opts)
return ret;
}
if (cmd == 'c')
- return sequencer_continue(opts);
+ return sequencer_continue(the_repository, opts);
if (cmd == 'a')
return sequencer_rollback(opts);
- return sequencer_pick_revisions(opts);
+ return sequencer_pick_revisions(the_repository, opts);
}
int cmd_revert(int argc, const char **argv, const char *prefix)
diff --git a/sequencer.c b/sequencer.c
index 0b8f18fd36..6cf3f65b0c 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -445,9 +445,9 @@ static struct tree *empty_tree(void)
return lookup_tree(the_repository,
the_repository->hash_algo->empty_tree);
}
-static int error_dirty_index(struct replay_opts *opts)
+static int error_dirty_index(struct index_state *istate, struct replay_opts
*opts)
{
- if (read_cache_unmerged())
+ if (read_index_unmerged(istate))
return error_resolve_conflict(_(action_name(opts)));
error(_("your local changes would be overwritten by %s."),
@@ -472,15 +472,18 @@ static void update_abort_safety_file(void)
write_file(git_path_abort_safety_file(), "%s", "");
}
-static int fast_forward_to(const struct object_id *to, const struct object_id
*from,
- int unborn, struct replay_opts *opts)
+static int fast_forward_to(struct repository *r,
+ const struct object_id *to,
+ const struct object_id *from,
+ int unborn,
+ struct replay_opts *opts)
{
struct ref_transaction *transaction;
struct strbuf sb = STRBUF_INIT;
struct strbuf err = STRBUF_INIT;
- read_index(_index);
- if (checkout_fast_forward(the_repository, from, to, 1))
+ read_index(r->index);
+ if (checkout_fast_forward(r, from, to, 1))
return -1; /* the callee should have complained already */
strbuf_addf(, _("%s: fast-forward"),
[PATCH v2 22/22] rebase-interactive.c: remove the_repository references
While at there add a forward declaration for struct strbuf.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/rebase--interactive.c | 2 +-
rebase-interactive.c | 6 +++---
rebase-interactive.h | 5 -
3 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index bd7d39e2af..dd2a55ab1d 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -240,7 +240,7 @@ int cmd_rebase__interactive(int argc, const char **argv,
const char *prefix)
break;
}
case EDIT_TODO:
- ret = edit_todo_list(flags);
+ ret = edit_todo_list(the_repository, flags);
break;
case SHOW_CURRENT_PATCH: {
struct child_process cmd = CHILD_PROCESS_INIT;
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 3cc19283ff..68aff1dac2 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -53,7 +53,7 @@ void append_todo_help(unsigned edit_todo, unsigned keep_empty,
}
}
-int edit_todo_list(unsigned flags)
+int edit_todo_list(struct repository *r, unsigned flags)
{
struct strbuf buf = STRBUF_INIT;
const char *todo_file = rebase_path_todo();
@@ -69,7 +69,7 @@ int edit_todo_list(unsigned flags)
strbuf_release();
- transform_todos(the_repository, flags | TODO_LIST_SHORTEN_IDS);
+ transform_todos(r, flags | TODO_LIST_SHORTEN_IDS);
if (strbuf_read_file(, todo_file, 0) < 0)
return error_errno(_("could not read '%s'."), todo_file);
@@ -85,7 +85,7 @@ int edit_todo_list(unsigned flags)
if (launch_sequence_editor(todo_file, NULL, NULL))
return -1;
- transform_todos(the_repository, flags & ~(TODO_LIST_SHORTEN_IDS));
+ transform_todos(r, flags & ~(TODO_LIST_SHORTEN_IDS));
return 0;
}
diff --git a/rebase-interactive.h b/rebase-interactive.h
index 971da03776..17b6c9f6d0 100644
--- a/rebase-interactive.h
+++ b/rebase-interactive.h
@@ -1,8 +1,11 @@
#ifndef REBASE_INTERACTIVE_H
#define REBASE_INTERACTIVE_H
+struct strbuf;
+struct repository;
+
void append_todo_help(unsigned edit_todo, unsigned keep_empty,
struct strbuf *buf);
-int edit_todo_list(unsigned flags);
+int edit_todo_list(struct repository *r, unsigned flags);
#endif
--
2.19.1.1231.g84aef82467
[PATCH v3 08/16] reflog: mark strings for translation
One string "nothing to delete?" is rephrased to be more helpful.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/reflog.c | 34 +++---
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/builtin/reflog.c b/builtin/reflog.c
index b5941c1ff3..5a74ccf7ab 100644
--- a/builtin/reflog.c
+++ b/builtin/reflog.c
@@ -13,11 +13,15 @@
/* NEEDSWORK: switch to using parse_options */
static const char reflog_expire_usage[] =
-"git reflog expire [--expire=] [--expire-unreachable=] [--rewrite]
[--updateref] [--stale-fix] [--dry-run | -n] [--verbose] [--all] ...";
+N_("git reflog expire [--expire=] "
+ "[--expire-unreachable=] "
+ "[--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] "
+ "[--verbose] [--all] ...");
static const char reflog_delete_usage[] =
-"git reflog delete [--rewrite] [--updateref] [--dry-run | -n] [--verbose]
...";
+N_("git reflog delete [--rewrite] [--updateref] "
+ "[--dry-run | -n] [--verbose] ...");
static const char reflog_exists_usage[] =
-"git reflog exists ";
+N_("git reflog exists ");
static timestamp_t default_reflog_expire;
static timestamp_t default_reflog_expire_unreachable;
@@ -556,7 +560,7 @@ static int cmd_reflog_expire(int argc, const char **argv,
const char *prefix)
break;
}
else if (arg[0] == '-')
- usage(reflog_expire_usage);
+ usage(_(reflog_expire_usage));
else
break;
}
@@ -569,7 +573,7 @@ static int cmd_reflog_expire(int argc, const char **argv,
const char *prefix)
if (cb.cmd.stalefix) {
repo_init_revisions(the_repository, , prefix);
if (flags & EXPIRE_REFLOGS_VERBOSE)
- printf("Marking reachable objects...");
+ printf(_("Marking reachable objects..."));
mark_reachable_objects(, 0, 0, NULL);
if (flags & EXPIRE_REFLOGS_VERBOSE)
putchar('\n');
@@ -598,7 +602,7 @@ static int cmd_reflog_expire(int argc, const char **argv,
const char *prefix)
char *ref;
struct object_id oid;
if (!dwim_log(argv[i], strlen(argv[i]), , )) {
- status |= error("%s points nowhere!", argv[i]);
+ status |= error(_("%s points nowhere!"), argv[i]);
continue;
}
set_reflog_expiry_param(, explicit_expiry, ref);
@@ -644,13 +648,13 @@ static int cmd_reflog_delete(int argc, const char **argv,
const char *prefix)
break;
}
else if (arg[0] == '-')
- usage(reflog_delete_usage);
+ usage(_(reflog_delete_usage));
else
break;
}
if (argc - i < 1)
- return error("Nothing to delete?");
+ return error(_("no reflog specified to delete"));
for ( ; i < argc; i++) {
const char *spec = strstr(argv[i], "@{");
@@ -659,12 +663,12 @@ static int cmd_reflog_delete(int argc, const char **argv,
const char *prefix)
int recno;
if (!spec) {
- status |= error("Not a reflog: %s", argv[i]);
+ status |= error(_("not a reflog: %s"), argv[i]);
continue;
}
if (!dwim_log(argv[i], spec - argv[i], , )) {
- status |= error("no reflog for '%s'", argv[i]);
+ status |= error(_("no reflog for '%s'"), argv[i]);
continue;
}
@@ -699,7 +703,7 @@ static int cmd_reflog_exists(int argc, const char **argv,
const char *prefix)
break;
}
else if (arg[0] == '-')
- usage(reflog_exists_usage);
+ usage(_(reflog_exists_usage));
else
break;
}
@@ -707,10 +711,10 @@ static int cmd_reflog_exists(int argc, const char **argv,
const char *prefix)
start = i;
if (argc - start != 1)
- usage(reflog_exists_usage);
+ usage(_(reflog_exists_usage));
if (check_refname_format(argv[start], REFNAME_ALLOW_ONELEVEL))
- die("invalid ref format: %s", argv[start]);
+ die(_("invalid ref format: %s"), argv[start]);
return !reflog_exists(argv[start]);
}
@@ -719,12 +723,12 @@ static int cmd_reflog_exists(int argc, const char **argv,
const char *prefix)
*/
static const char reflog_usage[] =
-"git reflog [ show | expire | delete | exists ]";
+N_("git reflog [ show | expire | delete | exists ]");
int cmd_reflog(int argc, const char **argv, const char *prefix)
{
if (argc > 1 && !strcmp(argv[1], "-h"))
-
Re: [PATCH v2 12/16] parse-options: replace opterror() with optname()
On Tue, Nov 6, 2018 at 3:07 PM Ramsay Jones wrote: > Also, this patch does not replace opterror() calls outside of > the 'parse-options.c' file with optname(). This tickles my > static-check.pl script, since optname() is an external function > which is only called from 'parse-options.c'. > > So, at present, optname() could be marked as a local 'static' > symbol. However, I could also imagine it being used by new callers > outside of 'parse-options.c' in the future. (maybe) Your call. ;-) I was making it static, but the compiler complained about undefined function and I would need to either move optname() up in parse-options.c or add a forward declaration (but I was going to _remove_ the declaration!) Since it could be potentially used by Jeff's series (and I think it does have potential in parse-options-cb.c), I'll just leave it exported and caress your static-check.pl script (how did it not catch optbug() not being used outside parse-options.c either)? -- Duy
[PATCH 02/22] wt-status.c: remove implicit dependency the_repository
Signed-off-by: Nguyễn Thái Ngọc Duy
---
ref-filter.c | 2 +-
wt-status.c | 18 ++
wt-status.h | 4 +++-
3 files changed, 14 insertions(+), 10 deletions(-)
diff --git a/ref-filter.c b/ref-filter.c
index 0c45ed9d94..c4eaf30313 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -1413,7 +1413,7 @@ char *get_head_description(void)
struct strbuf desc = STRBUF_INIT;
struct wt_status_state state;
memset(, 0, sizeof(state));
- wt_status_get_state(, 1);
+ wt_status_get_state(the_repository, , 1);
if (state.rebase_in_progress ||
state.rebase_interactive_in_progress) {
if (state.branch)
diff --git a/wt-status.c b/wt-status.c
index 6d401b2c24..e582c54238 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -755,7 +755,7 @@ void wt_status_collect(struct wt_status *s)
wt_status_collect_changes_index(s);
wt_status_collect_untracked(s);
- wt_status_get_state(>state, s->branch && !strcmp(s->branch, "HEAD"));
+ wt_status_get_state(s->repo, >state, s->branch && !strcmp(s->branch,
"HEAD"));
if (s->state.merge_in_progress && !has_unmerged(s))
s->committable = 1;
}
@@ -1482,7 +1482,8 @@ static int grab_1st_switch(struct object_id *ooid, struct
object_id *noid,
return 1;
}
-static void wt_status_get_detached_from(struct wt_status_state *state)
+static void wt_status_get_detached_from(struct repository *r,
+ struct wt_status_state *state)
{
struct grab_1st_switch_cbdata cb;
struct commit *commit;
@@ -1499,7 +1500,7 @@ static void wt_status_get_detached_from(struct
wt_status_state *state)
/* sha1 is a commit? match without further lookup */
(oideq(, ) ||
/* perhaps sha1 is a tag, try to dereference to a commit */
-((commit = lookup_commit_reference_gently(the_repository, ,
1)) != NULL &&
+((commit = lookup_commit_reference_gently(r, , 1)) != NULL &&
oideq(, >object.oid {
const char *from = ref;
if (!skip_prefix(from, "refs/tags/", ))
@@ -1556,30 +1557,31 @@ int wt_status_check_bisect(const struct worktree *wt,
return 0;
}
-void wt_status_get_state(struct wt_status_state *state,
+void wt_status_get_state(struct repository *r,
+struct wt_status_state *state,
int get_detached_from)
{
struct stat st;
struct object_id oid;
- if (!stat(git_path_merge_head(the_repository), )) {
+ if (!stat(git_path_merge_head(r), )) {
state->merge_in_progress = 1;
} else if (wt_status_check_rebase(NULL, state)) {
; /* all set */
- } else if (!stat(git_path_cherry_pick_head(the_repository), ) &&
+ } else if (!stat(git_path_cherry_pick_head(r), ) &&
!get_oid("CHERRY_PICK_HEAD", )) {
state->cherry_pick_in_progress = 1;
oidcpy(>cherry_pick_head_oid, );
}
wt_status_check_bisect(NULL, state);
- if (!stat(git_path_revert_head(the_repository), ) &&
+ if (!stat(git_path_revert_head(r), ) &&
!get_oid("REVERT_HEAD", )) {
state->revert_in_progress = 1;
oidcpy(>revert_head_oid, );
}
if (get_detached_from)
- wt_status_get_detached_from(state);
+ wt_status_get_detached_from(r, state);
}
static void wt_longstatus_print_state(struct wt_status *s)
diff --git a/wt-status.h b/wt-status.h
index 8375e816fb..3a95975032 100644
--- a/wt-status.h
+++ b/wt-status.h
@@ -134,7 +134,9 @@ void wt_status_prepare(struct repository *r, struct
wt_status *s);
void wt_status_print(struct wt_status *s);
void wt_status_collect(struct wt_status *s);
void wt_status_collect_free_buffers(struct wt_status *s);
-void wt_status_get_state(struct wt_status_state *state, int get_detached_from);
+void wt_status_get_state(struct repository *repo,
+struct wt_status_state *state,
+int get_detached_from);
int wt_status_check_rebase(const struct worktree *wt,
struct wt_status_state *state);
int wt_status_check_bisect(const struct worktree *wt,
--
2.19.1.1231.g84aef82467
[PATCH v2] format-patch: respect --stat in cover letter's diffstat
Commit 43662b23ab (format-patch: keep cover-letter diffstat wrapped in 72 columns - 2018-01-24) uncondtionally sets stat width to 72 when generating diffstat for the cover letter, ignoring --stat from command line. But it should only do so when stat width is still default (i.e. stat_width == 0). In order to fix this, we should only set stat_width if stat_width is zero. But it will never be. Commit 071dd0ba43 (format-patch: reduce patch diffstat width to 72 - 2018-02-01) makes sure that default stat width will be 72 (ignoring $COLUMNS, but could still be overriden by --stat). So all we need to do here is drop the assignment. Reported-by: Laszlo Ersek Helped-by: Leif Lindholm Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/log.c | 2 -- t/t4052-stat-output.sh | 48 +- 2 files changed, 33 insertions(+), 17 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 061d4fd864..1a39c6e52a 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1009,8 +1009,6 @@ static void show_diffstat(struct rev_info *rev, memcpy(, >diffopt, sizeof(opts)); opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT; - opts.stat_width = MAIL_DEFAULT_WRAP; - diff_setup_done(); diff_tree_oid(get_commit_tree_oid(origin), diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh index 6e2cf933f7..28c053849a 100755 --- a/t/t4052-stat-output.sh +++ b/t/t4052-stat-output.sh @@ -44,42 +44,50 @@ show --stat log -1 --stat EOF -while read cmd args +cat >expect.60 <<-'EOF' + ...a | 1 + +EOF +cat >expect.6030 <<-'EOF' + ...aaa | 1 + +EOF +cat >expect2.60 <<-'EOF' + ...a | 1 + + ...a | 1 + +EOF +cat >expect2.6030 <<-'EOF' + ...aaa | 1 + + ...aaa | 1 + +EOF +while read expect cmd args do - cat >expect <<-'EOF' -...a | 1 + - EOF test_expect_success "$cmd --stat=width: a long name is given more room when the bar is short" ' git $cmd $args --stat=40 >output && grep " | " output >actual && - test_cmp expect actual + test_cmp $expect.60 actual ' test_expect_success "$cmd --stat-width=width with long name" ' git $cmd $args --stat-width=40 >output && grep " | " output >actual && - test_cmp expect actual + test_cmp $expect.60 actual ' - cat >expect <<-'EOF' -...aaa | 1 + - EOF test_expect_success "$cmd --stat=...,name-width with long name" ' git $cmd $args --stat=60,30 >output && grep " | " output >actual && - test_cmp expect actual + test_cmp $expect.6030 actual ' test_expect_success "$cmd --stat-name-width with long name" ' git $cmd $args --stat-name-width=30 >output && grep " | " output >actual && - test_cmp expect actual + test_cmp $expect.6030 actual ' done <<\EOF -format-patch -1 --stdout -diff HEAD^ HEAD --stat -show --stat -log -1 --stat +expect2 format-patch --cover-letter -1 --stdout +expect diff HEAD^ HEAD --stat +expect show --stat +expect log -1 --stat EOF @@ -95,6 +103,16 @@ test_expect_success 'preparation for big change tests' ' git commit -m message abcd ' +cat >expect72 <<'EOF' + abcd | 1000 ++ + abcd | 1000 ++ +EOF +test_expect_success "format-patch --cover-letter ignores COLUMNS (big change)" ' + COLUMNS=200 git format-patch -1 --stdout --cover-letter >output && + grep " | " output >actual && + test_cmp expect72 actual +' + cat >expect72 <<'EOF' abcd | 1000 ++ EOF -- 2.19.1.1005.gac84295441
[PATCH 03/22] list-objects-filter.c: remove implicit dependency on the_index
While at there, since we have access to struct repository now,
eliminate the only the_repository reference in this file.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
list-objects-filter.c | 10 +++---
list-objects-filter.h | 2 ++
list-objects.c| 9 ++---
3 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/list-objects-filter.c b/list-objects-filter.c
index 765f3df3b0..a62624a1ce 100644
--- a/list-objects-filter.c
+++ b/list-objects-filter.c
@@ -34,6 +34,7 @@ struct filter_blobs_none_data {
};
static enum list_objects_filter_result filter_blobs_none(
+ struct repository *r,
enum list_objects_filter_situation filter_situation,
struct object *obj,
const char *pathname,
@@ -88,6 +89,7 @@ struct filter_trees_none_data {
};
static enum list_objects_filter_result filter_trees_none(
+ struct repository *r,
enum list_objects_filter_situation filter_situation,
struct object *obj,
const char *pathname,
@@ -144,6 +146,7 @@ struct filter_blobs_limit_data {
};
static enum list_objects_filter_result filter_blobs_limit(
+ struct repository *r,
enum list_objects_filter_situation filter_situation,
struct object *obj,
const char *pathname,
@@ -171,7 +174,7 @@ static enum list_objects_filter_result filter_blobs_limit(
assert(obj->type == OBJ_BLOB);
assert((obj->flags & SEEN) == 0);
- t = oid_object_info(the_repository, >oid, _length);
+ t = oid_object_info(r, >oid, _length);
if (t != OBJ_BLOB) { /* probably OBJ_NONE */
/*
* We DO NOT have the blob locally, so we cannot
@@ -249,6 +252,7 @@ struct filter_sparse_data {
};
static enum list_objects_filter_result filter_sparse(
+ struct repository *r,
enum list_objects_filter_situation filter_situation,
struct object *obj,
const char *pathname,
@@ -268,7 +272,7 @@ static enum list_objects_filter_result filter_sparse(
dtype = DT_DIR;
val = is_excluded_from_list(pathname, strlen(pathname),
filename, , _data->el,
- _index);
+ r->index);
if (val < 0)
val = filter_data->array_frame[filter_data->nr].defval;
@@ -331,7 +335,7 @@ static enum list_objects_filter_result filter_sparse(
dtype = DT_REG;
val = is_excluded_from_list(pathname, strlen(pathname),
filename, , _data->el,
- _index);
+ r->index);
if (val < 0)
val = frame->defval;
if (val > 0) {
diff --git a/list-objects-filter.h b/list-objects-filter.h
index 52b4a84da9..1d45a4ad57 100644
--- a/list-objects-filter.h
+++ b/list-objects-filter.h
@@ -4,6 +4,7 @@
struct list_objects_filter_options;
struct object;
struct oidset;
+struct repository;
/*
* During list-object traversal we allow certain objects to be
@@ -60,6 +61,7 @@ enum list_objects_filter_situation {
};
typedef enum list_objects_filter_result (*filter_object_fn)(
+ struct repository *r,
enum list_objects_filter_situation filter_situation,
struct object *obj,
const char *pathname,
diff --git a/list-objects.c b/list-objects.c
index c41cc80db5..0cfd646026 100644
--- a/list-objects.c
+++ b/list-objects.c
@@ -55,7 +55,8 @@ static void process_blob(struct traversal_context *ctx,
pathlen = path->len;
strbuf_addstr(path, name);
if ((obj->flags & NOT_USER_GIVEN) && ctx->filter_fn)
- r = ctx->filter_fn(LOFS_BLOB, obj,
+ r = ctx->filter_fn(ctx->revs->repo,
+ LOFS_BLOB, obj,
path->buf, >buf[pathlen],
ctx->filter_data);
if (r & LOFR_MARK_SEEN)
@@ -175,7 +176,8 @@ static void process_tree(struct traversal_context *ctx,
strbuf_addstr(base, name);
if ((obj->flags & NOT_USER_GIVEN) && ctx->filter_fn)
- r = ctx->filter_fn(LOFS_BEGIN_TREE, obj,
+ r = ctx->filter_fn(ctx->revs->repo,
+ LOFS_BEGIN_TREE, obj,
base->buf, >buf[baselen],
ctx->filter_data);
if (r & LOFR_MARK_SEEN)
@@ -191,7 +193,8 @@ static void process_tree(struct traversal_context *ctx,
process_tree_contents(ctx, tree, base);
if ((obj->flags & NOT_USER_GIVEN) && ctx->filter_fn) {
- r = ctx->filter_fn(LOFS_END_TREE, obj,
+ r = ctx->filter_fn(ctx->revs->repo,
+ LOFS_END_TREE, obj,
[PATCH 00/22] Kill the_index part 5
I lied about part 5 being final. This series contains most of my WIP series [2] except sha1-name.c, read-cache.c and merge-recursive.c because there will be conflicts on 'pu' so they will be in part 6. At least this only results in two small conflicts in sequencer.c. I did start pushing the_repository out of library code [1] in the bottom half of this series. There aren't big surprises except that cache-tree API now take _both_ struct repository and struct index_state. The reason is cache-tree can work on temporary indexes and repo->index is about $GIT_DIR/index. [1] Stefan and I make a good team. He keeps adding the_repository. I keep removing. We both hold hands leading commit counts (with Brian just a bit behind frantically replacing sha1 with oid to keep up). Meanwhile Junio is crying in the corner because of too many conflicts. :-D [2] https://public-inbox.org/git/20181019145237.16079-1-pclo...@gmail.com/ Nguyễn Thái Ngọc Duy (22): wt-status.c: remove implicit dependency on the_index wt-status.c: remove implicit dependency the_repository list-objects-filter.c: remove implicit dependency on the_index list-objects.c: reduce the_repository references notes-merge.c: remove implicit dependency on the_index notes-merge.c: remove implicit dependency the_repository transport.c: remove implicit dependency on the_index sequencer.c: remove implicit dependency on the_index sequencer.c: remove implicit dependency on the_repository blame.c: remove implicit dependency the_repository bisect.c: remove the_repository reference branch.c: remove the_repository reference bundle.c: remove the_repository references cache-tree.c: remove the_repository references delta-islands.c: remove the_repository references diff-lib.c: remove the_repository references line-log.c: remove the_repository reference notes-cache.c: remove the_repository references pack-check.c: remove the_repository references pack-*.c: remove the_repository references rerere.c: remove the_repository references rebase-interactive.c: remove the_repository references bisect.c | 48 ++-- bisect.h | 5 +- blame.c | 39 +-- branch.c | 21 +- branch.h | 8 +- builtin/am.c | 4 +- builtin/bisect--helper.c | 2 +- builtin/branch.c | 6 +- builtin/bundle.c | 7 +- builtin/checkout.c| 5 +- builtin/commit.c | 8 +- builtin/fsck.c| 3 +- builtin/merge-ours.c | 2 +- builtin/merge.c | 2 +- builtin/notes.c | 2 +- builtin/pack-objects.c| 6 +- builtin/pull.c| 3 +- builtin/push.c| 3 +- builtin/read-tree.c | 4 +- builtin/rebase--interactive.c | 19 +- builtin/rebase.c | 13 +- builtin/rerere.c | 10 +- builtin/reset.c | 4 +- builtin/revert.c | 8 +- bundle.c | 26 +- bundle.h | 9 +- cache-tree.c | 26 +- cache-tree.h | 4 +- combine-diff.c| 2 +- delta-islands.c | 24 +- delta-islands.h | 9 +- diff-lib.c| 7 +- diff.c| 12 +- diff.h| 5 +- diffcore-pickaxe.c| 4 +- grep.c| 2 +- line-log.c| 2 +- list-objects-filter.c | 10 +- list-objects-filter.h | 2 + list-objects.c| 13 +- notes-cache.c | 12 +- notes-cache.h | 6 +- notes-merge.c | 16 +- notes-merge.h | 5 +- pack-bitmap-write.c | 6 +- pack-bitmap.c | 13 +- pack-bitmap.h | 3 +- pack-check.c | 9 +- pack-objects.c| 6 +- pack-objects.h| 5 +- pack.h| 4 +- read-cache.c | 2 +- rebase-interactive.c | 6 +- rebase-interactive.h | 5 +- ref-filter.c | 2 +- rerere.c | 26 +- rerere.h | 6 +- sequencer.c | 453 +++--- sequencer.h | 27 +- transport.c | 9 +- transport.h | 3 +- unpack-trees.c| 2 +- userdiff.c| 5 +- userdiff.h| 4 +- wt-status.c | 94 --- wt-status.h | 21 +- 66 files changed, 650 insertions(+), 489 deletions(-) -- 2.19.1.1231.g84aef82467
[PATCH 01/22] wt-status.c: remove implicit dependency on the_index
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/commit.c | 2 +-
builtin/pull.c | 3 +-
builtin/rebase.c | 7 +++--
sequencer.c | 8 ++---
wt-status.c | 76 +++-
wt-status.h | 17 +++
6 files changed, 66 insertions(+), 47 deletions(-)
diff --git a/builtin/commit.c b/builtin/commit.c
index 074bd9a551..6637a928a7 100644
--- a/builtin/commit.c
+++ b/builtin/commit.c
@@ -185,7 +185,7 @@ static void determine_whence(struct wt_status *s)
static void status_init_config(struct wt_status *s, config_fn_t fn)
{
- wt_status_prepare(s);
+ wt_status_prepare(the_repository, s);
init_diff_ui_defaults();
git_config(fn, s);
determine_whence(s);
diff --git a/builtin/pull.c b/builtin/pull.c
index c21aa276f1..6026ce1a69 100644
--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -888,7 +888,8 @@ int cmd_pull(int argc, const char **argv, const char
*prefix)
die(_("Updating an unborn branch with changes added to
the index."));
if (!autostash)
- require_clean_work_tree(N_("pull with rebase"),
+ require_clean_work_tree(the_repository,
+ N_("pull with rebase"),
_("please commit or stash them."), 1, 0);
if (get_rebase_fork_point(_fork_point, repo, *refspecs))
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0ee06aa363..b9eb958454 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -983,7 +983,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
_file);
rollback_lock_file(_file);
- if (has_unstaged_changes(1)) {
+ if (has_unstaged_changes(the_repository, 1)) {
puts(_("You must edit all merge conflicts and then\n"
"mark them as resolved using git add"));
exit(1);
@@ -1351,7 +1351,8 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
update_index_if_able(_index, _file);
rollback_lock_file(_file);
- if (has_unstaged_changes(1) || has_uncommitted_changes(1)) {
+ if (has_unstaged_changes(the_repository, 1) ||
+ has_uncommitted_changes(the_repository, 1)) {
const char *autostash =
state_dir_path("autostash", );
struct child_process stash = CHILD_PROCESS_INIT;
@@ -1397,7 +1398,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
}
}
- if (require_clean_work_tree("rebase",
+ if (require_clean_work_tree(the_repository, "rebase",
_("Please commit or stash them."), 1, 1)) {
ret = 1;
goto cleanup;
diff --git a/sequencer.c b/sequencer.c
index 9e1ab3a2a7..0b8f18fd36 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2773,7 +2773,7 @@ static int do_exec(const char *command_line)
if (discard_cache() < 0 || read_cache() < 0)
return error(_("could not read index"));
- dirty = require_clean_work_tree("rebase", NULL, 1, 1);
+ dirty = require_clean_work_tree(the_repository, "rebase", NULL, 1, 1);
if (status) {
warning(_("execution failed: %s\n%s"
@@ -3714,10 +3714,10 @@ static int commit_staged_changes(struct replay_opts
*opts,
unsigned int flags = ALLOW_EMPTY | EDIT_MSG;
unsigned int final_fixup = 0, is_clean;
- if (has_unstaged_changes(1))
+ if (has_unstaged_changes(the_repository, 1))
return error(_("cannot rebase: You have unstaged changes."));
- is_clean = !has_uncommitted_changes(0);
+ is_clean = !has_uncommitted_changes(the_repository, 0);
if (file_exists(rebase_path_amend())) {
struct strbuf rev = STRBUF_INIT;
@@ -4847,7 +4847,7 @@ int complete_action(struct replay_opts *opts, unsigned
flags,
if (checkout_onto(opts, onto_name, oid_to_hex(), orig_head))
return -1;
;
- if (require_clean_work_tree("rebase", "", 1, 1))
+ if (require_clean_work_tree(the_repository, "rebase", "", 1, 1))
return -1;
return sequencer_continue(opts);
diff --git a/wt-status.c b/wt-status.c
index 187568a112..6d401b2c24 100644
--- a/wt-status.c
+++ b/wt-status.c
@@ -119,9 +119,10 @@ static void status_printf_more(struct wt_status *s, const
char *color,
va_end(ap);
}
-void wt_status_prepare(struct wt_status *s)
+void wt_status_prepare(struct repository *r, struct wt_status *s)
{
memset(s, 0, sizeof(*s));
+ s->repo = r;
memcpy(s->color_palette, default_wt_status_colors,
sizeof(default_wt_status_colors));
s->show_untracked_files =
[PATCH v3 13/16] parse-options.c: turn some die() to BUG()
These two strings are clearly not for the user to see. Reduce the
violence in one string while at there.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
parse-options.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 0bf817193d..3f5f985c1e 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -197,7 +197,7 @@ static int get_value(struct parse_opt_ctx_t *p,
return 0;
default:
- die("should not happen, someone must be hit on the forehead");
+ BUG("opt->type %d should not happen", opt->type);
}
}
@@ -424,7 +424,7 @@ void parse_options_start(struct parse_opt_ctx_t *ctx,
ctx->flags = flags;
if ((flags & PARSE_OPT_KEEP_UNKNOWN) &&
(flags & PARSE_OPT_STOP_AT_NON_OPTION))
- die("STOP_AT_NON_OPTION and KEEP_UNKNOWN don't go together");
+ BUG("STOP_AT_NON_OPTION and KEEP_UNKNOWN don't go together");
parse_options_check(options);
}
--
2.19.1.1231.g84aef82467
[PATCH v3 11/16] repack: mark more strings for translation
Two strings are slightly updated to be consistent with the rest: die()
starts with lowercase.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/repack.c | 26 +-
1 file changed, 13 insertions(+), 13 deletions(-)
diff --git a/builtin/repack.c b/builtin/repack.c
index c6a7943d5c..721f947218 100644
--- a/builtin/repack.c
+++ b/builtin/repack.c
@@ -197,7 +197,7 @@ static int write_oid(const struct object_id *oid, struct
packed_git *pack,
if (cmd->in == -1) {
if (start_command(cmd))
- die("Could not start pack-objects to repack promisor
objects");
+ die(_("could not start pack-objects to repack promisor
objects"));
}
xwrite(cmd->in, oid_to_hex(oid), GIT_SHA1_HEXSZ);
@@ -236,7 +236,7 @@ static void repack_promisor_objects(const struct
pack_objects_args *args,
char *promisor_name;
int fd;
if (line.len != 40)
- die("repack: Expecting 40 character sha1 lines only
from pack-objects.");
+ die(_("repack: Expecting 40 character sha1 lines only
from pack-objects."));
string_list_append(names, line.buf);
/*
@@ -247,13 +247,13 @@ static void repack_promisor_objects(const struct
pack_objects_args *args,
line.buf);
fd = open(promisor_name, O_CREAT|O_EXCL|O_WRONLY, 0600);
if (fd < 0)
- die_errno("unable to create '%s'", promisor_name);
+ die_errno(_("unable to create '%s'"), promisor_name);
close(fd);
free(promisor_name);
}
fclose(out);
if (finish_command())
- die("Could not finish pack-objects to repack promisor objects");
+ die(_("could not finish pack-objects to repack promisor
objects"));
}
#define ALL_INTO_ONE 1
@@ -408,7 +408,7 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
out = xfdopen(cmd.out, "r");
while (strbuf_getline_lf(, out) != EOF) {
if (line.len != 40)
- die("repack: Expecting 40 character sha1 lines only
from pack-objects.");
+ die(_("repack: Expecting 40 character sha1 lines only
from pack-objects"));
string_list_append(, line.buf);
}
fclose(out);
@@ -417,7 +417,7 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
return ret;
if (!names.nr && !po_args.quiet)
- printf("Nothing new to pack.\n");
+ printf_ln(_("Nothing new to pack."));
/*
* Ok we have prepared all new packfiles.
@@ -477,13 +477,13 @@ int cmd_repack(int argc, const char **argv, const char
*prefix)
if (rollback_failure.nr) {
int i;
fprintf(stderr,
- "WARNING: Some packs in use have been renamed
by\n"
- "WARNING: prefixing old- to their name, in
order to\n"
- "WARNING: replace them with the new version of
the\n"
- "WARNING: file. But the operation failed, and
the\n"
- "WARNING: attempt to rename them back to
their\n"
- "WARNING: original names also failed.\n"
- "WARNING: Please rename them in %s
manually:\n", packdir);
+ _("WARNING: Some packs in use have been renamed
by\n"
+ "WARNING: prefixing old- to their name, in
order to\n"
+ "WARNING: replace them with the new version
of the\n"
+ "WARNING: file. But the operation failed,
and the\n"
+ "WARNING: attempt to rename them back to
their\n"
+ "WARNING: original names also failed.\n"
+ "WARNING: Please rename them in %s
manually:\n"), packdir);
for (i = 0; i < rollback_failure.nr; i++)
fprintf(stderr, "WARNING: old-%s -> %s\n",
rollback_failure.items[i].string,
--
2.19.1.1231.g84aef82467
[PATCH v3 15/16] fsck: reduce word legos to help i18n
These messages will be marked for translation later. Reduce word legos
and give translators almost full phrases. describe_object() is updated
so that it can be called from printf() twice.
While at there, remove \n from the strings to reduce a bit of work
from translators.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/fsck.c | 65 +++---
1 file changed, 40 insertions(+), 25 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 06eb421720..1feb6142f4 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -52,16 +52,24 @@ static int name_objects;
static const char *describe_object(struct object *obj)
{
- static struct strbuf buf = STRBUF_INIT;
- char *name = name_objects ?
- lookup_decoration(fsck_walk_options.object_names, obj) : NULL;
+ static struct strbuf bufs[] = {
+ STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
+ };
+ static int b = 0;
+ struct strbuf *buf;
+ char *name = NULL;
- strbuf_reset();
- strbuf_addstr(, oid_to_hex(>oid));
+ if (name_objects)
+ name = lookup_decoration(fsck_walk_options.object_names, obj);
+
+ buf = bufs + b;
+ b = (b + 1) % ARRAY_SIZE(bufs);
+ strbuf_reset(buf);
+ strbuf_addstr(buf, oid_to_hex(>oid));
if (name)
- strbuf_addf(, " (%s)", name);
+ strbuf_addf(buf, " (%s)", name);
- return buf.buf;
+ return buf->buf;
}
static const char *printable_type(struct object *obj)
@@ -105,25 +113,29 @@ static int fsck_config(const char *var, const char
*value, void *cb)
return git_default_config(var, value, cb);
}
-static void objreport(struct object *obj, const char *msg_type,
- const char *err)
-{
- fprintf(stderr, "%s in %s %s: %s\n",
- msg_type, printable_type(obj), describe_object(obj), err);
-}
-
static int objerror(struct object *obj, const char *err)
{
errors_found |= ERROR_OBJECT;
- objreport(obj, "error", err);
+ fprintf_ln(stderr, "error in %s %s: %s",
+ printable_type(obj), describe_object(obj), err);
return -1;
}
static int fsck_error_func(struct fsck_options *o,
struct object *obj, int type, const char *message)
{
- objreport(obj, (type == FSCK_WARN) ? "warning" : "error", message);
- return (type == FSCK_WARN) ? 0 : 1;
+ switch (type) {
+ case FSCK_WARN:
+ fprintf_ln(stderr, "warning in %s %s: %s",
+ printable_type(obj), describe_object(obj), message);
+ return 0;
+ case FSCK_ERROR:
+ fprintf_ln(stderr, "error in %s %s: %s",
+ printable_type(obj), describe_object(obj), message);
+ return 1;
+ default:
+ BUG("%d (FSCK_IGNORE?) should never trigger this callback",
type);
+ }
}
static struct object_array pending;
@@ -165,10 +177,12 @@ static int mark_object(struct object *obj, int type, void
*data, struct fsck_opt
if (!(obj->flags & HAS_OBJ)) {
if (parent && !has_object_file(>oid)) {
- printf("broken link from %7s %s\n",
-printable_type(parent),
describe_object(parent));
- printf(" to %7s %s\n",
-printable_type(obj), describe_object(obj));
+ printf_ln("broken link from %7s %s\n"
+ " to %7s %s",
+ printable_type(parent),
+ describe_object(parent),
+ printable_type(obj),
+ describe_object(obj));
errors_found |= ERROR_REACHABLE;
}
return 1;
@@ -371,10 +385,11 @@ static int fsck_obj(struct object *obj, void *buffer,
unsigned long size)
struct tag *tag = (struct tag *) obj;
if (show_tags && tag->tagged) {
- printf("tagged %s %s", printable_type(tag->tagged),
- describe_object(tag->tagged));
- printf(" (%s) in %s\n", tag->tag,
- describe_object(>object));
+ printf_ln("tagged %s %s (%s) in %s",
+ printable_type(tag->tagged),
+ describe_object(tag->tagged),
+ tag->tag,
+ describe_object(>object));
}
}
--
2.19.1.1231.g84aef82467
[PATCH v3 10/16] remote.c: mark messages for translation
The two strings are slightly modified to be consistent with the rest:
die() and error() start with a lowercase.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
remote.c | 43 ++-
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/remote.c b/remote.c
index 257630ff21..11b33d1625 100644
--- a/remote.c
+++ b/remote.c
@@ -359,7 +359,7 @@ static int handle_config(const char *key, const char
*value, void *cb)
return 0;
/* Handle remote..* variables */
if (*name == '/') {
- warning("Config remote shorthand cannot begin with '/': %s",
+ warning(_("config remote shorthand cannot begin with '/': %s"),
name);
return 0;
}
@@ -406,7 +406,7 @@ static int handle_config(const char *key, const char
*value, void *cb)
if (!remote->receivepack)
remote->receivepack = v;
else
- error("more than one receivepack given, using the
first");
+ error(_("more than one receivepack given, using the
first"));
} else if (!strcmp(subkey, "uploadpack")) {
const char *v;
if (git_config_string(, key, value))
@@ -414,7 +414,7 @@ static int handle_config(const char *key, const char
*value, void *cb)
if (!remote->uploadpack)
remote->uploadpack = v;
else
- error("more than one uploadpack given, using the
first");
+ error(_("more than one uploadpack given, using the
first"));
} else if (!strcmp(subkey, "tagopt")) {
if (!strcmp(value, "--no-tags"))
remote->fetch_tags = -1;
@@ -680,7 +680,7 @@ static int match_name_with_pattern(const char *key, const
char *name,
size_t namelen;
int ret;
if (!kstar)
- die("Key '%s' of pattern had no '*'", key);
+ die(_("key '%s' of pattern had no '*'"), key);
klen = kstar - key;
ksuffixlen = strlen(kstar + 1);
namelen = strlen(name);
@@ -690,7 +690,7 @@ static int match_name_with_pattern(const char *key, const
char *name,
struct strbuf sb = STRBUF_INIT;
const char *vstar = strchr(value, '*');
if (!vstar)
- die("Value '%s' of pattern has no '*'", value);
+ die(_("value '%s' of pattern has no '*'"), value);
strbuf_add(, value, vstar - value);
strbuf_add(, name + klen, namelen - klen - ksuffixlen);
strbuf_addstr(, vstar + 1);
@@ -995,12 +995,12 @@ static int match_explicit_lhs(struct ref *src,
* way to delete 'other' ref at the remote end.
*/
if (try_explicit_object_name(rs->src, match) < 0)
- return error("src refspec %s does not match any.",
rs->src);
+ return error(_("src refspec %s does not match any"),
rs->src);
if (allocated_match)
*allocated_match = 1;
return 0;
default:
- return error("src refspec %s matches more than one.", rs->src);
+ return error(_("src refspec %s matches more than one"),
rs->src);
}
}
@@ -1030,7 +1030,7 @@ static int match_explicit(struct ref *src, struct ref
*dst,
if (!dst_value ||
((flag & REF_ISSYMREF) &&
!starts_with(dst_value, "refs/heads/")))
- die("%s cannot be resolved to branch.",
+ die(_("%s cannot be resolved to branch"),
matched_src->name);
}
@@ -1041,30 +1041,30 @@ static int match_explicit(struct ref *src, struct ref
*dst,
if (starts_with(dst_value, "refs/"))
matched_dst = make_linked_ref(dst_value, dst_tail);
else if (is_null_oid(_src->new_oid))
- error("unable to delete '%s': remote ref does not
exist",
+ error(_("unable to delete '%s': remote ref does not
exist"),
dst_value);
else if ((dst_guess = guess_ref(dst_value, matched_src))) {
matched_dst = make_linked_ref(dst_guess, dst_tail);
free(dst_guess);
} else
- error("unable to push to unqualified destination: %s\n"
- "The destination refspec neither matches an "
- "existing ref on the remote nor\n"
- "begins with refs/, and we are unable to "
- "guess a prefix based on the source ref.",
+ error(_("unable to push to unqualified destination:
%s\n"
+
[PATCH v3 01/16] git.c: mark more strings for translation
One string is slightly updated to keep consistency with the rest:
die() should begin with lowercase.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
git.c | 30 +++---
1 file changed, 15 insertions(+), 15 deletions(-)
diff --git a/git.c b/git.c
index adac132956..5fd30da093 100644
--- a/git.c
+++ b/git.c
@@ -338,27 +338,27 @@ static int handle_alias(int *argcp, const char ***argv)
if (ret >= 0) /* normal exit */
exit(ret);
- die_errno("while expanding alias '%s': '%s'",
- alias_command, alias_string + 1);
+ die_errno(_("while expanding alias '%s': '%s'"),
+ alias_command, alias_string + 1);
}
count = split_cmdline(alias_string, _argv);
if (count < 0)
- die("Bad alias.%s string: %s", alias_command,
+ die(_("bad alias.%s string: %s"), alias_command,
split_cmdline_strerror(count));
option_count = handle_options(_argv, , );
if (envchanged)
- die("alias '%s' changes environment variables.\n"
-"You can use '!git' in the alias to do this",
-alias_command);
+ die(_("alias '%s' changes environment variables.\n"
+ "You can use '!git' in the alias to do this"),
+ alias_command);
memmove(new_argv - option_count, new_argv,
count * sizeof(char *));
new_argv -= option_count;
if (count < 1)
- die("empty alias for %s", alias_command);
+ die(_("empty alias for %s"), alias_command);
if (!strcmp(alias_command, new_argv[0]))
- die("recursive alias: %s", alias_command);
+ die(_("recursive alias: %s"), alias_command);
trace_argv_printf(new_argv,
"trace: alias expansion: %s =>",
@@ -409,7 +409,7 @@ static int run_builtin(struct cmd_struct *p, int argc,
const char **argv)
if (!help && get_super_prefix()) {
if (!(p->option & SUPPORT_SUPER_PREFIX))
- die("%s doesn't support --super-prefix", p->cmd);
+ die(_("%s doesn't support --super-prefix"), p->cmd);
}
if (!help && p->option & NEED_WORK_TREE)
@@ -433,11 +433,11 @@ static int run_builtin(struct cmd_struct *p, int argc,
const char **argv)
/* Check for ENOSPC and EIO errors.. */
if (fflush(stdout))
- die_errno("write failure on standard output");
+ die_errno(_("write failure on standard output"));
if (ferror(stdout))
- die("unknown write failure on standard output");
+ die(_("unknown write failure on standard output"));
if (fclose(stdout))
- die_errno("close failed on standard output");
+ die_errno(_("close failed on standard output"));
return 0;
}
@@ -648,7 +648,7 @@ static void execv_dashed_external(const char **argv)
int status;
if (get_super_prefix())
- die("%s doesn't support --super-prefix", argv[0]);
+ die(_("%s doesn't support --super-prefix"), argv[0]);
if (use_pager == -1 && !is_builtin(argv[0]))
use_pager = check_pager_config(argv[0]);
@@ -760,7 +760,7 @@ int cmd_main(int argc, const char **argv)
if (skip_prefix(cmd, "git-", )) {
argv[0] = cmd;
handle_builtin(argc, argv);
- die("cannot handle %s as a builtin", cmd);
+ die(_("cannot handle %s as a builtin"), cmd);
}
/* Look for flags.. */
@@ -773,7 +773,7 @@ int cmd_main(int argc, const char **argv)
} else {
/* The user didn't specify a command; give them help */
commit_pager_choice();
- printf("usage: %s\n\n", git_usage_string);
+ printf(_("usage: %s\n\n"), git_usage_string);
list_common_cmds_help();
printf("\n%s\n", _(git_more_info_string));
exit(1);
--
2.19.1.1231.g84aef82467
[PATCH v3 12/16] parse-options: replace opterror() with optname()
Introduce optname() that does the early half of original opterror() to
come up with the name of the option reported back to the user, and use
it to kill opterror(). The callers of opterror() now directly call
error() using the string returned by opterror() instead.
There are a few issues with opterror()
- it tries to assemble an English sentence from pieces. This is not
great for translators because we give them pieces instead of a full
sentence.
- It's a wrapper around error() and needs some hack to let the
compiler know it always returns -1.
- Since it takes a string instead of printf format, one call site has
to assemble the string manually before passing to it.
Using error() directly solves the second and third problems.
It kind helps the first problem as well because "%s does foo" does
give a translator a full sentence in a sense and let them reorder if
needed. But it has limitations, if the subject part has to change
based on the rest of the sentence, that language is screwed. This is
also why I try to avoid calling optname() when 'flags' is known in
advance.
Mark of these strings for translation as well while at there.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/merge.c | 2 +-
builtin/revert.c| 3 ++-
parse-options-cb.c | 7 ---
parse-options.c | 46 +
parse-options.h | 5 +
ref-filter.c| 8 +---
t/t4211-line-log.sh | 2 +-
7 files changed, 40 insertions(+), 33 deletions(-)
diff --git a/builtin/merge.c b/builtin/merge.c
index 92ba7e1c6d..82248d43c3 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -128,7 +128,7 @@ static int option_read_message(struct parse_opt_ctx_t *ctx,
ctx->argc--;
arg = *++ctx->argv;
} else
- return opterror(opt, "requires a value", 0);
+ return error(_("option `%s' requires a value"), opt->long_name);
if (buf->len)
strbuf_addch(buf, '\n');
diff --git a/builtin/revert.c b/builtin/revert.c
index c93393c89b..11190d2ab4 100644
--- a/builtin/revert.c
+++ b/builtin/revert.c
@@ -69,7 +69,8 @@ static int option_parse_m(const struct option *opt,
replay->mainline = strtol(arg, , 10);
if (*end || replay->mainline <= 0)
- return opterror(opt, "expects a number greater than zero", 0);
+ return error(_("option `%s' expects a number greater than
zero"),
+opt->long_name);
return 0;
}
diff --git a/parse-options-cb.c b/parse-options-cb.c
index e8236534ac..813eb6301b 100644
--- a/parse-options-cb.c
+++ b/parse-options-cb.c
@@ -18,7 +18,8 @@ int parse_opt_abbrev_cb(const struct option *opt, const char
*arg, int unset)
} else {
v = strtol(arg, (char **), 10);
if (*arg)
- return opterror(opt, "expects a numerical value", 0);
+ return error(_("option `%s' expects a numerical value"),
+opt->long_name);
if (v && v < MINIMUM_ABBREV)
v = MINIMUM_ABBREV;
else if (v > 40)
@@ -54,8 +55,8 @@ int parse_opt_color_flag_cb(const struct option *opt, const
char *arg,
arg = unset ? "never" : (const char *)opt->defval;
value = git_config_colorbool(NULL, arg);
if (value < 0)
- return opterror(opt,
- "expects \"always\", \"auto\", or \"never\"", 0);
+ return error(_("option `%s' expects \"always\", \"auto\", or
\"never\""),
+opt->long_name);
*(int *)opt->value = value;
return 0;
}
diff --git a/parse-options.c b/parse-options.c
index 3b874a83a0..0bf817193d 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -32,7 +32,7 @@ static int get_arg(struct parse_opt_ctx_t *p, const struct
option *opt,
p->argc--;
*arg = *++p->argv;
} else
- return opterror(opt, "requires a value", flags);
+ return error(_("%s requires a value"), optname(opt, flags));
return 0;
}
@@ -49,7 +49,6 @@ static int opt_command_mode_error(const struct option *opt,
int flags)
{
const struct option *that;
- struct strbuf message = STRBUF_INIT;
struct strbuf that_name = STRBUF_INIT;
/*
@@ -67,13 +66,13 @@ static int opt_command_mode_error(const struct option *opt,
strbuf_addf(_name, "--%s", that->long_name);
else
strbuf_addf(_name, "-%c", that->short_name);
- strbuf_addf(, ": incompatible with %s", that_name.buf);
+ error(_("%s is incompatible with %s"),
+ optname(opt, flags), that_name.buf);
strbuf_release(_name);
- opterror(opt, message.buf, flags);
-
[PATCH v3 05/16] read-cache.c: turn die("internal error") to BUG()
Signed-off-by: Nguyễn Thái Ngọc Duy
---
read-cache.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index d57958233e..0c37f4885e 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -316,7 +316,7 @@ static int ce_match_stat_basic(const struct cache_entry
*ce, struct stat *st)
changed |= DATA_CHANGED;
return changed;
default:
- die("internal error: ce_mode is %o", ce->ce_mode);
+ BUG("unsupported ce_mode: %o", ce->ce_mode);
}
changed |= match_stat_data(>ce_stat_data, st);
@@ -2356,14 +2356,14 @@ void validate_cache_entries(const struct index_state
*istate)
for (i = 0; i < istate->cache_nr; i++) {
if (!istate) {
- die("internal error: cache entry is not allocated from
expected memory pool");
+ BUG("cache entry is not allocated from expected memory
pool");
} else if (!istate->ce_mem_pool ||
!mem_pool_contains(istate->ce_mem_pool,
istate->cache[i])) {
if (!istate->split_index ||
!istate->split_index->base ||
!istate->split_index->base->ce_mem_pool ||
!mem_pool_contains(istate->split_index->base->ce_mem_pool, istate->cache[i])) {
- die("internal error: cache entry is not
allocated from expected memory pool");
+ BUG("cache entry is not allocated from expected
memory pool");
}
}
}
--
2.19.1.1231.g84aef82467
[PATCH v3 09/16] remote.c: turn some error() or die() to BUG()
The first error, "internal error", is clearly a BUG(). The second two
are meant to catch calls with invalid parameters and should never
happen outside the test suite.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
remote.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/remote.c b/remote.c
index 81f4f01b00..257630ff21 100644
--- a/remote.c
+++ b/remote.c
@@ -620,7 +620,7 @@ static void handle_duplicate(struct ref *ref1, struct ref
*ref2)
* FETCH_HEAD_IGNORE entries always appear at
* the end of the list.
*/
- die(_("Internal error"));
+ BUG("Internal error");
}
}
free(ref2->peer_ref);
@@ -707,7 +707,7 @@ static void query_refspecs_multiple(struct refspec *rs,
int find_src = !query->src;
if (find_src && !query->dst)
- error("query_refspecs_multiple: need either src or dst");
+ BUG("query_refspecs_multiple: need either src or dst");
for (i = 0; i < rs->nr; i++) {
struct refspec_item *refspec = >items[i];
@@ -735,7 +735,7 @@ int query_refspecs(struct refspec *rs, struct refspec_item
*query)
char **result = find_src ? >src : >dst;
if (find_src && !query->dst)
- return error("query_refspecs: need either src or dst");
+ BUG("query_refspecs: need either src or dst");
for (i = 0; i < rs->nr; i++) {
struct refspec_item *refspec = >items[i];
--
2.19.1.1231.g84aef82467
[PATCH v3 14/16] parse-options.c: mark more strings for translation
One error is updated to start with lowercase to be consistent with the
rest.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
parse-options.c | 14 +++---
t/t0040-parse-options.sh | 4 ++--
2 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/parse-options.c b/parse-options.c
index 3f5f985c1e..86aad2e185 100644
--- a/parse-options.c
+++ b/parse-options.c
@@ -319,8 +319,8 @@ static int parse_long_opt(struct parse_opt_ctx_t *p, const
char *arg,
}
if (ambiguous_option) {
- error("Ambiguous option: %s "
- "(could be --%s%s or --%s%s)",
+ error(_("ambiguous option: %s "
+ "(could be --%s%s or --%s%s)"),
arg,
(ambiguous_flags & OPT_UNSET) ? "no-" : "",
ambiguous_option->long_name,
@@ -353,7 +353,7 @@ static void check_typos(const char *arg, const struct
option *options)
return;
if (starts_with(arg, "no-")) {
- error ("did you mean `--%s` (with two dashes ?)", arg);
+ error(_("did you mean `--%s` (with two dashes ?)"), arg);
exit(129);
}
@@ -361,7 +361,7 @@ static void check_typos(const char *arg, const struct
option *options)
if (!options->long_name)
continue;
if (starts_with(options->long_name, arg)) {
- error ("did you mean `--%s` (with two dashes ?)", arg);
+ error(_("did you mean `--%s` (with two dashes ?)"),
arg);
exit(129);
}
}
@@ -644,11 +644,11 @@ int parse_options(int argc, const char **argv, const char
*prefix,
break;
default: /* PARSE_OPT_UNKNOWN */
if (ctx.argv[0][1] == '-') {
- error("unknown option `%s'", ctx.argv[0] + 2);
+ error(_("unknown option `%s'"), ctx.argv[0] + 2);
} else if (isascii(*ctx.opt)) {
- error("unknown switch `%c'", *ctx.opt);
+ error(_("unknown switch `%c'"), *ctx.opt);
} else {
- error("unknown non-ascii option in string: `%s'",
+ error(_("unknown non-ascii option in string: `%s'"),
ctx.argv[0]);
}
usage_with_options(usagestr, options);
diff --git a/t/t0040-parse-options.sh b/t/t0040-parse-options.sh
index 17d0c18feb..e46b1e02f0 100755
--- a/t/t0040-parse-options.sh
+++ b/t/t0040-parse-options.sh
@@ -228,7 +228,7 @@ EOF
test_expect_success 'detect possible typos' '
test_must_fail test-tool parse-options -boolean >output 2>output.err &&
test_must_be_empty output &&
- test_cmp typo.err output.err
+ test_i18ncmp typo.err output.err
'
cat >typo.err <<\EOF
@@ -238,7 +238,7 @@ EOF
test_expect_success 'detect possible typos' '
test_must_fail test-tool parse-options -ambiguous >output 2>output.err
&&
test_must_be_empty output &&
- test_cmp typo.err output.err
+ test_i18ncmp typo.err output.err
'
test_expect_success 'keep some options as arguments' '
--
2.19.1.1231.g84aef82467
[PATCH v3 07/16] read-cache.c: add missing colon separators
typechange_fmt and added_fmt should have a colon before "needs
update". Align the statements to make it easier to read and see. Also
drop the unnecessary ().
Signed-off-by: Nguyễn Thái Ngọc Duy
---
read-cache.c | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 858befe738..8d99ae376c 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1492,11 +1492,11 @@ int refresh_index(struct index_state *istate, unsigned
int flags,
istate->cache_nr);
trace_performance_enter();
- modified_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n");
- deleted_fmt = (in_porcelain ? "D\t%s\n" : "%s: needs update\n");
- typechange_fmt = (in_porcelain ? "T\t%s\n" : "%s needs update\n");
- added_fmt = (in_porcelain ? "A\t%s\n" : "%s needs update\n");
- unmerged_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n");
+ modified_fmt = in_porcelain ? "M\t%s\n" : "%s: needs update\n";
+ deleted_fmt= in_porcelain ? "D\t%s\n" : "%s: needs update\n";
+ typechange_fmt = in_porcelain ? "T\t%s\n" : "%s: needs update\n";
+ added_fmt = in_porcelain ? "A\t%s\n" : "%s: needs update\n";
+ unmerged_fmt = in_porcelain ? "U\t%s\n" : "%s: needs merge\n";
for (i = 0; i < istate->cache_nr; i++) {
struct cache_entry *ce, *new_entry;
int cache_errno = 0;
--
2.19.1.1231.g84aef82467
[PATCH v3 04/16] attr.c: mark more string for translation
Signed-off-by: Nguyễn Thái Ngọc Duy
---
attr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/attr.c b/attr.c
index 60d284796d..3770bc1a11 100644
--- a/attr.c
+++ b/attr.c
@@ -372,8 +372,8 @@ static struct match_attr *parse_attr_line(const char *line,
const char *src,
if (strlen(ATTRIBUTE_MACRO_PREFIX) < namelen &&
starts_with(name, ATTRIBUTE_MACRO_PREFIX)) {
if (!macro_ok) {
- fprintf(stderr, "%s not allowed: %s:%d\n",
- name, src, lineno);
+ fprintf_ln(stderr, _("%s not allowed: %s:%d"),
+ name, src, lineno);
goto fail_return;
}
is_macro = 1;
--
2.19.1.1231.g84aef82467
[PATCH v3 06/16] read-cache.c: mark more strings for translation
There are a couple other improvements on these strings as well:
- add missing colon (as separator)
- quote paths
- provide more information on error messages
- keep first word in lowercase
Signed-off-by: Nguyễn Thái Ngọc Duy
---
read-cache.c| 57 +
t/t1450-fsck.sh | 2 +-
2 files changed, 30 insertions(+), 29 deletions(-)
diff --git a/read-cache.c b/read-cache.c
index 0c37f4885e..858befe738 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -672,7 +672,8 @@ static struct cache_entry *create_alias_ce(struct
index_state *istate,
struct cache_entry *new_entry;
if (alias->ce_flags & CE_ADDED)
- die("Will not add file alias '%s' ('%s' already exists in
index)", ce->name, alias->name);
+ die(_("will not add file alias '%s' ('%s' already exists in
index)"),
+ ce->name, alias->name);
/* Ok, create the new entry using the name of the existing alias */
len = ce_namelen(alias);
@@ -687,7 +688,7 @@ void set_object_name_for_intent_to_add_entry(struct
cache_entry *ce)
{
struct object_id oid;
if (write_object_file("", 0, blob_type, ))
- die("cannot create an empty blob in the object database");
+ die(_("cannot create an empty blob in the object database"));
oidcpy(>oid, );
}
@@ -708,7 +709,7 @@ int add_to_index(struct index_state *istate, const char
*path, struct stat *st,
newflags |= HASH_RENORMALIZE;
if (!S_ISREG(st_mode) && !S_ISLNK(st_mode) && !S_ISDIR(st_mode))
- return error("%s: can only add regular files, symbolic links or
git-directories", path);
+ return error(_("%s: can only add regular files, symbolic links
or git-directories"), path);
namelen = strlen(path);
if (S_ISDIR(st_mode)) {
@@ -763,7 +764,7 @@ int add_to_index(struct index_state *istate, const char
*path, struct stat *st,
if (!intent_only) {
if (index_path(istate, >oid, path, st, newflags)) {
discard_cache_entry(ce);
- return error("unable to index file %s", path);
+ return error(_("unable to index file '%s'"), path);
}
} else
set_object_name_for_intent_to_add_entry(ce);
@@ -782,7 +783,7 @@ int add_to_index(struct index_state *istate, const char
*path, struct stat *st,
discard_cache_entry(ce);
else if (add_index_entry(istate, ce, add_option)) {
discard_cache_entry(ce);
- return error("unable to add %s to index", path);
+ return error(_("unable to add '%s' to index"), path);
}
if (verbose && !was_same)
printf("add '%s'\n", path);
@@ -793,7 +794,7 @@ int add_file_to_index(struct index_state *istate, const
char *path, int flags)
{
struct stat st;
if (lstat(path, ))
- die_errno("unable to stat '%s'", path);
+ die_errno(_("unable to stat '%s'"), path);
return add_to_index(istate, path, , flags);
}
@@ -818,7 +819,7 @@ struct cache_entry *make_cache_entry(struct index_state
*istate,
int len;
if (!verify_path(path, mode)) {
- error("Invalid path '%s'", path);
+ error(_("invalid path '%s'"), path);
return NULL;
}
@@ -844,7 +845,7 @@ struct cache_entry *make_transient_cache_entry(unsigned int
mode, const struct o
int len;
if (!verify_path(path, mode)) {
- error("Invalid path '%s'", path);
+ error(_("invalid path '%s'"), path);
return NULL;
}
@@ -1297,12 +1298,12 @@ static int add_index_entry_with_check(struct
index_state *istate, struct cache_e
if (!ok_to_add)
return -1;
if (!verify_path(ce->name, ce->ce_mode))
- return error("Invalid path '%s'", ce->name);
+ return error(_("invalid path '%s'"), ce->name);
if (!skip_df_check &&
check_file_directory_conflict(istate, ce, pos, ok_to_replace)) {
if (!ok_to_replace)
- return error("'%s' appears as both a file and as a
directory",
+ return error(_("'%s' appears as both a file and as a
directory"),
ce->name);
pos = index_name_stage_pos(istate, ce->name, ce_namelen(ce),
ce_stage(ce));
pos = -pos-1;
@@ -1676,10 +1677,10 @@ static int verify_hdr(const struct cache_header *hdr,
unsigned long size)
int hdr_version;
if (hdr->hdr_signature != htonl(CACHE_SIGNATURE))
- return error("bad signature");
+ return error(_("bad signature 0x%08x"), hdr->hdr_signature);
hdr_version = ntohl(hdr->hdr_version);
if (hdr_version < INDEX_FORMAT_LB || INDEX_FORMAT_UB
[PATCH v3 02/16] alias.c: mark split_cmdline_strerror() strings for translation
This function can be part of translated messages. To make sure we
don't have a sentence with mixed languages, mark the strings for
translation, but only use translated strings in places we know we will
output translated strings.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
alias.c | 4 ++--
builtin/merge.c | 2 +-
git.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/alias.c b/alias.c
index a7e4e57130..c471538020 100644
--- a/alias.c
+++ b/alias.c
@@ -47,8 +47,8 @@ void list_aliases(struct string_list *list)
#define SPLIT_CMDLINE_BAD_ENDING 1
#define SPLIT_CMDLINE_UNCLOSED_QUOTE 2
static const char *split_cmdline_errors[] = {
- "cmdline ends with \\",
- "unclosed quote"
+ N_("cmdline ends with \\"),
+ N_("unclosed quote")
};
int split_cmdline(char *cmdline, const char ***argv)
diff --git a/builtin/merge.c b/builtin/merge.c
index 4aa6071598..92ba7e1c6d 100644
--- a/builtin/merge.c
+++ b/builtin/merge.c
@@ -577,7 +577,7 @@ static void parse_branch_merge_options(char *bmo)
argc = split_cmdline(bmo, );
if (argc < 0)
die(_("Bad branch.%s.mergeoptions string: %s"), branch,
- split_cmdline_strerror(argc));
+ _(split_cmdline_strerror(argc)));
REALLOC_ARRAY(argv, argc + 2);
MOVE_ARRAY(argv + 1, argv, argc + 1);
argc++;
diff --git a/git.c b/git.c
index 5fd30da093..c7e122cfc1 100644
--- a/git.c
+++ b/git.c
@@ -344,7 +344,7 @@ static int handle_alias(int *argcp, const char ***argv)
count = split_cmdline(alias_string, _argv);
if (count < 0)
die(_("bad alias.%s string: %s"), alias_command,
- split_cmdline_strerror(count));
+ _(split_cmdline_strerror(count)));
option_count = handle_options(_argv, , );
if (envchanged)
die(_("alias '%s' changes environment variables.\n"
--
2.19.1.1231.g84aef82467
[PATCH v3 03/16] archive.c: mark more strings for translation
Two messages also print extra information to be more useful
Signed-off-by: Nguyễn Thái Ngọc Duy
---
archive.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/archive.c b/archive.c
index 9d16b7fadf..d8f6e1ce30 100644
--- a/archive.c
+++ b/archive.c
@@ -385,12 +385,12 @@ static void parse_treeish_arg(const char **argv,
int refnamelen = colon - name;
if (!dwim_ref(name, refnamelen, , ))
- die("no such ref: %.*s", refnamelen, name);
+ die(_("no such ref: %.*s"), refnamelen, name);
free(ref);
}
if (get_oid(name, ))
- die("Not a valid object name");
+ die(_("not a valid object name: %s"), name);
commit = lookup_commit_reference_gently(ar_args->repo, , 1);
if (commit) {
@@ -403,7 +403,7 @@ static void parse_treeish_arg(const char **argv,
tree = parse_tree_indirect();
if (tree == NULL)
- die("not a tree object");
+ die(_("not a tree object: %s"), oid_to_hex());
if (prefix) {
struct object_id tree_oid;
@@ -413,7 +413,7 @@ static void parse_treeish_arg(const char **argv,
err = get_tree_entry(>object.oid, prefix, _oid,
);
if (err || !S_ISDIR(mode))
- die("current working directory is untracked");
+ die(_("current working directory is untracked"));
tree = parse_tree_indirect(_oid);
}
--
2.19.1.1231.g84aef82467
[PATCH v3 00/16] Mark more strings for translation
v3 is just small touchups here and there after the review comments
from v2.
Range-diff against v2:
1: e77c203313 ! 1: 936f84d4b3 git.c: mark more strings for translation
@@ -3,7 +3,7 @@
git.c: mark more strings for translation
One string is slightly updated to keep consistency with the rest:
-die() should with lowercase.
+die() should begin with lowercase.
diff --git a/git.c b/git.c
--- a/git.c
2: 12c9468696 = 2: 8ff2de14bf alias.c: mark split_cmdline_strerror()
strings for translation
3: bf662dc105 = 3: 8e147f6bff archive.c: mark more strings for translation
4: 1d032b0fdf = 4: 5d574460eb attr.c: mark more string for translation
5: 2c3ad8c262 = 5: 8e37efb756 read-cache.c: turn die("internal error") to
BUG()
6: df25ac78b1 = 6: 83b7b6d029 read-cache.c: mark more strings for
translation
7: 241e5a7450 = 7: 4bd085682d read-cache.c: add missing colon separators
8: 8f60728b0f = 8: fb72be3a1e reflog: mark strings for translation
9: 24d2ed6682 = 9: 711812d70b remote.c: turn some error() or die() to BUG()
10: 26e8cee291 = 10: 0213c1f5eb remote.c: mark messages for translation
11: 2409f76902 = 11: 85459c65ca repack: mark more strings for translation
12: afafe6771c ! 12: 21916a8fb4 parse-options: replace opterror() with
optname()
@@ -2,6 +2,11 @@
parse-options: replace opterror() with optname()
+Introduce optname() that does the early half of original opterror() to
+come up with the name of the option reported back to the user, and use
+it to kill opterror(). The callers of opterror() now directly call
+error() using the string returned by opterror() instead.
+
There are a few issues with opterror()
- it tries to assemble an English sentence from pieces. This is not
@@ -14,8 +19,7 @@
- Since it takes a string instead of printf format, one call site has
to assemble the string manually before passing to it.
-Kill it and produce the option name with optname(). The user will use
-error() directly. This solves the second and third problems.
+Using error() directly solves the second and third problems.
It kind helps the first problem as well because "%s does foo" does
give a translator a full sentence in a sense and let them reorder if
13: e8b4af8fac = 13: 8c9d2985b5 parse-options.c: turn some die() to BUG()
14: c3530e162e = 14: b81cdeda46 parse-options.c: mark more strings for
translation
15: 644c56780c ! 15: dd872fc39b fsck: reduce word legos to help i18n
@@ -19,7 +19,7 @@
- static struct strbuf buf = STRBUF_INIT;
- char *name = name_objects ?
- lookup_decoration(fsck_walk_options.object_names, obj) : NULL;
-+ static struct strbuf bufs[4] = {
++ static struct strbuf bufs[] = {
+ STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT
+ };
+ static int b = 0;
@@ -69,15 +69,18 @@
{
- objreport(obj, (type == FSCK_WARN) ? "warning" : "error", message);
- return (type == FSCK_WARN) ? 0 : 1;
-+ if (type == FSCK_WARN) {
++ switch (type) {
++ case FSCK_WARN:
+ fprintf_ln(stderr, "warning in %s %s: %s",
+ printable_type(obj), describe_object(obj), message);
+ return 0;
++ case FSCK_ERROR:
++ fprintf_ln(stderr, "error in %s %s: %s",
++ printable_type(obj), describe_object(obj), message);
++ return 1;
++ default:
++ BUG("%d (FSCK_IGNORE?) should never trigger this callback",
type);
+ }
-+
-+ fprintf_ln(stderr, "error in %s %s: %s",
-+ printable_type(obj), describe_object(obj), message);
-+ return 1;
}
static struct object_array pending;
16: 61af36567b ! 16: 8f1bbd1c65 fsck: mark strings for translation
@@ -28,22 +28,21 @@
return -1;
}
@@
- struct object *obj, int type, const char *message)
{
- if (type == FSCK_WARN) {
+ switch (type) {
+ case FSCK_WARN:
- fprintf_ln(stderr, "warning in %s %s: %s",
+ /* TRANSLATORS: e.g. warning in tree 01bfda:
*/
+ fprintf_ln(stderr, _("warning in %s %s: %s"),
printable_type(obj), describe_object(obj), message);
return 0;
- }
-
-- fprintf_ln(stderr, "error in %s %s: %s",
-+ /* TRANSLATORS: e.g. error in tree 01bfda: */
-+ fprintf_ln(stderr, _("error in %s %s: %s"),
- printable_type(obj), describe_object(obj), message);
- return 1;
- }
+ case FSCK_ERROR:
+- fprintf_ln(stderr, "error in %s %s: %s",
++ /* TRANSLATORS: e.g. error in tree 01bfda: */
++ fprintf_ln(stderr, _("error in %s %s: %s"),
+
[PATCH v3 16/16] fsck: mark strings for translation
Two die() are updated to start with lowercase to be consistent with
the rest.
Signed-off-by: Nguyễn Thái Ngọc Duy
---
builtin/fsck.c | 106 -
t/t1410-reflog.sh | 6 +--
t/t1450-fsck.sh| 50 -
t/t6050-replace.sh | 4 +-
t/t7415-submodule-names.sh | 6 +--
5 files changed, 90 insertions(+), 82 deletions(-)
diff --git a/builtin/fsck.c b/builtin/fsck.c
index 1feb6142f4..dcf72859a6 100644
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -85,7 +85,7 @@ static const char *printable_type(struct object *obj)
ret = type_name(obj->type);
if (!ret)
- ret = "unknown";
+ ret = _("unknown");
return ret;
}
@@ -116,7 +116,8 @@ static int fsck_config(const char *var, const char *value,
void *cb)
static int objerror(struct object *obj, const char *err)
{
errors_found |= ERROR_OBJECT;
- fprintf_ln(stderr, "error in %s %s: %s",
+ /* TRANSLATORS: e.g. error in tree 01bfda: */
+ fprintf_ln(stderr, _("error in %s %s: %s"),
printable_type(obj), describe_object(obj), err);
return -1;
}
@@ -126,11 +127,13 @@ static int fsck_error_func(struct fsck_options *o,
{
switch (type) {
case FSCK_WARN:
- fprintf_ln(stderr, "warning in %s %s: %s",
+ /* TRANSLATORS: e.g. warning in tree 01bfda:
*/
+ fprintf_ln(stderr, _("warning in %s %s: %s"),
printable_type(obj), describe_object(obj), message);
return 0;
case FSCK_ERROR:
- fprintf_ln(stderr, "error in %s %s: %s",
+ /* TRANSLATORS: e.g. error in tree 01bfda: */
+ fprintf_ln(stderr, _("error in %s %s: %s"),
printable_type(obj), describe_object(obj), message);
return 1;
default:
@@ -151,17 +154,18 @@ static int mark_object(struct object *obj, int type, void
*data, struct fsck_opt
*/
if (!obj) {
/* ... these references to parent->fld are safe here */
- printf("broken link from %7s %s\n",
- printable_type(parent), describe_object(parent));
- printf("broken link from %7s %s\n",
- (type == OBJ_ANY ? "unknown" : type_name(type)),
"unknown");
+ printf_ln(_("broken link from %7s %s"),
+ printable_type(parent), describe_object(parent));
+ printf_ln(_("broken link from %7s %s"),
+ (type == OBJ_ANY ? _("unknown") : type_name(type)),
+ _("unknown"));
errors_found |= ERROR_REACHABLE;
return 1;
}
if (type != OBJ_ANY && obj->type != type)
/* ... and the reference to parent is safe here */
- objerror(parent, "wrong object type in link");
+ objerror(parent, _("wrong object type in link"));
if (obj->flags & REACHABLE)
return 0;
@@ -177,8 +181,8 @@ static int mark_object(struct object *obj, int type, void
*data, struct fsck_opt
if (!(obj->flags & HAS_OBJ)) {
if (parent && !has_object_file(>oid)) {
- printf_ln("broken link from %7s %s\n"
- " to %7s %s",
+ printf_ln(_("broken link from %7s %s\n"
+ " to %7s %s"),
printable_type(parent),
describe_object(parent),
printable_type(obj),
@@ -246,8 +250,8 @@ static void check_reachable_object(struct object *obj)
return;
if (has_object_pack(>oid))
return; /* it is in pack - forget about it */
- printf("missing %s %s\n", printable_type(obj),
- describe_object(obj));
+ printf_ln(_("missing %s %s"), printable_type(obj),
+ describe_object(obj));
errors_found |= ERROR_REACHABLE;
return;
}
@@ -272,8 +276,8 @@ static void check_unreachable_object(struct object *obj)
* since this is something that is prunable.
*/
if (show_unreachable) {
- printf("unreachable %s %s\n", printable_type(obj),
- describe_object(obj));
+ printf_ln(_("unreachable %s %s"), printable_type(obj),
+ describe_object(obj));
return;
}
@@ -291,8 +295,8 @@ static void check_unreachable_object(struct object *obj)
*/
if (!(obj->flags & USED)) {
if (show_dangling)
- printf("dangling %s %s\n", printable_type(obj),
- describe_object(obj));
RE,
I have a deal for you, in your region.
[PATCH] coccicheck: introduce 'pending' semantic patches
From: SZEDER Gábor Teach `make coccicheck` to avoid patches named "*.pending.cocci" and handle them separately in a new `make coccicheck-pending` instead. This means that we can separate "critical" patches from "FYI" patches. The former target can continue causing Travis to fail its static analysis job, while the latter can let us keep an eye on ongoing (pending) transitions without them causing too much fallout. Document the intended use-cases around these two targets. As the process around the pending patches is not yet fully explored, leave that out. Based-on-work-by: SZEDER Gábor Signed-off-by: Stefan Beller Signed-off-by: Junio C Hamano --- I dialed back on the workflow, as we may want to explore it first before writing it down. Stefan Makefile | 7 +-- contrib/coccinelle/README | 41 +++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index bbfbb4292d..6bc4654828 100644 --- a/Makefile +++ b/Makefile @@ -2740,9 +2740,12 @@ endif then \ echo '' SPATCH result: $@; \ fi -coccicheck: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.cocci)) +coccicheck: $(addsuffix .patch,$(filter-out %.pending.cocci,$(wildcard contrib/coccinelle/*.cocci))) -.PHONY: coccicheck +# See contrib/coccinelle/README +coccicheck-pending: $(addsuffix .patch,$(wildcard contrib/coccinelle/*.pending.cocci)) + +.PHONY: coccicheck coccicheck-pending ### Installation rules diff --git a/contrib/coccinelle/README b/contrib/coccinelle/README index 9c2f8879c2..f0e80bd7f0 100644 --- a/contrib/coccinelle/README +++ b/contrib/coccinelle/README @@ -1,2 +1,43 @@ This directory provides examples of Coccinelle (http://coccinelle.lip6.fr/) semantic patches that might be useful to developers. + +There are two types of semantic patches: + + * Using the semantic transformation to check for bad patterns in the code; + The target 'make coccicheck' is designed to check for these patterns and + it is expected that any resulting patch indicates a regression. + The patches resulting from 'make coccicheck' are small and infrequent, + so once they are found, they can be sent to the mailing list as per usual. + + Example for introducing new patterns: + 67947c34ae (convert "hashcmp() != 0" to "!hasheq()", 2018-08-28) + b84c783882 (fsck: s/++i > 1/i++/, 2018-10-24) + + Example of fixes using this approach: + 248f66ed8e (run-command: use strbuf_addstr() for adding a string to + a strbuf, 2018-03-25) + f919ffebed (Use MOVE_ARRAY, 2018-01-22) + + These types of semantic patches are usually part of testing, c.f. + 0860a7641b (travis-ci: fail if Coccinelle static analysis found something + to transform, 2018-07-23) + + * Using semantic transformations in large scale refactorings throughout + the code base. + + When applying the semantic patch into a real patch, sending it to the + mailing list in the usual way, such a patch would be expected to have a + lot of textual and semantic conflicts as such large scale refactorings + change function signatures that are used widely in the code base. + A textual conflict would arise if surrounding code near any call of such + function changes. A semantic conflict arises when other patch series in + flight introduce calls to such functions. + + So to aid these large scale refactorings, semantic patches can be used. + However we do not want to store them in the same place as the checks for + bad patterns, as then automated builds would fail. + That is why semantic patches 'contrib/coccinelle/*.pending.cocci' + are ignored for checks, and can be applied using 'make coccicheck-pending'. + + This allows to expose plans of pending large scale refactorings without + impacting the bad pattern checks. -- 2.19.1.930.g4563a0d9d0-goog
lening aanvragen
-- Groet, groet aan jou, GELD BESCHIKBAAR VOOR LENING. Krijg het geld / de lening die u nodig hebt bij Funding Trusts Finance. Wij zijn particuliere kredietverstrekkers / investeerders en bieden zowel persoonlijke leningen, startleningen, educatieve / agrarische leningen, onroerend goed / bouwlening, onroerendgoedlening, schuldenvrije leningen, verhuur van mobiele woningen, lening met een hard geld, beveiligde / ongedekte lening, investeringsfinanciering, uitbreiding lening, Jv Capital / Rehab-leningen, aandelen / herfinancieringsleningen enz. aan geïnteresseerde personen uit welk land dan ook. We bieden leningen aan personen zowel nationaal als internationaal tegen een lage rente van 3%. Bent u geweigerd door banken of andere geldschieters? Funding Trusts Finance is er om u te helpen bij het archiveren van uw doel. Als je een lening van welk type dan ook nodig hebt, neem dan contact met ons op via het onderstaande e-mailadres en we zijn er om je te helpen krijgen wat je nodig hebt: i...@fundingtrustsfinance.com Je volledige naam: Adres: Telefoonnummer: Leningbedrag nodig: Looptijd: Bezetting: Maandelijks inkomensniveau: Geslacht: Geboortedatum: land: Doel: Zodra u deze informatie verstrekt, kunnen wij u de terugbetaling van de lening op basis van een maandelijkse basis aanbieden We wachten op je snelle reactie. Dank je. Met vriendelijke groet, Ronny Hens, E-mail: i...@fundingtrustsfinance.com
Re: [PATCH] Makefile: add pending semantic patches
On Thu, Nov 8, 2018 at 9:18 PM Junio C Hamano wrote:
>
> Stefan Beller writes:
>
> > From: SZEDER Gábor
> >
> > Add a description and place on how to use coccinelle for large refactorings
> > that happen only once.
> >
> > Based-on-work-by: SZEDER Gábor
> > Signed-off-by: Stefan Beller
> > ---
> >
> > I consider including this patch in a resend instead.
> > It outlays the basics of such a new workflow, which we can refine later.
>
> Thanks for tying loose ends.
>
> > diff --git a/contrib/coccinelle/README b/contrib/coccinelle/README
> > index 9c2f8879c2..fa09d1abcc 100644
> > --- a/contrib/coccinelle/README
> > +++ b/contrib/coccinelle/README
> > @@ -1,2 +1,62 @@
> > This directory provides examples of Coccinelle (http://coccinelle.lip6.fr/)
> > semantic patches that might be useful to developers.
> > +
> > +There are two types of semantic patches:
> > +
> > + * Using the semantic transformation to check for bad patterns in the code;
> > + This is what the original target 'make coccicheck' is designed to do and
> > + it is expected that any resulting patch indicates a regression.
> > + The patches resulting from 'make coccicheck' are small and infrequent,
> > + so once they are found, they can be sent to the mailing list as per
> > usual.
> > +
> > + Example for introducing new patterns:
> > + 67947c34ae (convert "hashcmp() != 0" to "!hasheq()", 2018-08-28)
> > + b84c783882 (fsck: s/++i > 1/i++/, 2018-10-24)
> > +
> > + Example of fixes using this approach:
> > + 248f66ed8e (run-command: use strbuf_addstr() for adding a string to
> > + a strbuf, 2018-03-25)
> > + f919ffebed (Use MOVE_ARRAY, 2018-01-22)
> > +
> > + These types of semantic patches are usually part of testing, c.f.
> > + 0860a7641b (travis-ci: fail if Coccinelle static analysis found
> > something
> > + to transform, 2018-07-23)
>
> Yup, and I think what we have in 'pu' (including your the_repository
> stuff) falls into this category.
My impression was that the_repository is strongly second category
as the_repository.cocci doesn't fix bad smells of code, but proposes
a refactoring that we agreed on doing.
> I've been paying attention to "make coccicheck" produced patches for
> the past few weeks, and so far, I found it _much_ _much_ more
> pleasant, compared to having to worry about merge conflicts with the
> topics in flight that changes day to day (not just because we add
> new topics or update existing topics, but also the order of the
> merge changes as topics mature at different rates and jumps over
> each other in master..pu history), that "make coccicheck" after
> topics are merged to integration branches fixes these issues up as
> needed.
So from your end we would not need the "pending" category as long
as the follow ups come in a timely manner?
>
> > + 3) Apply the semantic patch only partially, where needed for the patch
> > series
> > + that motivates the large scale refactoring and then build that series
> > + on top of it.
>
> It is not quite clear what "needed for the patch series" really
> means in the context of this paragraph. What are the changes that
> are not needed, that is still produced if we ran "make coccicheck"?
An example for "needed" would be
3f21279f50..bd8737176b
whereas "not needed" is what is in
"treewide: apply cocci patch".
The treewide conversion of e.g. unuse_commit_buffer to
repo_unuse_commit_buffer is nice, but "needed" only in
its followup patch that converts logmsg_reencode as that
calls into the unuse_commit_buffer.
> Are they wrong changes (e.g. a carelessly written read_cache() to
> read_index(_index) conversion may munge the implementation of
> read_cache(...) { return read_index(_index, ...); } and make
> inifinite recursion)? Or are they "correct but not immediately
> necessary" (e.g. because calling read_cache() does not hurt until
> that function gets removed, so rewriting the callers to call
> read_index() with _index may be correct but not immediately
> necessary)?
the latter. I assume correctness (of the semantic patch) to be a given,
but this is all about timing, i.e. how can I send the series without breaking
others in flight.
>
> > + By applying the semantic patch only partially where needed, the
> > series
> > + is less likely to conflict with other series in flight.
>
> That is correct.
>
> > + To make it possible to apply the semantic patch partially, there
> > needs
> > + to be mechanism for backwards compatibility to keep those places
> > working
> > + where the semantic patch is not applied. This can be done via a
> > + wrapper function that has the exact name and signature as the
> > function
> > + to be changed.
>
> OK, so this argues for leaving read_cache()-like things to help
> other in-flight topics, while a change to encourage the use of
> read_index() takes place. That also makes sense, and this directly
> relates to "less likely to conflict" benefit
Can I Trust You?
Dear friend, I am Abel Brent, a NATO soldier serving in Afghanistan. I and my Comrades, we are seeking your assistance to help us receive/invest our funds in your country in any lucrative business. Please if this proposal is acceptable by you, kindly respond back to me for more details. Thanks and waiting to hear from you Abel.
[PATCH] p3400: replace calls to `git checkout -b' by `git checkout -B'
p3400 makes a copy of the current repository to test git-rebase performance, and creates new branches in the copy with `git checkout -b'. If the original repository has branches with the same name as the script is trying to create, this operation will fail. This replaces these calls by `git checkout -B' to force the creation and update of these branches. Signed-off-by: Alban Gruin --- t/perf/p3400-rebase.sh | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/t/perf/p3400-rebase.sh b/t/perf/p3400-rebase.sh index ce271ca4c1..d202aaed06 100755 --- a/t/perf/p3400-rebase.sh +++ b/t/perf/p3400-rebase.sh @@ -6,9 +6,9 @@ test_description='Tests rebase performance' test_perf_default_repo test_expect_success 'setup rebasing on top of a lot of changes' ' - git checkout -f -b base && - git checkout -b to-rebase && - git checkout -b upstream && + git checkout -f -B base && + git checkout -B to-rebase && + git checkout -B upstream && for i in $(seq 100) do # simulate huge diffs @@ -35,8 +35,8 @@ test_perf 'rebase on top of a lot of unrelated changes' ' test_expect_success 'setup rebasing many changes without split-index' ' git config core.splitIndex false && - git checkout -b upstream2 to-rebase && - git checkout -b to-rebase2 upstream + git checkout -B upstream2 to-rebase && + git checkout -B to-rebase2 upstream ' test_perf 'rebase a lot of unrelated changes without split-index' ' -- 2.19.1
Re: [PATCH] Makefile: add pending semantic patches
On Thu, Nov 8, 2018 at 8:56 PM Martin Ågren wrote: > I haven't followed the original discussion too carefully, so I'll read > this like someone new to the topic probably would. Thanks! > A nit, perhaps, but I was genuinely confused at first. The subject is > "Makefile: add pending semantic patches", but this patch doesn't add > any. It adds Makefile-support for such patches though, and it defines > the entire concept of pending semantic patches. How about "coccicheck: > introduce 'pending' semantic patches"? > > > Add a description and place on how to use coccinelle for large refactorings > > that happen only once. > > A bit confused about "and place". Based on my understanding from reading > the remainder of this patch, maybe: > > Teach `make coccicheck` to avoid patches named "*.pending.cocci" and > handle them separately in a new `make coccicheck-pending` instead. > This means that we can separate "critical" patches from "FYI" patches. > The former target can continue causing Travis to fail its static > analysis job, while the latter can let us keep an eye on ongoing > (pending) transitions without them causing too much fallout. > > Document the intended use-cases and processes around these two > targets. Both suggested title and new commit message make sense. > > > This directory provides examples of Coccinelle (http://coccinelle.lip6.fr/) > > semantic patches that might be useful to developers. > > + > > +There are two types of semantic patches: > > + > > + * Using the semantic transformation to check for bad patterns in the code; > > + This is what the original target 'make coccicheck' is designed to do and > > Is it relevant that this was the "original" target? (Genuine question.) No. I can omit that part. > > > + it is expected that any resulting patch indicates a regression. > > + The patches resulting from 'make coccicheck' are small and infrequent, > > + so once they are found, they can be sent to the mailing list as per > > usual. > > + > > + Example for introducing new patterns: > > + 67947c34ae (convert "hashcmp() != 0" to "!hasheq()", 2018-08-28) > > + b84c783882 (fsck: s/++i > 1/i++/, 2018-10-24) > > + > > + Example of fixes using this approach: > > + 248f66ed8e (run-command: use strbuf_addstr() for adding a string to > > + a strbuf, 2018-03-25) > > + f919ffebed (Use MOVE_ARRAY, 2018-01-22) > > + > > + These types of semantic patches are usually part of testing, c.f. > > + 0860a7641b (travis-ci: fail if Coccinelle static analysis found > > something > > + to transform, 2018-07-23) > > Very nicely described, nice with the examples to quickly give a feeling > about how/when to use this. Thanks. > > > + * Using semantic transformations in large scale refactorings throughout > > + the code base. > > + > > + When applying the semantic patch into a real patch, sending it to the > > + mailing list in the usual way, such a patch would be expected to have a > > + lot of textual and semantic conflicts as such large scale refactorings > > + change function signatures that are used widely in the code base. > > + A textual conflict would arise if surrounding code near any call of such > > + function changes. A semantic conflict arises when other patch series in > > + flight introduce calls to such functions. > > OK, I'm with you. > > > + So to aid these large scale refactorings, semantic patches can be used, > > + using the process as follows: > > + > > + 1) Figure out what kind of large scale refactoring we need > > + -> This is usually done by another unrelated series. > > "This"? The figuring out, or the refactoring? Also, "unrelated"? The need and type of what kind of large scale refactoring are usually determined by a patch series, that is not refactoring for the sake of refactoring, but it wants to achieve a specific goal, unrelated to large refactorings per se. The large refactoring is just another tool that a developer can use to make their original series happen much faster. So "unrelated" == "not the large scale refactoring, as that may come as an preparatory series, but to have a preparatory series it may be good to showcase why we need the preparatory series" > > > + 2) Create the sematic patch needed for the large scale refactoring > > s/sematic/semantic/ yup. > > > + and store it in contrib/coccinelle/*.pending.cocci > > + -> The suffix containing 'pending' is important to differentiate > > + this case from the other use case of checking for bad patterns. > > Good. > > > + 3) Apply the semantic patch only partially, where needed for the patch > > series > > + that motivates the large scale refactoring and then build that series > > + on top of it. > > + By applying the semantic patch only partially where needed, the > > series > > + is less likely to conflict with other series in flight. > > + To make it possible to apply the
Re: [PATCH 2/2] built-in rebase: reinstate `checkout -q` behavior where appropriate
On Fri, Nov 09, 2018 at 06:21:41PM +0100, Johannes Schindelin wrote: > Actually, you got me thinking about the desc.buffer. And I think there is > one corner case where it could cause a problem: `struct tree_desc desc[2]` > does not initialize the buffers to NULL. And what if > fill_tree_descriptor() function returns NULL? Then the buffer is still > uninitialized. > > In practice, our *current* version of fill_tree_descriptor() never returns > NULL if the oid parameter is non-NULL. It would die() in the error case > instead (bad!). So to prepare for a less bad version, I'd rather > initialize the `desc` array and be on the safe (and easier to reason > about) side. Yeah, I agree with all of that. One solution would just be to increment only after success: if (fill_tree_descriptor([nr], ..) < 0) goto error; nr++; /* now we know it's valid! */ But there are lots of alternatives. :) -Peff
Re: [PATCH v1 1/1] Upcast size_t variables to uintmax_t when printing
On Fri, Nov 9, 2018 at 9:46 AM wrote: > When printing variables which contains a size, today "unsigned long" > is used at many places. > In order to be able to change the type from "unsigned long" into size_t > some day the future, we need to have a way to print 64 bit variables s/day/& in/ > on a system that has "unsigned long" defined to be 32 bit, link Win64. s/link/like/ > Upcast all those variables into uintmax_t before they are printed. > This is to prepare for a bigger change, when "unsligned long" s/unsligned/unsigned/ > will be converted into size_t for variables which may be > 4Gib. > > Signed-off-by: Torsten Bögershausen
[PATCH v2 1/1] remote-curl.c: xcurl_off_t is not portable (on 32 bit platfoms)
From: Torsten Bögershausen
When setting
DEVELOPER = 1
DEVOPTS = extra-all
"gcc (Raspbian 6.3.0-18+rpi1+deb9u1) 6.3.0 20170516" errors out with
"comparison is always false due to limited range of data type"
"[-Werror=type-limits]"
It turns out that the function xcurl_off_t() has 2 flavours:
- It gives a warning 32 bit systems, like Linux
- It takes the signed ssize_t as a paramter, but the only caller is using
a size_t (which is typically unsigned these days)
The original motivation of this function is to make sure that sizes > 2GiB
are handled correctly. The curl documentation says:
"For any given platform/compiler curl_off_t must be typedef'ed to a 64-bit
wide signed integral data type"
On a 32 bit system "size_t" can be promoted into a 64 bit signed value
without loss of data, and therefore we may see the
"comparison is always false" warning.
On a 64 bit system it may happen, at least in theory, that size_t is > 2^63,
and then the promotion from an unsigned "size_t" into a signed "curl_off_t"
may be a problem.
One solution to suppress a possible compiler warning could be to remove
the function xcurl_off_t().
However, to be on the very safe side, we keep it and improve it:
- The len parameter is changed from ssize_t to size_t
- A temporally variable "size" is used, promoted int uintmax_t and the comopared
with "maximum_signed_value_of_type(curl_off_t)".
Thanks to Junio C Hamano for this hint.
Signed-off-by: Torsten Bögershausen
---
This is a re-semd, the orignal patch was part of a 2
patch-series.
This patch needed some rework, and here should be
the polished version.
remote-curl.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/remote-curl.c b/remote-curl.c
index 762a55a75f..1220dffcdc 100644
--- a/remote-curl.c
+++ b/remote-curl.c
@@ -617,10 +617,11 @@ static int probe_rpc(struct rpc_state *rpc, struct
slot_results *results)
return err;
}
-static curl_off_t xcurl_off_t(ssize_t len) {
- if (len > maximum_signed_value_of_type(curl_off_t))
+static curl_off_t xcurl_off_t(size_t len) {
+ uintmax_t size = len;
+ if (size > maximum_signed_value_of_type(curl_off_t))
die("cannot handle pushes this big");
- return (curl_off_t) len;
+ return (curl_off_t)size;
}
static int post_rpc(struct rpc_state *rpc)
--
2.19.0.271.gfe8321ec05
Re: [PATCH 2/2] built-in rebase: reinstate `checkout -q` behavior where appropriate
Hi Peff,
On Fri, 9 Nov 2018, Jeff King wrote:
> On Fri, Nov 09, 2018 at 01:34:19AM -0800, Johannes Schindelin via
> GitGitGadget wrote:
>
> > From: Johannes Schindelin
> >
> > When we converted a `git checkout -q $onto^0` call to use
> > `reset_head()`, we inadvertently incurred a change from a twoway_merge
> > to a oneway_merge, as if we wanted a `git reset --hard` instead.
> >
> > This has performance ramifications under certain, though, as the
> > oneway_merge needs to lstat() every single index entry whereas
> > twoway_merge does not.
> >
> > So let's go back to the old behavior.
>
> Makes sense. I didn't think too hard about any possible gotchas with the
> twoway/oneway switch, but if that's what git-checkout was doing before,
> it seems obviously safe.
Right.
> > diff --git a/builtin/rebase.c b/builtin/rebase.c
> > index 6f6d7de156..c1cc50f3f8 100644
> > --- a/builtin/rebase.c
> > +++ b/builtin/rebase.c
> > @@ -523,11 +523,12 @@ finished_rebase:
> > #define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
> >
> > static int reset_head(struct object_id *oid, const char *action,
> > - const char *switch_to_branch, int detach_head,
> > + const char *switch_to_branch,
> > + int detach_head, int reset_hard,
>
> It might be worth switching to a single flag variable here. It would
> make calls like this:
>
> > - if (reset_head(>object.oid, "checkout", NULL, 1,
> > + if (reset_head(>object.oid, "checkout", NULL, 1, 0,
> > NULL, msg.buf))
>
> a little more self-documenting (if a little more verbose).
I thought about that, but for just two flags? Well, let me try it and see
whether I like the result ;-)
> > - if (!oid) {
> > - if (get_oid("HEAD", _oid)) {
> > - rollback_lock_file();
> > - return error(_("could not determine HEAD revision"));
> > - }
> > - oid = _oid;
> > + if (get_oid("HEAD", _oid)) {
> > + rollback_lock_file();
> > + return error(_("could not determine HEAD revision"));
> > }
>
> This one could actually turn into:
>
> ret = error(...);
> goto leave_reset_head;
>
> now. We don't have to worry about an uninitialized desc.buffer anymore
> (as I mentioned in the previous email), because "nr" would be 0.
>
> It doesn't save any lines, though (but maybe having a single
> cleanup/exit point would make things easier to read; I dunno).
But you're right, of course. Consistency makes for easier-to-read code.
> Take all my comments as observations, not objections. This looks OK to
> me either way.
Actually, you got me thinking about the desc.buffer. And I think there is
one corner case where it could cause a problem: `struct tree_desc desc[2]`
does not initialize the buffers to NULL. And what if
fill_tree_descriptor() function returns NULL? Then the buffer is still
uninitialized.
In practice, our *current* version of fill_tree_descriptor() never returns
NULL if the oid parameter is non-NULL. It would die() in the error case
instead (bad!). So to prepare for a less bad version, I'd rather
initialize the `desc` array and be on the safe (and easier to reason
about) side.
Thank you for your helpful review,
Dscho
Re: [PATCH 1/2] rebase: consolidate clean-up code before leaving reset_head()
Hi Peff,
On Fri, 9 Nov 2018, Jeff King wrote:
> On Fri, Nov 09, 2018 at 01:34:17AM -0800, Johannes Schindelin via
> GitGitGadget wrote:
>
> > diff --git a/builtin/rebase.c b/builtin/rebase.c
> > index 0ee06aa363..6f6d7de156 100644
> > --- a/builtin/rebase.c
> > +++ b/builtin/rebase.c
> > @@ -569,16 +569,13 @@ static int reset_head(struct object_id *oid, const
> > char *action,
> > }
> >
> > if (!fill_tree_descriptor(, oid)) {
> > - error(_("failed to find tree of %s"), oid_to_hex(oid));
> > - rollback_lock_file();
> > - free((void *)desc.buffer);
> > - return -1;
> > + ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
> > + goto leave_reset_head;
> > }
>
> If fill_tree_descriptor() fails, what is left in desc.buffer? Looking at
> the implementation, I think it's always NULL or a valid buffer. But I
> think all code paths actually die() unless we pass a NULL oid (and in
> that case desc.buffer would be NULL, too).
>
> So I think the original here that calls free() doesn't ever do anything
> but it did not hurt. After your patch, the leave_reset_head code would
> continue to call free(), and that's OK.
Right, that was my thinking, too.
> There are a few earlier conditionals in reset_head() that do only
> rollback_lock_file() that could similarly be converted to use the goto.
> But they would need desc.buffer to be initialized to NULL. I could go
> either way on converting them or not.
Whoops. I should have checked more carefully?
> > @@ -586,10 +583,9 @@ static int reset_head(struct object_id *oid, const
> > char *action,
> >
> > if (write_locked_index(the_repository->index, , COMMIT_LOCK) < 0)
> > ret = error(_("could not write index"));
> > - free((void *)desc.buffer);
> >
> > if (ret)
> > - return ret;
> > + goto leave_reset_head;
> >
> > reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
> > strbuf_addf(, "%s: ", reflog_action ? reflog_action : "rebase");
> > @@ -622,7 +618,10 @@ static int reset_head(struct object_id *oid, const
> > char *action,
> > UPDATE_REFS_MSG_ON_ERR);
> > }
> >
> > +leave_reset_head:
> > strbuf_release();
> > + rollback_lock_file();
> > + free((void *)desc.buffer);
> > return ret;
>
> We get here on success, too. So we may call rollback_lock_file() on an
> already-committed lock. This is explicitly documented as a no-op by the
> lock code, so that's OK.
Indeed. I did not check the documentation, but the code, and came to the
same conclusion.
> So overall looks good to me.
Thanks!
Dscho
Re: [PATCH v4 0/3] range-diff fixes
Hi Ævar, On Fri, 9 Nov 2018, Ævar Arnfjörð Bjarmason wrote: > Addresses feedback on v3, especially Eric's suggestion to split out > the behavior change (which I was not aware of) into a 3/3. For the record, I am fine with this iteration, too. Ciao, Dscho
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
On Fri, Nov 9, 2018 at 11:19 AM Jeff King wrote: > > The form `/abc/def` would not be confused with anything > > that it is not, I would think. The only thing against this form (at least > > that I can think of) is that some people use this way to talk about paths > > that vary between different setups, with the implicit assumption that the > > reader will "interpolate" this *before* applying. So for example, when I > > tell a user to please edit their /config, I implicitly assume > > that they know to not type out, literally, but instead fill in > > the correct path. > > So yeah, some alternative syntax that is verbose but distinct makes > sense to me. We use %-substitution elsewhere. Could we do something with > that? "%RUNTIME_PREFIX%" gives me too many flashbacks, but something > like "%(RUNTIME_PREFIX)" matches our formatting language. FWIW I don't have any preference, as long as the variable can still have a name (that is not a symbol). A side question regardless of syntax. What do we do with %(unrecognized name)/foo? I see three options - expand to empty, so "/foo" - keep it and try the literal path "%(unrecognized name)/foo" - somehow tell the caller that the path is invalid and treat it like non-existing path, even if there is some real thing at "%(unrecognized name)/foo" The last option is more predictable. But we need to be more careful about the syntax because if "%(some path like this)" actually exists somewhere, then it will be broken. And I think it's also more work. -- Duy
Re: [RFC PATCH] index-pack: improve performance on NFS
On Fri, Nov 9, 2018 at 2:46 PM Ævar Arnfjörð Bjarmason wrote: > I'm planning to re-submit mine with some minor changes after the great > Documentation/config* move lands. > > As noted in > https://public-inbox.org/git/87bm7clf4o@evledraar.gmail.com/ and > https://public-inbox.org/git/87h8gq5zmc@evledraar.gmail.com/ I think > it's regardless of Jeff's optimization is. O(nothing) is always faster > than O(something), particularly (as explained in that E-Mail) on NFS. Is it really worth adding more code to maintain just to shave a couple seconds (or a few percent clone time)? -- Duy
[PATCH v1 1/1] Upcast size_t variables to uintmax_t when printing
From: Torsten Bögershausen
When printing variables which contains a size, today "unsigned long"
is used at many places.
In order to be able to change the type from "unsigned long" into size_t
some day the future, we need to have a way to print 64 bit variables
on a system that has "unsigned long" defined to be 32 bit, link Win64.
Upcast all those variables into uintmax_t before they are printed.
This is to prepare for a bigger change, when "unsligned long"
will be converted into size_t for variables which may be > 4Gib.
Signed-off-by: Torsten Bögershausen
---
archive-tar.c | 2 +-
builtin/cat-file.c | 4 ++--
builtin/fast-export.c | 2 +-
builtin/index-pack.c | 9 +
builtin/ls-tree.c | 2 +-
builtin/pack-objects.c | 12 ++--
diff.c | 2 +-
fast-import.c | 4 ++--
http-push.c| 2 +-
ref-filter.c | 2 +-
sha1-file.c| 6 +++---
11 files changed, 24 insertions(+), 23 deletions(-)
diff --git a/archive-tar.c b/archive-tar.c
index 7a535cba24..a58e1a8ebf 100644
--- a/archive-tar.c
+++ b/archive-tar.c
@@ -202,7 +202,7 @@ static void prepare_header(struct archiver_args *args,
unsigned int mode, unsigned long size)
{
xsnprintf(header->mode, sizeof(header->mode), "%07o", mode & 0);
- xsnprintf(header->size, sizeof(header->size), "%011lo", S_ISREG(mode) ?
size : 0);
+ xsnprintf(header->size, sizeof(header->size), "%011"PRIoMAX ,
S_ISREG(mode) ? (uintmax_t)size : (uintmax_t)0);
xsnprintf(header->mtime, sizeof(header->mtime), "%011lo", (unsigned
long) args->time);
xsnprintf(header->uid, sizeof(header->uid), "%07o", 0);
diff --git a/builtin/cat-file.c b/builtin/cat-file.c
index 8d97c84725..05decee33f 100644
--- a/builtin/cat-file.c
+++ b/builtin/cat-file.c
@@ -92,7 +92,7 @@ static int cat_one_file(int opt, const char *exp_type, const
char *obj_name,
oi.sizep =
if (oid_object_info_extended(the_repository, , , flags)
< 0)
die("git cat-file: could not get object info");
- printf("%lu\n", size);
+ printf("%"PRIuMAX"\n", (uintmax_t)size);
return 0;
case 'e':
@@ -238,7 +238,7 @@ static void expand_atom(struct strbuf *sb, const char
*atom, int len,
if (data->mark_query)
data->info.sizep = >size;
else
- strbuf_addf(sb, "%lu", data->size);
+ strbuf_addf(sb, "%"PRIuMAX , (uintmax_t)data->size);
} else if (is_atom("objectsize:disk", atom, len)) {
if (data->mark_query)
data->info.disk_sizep = >disk_size;
diff --git a/builtin/fast-export.c b/builtin/fast-export.c
index 456797c12a..5790f0d554 100644
--- a/builtin/fast-export.c
+++ b/builtin/fast-export.c
@@ -253,7 +253,7 @@ static void export_blob(const struct object_id *oid)
mark_next_object(object);
- printf("blob\nmark :%"PRIu32"\ndata %lu\n", last_idnum, size);
+ printf("blob\nmark :%"PRIu32"\ndata %"PRIuMAX"\n", last_idnum,
(uintmax_t)size);
if (size && fwrite(buf, size, 1, stdout) != 1)
die_errno("could not write blob '%s'", oid_to_hex(oid));
printf("\n");
diff --git a/builtin/index-pack.c b/builtin/index-pack.c
index 2004e25da2..2a8ada432b 100644
--- a/builtin/index-pack.c
+++ b/builtin/index-pack.c
@@ -450,7 +450,8 @@ static void *unpack_entry_data(off_t offset, unsigned long
size,
int hdrlen;
if (!is_delta_type(type)) {
- hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %lu", type_name(type),
size) + 1;
+ hdrlen = xsnprintf(hdr, sizeof(hdr), "%s %"PRIuMAX,
+ type_name(type),(uintmax_t)size) + 1;
the_hash_algo->init_fn();
the_hash_algo->update_fn(, hdr, hdrlen);
} else
@@ -1628,10 +1629,10 @@ static void show_pack_info(int stat_only)
chain_histogram[obj_stat[i].delta_depth - 1]++;
if (stat_only)
continue;
- printf("%s %-6s %lu %lu %"PRIuMAX,
+ printf("%s %-6s %"PRIuMAX" %"PRIuMAX" %"PRIuMAX,
oid_to_hex(>idx.oid),
- type_name(obj->real_type), obj->size,
- (unsigned long)(obj[1].idx.offset - obj->idx.offset),
+ type_name(obj->real_type), (uintmax_t)obj->size,
+ (uintmax_t)(obj[1].idx.offset - obj->idx.offset),
(uintmax_t)obj->idx.offset);
if (is_delta_type(obj->type)) {
struct object_entry *bobj =
[obj_stat[i].base_object_no];
diff --git a/builtin/ls-tree.c b/builtin/ls-tree.c
index fe3b952cb3..7d581d6463 100644
--- a/builtin/ls-tree.c
+++ b/builtin/ls-tree.c
@@ -100,7 +100,7 @@ static int show_tree(const struct
Re: [RFC PATCH] index-pack: improve performance on NFS
On Wed, Nov 07 2018, Geert Jansen wrote: > On Mon, Oct 29, 2018 at 07:27:39PM -0400, Jeff King wrote: > >> On Mon, Oct 29, 2018 at 08:36:07PM +0100, Ævar Arnfjörð Bjarmason wrote: >> > * Re-roll my 4 patch series to include the patch you have in >> ><20181027093300.ga23...@sigill.intra.peff.net> >> >> I don't think it's quite ready for inclusion as-is. I hope to brush it >> up a bit, but I have quite a backlog of stuff to review, as well. > > We're still quite keen to get this patch included. Is there anything I can do > to help? > > Also I just re-read your comments on maximum cache size. I think you were > arguing both sides of the equation and I wasn't sure where you'd ended up. :) > A larger cache size potentially takes more time to fill up especially on NFS > while a smaller cache size obviously would less effective. That said a small > cache is still effective for the "clone" case where the repo is empty. > > It also occurred to me that as a performance optimization your patch could > read > the the loose object directories in parallel using a thread pool. At least on > Amazon EFS this should result in al almost linear performance increase. I'm > not > sure how much this would help for local file systems. In any case this may be > best done as a follow-up patch (that I'd be happy to volunteer for). I'm planning to re-submit mine with some minor changes after the great Documentation/config* move lands. As noted in https://public-inbox.org/git/87bm7clf4o@evledraar.gmail.com/ and https://public-inbox.org/git/87h8gq5zmc@evledraar.gmail.com/ I think it's regardless of Jeff's optimization is. O(nothing) is always faster than O(something), particularly (as explained in that E-Mail) on NFS. You didn't answer my question in https://public-inbox.org/git/20181030024925.gc8...@amazon.com/ about whether for your purposes you're interested in this for something where it needs to work out of the box on some random Amazon's customer's "git", or if it's something in-house and you just don't want to turn off collision checking. That would be useful to know. I've turned on core.checkCollisions=false in production at our site. Cloning for some large repositories went from ~200 minutes to ~5m, and some pushes from ~5 minutes to ~10 seconds. Those numbers will be very similar (but slightly higher, maybe 1-5 seconds higher in the latter case) with Jeff's (depending on the push).
Re: [PATCH v4 3/3] range-diff: make diff option behavior (e.g. --stat) consistent
On Friday, November 9, 2018 3:18:03 AM MST Ævar Arnfjörð Bjarmason wrote: > > But we should behave consistently with "diff" in anticipation of such > output being useful in the future, because it would make for confusing > UI if two "diff" and "range-diff" behaved differently when it came to 's/ two//' > how they interpret diff options. >
Re: Git Evolve
Hi,
On Tue, 2 Oct 2018, Ævar Arnfjörð Bjarmason wrote:
> On Tue, Oct 02 2018, Taylor Blau wrote:
>
> > Hi Stefan,
> >
> > On Sat, Sep 29, 2018 at 04:00:04PM -0700, Stefan Xenos wrote:
> >> Hello, List!
> >>
> >> I'm interested in porting something like Mercurial's evolve command to
> >> Git.
> >
> > Welcome to Git :-). I think that the discussion in this thread is good,
> > but it's not why I'm replying. I have also wanted a Mercurial feature in
> > Git, but a different one than yours.
> >
> > Specifically, I've wanted the 'hg absorb' command. My understanding of
> > the commands functionality is that it builds a sort of flamegraph-esque
> > view of the blame, and then cascades downwards parts of a change. I am
> > sure that I'm not doing the command justice, so I'll defer to [1] where
> > it is explained in more detail.
> >
> > The benefit of this command is that it gives you a way to--without
> > ambiguity--absorb changes into earlier commits, and in fact, the
> > earliest commit that they make sense to belong to.
> >
> > This would simplify my workflow greatly when re-rolling patches, as I
> > often want to rewrite a part of an earlier commit. This is certainly
> > possible by a number of different `git rebase` invocations (e.g., (1)
> > create fixup commits, and then re-order them, or (2) mark points in your
> > history as 'edit', and rewrite them in a detached state, and I'm sure
> > many more).
> >
> > I'm curious if you or anyone else has thought about how this might work
> > in Git.
>
> I've wanted a "git absorb" for a while, but have done no actual work on
> it, I just found out about it.
>
> I think a combination of these two heuristics would probably do the
> trick:
>
> 1. If a change in your "git diff" output has a hunk whose lines overlap
> with an earlier commit in the @{u}.. range, we do the equivalent of
> "git add -p", select that hunk, and "git commit --fixup commit>". We fixup the most recent commit that matches (otherwise
> commit>we'd conflict).
>
> 2. Have some mode where we fall back from #1 and consider changes to
> entire files, if that's unambiguous.
>
> The neat thing about this would be that you could tweak how promiscuous
> #1 would be via the -U option to git-diff, and #2 would just be a
> special case of -U9 (we should really add a -Uinf...).
>
> Then once you ran this you could run "git rebase -i --autosquash" to see
> how the TODO list would look, and optionally have some "git absorb
> --now" or whatever to do the "git add -p", "git commit --fixup" and "git
> rebase --autosquash" all in one go.
This is essentially what the script does that I sent to this here mailing
list back in May:
https://public-inbox.org/git/nycvar.qro.7.76.6.1805052220360...@tvgsbejvaqbjf.bet/
I used this quite a lot, but I still find it slow (especially on Windows),
so I am still in search for a better solution. And yes, I was intrigued by
`hg absorb` when it was presented at GitMerge last year, and wanted to
have the same.
In the meantime, what I often do is to call `git log -L :`
where and are taken from the hunk(s) of the current diff.
Actually, I have a script to do that, hidden behind a Git alias. Then I
inspect the diffs in that log and call `git commit --fixup` with the one I
deem most appropriate.
Note that it sometimes fails because of semantic dependencies. That is,
even if my current change overlaps with an earlier change, it may be too
early to be squashed in.
As an example: imagine that I moved a git_config() call from some function
into the cmd_() function. Only: I intended to move it, but in
fact, I just added the call to the latter function. Eventually, I figure
it out! So I want to make a fixup! commit. My script, as well as Ævar's
suggestion, as well as literally all the other attempts to solve this that
I am aware of, would try to squash this change into whichever commit
introduced the function that originally called git_config(). But that is
wrong. It should be squashed into the commit that added the git_config()
call to the cmd_() function.
Ciao,
Dscho
>
> > [1]: http://files.lihdd.net/hgabsorb-note.pdf
>
Re: GPG signing is bent on WIndows
Hi Jeff, On Wed, 26 Sep 2018, Jeffrey Walton wrote: > Several weeks ago I updated to the latest Git for Windows (when > prompted by the version check). At the time I noticed: > > $ git commit -S -am "Fix unset MAKE variable in test scripts" > gpg: signing failed: No pinentry > gpg: signing failed: No pinentry > error: gpg failed to sign the data > fatal: failed to write commit object > > I got to look at it today. On Windows: > > $ cat ~/.gnupg/gpg-agent.conf > pinentry-program > /usr/local/MacGPG2/libexec/pinentry-mac.app/Contents/MacOS/pinentry-mac Git for Windows is partially based off of MSYS2, which upgraded GPG from v1 to v2, and one of the consequences is that v2 handles interaction with the user differently. My guess is that you copied your config from a Mac, and the path is simply incorrect. I would wager a bet that it starts working when you remove that line with the incorrect path, as the default should work plenty fine for you. Ciao, Johannes
Re: [GSoC][PATCH v8 14/20] stash: convert create to builtin
Hi Paul, On Wed, 26 Sep 2018, Paul-Sebastian Ungureanu wrote: > Sorry for the late reply. I had a lot on my plate for the last couple of > weeks. I understand. University can be busy times. > > > + > > > + git_config(git_diff_basic_config, NULL); > > > > Is this not called in as part of `git_config(git_default_config, NULL);` > > in cmd_stash() already? > > > > *clicketyclick* > > > > I guess not. But then, maybe it would make sense to run with > > `git_diff_basic_config` from the get go, to avoid having to run > > `git_config()` twice. > > I am not sure I got it right, but if I did: > > Running `git_config` with `git_diff_basic_config` from the > beginning wouldn't be pointless when we would use any other > command than `create`, `push` and `save`? Although it might > confuse the reader a little bit, the stash should run without > problems. In my mind, this would simplify the code. Which is always easier on the reader... ;-) Ciao, Dscho
Re: [GSoC][PATCH v8 18/20] stash: convert `stash--helper.c` into `stash.c`
Hi Paul,
On Wed, 26 Sep 2018, Paul-Sebastian Ungureanu wrote:
> Hi,
>
> > > @@ -1443,9 +1448,10 @@ static int push_stash(int argc, const char
> > > **argv, const char *prefix)
> > > OPT_END()
> > >};
> > > - argc = parse_options(argc, argv, prefix, options,
> > > - git_stash_helper_push_usage,
> > > - 0);
> > > + if (argc)
> > > + argc = parse_options(argc, argv, prefix, options,
> > > + git_stash_push_usage,
> > > + 0);
> >
> > Is this `if (argc)` guard necessary?
>
> Yes because without it, there would be a seg fault when
> calling `git stash`. Why?
>
> After spawning `git stash`, in `push_stash()`: `argc` would be
> 0 (and `argv` would be `NULL`).
I *think* what you want to do, then, is to pass PARSE_OPT_KEEP_ARGV0 in
the first parse_options() call. In general, this needs to be done any time
you might want to call `parse_options()` on the remaining options.
Looking forward to v10,
Dscho
> `parse_options()` calls `parse_options_start()` which does the following:
>
> ctx->argc = ctx->total = argc - 1;
> ctx->argv = argv + 1;
>
> So, `ctx->argc` would be `-1`. After we are back to `parse_options()`,
> `parse_options_step()` would be called. In `parse_options_step()`:
>
> for (; ctx->argc; ctx->argc--, ctx->argv++)
>
> Which is an infinite loop, because `ctx->argc` is already -1.
>
> This check is also done for `git notes list` (function `list()`
> inside `builtin/notes.c`). Now, that I relook at it, it seems to me
> that this is a bug. Probably a better solution would be to check at the
> beginning of `parse_options()` and return early if `argc` is less then one.
>
> > > @@ -1536,7 +1544,44 @@ int cmd_stash__helper(int argc, const char **argv,
> > > const char *prefix)
> > > return !!push_stash(argc, argv, prefix);
> > >else if (!strcmp(argv[0], "save"))
> > > return !!save_stash(argc, argv, prefix);
> > > + else if (*argv[0] != '-')
> > > + usage_msg_opt(xstrfmt(_("unknown subcommand: %s"), argv[0]),
> > > + git_stash_usage, options);
> > > +
> > > + if (strcmp(argv[0], "-p")) {
> > > + while (++i < argc && strcmp(argv[i], "--")) {
> > > + /*
> > > + * `akpqu` is a string which contains all short
> > > options,
> > > + * except `-m` which is verified separately.
> > > + */
> > > + if ((strlen(argv[i]) == 2) && *argv[i] == '-' &&
> > > + strchr("akpqu", argv[i][1]))
> >
> > I *think* this is missing the `n`.
>
> I guess that by `n` you are referring to the short option of
> `--no-keep-index`, which is missing because it was also omitted
> in `stash.sh`. I am not sure whether it is worth adding it. In
> case `stash` will learn any other option starting with `n`, this
> might create confusion amongst users.
>
> Best regards,
> Paul
>
Re: [PATCH] travis-ci: install packages in 'ci/install-dependencies.sh'
Hi, On Fri, 9 Nov 2018, Junio C Hamano wrote: > SZEDER Gábor writes: > > >> > I'm not sure about the last paragraph, because: > >> > > >> > - It talks about presumed benefits for a currently still > >> > work-in-progress patch series of an other contributor, and I'm not > >> > really sure that that's a good thing. Perhaps I should have > >> > rather put it below the '---'. > >> > > >> > - I'm confused about the name of this Azure thing. The cover letter > >> > mentions "Azure Pipelines", the file is called > >> > 'azure-pipelines.yml', but the relevant patch I link to talks > >> > about "Azure DevOps" in the commit message. > >> > > >> > Anyway, keep that last paragraph or drop it as you see fit. > >> > >> I hope we'll hear from Dscho in one or two revolutions of the Earth > >> ;-) > > > > ... revolutions around what? :) > > Originally I meant its own axis, but perhaps the moon. I see, you had fun talking about a revolution [*1*]... I am much in favor of this patch, as it indeed will simplify the integration into Azure Pipelines. As to the DevOps vs Pipelines thing: DevOps is the umbrella, it consists of much more than just the automated builds (Pipelines), it also has Boards, Packages, etc, but I don't think we will ever use that in the Git project, as our workflow is based out of a mailing list. Ciao, Dscho Footnote *1*: https://www.youtube.com/watch?v=Xv8FBjo1Y8I
Re: What's cooking in git.git (Nov 2018, #03; Wed, 7)
Hi Phillip, On Thu, 8 Nov 2018, Phillip Wood wrote: > On 07/11/2018 09:41, Junio C Hamano wrote: > > 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. > > > > 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 > > > > -- > > > > * pw/add-p-select (2018-07-26) 4 commits > > - add -p: optimize line selection for short hunks > > - add -p: allow line selection to be inverted > > - add -p: select modified lines correctly > > - add -p: select individual hunk lines > > > > "git add -p" interactive interface learned to let users choose > > individual added/removed lines to be used in the operation, instead > > of accepting or rejecting a whole hunk. > > > > Will discard. > > No further feedbacks on the topic for quite some time. > > Unfortunately I've not found time to work on this recently and don't see > myself doing so before the new year so I think it makes sense to drop it. > Hopefully I can come up with something next year, it would be nice for users > to avoid having to edit patches where possible. If I understand correctly, this patch series essentially imitates on the console what you can do in Git GUI by selecting multiple lines in the diff and then "Stage Selected Lines"? If my understanding is correct, then yes, I would be very much in favor of having this feature, too. Thanks, Dscho
[PATCH v2 0/1] Update .mailmap
I noticed that there were a couple of duplicate entries in git shortlog -nse v2.19.1.., and then continued a bit to remove the duplicate entries even for v2.10.0.. Changes relative to v1: * Fixed the commit message (it talked about the opposite commit range than intended). * Added a formerly missing space between the email addresses of Masaya. Johannes Schindelin (1): Update .mailmap .mailmap | 13 + 1 file changed, 13 insertions(+) base-commit: 8858448bb49332d353febc078ce4a3abcc962efe Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-71%2Fdscho%2Fmailmap-v2 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-71/dscho/mailmap-v2 Pull-Request: https://github.com/gitgitgadget/git/pull/71 Range-diff vs v1: 1: b590a9bebf ! 1: c121ebc474 Update .mailmap @@ -2,7 +2,7 @@ Update .mailmap -This patch makes the output of `git shortlog -nse v2.10.0` +This patch makes the output of `git shortlog -nse v2.10.0..master` duplicate-free. Signed-off-by: Johannes Schindelin @@ -47,7 +47,7 @@ Mark Rada Martin Langhoff Martin von Zweigbergk -+Masaya Suzuki ++Masaya Suzuki Matt Draisey Matt Kraai Matt McCutchen -- gitgitgadget
[PATCH v2 1/1] Update .mailmap
From: Johannes Schindelin This patch makes the output of `git shortlog -nse v2.10.0..master` duplicate-free. Signed-off-by: Johannes Schindelin --- .mailmap | 13 + 1 file changed, 13 insertions(+) diff --git a/.mailmap b/.mailmap index bef3352b0d..eb7b5fc7b9 100644 --- a/.mailmap +++ b/.mailmap @@ -21,6 +21,8 @@ Anders Kaseorg Aneesh Kumar K.V Amos Waterland Amos Waterland +Ben Peart +Ben Peart Ben Walton Benoit Sigoure Bernt Hansen @@ -32,6 +34,7 @@ Bryan Larsen Cheng Renquan Chris Shoemaker Chris Wright +Christian Ludwig Cord Seele Christian Couder Christian Stimming @@ -51,6 +54,7 @@ David Reiss David S. Miller David Turner David Turner +Derrick Stolee Deskin Miller Dirk Süsserott Eric Blake @@ -98,6 +102,7 @@ Jens Axboe Jens Lindström Jens Lindstrom Jim Meyering Joachim Berdal Haga +Joachim Jablon Johannes Schindelin Johannes Sixt Johannes Sixt @@ -150,6 +155,7 @@ Mark Levedahl Mark Rada Martin Langhoff Martin von Zweigbergk +Masaya Suzuki Matt Draisey Matt Kraai Matt McCutchen @@ -157,6 +163,7 @@ Matthias Kestenholz Matthias Rüster Matthias Ruester Matthias Urlichs Matthias Urlichs +Matthieu Moy Michael Coleman Michael J Gruber Michael J Gruber @@ -180,7 +187,11 @@ Nick Stokoe Nick Woolley Nick Stokoe Nick Woolley Nicolas Morey-Chaisemartin Nicolas Morey-Chaisemartin +Nicolas Morey-Chaisemartin +Nicolas Morey-Chaisemartin +Nicolas Morey-Chaisemartin Nicolas Sebrecht +Orgad Shaneh Paolo Bonzini Pascal Obry Pascal Obry @@ -200,6 +211,7 @@ Philipp A. Hartmann Philippe Bruhat Ralf Thielow Ramsay Jones +Randall S. Becker René Scharfe René Scharfe Rene Scharfe Richard Hansen @@ -238,6 +250,7 @@ Steven Walter Sven Verdoolaege Sven Verdoolaege SZEDER Gábor +Tao Qingyun <845767...@qq.com> Tay Ray Chuan Ted Percival Theodore Ts'o -- gitgitgadget
Re: [PATCH 1/1] Update .mailmap
Hi Junio, On Fri, 9 Nov 2018, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" > writes: > > > From: Johannes Schindelin > > > > This patch makes the output of `git shortlog -nse v2.10.0` > > duplicate-free. > > Did you mean "v2.10.0..master" or did you really mean this covers > authors recorded up to v2.10.0? Judging from the cover letter, I > think you meant the former. D'oh. Yes, I meant v2.10.0..master. > Can you say a bit more about how one among multiple addresses for > each person was chosen in the log message? E.g. "After asking each > author which one is the preferred one", "Picked the one with the > most recent committer timestamps", "There were two for each but one > of them were bouncing", etc. I looked at each of the authors' mails and tried to determine which one was the preferred one. For example, Masaya sent a patch from their gmail account but signed off using their google account, so I figured the latter was preferable. Nicolas, on the other hand, already had a couple of entries in .mailmap, so I picked the one that seemed to be preferred judging by the .mailmap even if it was not in use lately. For Christian, it was gmail vs googlemail, and I picked the former, as it is more common these days. For Ben and Stolee, I used their Microsoft accounts (preferring the one Ben used originally). For Joachim, Matthieu, Randall and Tao, I used the more personalized email address. For Orgad, I used his personal one rather than his work email. > > @@ -150,6 +155,7 @@ Mark Levedahl > > Mark Rada > > Martin Langhoff > > Martin von Zweigbergk > > > > +Masaya Suzuki > > It is a bit surprising that we can take an entry without SP in > between two addresses and still behave sensibley, but it probably > makes more sense to add one just to look similar to other entries. Whoops. Thanks, fixed! > Thanks for working on this. You're welcome. v2 about to be sent, Dscho
[no subject]
Dear friend, I am very gald and happy that you answerred this message very fast, My name is Veronica Ali. a France Nationality, I am a widow, currently hospitalized due to cancer illness . Meanwhile, I have decided to donate my fund to you as a reliable individual that will use this money wisely, €3,800.000 Million Euros. to help the poor and less privileged. So if you are willing to accept this offer and do exactly as I will instruct, then get back to me for more details. Mrs Veronica Ali.
Re: [ANNOUNCE] Git Merge Contributor's Summit Jan 31, 2019, Brussels
On Fri, Nov 09, 2018 at 10:44:10AM +, Luca Milanesio wrote: > > On 9 Nov 2018, at 10:42, Jeff King wrote: > > > > Git Merge 2019 is happening on February 1st. There will be a > > Contributor's Summit the day before. Here are the details: > > > > When: Thursday, January 31, 2019. 10am-5pm. > > Where: The Egg[1], Brussels, Belgium > > What: Round-table discussion about Git > > Who: All contributors to Git or related projects in the Git ecosystem > > are invited; if you're not sure if you qualify, please ask! > > Hi Jeff, > is Gerrit included in the "Git ecosystem"? Yeah, I think so. At least the Git parts of it. :) -Peff
Re: [ANNOUNCE] Git Merge Contributor's Summit Jan 31, 2019, Brussels
> On 9 Nov 2018, at 10:42, Jeff King wrote: > > Git Merge 2019 is happening on February 1st. There will be a > Contributor's Summit the day before. Here are the details: > > When: Thursday, January 31, 2019. 10am-5pm. > Where: The Egg[1], Brussels, Belgium > What: Round-table discussion about Git > Who: All contributors to Git or related projects in the Git ecosystem > are invited; if you're not sure if you qualify, please ask! Hi Jeff, is Gerrit included in the "Git ecosystem"? Luca. > > This email is just to announce the date so people can start planning. > You'll need to register eventually, but I don't have the invite codes > yet. Registration should be similar to past years. In particular, if > you're coming to the contrib summit, DON'T register for the main > conference yet. I think the codes I get will cover both (and as with > previous years, contrib summit attendees will have the option of a > complimentary pass to the main conference, or can pay €99 that goes to > Software Freedom Conservancy). > > The content / agenda is whatever we choose. There's some more discussion > on format in this thread from August: > > https://public-inbox.org/git/20180813163108.ga6...@sigill.intra.peff.net/ > > though it sounds like people are mostly on board with what we've done in > past years. I'm looking into getting better A/V for remote attendance, > but I'm not sure yet what will be possible. > > Any thoughts or discussion on format, content, etc are welcome. The only > thing set so far is the time and place. :) > > -Peff > > [1] This is the same venue as 2017: https://goo.gl/maps/E36qCGJhK8J2
[ANNOUNCE] Git Merge Contributor's Summit Jan 31, 2019, Brussels
Git Merge 2019 is happening on February 1st. There will be a Contributor's Summit the day before. Here are the details: When: Thursday, January 31, 2019. 10am-5pm. Where: The Egg[1], Brussels, Belgium What: Round-table discussion about Git Who: All contributors to Git or related projects in the Git ecosystem are invited; if you're not sure if you qualify, please ask! This email is just to announce the date so people can start planning. You'll need to register eventually, but I don't have the invite codes yet. Registration should be similar to past years. In particular, if you're coming to the contrib summit, DON'T register for the main conference yet. I think the codes I get will cover both (and as with previous years, contrib summit attendees will have the option of a complimentary pass to the main conference, or can pay €99 that goes to Software Freedom Conservancy). The content / agenda is whatever we choose. There's some more discussion on format in this thread from August: https://public-inbox.org/git/20180813163108.ga6...@sigill.intra.peff.net/ though it sounds like people are mostly on board with what we've done in past years. I'm looking into getting better A/V for remote attendance, but I'm not sure yet what will be possible. Any thoughts or discussion on format, content, etc are welcome. The only thing set so far is the time and place. :) -Peff [1] This is the same venue as 2017: https://goo.gl/maps/E36qCGJhK8J2
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
On Fri, Nov 09, 2018 at 02:05:48AM +, Joseph Moisan wrote: > Can someone please tell me how to unsubscribe from this email. I am > no longer interested in receiving these emails, and cannot find how to > unsubscribe. Details are at http://vger.kernel.org/vger-lists.html#git. -Peff
Re: [PATCH 1/1] mingw: handle absolute paths in expand_user_path()
On Thu, Nov 08, 2018 at 04:45:16PM +0100, Johannes Schindelin wrote: > > One thing I had in mind when proposing $VARIABLE is that it opens up a > > namespace for us to expand more things (*) for example $GIT_DIR (from > > ~/.gitconfig). > > > > (*) but in a controlled way, it may look like an environment variable, > > but we only accept a selected few. And it's only expanded at the > > beginning of a path. > > I understand that desire, and I even agree. But the fact that it looks > like an environment variable, but is not, is actually a point in favor of > *not* doing this. It would violate the principle of least astonishment. I agree that it is potentially surprising. OTOH, it is at least pretty obvious when you see in the wild something like: [core] foo = $RUNTIME_PREFIX/bar what it is _trying_ to do. My big concern with "~"-based things is that they're kind of subtle. If I saw: [core] foo = ~~/bar I'm not sure what I would think it does. ;) > The form `/abc/def` would not be confused with anything > that it is not, I would think. The only thing against this form (at least > that I can think of) is that some people use this way to talk about paths > that vary between different setups, with the implicit assumption that the > reader will "interpolate" this *before* applying. So for example, when I > tell a user to please edit their /config, I implicitly assume > that they know to not type out, literally, but instead fill in > the correct path. So yeah, some alternative syntax that is verbose but distinct makes sense to me. We use %-substitution elsewhere. Could we do something with that? "%RUNTIME_PREFIX%" gives me too many flashbacks, but something like "%(RUNTIME_PREFIX)" matches our formatting language. I dunno. I actually do not think "$FOO" is so bad, as long as we clearly document that: - we do not allow arbitrary $FOO from the environment (though I am actually not all that opposed to doing so) - we add some special $FOOs that are not actually environment variables -Peff
[PATCH v4 0/3] range-diff fixes
Addresses feedback on v3, especially Eric's suggestion to split out
the behavior change (which I was not aware of) into a 3/3. Diff with
v3:
1: 23295d7806 ! 1: 5399e57513 range-diff doc: add a section about output
stability
@@ -29,11 +29,11 @@
+be machine-readable.
+
+This is particularly true when passing in diff options. Currently some
-+options like `--stat` can as an emergent effect produce output that's
-+quite useless in the context of `range-diff`. Future versions of
-+`range-diff` may learn to interpret such options in a manner specifc
-+to `range-diff` (e.g. for `--stat` summarizing how the diffstat
-+changed).
++options like `--stat` can, as an emergent effect, produce output
++that's quite useless in the context of `range-diff`. Future versions
++of `range-diff` may learn to interpret such options in a manner
++specific to `range-diff` (e.g. for `--stat` producing human-readable
++output which summarizes how the diffstat changed).
CONFIGURATION
-
2: b21bd273f5 ! 2: e56975df6c range-diff: fix regression in passing along
diff options
@@ -4,8 +4,10 @@
In 73a834e9e2 ("range-diff: relieve callers of low-level configuration
burden", 2018-07-22) we broke passing down options like --no-patch,
---stat etc. Fix that regression, and add a test for some of these
-options being passed down.
+--stat etc.
+
+Fix that regression, and add a test asserting the pre-73a834e9e2
+behavior for some of these diff options.
As noted in a change leading up to this ("range-diff doc: add a
section about output stability", 2018-11-07) the output is not meant
@@ -14,7 +16,9 @@
See
https://public-inbox.org/git/nycvar.qro.7.76.6.1811071202480...@tvgsbejvaqbjf.bet/
-for a further explanation of the regression.
+for a further explanation of the regression. The fix here is not the
+same as in Johannes's on-list patch, for reasons that'll be explained
+in a follow-up commit.
The quoting of "EOF" here mirrors that of an earlier test. Perhaps
that should be fixed, but let's leave that up to a later cleanup
@@ -30,8 +34,7 @@
memcpy(, diffopt, sizeof(opts));
- opts.output_format = DIFF_FORMAT_PATCH;
-+ if (!opts.output_format)
-+ opts.output_format = DIFF_FORMAT_PATCH;
++ opts.output_format |= DIFF_FORMAT_PATCH;
opts.flags.suppress_diff_headers = 1;
opts.flags.dual_color_diffed_diffs = dual_color;
opts.output_prefix = output_prefix_cb;
@@ -55,20 +58,43 @@
+'
+
+test_expect_success 'changed commit with --stat diff option' '
++ four_spaces="" &&
+ git range-diff --no-color --stat topic...changed >actual &&
+ cat >expected <<-EOF &&
+ 1: 4de457d = 1: a4b s/5/A/
+ a => b | 0
+ 1 file changed, 0 insertions(+), 0 deletions(-)
++ $four_spaces
+ 2: fccce22 = 2: f51d370 s/4/A/
+ a => b | 0
+ 1 file changed, 0 insertions(+), 0 deletions(-)
++ $four_spaces
+ 3: 147e64e ! 3: 0559556 s/11/B/
+ a => b | 0
+ 1 file changed, 0 insertions(+), 0 deletions(-)
++ $four_spaces
++ @@ -10,7 +10,7 @@
++9
++10
++ -11
++ -+B
++ ++BB
++12
++13
++14
+ 4: a63e992 ! 4: d966c5c s/12/B/
+ a => b | 0
+ 1 file changed, 0 insertions(+), 0 deletions(-)
++ $four_spaces
++ @@ -8,7 +8,7 @@
++ @@
++9
++10
++ - B
++ + BB
++ -12
++ +B
++13
+ EOF
+ test_cmp expected actual
+'
-: -- > 3: edfef733c7 range-diff: make diff option behavior (e.g.
--stat) consistent
Ævar Arnfjörð Bjarmason (3):
range-diff doc: add a section about output stability
range-diff: fix regression in passing along diff options
range-diff: make diff option behavior (e.g. --stat) consistent
Documentation/git-range-diff.txt | 17 +
range-diff.c | 3 ++-
t/t3206-range-diff.sh| 31 +++
3 files changed, 50 insertions(+), 1 deletion(-)
--
2.19.1.1182.g4ecb1133ce
[PATCH v4 3/3] range-diff: make diff option behavior (e.g. --stat) consistent
Make the behavior when diff options (e.g. "--stat") are passed
consistent with how "diff" behaves.
Before 73a834e9e2 ("range-diff: relieve callers of low-level
configuration burden", 2018-07-22) running range-diff with "--stat"
would produce stat output and the diff output, as opposed to how
"diff" behaves where once "--stat" is specified "--patch" also needs
to be provided to emit the patch output.
As noted in a previous change ("range-diff doc: add a section about
output stability", 2018-11-07) the "--stat" output with "range-diff"
is useless at the moment.
But we should behave consistently with "diff" in anticipation of such
output being useful in the future, because it would make for confusing
UI if two "diff" and "range-diff" behaved differently when it came to
how they interpret diff options.
The new behavior is also consistent with the existing documentation
added in ba931edd28 ("range-diff: populate the man page",
2018-08-13). See "[...]also accepts the regular diff options[...]" in
git-range-diff(1).
Signed-off-by: Ævar Arnfjörð Bjarmason
---
range-diff.c | 3 ++-
t/t3206-range-diff.sh | 22 --
2 files changed, 2 insertions(+), 23 deletions(-)
diff --git a/range-diff.c b/range-diff.c
index ea317f92f9..72bde281f3 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -453,7 +453,8 @@ int show_range_diff(const char *range1, const char *range2,
struct strbuf indent = STRBUF_INIT;
memcpy(, diffopt, sizeof(opts));
- opts.output_format |= DIFF_FORMAT_PATCH;
+ if (!opts.output_format)
+ opts.output_format |= DIFF_FORMAT_PATCH;
opts.flags.suppress_diff_headers = 1;
opts.flags.dual_color_diffed_diffs = dual_color;
opts.output_prefix = output_prefix_cb;
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index ab44e085d5..9352f65280 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -140,37 +140,15 @@ test_expect_success 'changed commit with --stat diff
option' '
1: 4de457d = 1: a4b s/5/A/
a => b | 0
1 file changed, 0 insertions(+), 0 deletions(-)
- $four_spaces
2: fccce22 = 2: f51d370 s/4/A/
a => b | 0
1 file changed, 0 insertions(+), 0 deletions(-)
- $four_spaces
3: 147e64e ! 3: 0559556 s/11/B/
a => b | 0
1 file changed, 0 insertions(+), 0 deletions(-)
- $four_spaces
- @@ -10,7 +10,7 @@
- 9
- 10
--11
- -+B
- ++BB
- 12
- 13
- 14
4: a63e992 ! 4: d966c5c s/12/B/
a => b | 0
1 file changed, 0 insertions(+), 0 deletions(-)
- $four_spaces
- @@ -8,7 +8,7 @@
-@@
- 9
- 10
- - B
- + BB
--12
-+B
- 13
EOF
test_cmp expected actual
'
--
2.19.1.1182.g4ecb1133ce
[PATCH v4 1/3] range-diff doc: add a section about output stability
The range-diff command is already advertised as porcelain, but let's make it really clear that the output is completely subject to change, particularly when it comes to diff options such as --stat. Right now that option doesn't work, but fixing that is the subject of a later change. Signed-off-by: Ævar Arnfjörð Bjarmason --- Documentation/git-range-diff.txt | 17 + 1 file changed, 17 insertions(+) diff --git a/Documentation/git-range-diff.txt b/Documentation/git-range-diff.txt index f693930fdb..8a6ea2c6c5 100644 --- a/Documentation/git-range-diff.txt +++ b/Documentation/git-range-diff.txt @@ -78,6 +78,23 @@ between patches", i.e. to compare the author, commit message and diff of corresponding old/new commits. There is currently no means to tweak the diff options passed to `git log` when generating those patches. +OUTPUT STABILITY + + +The output of the `range-diff` command is subject to change. It is +intended to be human-readable porcelain output, not something that can +be used across versions of Git to get a textually stable `range-diff` +(as opposed to something like the `--stable` option to +linkgit:git-patch-id[1]). There's also no equivalent of +linkgit:git-apply[1] for `range-diff`, the output is not intended to +be machine-readable. + +This is particularly true when passing in diff options. Currently some +options like `--stat` can, as an emergent effect, produce output +that's quite useless in the context of `range-diff`. Future versions +of `range-diff` may learn to interpret such options in a manner +specific to `range-diff` (e.g. for `--stat` producing human-readable +output which summarizes how the diffstat changed). CONFIGURATION - -- 2.19.1.1182.g4ecb1133ce
[PATCH v4 2/3] range-diff: fix regression in passing along diff options
In 73a834e9e2 ("range-diff: relieve callers of low-level configuration
burden", 2018-07-22) we broke passing down options like --no-patch,
--stat etc.
Fix that regression, and add a test asserting the pre-73a834e9e2
behavior for some of these diff options.
As noted in a change leading up to this ("range-diff doc: add a
section about output stability", 2018-11-07) the output is not meant
to be stable. So this regression test will likely need to be tweaked
once we get a "proper" --stat option.
See
https://public-inbox.org/git/nycvar.qro.7.76.6.1811071202480...@tvgsbejvaqbjf.bet/
for a further explanation of the regression. The fix here is not the
same as in Johannes's on-list patch, for reasons that'll be explained
in a follow-up commit.
The quoting of "EOF" here mirrors that of an earlier test. Perhaps
that should be fixed, but let's leave that up to a later cleanup
change.
Signed-off-by: Ævar Arnfjörð Bjarmason
---
range-diff.c | 2 +-
t/t3206-range-diff.sh | 53 +++
2 files changed, 54 insertions(+), 1 deletion(-)
diff --git a/range-diff.c b/range-diff.c
index bd8083f2d1..ea317f92f9 100644
--- a/range-diff.c
+++ b/range-diff.c
@@ -453,7 +453,7 @@ int show_range_diff(const char *range1, const char *range2,
struct strbuf indent = STRBUF_INIT;
memcpy(, diffopt, sizeof(opts));
- opts.output_format = DIFF_FORMAT_PATCH;
+ opts.output_format |= DIFF_FORMAT_PATCH;
opts.flags.suppress_diff_headers = 1;
opts.flags.dual_color_diffed_diffs = dual_color;
opts.output_prefix = output_prefix_cb;
diff --git a/t/t3206-range-diff.sh b/t/t3206-range-diff.sh
index 6aae364171..ab44e085d5 100755
--- a/t/t3206-range-diff.sh
+++ b/t/t3206-range-diff.sh
@@ -122,6 +122,59 @@ test_expect_success 'changed commit' '
test_cmp expected actual
'
+test_expect_success 'changed commit with --no-patch diff option' '
+ git range-diff --no-color --no-patch topic...changed >actual &&
+ cat >expected <<-EOF &&
+ 1: 4de457d = 1: a4b s/5/A/
+ 2: fccce22 = 2: f51d370 s/4/A/
+ 3: 147e64e ! 3: 0559556 s/11/B/
+ 4: a63e992 ! 4: d966c5c s/12/B/
+ EOF
+ test_cmp expected actual
+'
+
+test_expect_success 'changed commit with --stat diff option' '
+ four_spaces="" &&
+ git range-diff --no-color --stat topic...changed >actual &&
+ cat >expected <<-EOF &&
+ 1: 4de457d = 1: a4b s/5/A/
+a => b | 0
+1 file changed, 0 insertions(+), 0 deletions(-)
+ $four_spaces
+ 2: fccce22 = 2: f51d370 s/4/A/
+a => b | 0
+1 file changed, 0 insertions(+), 0 deletions(-)
+ $four_spaces
+ 3: 147e64e ! 3: 0559556 s/11/B/
+a => b | 0
+1 file changed, 0 insertions(+), 0 deletions(-)
+ $four_spaces
+ @@ -10,7 +10,7 @@
+ 9
+ 10
+-11
+ -+B
+ ++BB
+ 12
+ 13
+ 14
+ 4: a63e992 ! 4: d966c5c s/12/B/
+a => b | 0
+1 file changed, 0 insertions(+), 0 deletions(-)
+ $four_spaces
+ @@ -8,7 +8,7 @@
+@@
+ 9
+ 10
+ - B
+ + BB
+-12
++B
+ 13
+ EOF
+ test_cmp expected actual
+'
+
test_expect_success 'changed commit with sm config' '
git range-diff --no-color --submodule=log topic...changed >actual &&
cat >expected <<-EOF &&
--
2.19.1.1182.g4ecb1133ce
Re: [RFC PATCH] index-pack: improve performance on NFS
On Thu, Nov 08 2018, Ævar Arnfjörð Bjarmason wrote:
> On Thu, Nov 08 2018, Jeff King wrote:
>
>> On Wed, Nov 07, 2018 at 10:55:24PM +, Geert Jansen wrote:
>>
>>> On Mon, Oct 29, 2018 at 07:27:39PM -0400, Jeff King wrote:
>>>
>>> > On Mon, Oct 29, 2018 at 08:36:07PM +0100, Ævar Arnfjörð Bjarmason wrote:
>>> > > * Re-roll my 4 patch series to include the patch you have in
>>> > ><20181027093300.ga23...@sigill.intra.peff.net>
>>> >
>>> > I don't think it's quite ready for inclusion as-is. I hope to brush it
>>> > up a bit, but I have quite a backlog of stuff to review, as well.
>>>
>>> We're still quite keen to get this patch included. Is there anything I can
>>> do
>>> to help?
>>
>> Yes, testing and review. :)
>>
>> I won't send the series out just yet, as I suspect it could use another
>> read-through on my part. But if you want to peek at it or try some
>> timings, it's available at:
>>
>> https://github.com/peff/git jk/loose-cache
>
> Just a comment on this from the series:
>
> Note that it is possible for this to actually be _slower_. We'll do a
> full readdir() to fill the cache, so if you have a very large number of
> loose objects and a very small number of lookups, that readdir() may end
> up more expensive.
>
> In practice, though, having a large number of loose objects is already a
> performance problem, which should be fixed by repacking or pruning via
> git-gc. So on balance, this should be a good tradeoff.
>
> Our biggest repo has a very large number of loose objects at any given
> time, but the vast majority of these are because gc *is* happening very
> frequently and the default expiry policy of 2wks is in effect.
>
> Having a large number of loose objects is not per-se a performance
> problem.
>
> It's a problem if you end up "faulting" to from packs to the loose
> object directory a lot because those objects are still reachable, but if
> they're not reachable that number can grow very large if your ref churn
> is large (so lots of expired loose object production).
>
> Anyway, the series per-se looks good to me. It's particularly nice to
> have some of the ODB cleanup + cleanup in fetch-pack.c
>
> Just wanted to note that in our default (reasonable) config we do
> produce scenarios where this change can still be somewhat pathological,
> so I'm still interested in disabling it entirely given the
> implausibility of what it's guarding against.
Some actual numbers for this for a fairly small repo on NFS, "cold"
cache (hadn't run it in a bit):
$ time (find objects/?? -type f|wc -l)
862
real0m1.927s
Warm cache:
$ time (find objects/?? -type f|wc -l)
872
real0m0.151s
Cold cache on a bigger monorepo:
$ time (find objects/?? -type f|wc -l)
real0m4.336s
Warm cache on a bigger monorepo (more ref churn):
$ time (find objects/?? -type f|wc -l)
49746
real0m1.082s
This on a server where bulk sustained writes of large files are really
fast (up to 1GB/s). It's just these metadata ops that are slow.
I also get cold cache times of up to 6 seconds on:
time (find $(ls -d objects/??|sort -R) -type f | wc -l)
As opposed max of ~4s without -R, so I suspect there may be some
client/server optimization where things are iterated over in recursive
glob order (pre-fetched?), whereas the cache will try to fill buckets is
it encounters loose objects, so iterate over objects/{00..ff} randomly.
I'm not really leading up to any point here I haven't made already. I
was just curious to try to find some upper bound of overhead if say a
pack with 512 objects is pushed. In that case it's very likely that we
need to fill at least 200/256 buckets.
Re: [PATCH 2/2] built-in rebase: reinstate `checkout -q` behavior where appropriate
On Fri, Nov 09, 2018 at 01:34:19AM -0800, Johannes Schindelin via GitGitGadget
wrote:
> From: Johannes Schindelin
>
> When we converted a `git checkout -q $onto^0` call to use
> `reset_head()`, we inadvertently incurred a change from a twoway_merge
> to a oneway_merge, as if we wanted a `git reset --hard` instead.
>
> This has performance ramifications under certain, though, as the
> oneway_merge needs to lstat() every single index entry whereas
> twoway_merge does not.
>
> So let's go back to the old behavior.
Makes sense. I didn't think too hard about any possible gotchas with the
twoway/oneway switch, but if that's what git-checkout was doing before,
it seems obviously safe.
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 6f6d7de156..c1cc50f3f8 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -523,11 +523,12 @@ finished_rebase:
> #define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
>
> static int reset_head(struct object_id *oid, const char *action,
> - const char *switch_to_branch, int detach_head,
> + const char *switch_to_branch,
> + int detach_head, int reset_hard,
It might be worth switching to a single flag variable here. It would
make calls like this:
> - if (reset_head(>object.oid, "checkout", NULL, 1,
> + if (reset_head(>object.oid, "checkout", NULL, 1, 0,
> NULL, msg.buf))
a little more self-documenting (if a little more verbose).
> - if (!oid) {
> - if (get_oid("HEAD", _oid)) {
> - rollback_lock_file();
> - return error(_("could not determine HEAD revision"));
> - }
> - oid = _oid;
> + if (get_oid("HEAD", _oid)) {
> + rollback_lock_file();
> + return error(_("could not determine HEAD revision"));
> }
This one could actually turn into:
ret = error(...);
goto leave_reset_head;
now. We don't have to worry about an uninitialized desc.buffer anymore
(as I mentioned in the previous email), because "nr" would be 0.
It doesn't save any lines, though (but maybe having a single
cleanup/exit point would make things easier to read; I dunno).
Take all my comments as observations, not objections. This looks OK to
me either way.
-Peff
Re: [PATCH 1/2] rebase: consolidate clean-up code before leaving reset_head()
On Fri, Nov 09, 2018 at 01:34:17AM -0800, Johannes Schindelin via GitGitGadget
wrote:
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 0ee06aa363..6f6d7de156 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -569,16 +569,13 @@ static int reset_head(struct object_id *oid, const char
> *action,
> }
>
> if (!fill_tree_descriptor(, oid)) {
> - error(_("failed to find tree of %s"), oid_to_hex(oid));
> - rollback_lock_file();
> - free((void *)desc.buffer);
> - return -1;
> + ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
> + goto leave_reset_head;
> }
If fill_tree_descriptor() fails, what is left in desc.buffer? Looking at
the implementation, I think it's always NULL or a valid buffer. But I
think all code paths actually die() unless we pass a NULL oid (and in
that case desc.buffer would be NULL, too).
So I think the original here that calls free() doesn't ever do anything
but it did not hurt. After your patch, the leave_reset_head code would
continue to call free(), and that's OK.
There are a few earlier conditionals in reset_head() that do only
rollback_lock_file() that could similarly be converted to use the goto.
But they would need desc.buffer to be initialized to NULL. I could go
either way on converting them or not.
> @@ -586,10 +583,9 @@ static int reset_head(struct object_id *oid, const char
> *action,
>
> if (write_locked_index(the_repository->index, , COMMIT_LOCK) < 0)
> ret = error(_("could not write index"));
> - free((void *)desc.buffer);
>
> if (ret)
> - return ret;
> + goto leave_reset_head;
>
> reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
> strbuf_addf(, "%s: ", reflog_action ? reflog_action : "rebase");
> @@ -622,7 +618,10 @@ static int reset_head(struct object_id *oid, const char
> *action,
>UPDATE_REFS_MSG_ON_ERR);
> }
>
> +leave_reset_head:
> strbuf_release();
> + rollback_lock_file();
> + free((void *)desc.buffer);
> return ret;
We get here on success, too. So we may call rollback_lock_file() on an
already-committed lock. This is explicitly documented as a no-op by the
lock code, so that's OK.
So overall looks good to me.
-Peff
[PATCH 0/2] Fix built-in rebase perf regression
In our tests with large repositories, we noticed a serious regression of the performance of git rebase when using the built-in vs the shell script version. It boils down to an incorrect conversion of a git checkout -q: instead of using a twoway_merge as git checkout does, we used a oneway_merge as git reset does. The latter, however, calls lstat() on all files listed in the index, while the former essentially looks only at the files that are different between the given two revisions. Let's reinstate the original behavior by introducing a flag to the reset_head() function to indicate whether we want to emulate reset --hard (in which case we use the oneway_merge, otherwise we use twoway_merge). Johannes Schindelin (2): rebase: consolidate clean-up code before leaving reset_head() built-in rebase: reinstate `checkout -q` behavior where appropriate builtin/rebase.c | 60 ++-- 1 file changed, 33 insertions(+), 27 deletions(-) base-commit: 8858448bb49332d353febc078ce4a3abcc962efe Published-As: https://github.com/gitgitgadget/git/releases/tags/pr-72%2Fdscho%2Fbuiltin-rebase-perf-regression-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-72/dscho/builtin-rebase-perf-regression-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/72 -- gitgitgadget
[PATCH 2/2] built-in rebase: reinstate `checkout -q` behavior where appropriate
From: Johannes Schindelin
When we converted a `git checkout -q $onto^0` call to use
`reset_head()`, we inadvertently incurred a change from a twoway_merge
to a oneway_merge, as if we wanted a `git reset --hard` instead.
This has performance ramifications under certain, though, as the
oneway_merge needs to lstat() every single index entry whereas
twoway_merge does not.
So let's go back to the old behavior.
Signed-off-by: Johannes Schindelin
---
builtin/rebase.c | 45 ++---
1 file changed, 26 insertions(+), 19 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 6f6d7de156..c1cc50f3f8 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -523,11 +523,12 @@ finished_rebase:
#define GIT_REFLOG_ACTION_ENVIRONMENT "GIT_REFLOG_ACTION"
static int reset_head(struct object_id *oid, const char *action,
- const char *switch_to_branch, int detach_head,
+ const char *switch_to_branch,
+ int detach_head, int reset_hard,
const char *reflog_orig_head, const char *reflog_head)
{
struct object_id head_oid;
- struct tree_desc desc;
+ struct tree_desc desc[2];
struct lock_file lock = LOCK_INIT;
struct unpack_trees_options unpack_tree_opts;
struct tree *tree;
@@ -536,7 +537,7 @@ static int reset_head(struct object_id *oid, const char
*action,
size_t prefix_len;
struct object_id *orig = NULL, oid_orig,
*old_orig = NULL, oid_old_orig;
- int ret = 0;
+ int ret = 0, nr = 0;
if (switch_to_branch && !starts_with(switch_to_branch, "refs/"))
BUG("Not a fully qualified branch: '%s'", switch_to_branch);
@@ -544,20 +545,20 @@ static int reset_head(struct object_id *oid, const char
*action,
if (hold_locked_index(, LOCK_REPORT_ON_ERROR) < 0)
return -1;
- if (!oid) {
- if (get_oid("HEAD", _oid)) {
- rollback_lock_file();
- return error(_("could not determine HEAD revision"));
- }
- oid = _oid;
+ if (get_oid("HEAD", _oid)) {
+ rollback_lock_file();
+ return error(_("could not determine HEAD revision"));
}
+ if (!oid)
+ oid = _oid;
+
memset(_tree_opts, 0, sizeof(unpack_tree_opts));
setup_unpack_trees_porcelain(_tree_opts, action);
unpack_tree_opts.head_idx = 1;
unpack_tree_opts.src_index = the_repository->index;
unpack_tree_opts.dst_index = the_repository->index;
- unpack_tree_opts.fn = oneway_merge;
+ unpack_tree_opts.fn = reset_hard ? oneway_merge : twoway_merge;
unpack_tree_opts.update = 1;
unpack_tree_opts.merge = 1;
if (!detach_head)
@@ -568,12 +569,17 @@ static int reset_head(struct object_id *oid, const char
*action,
return error(_("could not read index"));
}
- if (!fill_tree_descriptor(, oid)) {
+ if (!reset_hard && !fill_tree_descriptor([nr++], _oid)) {
+ ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
+ goto leave_reset_head;
+ }
+
+ if (!fill_tree_descriptor([nr++], oid)) {
ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
goto leave_reset_head;
}
- if (unpack_trees(1, , _tree_opts)) {
+ if (unpack_trees(nr, desc, _tree_opts)) {
ret = -1;
goto leave_reset_head;
}
@@ -621,7 +627,8 @@ static int reset_head(struct object_id *oid, const char
*action,
leave_reset_head:
strbuf_release();
rollback_lock_file();
- free((void *)desc.buffer);
+ while (nr)
+ free((void *)desc[--nr].buffer);
return ret;
}
@@ -999,7 +1006,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
rerere_clear(_rr);
string_list_clear(_rr, 1);
- if (reset_head(NULL, "reset", NULL, 0, NULL, NULL) < 0)
+ if (reset_head(NULL, "reset", NULL, 0, 1, NULL, NULL) < 0)
die(_("could not discard worktree changes"));
if (read_basic_state())
exit(1);
@@ -1015,7 +1022,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
if (read_basic_state())
exit(1);
if (reset_head(_head, "reset",
- options.head_name, 0, NULL, NULL) < 0)
+ options.head_name, 0, 1, NULL, NULL) < 0)
die(_("could not move back to %s"),
oid_to_hex(_head));
ret = finish_rebase();
@@ -1379,7 +1386,7 @@ int cmd_rebase(int argc, const char **argv, const char
*prefix)
write_file(autostash, "%s",
[PATCH 1/2] rebase: consolidate clean-up code before leaving reset_head()
From: Johannes Schindelin
The same clean-up code is repeated quite a few times; Let's DRY up the
code some.
Signed-off-by: Johannes Schindelin
---
builtin/rebase.c | 17 -
1 file changed, 8 insertions(+), 9 deletions(-)
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 0ee06aa363..6f6d7de156 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -569,16 +569,13 @@ static int reset_head(struct object_id *oid, const char
*action,
}
if (!fill_tree_descriptor(, oid)) {
- error(_("failed to find tree of %s"), oid_to_hex(oid));
- rollback_lock_file();
- free((void *)desc.buffer);
- return -1;
+ ret = error(_("failed to find tree of %s"), oid_to_hex(oid));
+ goto leave_reset_head;
}
if (unpack_trees(1, , _tree_opts)) {
- rollback_lock_file();
- free((void *)desc.buffer);
- return -1;
+ ret = -1;
+ goto leave_reset_head;
}
tree = parse_tree_indirect(oid);
@@ -586,10 +583,9 @@ static int reset_head(struct object_id *oid, const char
*action,
if (write_locked_index(the_repository->index, , COMMIT_LOCK) < 0)
ret = error(_("could not write index"));
- free((void *)desc.buffer);
if (ret)
- return ret;
+ goto leave_reset_head;
reflog_action = getenv(GIT_REFLOG_ACTION_ENVIRONMENT);
strbuf_addf(, "%s: ", reflog_action ? reflog_action : "rebase");
@@ -622,7 +618,10 @@ static int reset_head(struct object_id *oid, const char
*action,
UPDATE_REFS_MSG_ON_ERR);
}
+leave_reset_head:
strbuf_release();
+ rollback_lock_file();
+ free((void *)desc.buffer);
return ret;
}
--
gitgitgadget
[PATCH v3 02/16] sequencer: make the todo_list structure public
This makes the structures todo_list and todo_item, and the functions
todo_list_release() and parse_insn_buffer(), accessible outside of
sequencer.c.
Signed-off-by: Alban Gruin
---
sequencer.c | 67 +
sequencer.h | 49 +++
2 files changed, 60 insertions(+), 56 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 07cc91d8db..7adbeaa27d 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -1443,32 +1443,6 @@ static int allow_empty(struct replay_opts *opts, struct
commit *commit)
return 1;
}
-/*
- * Note that ordering matters in this enum. Not only must it match the mapping
- * below, it is also divided into several sections that matter. When adding
- * new commands, make sure you add it in the right section.
- */
-enum todo_command {
- /* commands that handle commits */
- TODO_PICK = 0,
- TODO_REVERT,
- TODO_EDIT,
- TODO_REWORD,
- TODO_FIXUP,
- TODO_SQUASH,
- /* commands that do something else than handling a single commit */
- TODO_EXEC,
- TODO_BREAK,
- TODO_LABEL,
- TODO_RESET,
- TODO_MERGE,
- /* commands that do nothing but are counted for reporting progress */
- TODO_NOOP,
- TODO_DROP,
- /* comments (not counted for reporting progress) */
- TODO_COMMENT
-};
-
static struct {
char c;
const char *str;
@@ -1939,26 +1913,7 @@ enum todo_item_flags {
TODO_EDIT_MERGE_MSG = 1
};
-struct todo_item {
- enum todo_command command;
- struct commit *commit;
- unsigned int flags;
- const char *arg;
- int arg_len;
- size_t offset_in_buf;
-};
-
-struct todo_list {
- struct strbuf buf;
- struct todo_item *items;
- int nr, alloc, current;
- int done_nr, total_nr;
- struct stat_data stat;
-};
-
-#define TODO_LIST_INIT { STRBUF_INIT }
-
-static void todo_list_release(struct todo_list *todo_list)
+void todo_list_release(struct todo_list *todo_list)
{
strbuf_release(_list->buf);
FREE_AND_NULL(todo_list->items);
@@ -2060,7 +2015,7 @@ static int parse_insn_line(struct todo_item *item, const
char *bol, char *eol)
return !item->commit;
}
-static int parse_insn_buffer(char *buf, struct todo_list *todo_list)
+int todo_list_parse_insn_buffer(char *buf, struct todo_list *todo_list)
{
struct todo_item *item;
char *p = buf, *next_p;
@@ -2158,7 +2113,7 @@ static int read_populate_todo(struct todo_list *todo_list,
return error(_("could not stat '%s'"), todo_file);
fill_stat_data(_list->stat, );
- res = parse_insn_buffer(todo_list->buf.buf, todo_list);
+ res = todo_list_parse_insn_buffer(todo_list->buf.buf, todo_list);
if (res) {
if (is_rebase_i(opts))
return error(_("please fix this using "
@@ -2189,7 +2144,7 @@ static int read_populate_todo(struct todo_list *todo_list,
FILE *f = fopen_or_warn(rebase_path_msgtotal(), "w");
if (strbuf_read_file(, rebase_path_done(), 0) > 0 &&
- !parse_insn_buffer(done.buf.buf, ))
+ !todo_list_parse_insn_buffer(done.buf.buf,
))
todo_list->done_nr = count_commands();
else
todo_list->done_nr = 0;
@@ -4454,7 +4409,7 @@ int sequencer_add_exec_commands(const char *commands)
if (strbuf_read_file(_list.buf, todo_file, 0) < 0)
return error(_("could not read '%s'."), todo_file);
- if (parse_insn_buffer(todo_list.buf.buf, _list)) {
+ if (todo_list_parse_insn_buffer(todo_list.buf.buf, _list)) {
todo_list_release(_list);
return error(_("unusable todo list: '%s'"), todo_file);
}
@@ -4510,7 +4465,7 @@ int transform_todos(unsigned flags)
if (strbuf_read_file(_list.buf, todo_file, 0) < 0)
return error(_("could not read '%s'."), todo_file);
- if (parse_insn_buffer(todo_list.buf.buf, _list)) {
+ if (todo_list_parse_insn_buffer(todo_list.buf.buf, _list)) {
todo_list_release(_list);
return error(_("unusable todo list: '%s'"), todo_file);
}
@@ -4596,7 +4551,7 @@ int check_todo_list(void)
goto leave_check;
}
advise_to_edit_todo = res =
- parse_insn_buffer(todo_list.buf.buf, _list);
+ todo_list_parse_insn_buffer(todo_list.buf.buf, _list);
if (res || check_level == MISSING_COMMIT_CHECK_IGNORE)
goto leave_check;
@@ -4615,7 +4570,7 @@ int check_todo_list(void)
goto leave_check;
}
strbuf_release(_file);
- res = !!parse_insn_buffer(todo_list.buf.buf, _list);
+ res = !!todo_list_parse_insn_buffer(todo_list.buf.buf, _list);
/* Find commits in
[PATCH v3 09/16] sequencer: change complete_action() to use the refactored functions
complete_action() used functions that read the todo-list file, made some
changes to it, and wrote it back to the disk.
The previous commits were dedicated to separate the part that deals with
the file from the actual logic of these functions. Now that this is
done, we can call directly the "logic" functions to avoid useless file
access.
The parsing of the list has to be done by the caller. If the buffer of
the todo list provided by the caller is empty, a `noop' command is
directly added to the todo list, without touching to the buffer.
Signed-off-by: Alban Gruin
---
builtin/rebase--interactive.c | 16 ++-
sequencer.c | 80 +++
sequencer.h | 2 +-
3 files changed, 40 insertions(+), 58 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index c740a7dd5d..99cbd1e8e3 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -71,7 +71,6 @@ static int do_interactive_rebase(struct replay_opts *opts,
unsigned flags,
const char *head_hash = NULL;
char *revisions = NULL, *shortrevisions = NULL;
struct argv_array make_script_args = ARGV_ARRAY_INIT;
- FILE *todo_list_file;
struct todo_list todo_list = TODO_LIST_INIT;
if (prepare_branch_to_be_rebased(opts, switch_to))
@@ -94,14 +93,6 @@ static int do_interactive_rebase(struct replay_opts *opts,
unsigned flags,
if (!upstream && squash_onto)
write_file(path_squash_onto(), "%s\n", squash_onto);
- todo_list_file = fopen(rebase_path_todo(), "w");
- if (!todo_list_file) {
- free(revisions);
- free(shortrevisions);
-
- return error_errno(_("could not open %s"), rebase_path_todo());
- }
-
argv_array_pushl(_script_args, "", revisions, NULL);
if (restrict_revision)
argv_array_push(_script_args, restrict_revision);
@@ -109,15 +100,16 @@ static int do_interactive_rebase(struct replay_opts
*opts, unsigned flags,
ret = sequencer_make_script(_list.buf,
make_script_args.argc,
make_script_args.argv,
flags);
- fputs(todo_list.buf.buf, todo_list_file);
- fclose(todo_list_file);
if (ret)
error(_("could not generate todo list"));
else {
discard_cache();
+ if (todo_list_parse_insn_buffer(todo_list.buf.buf, _list))
+ BUG("unusable todo list");
+
ret = complete_action(opts, flags, shortrevisions, onto_name,
onto,
- head_hash, commands, autosquash);
+ head_hash, commands, autosquash,
_list);
}
free(revisions);
diff --git a/sequencer.c b/sequencer.c
index 3389a753b6..64a99ab84f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4700,93 +4700,83 @@ static int skip_unnecessary_picks(struct object_id
*output_oid)
return 0;
}
+static int todo_list_rearrange_squash(struct todo_list *todo_list);
+
int complete_action(struct replay_opts *opts, unsigned flags,
const char *shortrevisions, const char *onto_name,
const char *onto, const char *orig_head, struct string_list
*commands,
- unsigned autosquash)
+ unsigned autosquash, struct todo_list *todo_list)
{
const char *shortonto, *todo_file = rebase_path_todo();
- struct todo_list todo_list = TODO_LIST_INIT;
- struct strbuf *buf = &(todo_list.buf);
+ struct todo_list new_todo = TODO_LIST_INIT;
+ struct strbuf *buf = _list->buf;
struct object_id oid;
- struct stat st;
get_oid(onto, );
shortonto = find_unique_abbrev(, DEFAULT_ABBREV);
- if (!lstat(todo_file, ) && st.st_size == 0 &&
- write_message("noop\n", 5, todo_file, 0))
- return -1;
+ if (buf->len == 0) {
+ struct todo_item *item = append_new_todo(todo_list);
+ item->command = TODO_NOOP;
+ item->commit = NULL;
+ item->arg = NULL;
+ item->arg_len = item->flags = item->offset_in_buf = 0;
+ }
- if (autosquash && rearrange_squash_in_todo_file())
+ if (autosquash && todo_list_rearrange_squash(todo_list))
return -1;
if (commands->nr)
- sequencer_add_exec_commands(commands);
-
- if (strbuf_read_file(buf, todo_file, 0) < 0)
- return error_errno(_("could not read '%s'."), todo_file);
-
- if (todo_list_parse_insn_buffer(buf->buf, _list)) {
- todo_list_release(_list);
- return error(_("unusable todo list: '%s'"), todo_file);
- }
+ todo_list_add_exec_commands(todo_list, commands);
- if (count_commands(_list) == 0) {
+ if
[PATCH v3 01/16] sequencer: changes in parse_insn_buffer()
This clears the number of items of a todo_list before parsing it to
allow to parse the same list multiple times without issues. As its
items are not dynamically allocated, or don’t need to allocate memory,
no additionnal memory management is required here.
Furthermore, if a line is invalid, the type of the corresponding
command is set to a garbage value, and its argument is defined properly.
This will allow to recreate the text of a todo list from its commands,
even if one of them is incorrect.
Signed-off-by: Alban Gruin
---
No changes since v2.
sequencer.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index 9e1ab3a2a7..07cc91d8db 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2066,6 +2066,8 @@ static int parse_insn_buffer(char *buf, struct todo_list
*todo_list)
char *p = buf, *next_p;
int i, res = 0, fixup_okay = file_exists(rebase_path_done());
+ todo_list->current = todo_list->nr = 0;
+
for (i = 1; *p; i++, p = next_p) {
char *eol = strchrnul(p, '\n');
@@ -2079,7 +2081,10 @@ static int parse_insn_buffer(char *buf, struct todo_list
*todo_list)
if (parse_insn_line(item, p, eol)) {
res = error(_("invalid line %d: %.*s"),
i, (int)(eol - p), p);
- item->command = TODO_NOOP;
+ item->command = TODO_COMMENT + 1;
+ item->arg = p;
+ item->arg_len = (int)(eol - p);
+ item->commit = NULL;
}
if (fixup_okay)
--
2.19.1.872.ga867da739e
[PATCH v3 13/16] rebase-interactive: rewrite edit_todo_list() to handle the initial edit
edit_todo_list() is changed to work on a todo_list, and to handle the
initial edition of the todo list (ie. making a backup of the todo
list).
It does not check for dropped commits yet, as todo_list_check() does not
take the commits that have already been processed by the rebase (ie. the
todo list is edited in the middle of a rebase session).
Signed-off-by: Alban Gruin
---
builtin/rebase--interactive.c | 23 -
rebase-interactive.c | 48 ++-
rebase-interactive.h | 4 ++-
sequencer.c | 3 +--
sequencer.h | 1 +
5 files changed, 52 insertions(+), 27 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index 99cbd1e8e3..6871990544 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -13,6 +13,27 @@ static GIT_PATH_FUNC(path_state_dir, "rebase-merge/")
static GIT_PATH_FUNC(path_squash_onto, "rebase-merge/squash-onto")
static GIT_PATH_FUNC(path_interactive, "rebase-merge/interactive")
+static int edit_todo_file(unsigned flags)
+{
+ const char *todo_file = rebase_path_todo();
+ struct todo_list todo_list = TODO_LIST_INIT,
+ new_todo = TODO_LIST_INIT;
+
+ if (strbuf_read_file(_list.buf, todo_file, 0) < 0)
+ return error_errno(_("could not read '%s'."), todo_file);
+
+ strbuf_stripspace(_list.buf, 1);
+ if (!edit_todo_list(_list, _todo, NULL, NULL, flags) &&
+ todo_list_write_to_file(_todo, todo_file, NULL, NULL, -1,
+ flags & ~(TODO_LIST_SHORTEN_IDS)) < 0)
+ return error_errno(_("could not write '%s'"), todo_file);
+
+ todo_list_release(_list);
+ todo_list_release(_todo);
+
+ return 0;
+}
+
static int get_revision_ranges(const char *upstream, const char *onto,
const char **head_hash,
char **revisions, char **shortrevisions)
@@ -241,7 +262,7 @@ int cmd_rebase__interactive(int argc, const char **argv,
const char *prefix)
break;
}
case EDIT_TODO:
- ret = edit_todo_list(flags);
+ ret = edit_todo_file(flags);
break;
case SHOW_CURRENT_PATCH: {
struct child_process cmd = CHILD_PROCESS_INIT;
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 3b7b5e3382..e0fa88b90e 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -87,35 +87,37 @@ void append_todo_help(unsigned keep_empty, int
command_count,
}
}
-int edit_todo_list(unsigned flags)
+int edit_todo_list(struct todo_list *todo_list, struct todo_list *new_todo,
+ const char *shortrevisions, const char *shortonto,
+ unsigned flags)
{
const char *todo_file = rebase_path_todo();
- struct todo_list todo_list = TODO_LIST_INIT;
- int res = 0;
-
- if (strbuf_read_file(_list.buf, todo_file, 0) < 0)
- return error_errno(_("could not read '%s'."), todo_file);
-
- strbuf_stripspace(_list.buf, 1);
- todo_list_parse_insn_buffer(todo_list.buf.buf, _list);
- if (todo_list_write_to_file(_list, todo_file, NULL, NULL, -1,
- flags | TODO_LIST_SHORTEN_IDS |
TODO_LIST_APPEND_TODO_HELP)) {
- todo_list_release(_list);
- return -1;
+ unsigned initial = shortrevisions && shortonto;
+
+ if (initial) {
+ todo_list_write_to_file(todo_list, todo_file, shortrevisions,
shortonto, -1,
+ flags | TODO_LIST_SHORTEN_IDS |
TODO_LIST_APPEND_TODO_HELP);
+
+ if (copy_file(rebase_path_todo_backup(), todo_file, 0666))
+ return error(_("could not copy '%s' to '%s'."),
todo_file,
+rebase_path_todo_backup());
+ } else {
+ todo_list_parse_insn_buffer(todo_list->buf.buf, todo_list);
+ todo_list_write_to_file(todo_list, todo_file, NULL, NULL, -1,
+ flags | TODO_LIST_SHORTEN_IDS |
TODO_LIST_APPEND_TODO_HELP);
}
- strbuf_reset(_list.buf);
- if (launch_sequence_editor(todo_file, _list.buf, NULL)) {
- todo_list_release(_list);
- return -1;
- }
+ if (launch_sequence_editor(todo_file, _todo->buf, NULL))
+ return -2;
- if (!todo_list_parse_insn_buffer(todo_list.buf.buf, _list))
- res = todo_list_write_to_file(_list, todo_file, NULL,
NULL, -1,
- flags & ~(TODO_LIST_SHORTEN_IDS));
+ strbuf_stripspace(_todo->buf, 1);
+ if (initial && new_todo->buf.len == 0)
+ return -3;
- todo_list_release(_list);
- return res;
+ if (!initial)
+
[PATCH v3 08/16] sequencer: make sequencer_make_script() write its script to a strbuf
This makes sequencer_make_script() write its script to a strbuf (ie. the
buffer of a todo_list) instead of a FILE. This reduce the amount of
read/write made by rebase interactive.
Signed-off-by: Alban Gruin
---
builtin/rebase--interactive.c | 13 +++-
sequencer.c | 38 ---
sequencer.h | 2 +-
3 files changed, 26 insertions(+), 27 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index a6d83a684e..c740a7dd5d 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -71,7 +71,8 @@ static int do_interactive_rebase(struct replay_opts *opts,
unsigned flags,
const char *head_hash = NULL;
char *revisions = NULL, *shortrevisions = NULL;
struct argv_array make_script_args = ARGV_ARRAY_INIT;
- FILE *todo_list;
+ FILE *todo_list_file;
+ struct todo_list todo_list = TODO_LIST_INIT;
if (prepare_branch_to_be_rebased(opts, switch_to))
return -1;
@@ -93,8 +94,8 @@ static int do_interactive_rebase(struct replay_opts *opts,
unsigned flags,
if (!upstream && squash_onto)
write_file(path_squash_onto(), "%s\n", squash_onto);
- todo_list = fopen(rebase_path_todo(), "w");
- if (!todo_list) {
+ todo_list_file = fopen(rebase_path_todo(), "w");
+ if (!todo_list_file) {
free(revisions);
free(shortrevisions);
@@ -105,10 +106,11 @@ static int do_interactive_rebase(struct replay_opts
*opts, unsigned flags,
if (restrict_revision)
argv_array_push(_script_args, restrict_revision);
- ret = sequencer_make_script(todo_list,
+ ret = sequencer_make_script(_list.buf,
make_script_args.argc,
make_script_args.argv,
flags);
- fclose(todo_list);
+ fputs(todo_list.buf.buf, todo_list_file);
+ fclose(todo_list_file);
if (ret)
error(_("could not generate todo list"));
@@ -120,6 +122,7 @@ static int do_interactive_rebase(struct replay_opts *opts,
unsigned flags,
free(revisions);
free(shortrevisions);
+ todo_list_release(_list);
argv_array_clear(_script_args);
return ret;
diff --git a/sequencer.c b/sequencer.c
index fce97e5f11..3389a753b6 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4109,7 +4109,7 @@ static const char *label_oid(struct object_id *oid, const
char *label,
}
static int make_script_with_merges(struct pretty_print_context *pp,
- struct rev_info *revs, FILE *out,
+ struct rev_info *revs, struct strbuf *out,
unsigned flags)
{
int keep_empty = flags & TODO_LIST_KEEP_EMPTY;
@@ -4254,7 +4254,7 @@ static int make_script_with_merges(struct
pretty_print_context *pp,
* gathering commits not yet shown, reversing the list on the fly,
* then outputting that list (labeling revisions as needed).
*/
- fprintf(out, "%s onto\n", cmd_label);
+ strbuf_addf(out, "%s onto\n", cmd_label);
for (iter = tips; iter; iter = iter->next) {
struct commit_list *list = NULL, *iter2;
@@ -4264,9 +4264,9 @@ static int make_script_with_merges(struct
pretty_print_context *pp,
entry = oidmap_get(, >object.oid);
if (entry)
- fprintf(out, "\n%c Branch %s\n", comment_line_char,
entry->string);
+ strbuf_addf(out, "\n%c Branch %s\n", comment_line_char,
entry->string);
else
- fprintf(out, "\n");
+ strbuf_addch(out, '\n');
while (oidset_contains(, >object.oid) &&
!oidset_contains(, >object.oid)) {
@@ -4279,8 +4279,8 @@ static int make_script_with_merges(struct
pretty_print_context *pp,
}
if (!commit)
- fprintf(out, "%s %s\n", cmd_reset,
- rebase_cousins ? "onto" : "[new root]");
+ strbuf_addf(out, "%s %s\n", cmd_reset,
+ rebase_cousins ? "onto" : "[new root]");
else {
const char *to = NULL;
@@ -4293,12 +4293,12 @@ static int make_script_with_merges(struct
pretty_print_context *pp,
);
if (!to || !strcmp(to, "onto"))
- fprintf(out, "%s onto\n", cmd_reset);
+ strbuf_addf(out, "%s onto\n", cmd_reset);
else {
strbuf_reset();
pretty_print_commit(pp, commit, );
- fprintf(out, "%s %s # %s\n",
-
[PATCH v3 15/16] sequencer: fix a call to error() in transform_todo_file()
This replaces a call to error() by a call to error_errno() after writing
the content of the todo list to the disk in transform_todo_file().
Signed-off-by: Alban Gruin
---
No changes since v2.
sequencer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sequencer.c b/sequencer.c
index 02afd2f5cd..a55df3526f 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4464,7 +4464,7 @@ int sequencer_add_exec_commands(struct string_list
*commands)
int res;
if (strbuf_read_file(_list.buf, todo_file, 0) < 0)
- return error(_("could not read '%s'."), todo_file);
+ return error_errno(_("could not read '%s'."), todo_file);
if (todo_list_parse_insn_buffer(todo_list.buf.buf, _list)) {
todo_list_release(_list);
--
2.19.1.872.ga867da739e
[PATCH v3 16/16] rebase--interactive: move transform_todo_file() to rebase--interactive.c
As transform_todo_file() is only needed inside of rebase--interactive.c,
it is moved there from sequencer.c.
Signed-off-by: Alban Gruin
---
builtin/rebase--interactive.c | 20
sequencer.c | 20
sequencer.h | 1 -
3 files changed, 20 insertions(+), 21 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index 6871990544..580c6a3822 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -34,6 +34,26 @@ static int edit_todo_file(unsigned flags)
return 0;
}
+static int transform_todo_file(unsigned flags)
+{
+ const char *todo_file = rebase_path_todo();
+ struct todo_list todo_list = TODO_LIST_INIT;
+ int res;
+
+ if (strbuf_read_file(_list.buf, todo_file, 0) < 0)
+ return error_errno(_("could not read '%s'."), todo_file);
+
+ if (todo_list_parse_insn_buffer(todo_list.buf.buf, _list)) {
+ todo_list_release(_list);
+ return error(_("unusable todo list: '%s'"), todo_file);
+ }
+
+ res = todo_list_write_to_file(_list, todo_file,
+ NULL, NULL, -1, flags);
+ todo_list_release(_list);
+ return res;
+}
+
static int get_revision_ranges(const char *upstream, const char *onto,
const char **head_hash,
char **revisions, char **shortrevisions)
diff --git a/sequencer.c b/sequencer.c
index a55df3526f..896dd04150 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4542,26 +4542,6 @@ int todo_list_write_to_file(struct todo_list *todo_list,
const char *file,
return res;
}
-int transform_todo_file(unsigned flags)
-{
- const char *todo_file = rebase_path_todo();
- struct todo_list todo_list = TODO_LIST_INIT;
- int res;
-
- if (strbuf_read_file(_list.buf, todo_file, 0) < 0)
- return error(_("could not read '%s'."), todo_file);
-
- if (todo_list_parse_insn_buffer(todo_list.buf.buf, _list)) {
- todo_list_release(_list);
- return error(_("unusable todo list: '%s'"), todo_file);
- }
-
- res = todo_list_write_to_file(_list, todo_file,
- NULL, NULL, -1, flags);
- todo_list_release(_list);
- return res;
-}
-
static const char edit_todo_list_advice[] =
N_("You can fix this with 'git rebase --edit-todo' "
"and then run 'git rebase --continue'.\n"
diff --git a/sequencer.h b/sequencer.h
index f6751d53b9..ee59233344 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -142,7 +142,6 @@ int sequencer_make_script(struct strbuf *out, int argc,
const char **argv,
unsigned flags);
int sequencer_add_exec_commands(struct string_list *commands);
-int transform_todo_file(unsigned flags);
int check_todo_list_from_file(void);
int complete_action(struct replay_opts *opts, unsigned flags,
const char *shortrevisions, const char *onto_name,
--
2.19.1.872.ga867da739e
[PATCH v3 10/16] sequencer: refactor skip_unnecessary_picks() to work on a todo_list
This refactors skip_unnecessary_picks() to work on a todo_list. The
file-handling logic is completely dropped here, as its only usage is
made by complete_action().
Instead of truncating the todo list’s buffer, the items are moved to
the beginning of the list, eliminating the need to reparse the list.
This also means its buffer cannot be directly written to the disk.
rewrite_file() is then removed, as it is now unused.
Signed-off-by: Alban Gruin
---
sequencer.c | 79 -
1 file changed, 17 insertions(+), 62 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index 64a99ab84f..1c405763c3 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4607,52 +4607,21 @@ int check_todo_list_from_file(void)
return res;
}
-static int rewrite_file(const char *path, const char *buf, size_t len)
-{
- int rc = 0;
- int fd = open(path, O_WRONLY | O_TRUNC);
- if (fd < 0)
- return error_errno(_("could not open '%s' for writing"), path);
- if (write_in_full(fd, buf, len) < 0)
- rc = error_errno(_("could not write to '%s'"), path);
- if (close(fd) && !rc)
- rc = error_errno(_("could not close '%s'"), path);
- return rc;
-}
-
/* skip picking commits whose parents are unchanged */
-static int skip_unnecessary_picks(struct object_id *output_oid)
+static int skip_unnecessary_picks(struct todo_list *todo_list,
+ struct object_id *output_oid)
{
- const char *todo_file = rebase_path_todo();
- struct strbuf buf = STRBUF_INIT;
- struct todo_list todo_list = TODO_LIST_INIT;
struct object_id *parent_oid;
- int fd, i;
-
- if (!read_oneliner(, rebase_path_onto(), 0))
- return error(_("could not read 'onto'"));
- if (get_oid(buf.buf, output_oid)) {
- strbuf_release();
- return error(_("need a HEAD to fixup"));
- }
- strbuf_release();
-
- if (strbuf_read_file_or_whine(_list.buf, todo_file) < 0)
- return -1;
- if (todo_list_parse_insn_buffer(todo_list.buf.buf, _list) < 0) {
- todo_list_release(_list);
- return -1;
- }
+ int i;
- for (i = 0; i < todo_list.nr; i++) {
- struct todo_item *item = todo_list.items + i;
+ for (i = 0; i < todo_list->nr; i++) {
+ struct todo_item *item = todo_list->items + i;
if (item->command >= TODO_NOOP)
continue;
if (item->command != TODO_PICK)
break;
if (parse_commit(item->commit)) {
- todo_list_release(_list);
return error(_("could not parse commit '%s'"),
oid_to_hex(>commit->object.oid));
}
@@ -4666,37 +4635,21 @@ static int skip_unnecessary_picks(struct object_id
*output_oid)
oidcpy(output_oid, >commit->object.oid);
}
if (i > 0) {
- int offset = get_item_line_offset(_list, i);
const char *done_path = rebase_path_done();
- fd = open(done_path, O_CREAT | O_WRONLY | O_APPEND, 0666);
- if (fd < 0) {
- error_errno(_("could not open '%s' for writing"),
- done_path);
- todo_list_release(_list);
- return -1;
- }
- if (write_in_full(fd, todo_list.buf.buf, offset) < 0) {
+ if (todo_list_write_to_file(todo_list, done_path, NULL, NULL,
i, 0)) {
error_errno(_("could not write to '%s'"), done_path);
- todo_list_release(_list);
- close(fd);
return -1;
}
- close(fd);
- if (rewrite_file(rebase_path_todo(), todo_list.buf.buf + offset,
-todo_list.buf.len - offset) < 0) {
- todo_list_release(_list);
- return -1;
- }
+ MOVE_ARRAY(todo_list->items, todo_list->items + i,
todo_list->nr - i);
+ todo_list->nr -= i;
+ todo_list->current = 0;
- todo_list.current = i;
- if (is_fixup(peek_command(_list, 0)))
- record_in_rewritten(output_oid,
peek_command(_list, 0));
+ if (is_fixup(peek_command(todo_list, 0)))
+ record_in_rewritten(output_oid, peek_command(todo_list,
0));
}
- todo_list_release(_list);
-
return 0;
}
@@ -4770,6 +4723,11 @@ int complete_action(struct replay_opts *opts, unsigned
flags,
return -1;
}
+ if (opts->allow_ff && skip_unnecessary_picks(_todo, )) {
+ todo_list_release(_todo);
+ return
[PATCH v3 06/16] sequencer: refactor sequencer_add_exec_commands() to work on a todo_list
This refactors sequencer_add_exec_commands() to work on a todo_list to
avoid redundant reads and writes to the disk.
Instead of just inserting the `exec' command between the other commands,
and re-parsing the buffer at the end the exec command is appended to the
buffer once, and a new list of items is created. Items from the old
list are copied across and new `exec' items are appended when
necessary. This eliminates the need to reparse the buffer, but this
also means we have to use todo_list_write_to_disk() to write the file().
todo_list_add_exec_commands() and sequencer_add_exec_commands() are
modified to take a string list instead of a string -- one item for each
command. This makes it easier to insert a new command to the todo list
for each command to execute.
sequencer_add_exec_commands() still reads the todo list from the disk,
as it is needed by rebase -p.
complete_action() still uses sequencer_add_exec_commands() for now.
This will be changed in a future commit.
Signed-off-by: Alban Gruin
---
builtin/rebase--interactive.c | 15 +++--
sequencer.c | 111 +-
sequencer.h | 4 +-
3 files changed, 83 insertions(+), 47 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index c1d87c0fe6..1fb5a43c0d 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -65,7 +65,7 @@ static int do_interactive_rebase(struct replay_opts *opts,
unsigned flags,
const char *onto, const char *onto_name,
const char *squash_onto, const char *head_name,
const char *restrict_revision, char
*raw_strategies,
-const char *cmd, unsigned autosquash)
+struct string_list *commands, unsigned
autosquash)
{
int ret;
const char *head_hash = NULL;
@@ -115,7 +115,7 @@ static int do_interactive_rebase(struct replay_opts *opts,
unsigned flags,
else {
discard_cache();
ret = complete_action(opts, flags, shortrevisions, onto_name,
onto,
- head_hash, cmd, autosquash);
+ head_hash, commands, autosquash);
}
free(revisions);
@@ -138,6 +138,7 @@ int cmd_rebase__interactive(int argc, const char **argv,
const char *prefix)
const char *onto = NULL, *onto_name = NULL, *restrict_revision = NULL,
*squash_onto = NULL, *upstream = NULL, *head_name = NULL,
*switch_to = NULL, *cmd = NULL;
+ struct string_list commands = STRING_LIST_INIT_DUP;
char *raw_strategies = NULL;
enum {
NONE = 0, CONTINUE, SKIP, EDIT_TODO, SHOW_CURRENT_PATCH,
@@ -220,6 +221,12 @@ int cmd_rebase__interactive(int argc, const char **argv,
const char *prefix)
warning(_("--[no-]rebase-cousins has no effect without "
"--rebase-merges"));
+ if (cmd && *cmd) {
+ string_list_split(, cmd, '\n', -1);
+ if (strlen(commands.items[commands.nr - 1].string) == 0)
+ --commands.nr;
+ }
+
switch (command) {
case NONE:
if (!onto && !upstream)
@@ -227,7 +234,7 @@ int cmd_rebase__interactive(int argc, const char **argv,
const char *prefix)
ret = do_interactive_rebase(, flags, switch_to, upstream,
onto,
onto_name, squash_onto, head_name,
restrict_revision,
- raw_strategies, cmd, autosquash);
+ raw_strategies, ,
autosquash);
break;
case SKIP: {
struct string_list merge_rr = STRING_LIST_INIT_DUP;
@@ -261,7 +268,7 @@ int cmd_rebase__interactive(int argc, const char **argv,
const char *prefix)
ret = rearrange_squash();
break;
case ADD_EXEC:
- ret = sequencer_add_exec_commands(cmd);
+ ret = sequencer_add_exec_commands();
break;
default:
BUG("invalid command '%d'", command);
diff --git a/sequencer.c b/sequencer.c
index 900899ef20..11692d0b98 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -4394,24 +4394,29 @@ int sequencer_make_script(FILE *out, int argc, const
char **argv,
return 0;
}
-/*
- * Add commands after pick and (series of) squash/fixup commands
- * in the todo list.
- */
-int sequencer_add_exec_commands(const char *commands)
+static void todo_list_add_exec_commands(struct todo_list *todo_list,
+ struct string_list *commands)
{
- const char *todo_file = rebase_path_todo();
- struct todo_list todo_list = TODO_LIST_INIT;
- struct strbuf *buf = _list.buf;
- size_t offset = 0,
[PATCH v3 05/16] sequencer: refactor check_todo_list() to work on a todo_list
This refactors check_todo_list() to work on a todo_list to avoid
redundant reads and writes to the disk. The function is renamed
todo_list_check(). The parsing of the two todo lists is left to the
caller.
As rebase -p still need to check the todo list from the disk, a new
function is introduced, check_todo_list_from_file(). It reads the file
from the disk, parses it, pass the todo_list to todo_list_check(), and
writes it back to the disk.
As get_missing_commit_check_level() and the enum
missing_commit_check_level are no longer needed inside of sequencer.c,
they are moved to rebase-interactive.c, and made static again.
Signed-off-by: Alban Gruin
---
No changes since v2.
builtin/rebase--interactive.c | 2 +-
rebase-interactive.c | 90 -
rebase-interactive.h | 1 +
sequencer.c | 120 +++---
sequencer.h | 9 +--
5 files changed, 115 insertions(+), 107 deletions(-)
diff --git a/builtin/rebase--interactive.c b/builtin/rebase--interactive.c
index abdf6126df..c1d87c0fe6 100644
--- a/builtin/rebase--interactive.c
+++ b/builtin/rebase--interactive.c
@@ -255,7 +255,7 @@ int cmd_rebase__interactive(int argc, const char **argv,
const char *prefix)
ret = transform_todo_file(flags);
break;
case CHECK_TODO_LIST:
- ret = check_todo_list();
+ ret = check_todo_list_from_file();
break;
case REARRANGE_SQUASH:
ret = rearrange_squash();
diff --git a/rebase-interactive.c b/rebase-interactive.c
index 4cd487a450..3adcf39e07 100644
--- a/rebase-interactive.c
+++ b/rebase-interactive.c
@@ -1,8 +1,32 @@
#include "cache.h"
#include "commit.h"
-#include "rebase-interactive.h"
#include "sequencer.h"
+#include "rebase-interactive.h"
#include "strbuf.h"
+#include "commit-slab.h"
+#include "config.h"
+
+enum missing_commit_check_level {
+ MISSING_COMMIT_CHECK_IGNORE = 0,
+ MISSING_COMMIT_CHECK_WARN,
+ MISSING_COMMIT_CHECK_ERROR
+};
+
+static enum missing_commit_check_level get_missing_commit_check_level(void)
+{
+ const char *value;
+
+ if (git_config_get_value("rebase.missingcommitscheck", ) ||
+ !strcasecmp("ignore", value))
+ return MISSING_COMMIT_CHECK_IGNORE;
+ if (!strcasecmp("warn", value))
+ return MISSING_COMMIT_CHECK_WARN;
+ if (!strcasecmp("error", value))
+ return MISSING_COMMIT_CHECK_ERROR;
+ warning(_("unrecognized setting %s for option "
+ "rebase.missingCommitsCheck. Ignoring."), value);
+ return MISSING_COMMIT_CHECK_IGNORE;
+}
void append_todo_help(unsigned edit_todo, unsigned keep_empty,
struct strbuf *buf)
@@ -89,3 +113,67 @@ int edit_todo_list(unsigned flags)
return 0;
}
+
+define_commit_slab(commit_seen, unsigned char);
+/*
+ * Check if the user dropped some commits by mistake
+ * Behaviour determined by rebase.missingCommitsCheck.
+ * Check if there is an unrecognized command or a
+ * bad SHA-1 in a command.
+ */
+int todo_list_check(struct todo_list *old_todo, struct todo_list *new_todo)
+{
+ enum missing_commit_check_level check_level =
get_missing_commit_check_level();
+ struct strbuf missing = STRBUF_INIT;
+ int res = 0, i;
+ struct commit_seen commit_seen;
+
+ init_commit_seen(_seen);
+
+ if (check_level == MISSING_COMMIT_CHECK_IGNORE)
+ goto leave_check;
+
+ /* Mark the commits in git-rebase-todo as seen */
+ for (i = 0; i < new_todo->nr; i++) {
+ struct commit *commit = new_todo->items[i].commit;
+ if (commit)
+ *commit_seen_at(_seen, commit) = 1;
+ }
+
+ /* Find commits in git-rebase-todo.backup yet unseen */
+ for (i = old_todo->nr - 1; i >= 0; i--) {
+ struct todo_item *item = old_todo->items + i;
+ struct commit *commit = item->commit;
+ if (commit && !*commit_seen_at(_seen, commit)) {
+ strbuf_addf(, " - %s %.*s\n",
+ find_unique_abbrev(>object.oid,
DEFAULT_ABBREV),
+ item->arg_len, item->arg);
+ *commit_seen_at(_seen, commit) = 1;
+ }
+ }
+
+ /* Warn about missing commits */
+ if (!missing.len)
+ goto leave_check;
+
+ if (check_level == MISSING_COMMIT_CHECK_ERROR)
+ res = 1;
+
+ fprintf(stderr,
+ _("Warning: some commits may have been dropped accidentally.\n"
+ "Dropped commits (newer to older):\n"));
+
+ /* Make the list user-friendly and display */
+ fputs(missing.buf, stderr);
+ strbuf_release();
+
+ fprintf(stderr, _("To avoid this message, use \"drop\" to "
+ "explicitly remove a
