`git stash --help` tries to pull up nonexistent file gitstack.html
Looks like a simple typo. Joseph Musser -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv2] checkout: do not mention detach advice for explicit --detach option
When a user asked for a detached HEAD specifically with `--detach`, we do not need to give advice on what a detached HEAD state entails as we can assume they know what they're getting into as they asked for it. Signed-off-by: Stefan Beller --- jrnieder wrote: > Examples? What do you mean by example? builtin/checkout.c | 2 +- t/t2020-checkout-detach.sh | 13 + 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 8d852d4..85408b1 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -658,7 +658,7 @@ static void update_refs_for_switch(const struct checkout_opts *opts, update_ref(msg.buf, "HEAD", new->commit->object.oid.hash, NULL, REF_NODEREF, UPDATE_REFS_DIE_ON_ERR); if (!opts->quiet) { - if (old->path && advice_detached_head) + if (old->path && advice_detached_head && !opts->force_detach) detach_advice(new->name); describe_detached_head(_("HEAD is now at"), new->commit); } diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh index 5d68729..941ea49 100755 --- a/t/t2020-checkout-detach.sh +++ b/t/t2020-checkout-detach.sh @@ -163,4 +163,17 @@ test_expect_success 'tracking count is accurate after orphan check' ' test_i18ncmp expect stdout ' +test_expect_success 'no advice given for explicit detached head state' ' + git config advice.detachedHead false && + git checkout child && + git checkout --detach HEAD >expect && + git config advice.detachedHead true && + git checkout child && + git checkout --detach HEAD >actual && + test_cmp expect actual && + git checkout child && + git checkout HEAD >actual && + ! test_cmp expect actual +' + test_done -- 2.9.2.730.g46b112d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv4 6/8] clone: clarify option_reference as required
In the next patch we introduce optional references; To better distinguish between optional and required references we rename the variable. Signed-off-by: Stefan Beller --- builtin/clone.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index 24b17539..7ccbdef 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -50,7 +50,7 @@ static int option_verbosity; static int option_progress = -1; static enum transport_family family; static struct string_list option_config = STRING_LIST_INIT_NODUP; -static struct string_list option_reference = STRING_LIST_INIT_NODUP; +static struct string_list option_required_reference = STRING_LIST_INIT_NODUP; static int option_dissociate; static int max_jobs = -1; @@ -79,7 +79,7 @@ static struct option builtin_clone_options[] = { N_("number of submodules cloned in parallel")), OPT_STRING(0, "template", &option_template, N_("template-directory"), N_("directory from which templates will be used")), - OPT_STRING_LIST(0, "reference", &option_reference, N_("repo"), + OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"), N_("reference repository")), OPT_BOOL(0, "dissociate", &option_dissociate, N_("use --reference only while cloning")), @@ -297,7 +297,7 @@ static int add_one_reference(struct string_list_item *item, void *cb_data) static void setup_reference(void) { - for_each_string_list(&option_reference, add_one_reference, NULL); + for_each_string_list(&option_required_reference, add_one_reference, NULL); } static void copy_alternates(struct strbuf *src, struct strbuf *dst, @@ -949,7 +949,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) git_config_set(key.buf, repo); strbuf_reset(&key); - if (option_reference.nr) + if (option_required_reference.nr) setup_reference(); fetch_pattern = value.buf; -- 2.9.2.737.g4a14654 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv4 7/8] clone: implement optional references
In a later patch we want to try to create alternates for submodules, but they might not exist in the referenced superproject. So add a way to skip the non existing references and report them. Signed-off-by: Stefan Beller --- Documentation/git-clone.txt | 5 - builtin/clone.c | 29 ++--- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt index ec41d3d..e316c4b 100644 --- a/Documentation/git-clone.txt +++ b/Documentation/git-clone.txt @@ -90,13 +90,16 @@ If you want to break the dependency of a repository cloned with `-s` on its source repository, you can simply run `git repack -a` to copy all objects from the source repository into a pack in the cloned repository. ---reference :: +--reference[-if-able] :: If the reference repository is on the local machine, automatically setup `.git/objects/info/alternates` to obtain objects from the reference repository. Using an already existing repository as an alternate will require fewer objects to be copied from the repository being cloned, reducing network and local storage costs. + When using the `--reference-if-able`, a non existing + directory is skipped with a warning instead of aborting + the clone. + *NOTE*: see the NOTE for the `--shared` option, and also the `--dissociate` option. diff --git a/builtin/clone.c b/builtin/clone.c index 7ccbdef..fdb2ca9 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -51,6 +51,7 @@ static int option_progress = -1; static enum transport_family family; static struct string_list option_config = STRING_LIST_INIT_NODUP; static struct string_list option_required_reference = STRING_LIST_INIT_NODUP; +static struct string_list option_optional_reference = STRING_LIST_INIT_NODUP; static int option_dissociate; static int max_jobs = -1; @@ -81,6 +82,8 @@ static struct option builtin_clone_options[] = { N_("directory from which templates will be used")), OPT_STRING_LIST(0, "reference", &option_required_reference, N_("repo"), N_("reference repository")), + OPT_STRING_LIST(0, "reference-if-able", &option_optional_reference, + N_("repo"), N_("reference repository")), OPT_BOOL(0, "dissociate", &option_dissociate, N_("use --reference only while cloning")), OPT_STRING('o', "origin", &option_origin, N_("name"), @@ -283,13 +286,20 @@ static void strip_trailing_slashes(char *dir) static int add_one_reference(struct string_list_item *item, void *cb_data) { struct strbuf sb = STRBUF_INIT; + int *required = cb_data; char *ref_git = compute_alternate_path(item->string, &sb); - if (!ref_git) - die("%s", sb.buf); - - strbuf_addf(&sb, "%s/objects", ref_git); - add_to_alternates_file(sb.buf); + if (!ref_git) { + if (*required) + die("%s", sb.buf); + else + fprintf(stderr, + _("info: Could not add alternate for '%s': %s\n"), + item->string, sb.buf); + } else { + strbuf_addf(&sb, "%s/objects", ref_git); + add_to_alternates_file(sb.buf); + } strbuf_release(&sb); return 0; @@ -297,7 +307,12 @@ static int add_one_reference(struct string_list_item *item, void *cb_data) static void setup_reference(void) { - for_each_string_list(&option_required_reference, add_one_reference, NULL); + int required = 1; + for_each_string_list(&option_required_reference, +add_one_reference, &required); + required = 0; + for_each_string_list(&option_optional_reference, +add_one_reference, &required); } static void copy_alternates(struct strbuf *src, struct strbuf *dst, @@ -949,7 +964,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) git_config_set(key.buf, repo); strbuf_reset(&key); - if (option_required_reference.nr) + if (option_required_reference.nr || option_optional_reference.nr) setup_reference(); fetch_pattern = value.buf; -- 2.9.2.737.g4a14654 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv4 5/8] clone: factor out checking for an alternate path
In a later patch we want to determine if a path is suitable as an alternate from other commands than builtin/clone. Move the checking functionality of `add_one_reference` to `compute_alternate_path` that is defined in cache.h. Signed-off-by: Stefan Beller --- builtin/clone.c | 42 ++-- cache.h | 1 + sha1_file.c | 74 + 3 files changed, 82 insertions(+), 35 deletions(-) diff --git a/builtin/clone.c b/builtin/clone.c index f044a8c..24b17539 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -282,44 +282,16 @@ static void strip_trailing_slashes(char *dir) static int add_one_reference(struct string_list_item *item, void *cb_data) { - char *ref_git; - const char *repo; - struct strbuf alternate = STRBUF_INIT; - - /* Beware: read_gitfile(), real_path() and mkpath() return static buffer */ - ref_git = xstrdup(real_path(item->string)); - - repo = read_gitfile(ref_git); - if (!repo) - repo = read_gitfile(mkpath("%s/.git", ref_git)); - if (repo) { - free(ref_git); - ref_git = xstrdup(repo); - } - - if (!repo && is_directory(mkpath("%s/.git/objects", ref_git))) { - char *ref_git_git = mkpathdup("%s/.git", ref_git); - free(ref_git); - ref_git = ref_git_git; - } else if (!is_directory(mkpath("%s/objects", ref_git))) { - struct strbuf sb = STRBUF_INIT; - if (get_common_dir(&sb, ref_git)) - die(_("reference repository '%s' as a linked checkout is not supported yet."), - item->string); - die(_("reference repository '%s' is not a local repository."), - item->string); - } + struct strbuf sb = STRBUF_INIT; + char *ref_git = compute_alternate_path(item->string, &sb); - if (!access(mkpath("%s/shallow", ref_git), F_OK)) - die(_("reference repository '%s' is shallow"), item->string); + if (!ref_git) + die("%s", sb.buf); - if (!access(mkpath("%s/info/grafts", ref_git), F_OK)) - die(_("reference repository '%s' is grafted"), item->string); + strbuf_addf(&sb, "%s/objects", ref_git); + add_to_alternates_file(sb.buf); - strbuf_addf(&alternate, "%s/objects", ref_git); - add_to_alternates_file(alternate.buf); - strbuf_release(&alternate); - free(ref_git); + strbuf_release(&sb); return 0; } diff --git a/cache.h b/cache.h index 95a0bd3..35f41f7 100644 --- a/cache.h +++ b/cache.h @@ -1344,6 +1344,7 @@ extern struct alternate_object_database { } *alt_odb_list; extern void prepare_alt_odb(void); extern void read_info_alternates(const char * relative_base, int depth); +extern char *compute_alternate_path(const char *path, struct strbuf *err); extern void add_to_alternates_file(const char *reference); typedef int alt_odb_fn(struct alternate_object_database *, void *); extern int foreach_alt_odb(alt_odb_fn, void*); diff --git a/sha1_file.c b/sha1_file.c index 02940f1..7351d8c 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -418,6 +418,80 @@ void add_to_alternates_file(const char *reference) free(alts); } +/* + * Compute the exact path an alternate is at and returns it. In case of + * error NULL is returned and the human readable error is added to `err` + * `path` may be relative and should point to $GITDIR. + * `err` must not be null. + */ +char *compute_alternate_path(const char *path, struct strbuf *err) +{ + char *ref_git = NULL; + const char *repo, *ref_git_s; + struct strbuf err_buf = STRBUF_INIT; + + ref_git_s = real_path_if_valid(path); + if (!ref_git_s) { + strbuf_addf(&err_buf, _("path '%s' does not exist"), path); + goto out; + } else + /* +* Beware: read_gitfile(), real_path() and mkpath() +* return static buffer +*/ + ref_git = xstrdup(ref_git_s); + + repo = read_gitfile(ref_git); + if (!repo) + repo = read_gitfile(mkpath("%s/.git", ref_git)); + if (repo) { + free(ref_git); + ref_git = xstrdup(repo); + } + + if (!repo && is_directory(mkpath("%s/.git/objects", ref_git))) { + char *ref_git_git = mkpathdup("%s/.git", ref_git); + free(ref_git); + ref_git = ref_git_git; + } else if (!is_directory(mkpath("%s/objects", ref_git))) { + struct strbuf sb = STRBUF_INIT; + if (get_common_dir(&sb, ref_git)) { + strbuf_addf(&err_buf, + _("reference repository '%s' as a linked " + "checkout is not supported yet."), + path); +
[PATCHv4 8/8] clone: recursive and reference option triggers submodule alternates
When `--recursive` and `--reference` is given, it is reasonable to expect that the submodules are created with references to the submodules of the given alternate for the superproject. An initial attempt to do this was presented to the mailing list, which used flags that are passed around ("--super-reference") that instructed the submodule clone to look for a reference in the submodules of the referenced superproject. This is not well thought out, as any further `submodule update` should also respect the initial setup. When a new submodule is added to the superproject and the alternate of the superproject does not know about that submodule yet, we rather error out informing the user instead of being unclear if we did or did not use a submodules alternate. To solve this problem introduce new options that store the configuration for what the user wanted originally. Signed-off-by: Stefan Beller --- Documentation/config.txt | 12 ++ builtin/clone.c| 19 + builtin/submodule--helper.c| 87 ++ t/t7408-submodule-reference.sh | 43 + 4 files changed, 161 insertions(+) diff --git a/Documentation/config.txt b/Documentation/config.txt index 0bcb679..e09d32c 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2847,6 +2847,18 @@ submodule.fetchJobs:: in parallel. A value of 0 will give some reasonable default. If unset, it defaults to 1. +submodule.alternateLocation:: + Specifies how the submodules obtain alternates when submodules are + cloned. Possible values are `no`, `superproject`. + By default `no` is assumed, which doesn't add references. When the + value is set to `superproject` the submodule to be cloned computes + its alternates location relative to the superprojects alternate. + +submodule.alternateErrorStrategy + Specifies how to treat errors with the alternates for a submodule + as computed via `submodule.alternateLocation`. Possible values are + `ignore`, `info`, `die`. + tag.forceSignAnnotated:: A boolean to specify whether annotated tags created should be GPG signed. If `--annotate` is specified on the command line, it takes diff --git a/builtin/clone.c b/builtin/clone.c index fdb2ca9..0593aee 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -944,6 +944,25 @@ int cmd_clone(int argc, const char **argv, const char *prefix) else fprintf(stderr, _("Cloning into '%s'...\n"), dir); } + + if (option_recursive) { + if (option_required_reference.nr && + option_optional_reference.nr) + die(_("clone --recursive is not compatible with " + "both --reference and --reference-if-able")); + else if (option_required_reference.nr) { + string_list_append(&option_config, + "submodule.alternateLocation=superproject"); + string_list_append(&option_config, + "submodule.alternateErrorStrategy=die"); + } else if (option_optional_reference.nr) { + string_list_append(&option_config, + "submodule.alternateLocation=superproject"); + string_list_append(&option_config, + "submodule.alternateErrorStrategy=info"); + } + } + init_db(option_template, INIT_DB_QUIET); write_config(&option_config); diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index a10a777..a71c903 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -472,6 +472,90 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url return run_command(&cp); } +struct submodule_alternate_setup { + const char *submodule_name; + enum SUBMODULE_ALTERNATE_ERROR_MODE { + SUBMODULE_ALTERNATE_ERROR_DIE, + SUBMODULE_ALTERNATE_ERROR_INFO, + SUBMODULE_ALTERNATE_ERROR_IGNORE + } error_mode; + struct string_list *reference; +}; +#define SUBMODULE_ALTERNATE_SETUP_INIT { NULL, \ + SUBMODULE_ALTERNATE_ERROR_IGNORE, NULL } + +int add_possible_reference_from_superproject( + struct alternate_object_database *alt, void *sas_cb) +{ + struct submodule_alternate_setup *sas = sas_cb; + + /* directory name, minus trailing slash */ + size_t namelen = alt->name - alt->base - 1; + struct strbuf name = STRBUF_INIT; + strbuf_add(&name, alt->base, namelen); + + /* +* If the alternate object store is another repository, try the +* standard layout with .git/modules//objects +*/ + if (ends_with(name.buf, ".git/objects")) { + char *sm_alt
[PATCHv4 2/8] t7408: merge short tests, factor out testing method
Tests consisting of one line each can be consolidated to have fewer tests to run as well as fewer lines of code. When having just a few git commands, do not create a new shell but use the -C flag in Git to execute in the correct directory. Signed-off-by: Stefan Beller --- t/t7408-submodule-reference.sh | 48 ++ 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh index b84c6748..dff47af 100755 --- a/t/t7408-submodule-reference.sh +++ b/t/t7408-submodule-reference.sh @@ -8,6 +8,15 @@ test_description='test clone --reference' base_dir=$(pwd) +test_alternate_is_used () { + alternates_file="$1" && + working_dir="$2" && + test_line_count = 1 "$alternates_file" && + echo "0 objects, 0 kilobytes" >expect && + git -C "$working_dir" count-objects >actual && + test_cmp expect actual +} + test_expect_success 'preparing first repository' ' test_create_repo A && ( @@ -40,16 +49,14 @@ test_expect_success 'preparing superproject' ' ) ' -test_expect_success 'submodule add --reference' ' +test_expect_success 'submodule add --reference uses alternates' ' ( cd super && git submodule add --reference ../B "file://$base_dir/A" sub && - git commit -m B-super-added - ) -' - -test_expect_success 'after add: existence of info/alternates' ' - test_line_count = 1 super/.git/modules/sub/objects/info/alternates + git commit -m B-super-added && + git repack -ad + ) && + test_alternate_is_used super/.git/modules/sub/objects/info/alternates super/sub ' test_expect_success 'that reference gets used with add' ' @@ -61,23 +68,18 @@ test_expect_success 'that reference gets used with add' ' ) ' -test_expect_success 'cloning superproject' ' - git clone super super-clone -' - -test_expect_success 'update with reference' ' - cd super-clone && git submodule update --init --reference ../B -' - -test_expect_success 'after update: existence of info/alternates' ' - test_line_count = 1 super-clone/.git/modules/sub/objects/info/alternates -' +# The tests up to this point, and repositories created by them +# (A, B, super and super/sub), are about setting up the stage +# for subsequent tests and meant to be kept throughout the +# remainder of the test. +# Tests from here on, if they create their own test repository, +# are expected to clean after themselves. -test_expect_success 'that reference gets used with update' ' - cd super-clone/sub && - echo "0 objects, 0 kilobytes" >expected && - git count-objects >current && - diff expected current +test_expect_success 'updating superproject keeps alternates' ' + test_when_finished "rm -rf super-clone" && + git clone super super-clone && + git -C super-clone submodule update --init --reference ../B && + test_alternate_is_used super-clone/.git/modules/sub/objects/info/alternates super-clone/sub ' test_done -- 2.9.2.737.g4a14654 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv4 3/8] submodule--helper module-clone: allow multiple references
Allow users to pass in multiple references, just as clone accepts multiple references as well. Signed-off-by: Stefan Beller --- builtin/submodule--helper.c | 20 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index b09632e..db9270e 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -442,7 +442,7 @@ static int module_name(int argc, const char **argv, const char *prefix) } static int clone_submodule(const char *path, const char *gitdir, const char *url, - const char *depth, const char *reference, int quiet) + const char *depth, struct string_list *reference, int quiet) { struct child_process cp; child_process_init(&cp); @@ -453,8 +453,12 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url argv_array_push(&cp.args, "--quiet"); if (depth && *depth) argv_array_pushl(&cp.args, "--depth", depth, NULL); - if (reference && *reference) - argv_array_pushl(&cp.args, "--reference", reference, NULL); + if (reference->nr) { + struct string_list_item *item; + for_each_string_list_item(item, reference) + argv_array_pushl(&cp.args, "--reference", +item->string, NULL); + } if (gitdir && *gitdir) argv_array_pushl(&cp.args, "--separate-git-dir", gitdir, NULL); @@ -470,13 +474,13 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url static int module_clone(int argc, const char **argv, const char *prefix) { - const char *name = NULL, *url = NULL; - const char *reference = NULL, *depth = NULL; + const char *name = NULL, *url = NULL, *depth = NULL; int quiet = 0; FILE *submodule_dot_git; char *p, *path = NULL, *sm_gitdir; struct strbuf rel_path = STRBUF_INIT; struct strbuf sb = STRBUF_INIT; + struct string_list reference = STRING_LIST_INIT_NODUP; struct option module_clone_options[] = { OPT_STRING(0, "prefix", &prefix, @@ -491,8 +495,8 @@ static int module_clone(int argc, const char **argv, const char *prefix) OPT_STRING(0, "url", &url, N_("string"), N_("url where to clone the submodule from")), - OPT_STRING(0, "reference", &reference, - N_("string"), + OPT_STRING_LIST(0, "reference", &reference, + N_("repo"), N_("reference repository")), OPT_STRING(0, "depth", &depth, N_("string"), @@ -528,7 +532,7 @@ static int module_clone(int argc, const char **argv, const char *prefix) if (!file_exists(sm_gitdir)) { if (safe_create_leading_directories_const(sm_gitdir) < 0) die(_("could not create directory '%s'"), sm_gitdir); - if (clone_submodule(path, sm_gitdir, url, depth, reference, quiet)) + if (clone_submodule(path, sm_gitdir, url, depth, &reference, quiet)) die(_("clone of '%s' into submodule path '%s' failed"), url, path); } else { -- 2.9.2.737.g4a14654 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv4 4/8] submodule--helper update-clone: allow multiple references
Allow the user to pass in multiple references to update_clone. Currently this is only internal API, but once the shell script is replaced by a C version, this is needed. This fixes an API bug between the shell script and the helper. Currently the helper accepts "--reference" "--reference=foo" as a OPT_STRING whose value happens to be "--reference=foo", and then uses if (suc->reference) argv_array_push(&child->args, suc->reference) where suc->reference _is_ "--reference=foo" when invoking the underlying "git clone", it cancels out. With this change we omit one of the "--reference" arguments when passing references from the shell script to the helper. Signed-off-by: Stefan Beller --- builtin/submodule--helper.c | 14 +- git-submodule.sh| 2 +- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index db9270e..a10a777 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -584,7 +584,7 @@ struct submodule_update_clone { /* configuration parameters which are passed on to the children */ int quiet; int recommend_shallow; - const char *reference; + struct string_list references; const char *depth; const char *recursive_prefix; const char *prefix; @@ -600,7 +600,8 @@ struct submodule_update_clone { int failed_clones_nr, failed_clones_alloc; }; #define SUBMODULE_UPDATE_CLONE_INIT {0, MODULE_LIST_INIT, 0, \ - SUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, NULL, NULL, NULL, NULL, \ + SUBMODULE_UPDATE_STRATEGY_INIT, 0, -1, STRING_LIST_INIT_DUP, \ + NULL, NULL, NULL, \ STRING_LIST_INIT_DUP, 0, NULL, 0, 0} @@ -710,8 +711,11 @@ static int prepare_to_clone_next_submodule(const struct cache_entry *ce, argv_array_pushl(&child->args, "--path", sub->path, NULL); argv_array_pushl(&child->args, "--name", sub->name, NULL); argv_array_pushl(&child->args, "--url", url, NULL); - if (suc->reference) - argv_array_push(&child->args, suc->reference); + if (suc->references.nr) { + struct string_list_item *item; + for_each_string_list_item(item, &suc->references) + argv_array_pushl(&child->args, "--reference", item->string, NULL); + } if (suc->depth) argv_array_push(&child->args, suc->depth); @@ -830,7 +834,7 @@ static int update_clone(int argc, const char **argv, const char *prefix) OPT_STRING(0, "update", &update, N_("string"), N_("rebase, merge, checkout or none")), - OPT_STRING(0, "reference", &suc.reference, N_("repo"), + OPT_STRING_LIST(0, "reference", &suc.references, N_("repo"), N_("reference repository")), OPT_STRING(0, "depth", &suc.depth, "", N_("Create a shallow clone truncated to the " diff --git a/git-submodule.sh b/git-submodule.sh index b57f87d..a1cc71b 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -576,7 +576,7 @@ cmd_update() ${wt_prefix:+--prefix "$wt_prefix"} \ ${prefix:+--recursive-prefix "$prefix"} \ ${update:+--update "$update"} \ - ${reference:+--reference "$reference"} \ + ${reference:+"$reference"} \ ${depth:+--depth "$depth"} \ ${recommend_shallow:+"$recommend_shallow"} \ ${jobs:+$jobs} \ -- 2.9.2.737.g4a14654 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv4 0/6] git clone: Marry --recursive and --reference
v4: Thanks to Junios critial questions regarding the design, I took a step back to look at the bigger picture, again. new patches: clone: factor out checking for an alternate path clone: recursive and reference option triggers submodule alternates The last patch redesigns completely how we approach the problem. Now there are no new command line options (that relate to the problem of marrying --recursive and --reference), but instead we communicate everything via configuration options to have a lasting effect (i.e. submodule update remembers the decision of the initial setup) Thanks, Stefan v3: Thanks to Junios critial questions regarding the design, I took a step back to look at the bigger picture. --super-reference sounds confusing. (what is the super referring to?) So drop that approach. Instead we'll compute where the reference might be in the superproject scope and ask the submodule clone operation to consider an optional reference. If the referenced alternate is not there, we'll just warn about it and carry on. * fixed the style in patch 2. * fixed another bug in the last patch, that is unrelated, but would have helped me a lot. Thanks, Stefan Documentation/git-clone.txt| 9 - builtin/clone.c| 36 builtin/submodule--helper.c| 105 - git-submodule.sh | 2 +- t/t7408-submodule-reference.sh | 162 -- 5 files changed, 217 insertions(+), 97 deletions(-) v2: * fixed the p1,2 cleanups * added documentation to patches 5,6 * improved commit message in v4 Thanks, Stefan v1: Currently when cloning a superproject with --recursive and --reference only the superproject learns about its alternates. The submodules are cloned independently, which may incur lots of network costs. Assume that the reference repository has the submodules at the same paths as the to-be-cloned submodule and try to setup alternates from there. Some submodules in the referenced superproject may not be there, (they are just not initialized/cloned/checked out), which yields an error for now. In future work we may want to soften the alternate check and not die in the clone when one of the given alternates doesn't exist. patch 1,2 are modernizing style of t7408, patches 3,4 are not strictly necessary, but I think it is a good thing to not leave the submodule related C code in a crippled state (i.e. allowing only one reference). The shell code would also need this update, but it looked ugly to me, so I postpone it until more of the submodule code is written in C. Thanks, Stefan Stefan Beller (6): t7408: modernize style t7408: merge short tests, factor out testing method submodule--helper module-clone: allow multiple references submodule--helper update-clone: allow multiple references submodule update: add super-reference flag clone: reference flag is used for submodules as well builtin/clone.c| 22 -- builtin/submodule--helper.c| 45 git-submodule.sh | 12 +++- t/t7408-submodule-reference.sh | 153 +++-- 4 files changed, 147 insertions(+), 85 deletions(-) -- 2.9.2.572.g9d9644e.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv4 1/8] t7408: modernize style
No functional change intended. This commit only changes formatting to the style we recently use, e.g. starting the body of a test with a single quote on the same line as the header, and then having the test indented in the following lines. Whenever we change directories, we do that in subshells. Signed-off-by: Stefan Beller --- t/t7408-submodule-reference.sh | 140 + 1 file changed, 71 insertions(+), 69 deletions(-) diff --git a/t/t7408-submodule-reference.sh b/t/t7408-submodule-reference.sh index eaea19b..b84c6748 100755 --- a/t/t7408-submodule-reference.sh +++ b/t/t7408-submodule-reference.sh @@ -8,74 +8,76 @@ test_description='test clone --reference' base_dir=$(pwd) -U=$base_dir/UPLOAD_LOG - -test_expect_success 'preparing first repository' \ -'test_create_repo A && cd A && -echo first > file1 && -git add file1 && -git commit -m A-initial' - -cd "$base_dir" - -test_expect_success 'preparing second repository' \ -'git clone A B && cd B && -echo second > file2 && -git add file2 && -git commit -m B-addition && -git repack -a -d && -git prune' - -cd "$base_dir" - -test_expect_success 'preparing superproject' \ -'test_create_repo super && cd super && -echo file > file && -git add file && -git commit -m B-super-initial' - -cd "$base_dir" - -test_expect_success 'submodule add --reference' \ -'cd super && git submodule add --reference ../B "file://$base_dir/A" sub && -git commit -m B-super-added' - -cd "$base_dir" - -test_expect_success 'after add: existence of info/alternates' \ -'test_line_count = 1 super/.git/modules/sub/objects/info/alternates' - -cd "$base_dir" - -test_expect_success 'that reference gets used with add' \ -'cd super/sub && -echo "0 objects, 0 kilobytes" > expected && -git count-objects > current && -diff expected current' - -cd "$base_dir" - -test_expect_success 'cloning superproject' \ -'git clone super super-clone' - -cd "$base_dir" - -test_expect_success 'update with reference' \ -'cd super-clone && git submodule update --init --reference ../B' - -cd "$base_dir" - -test_expect_success 'after update: existence of info/alternates' \ -'test_line_count = 1 super-clone/.git/modules/sub/objects/info/alternates' - -cd "$base_dir" - -test_expect_success 'that reference gets used with update' \ -'cd super-clone/sub && -echo "0 objects, 0 kilobytes" > expected && -git count-objects > current && -diff expected current' - -cd "$base_dir" +test_expect_success 'preparing first repository' ' + test_create_repo A && + ( + cd A && + echo first >file1 && + git add file1 && + git commit -m A-initial + ) +' + +test_expect_success 'preparing second repository' ' + git clone A B && + ( + cd B && + echo second >file2 && + git add file2 && + git commit -m B-addition && + git repack -a -d && + git prune + ) +' + +test_expect_success 'preparing superproject' ' + test_create_repo super && + ( + cd super && + echo file >file && + git add file && + git commit -m B-super-initial + ) +' + +test_expect_success 'submodule add --reference' ' + ( + cd super && + git submodule add --reference ../B "file://$base_dir/A" sub && + git commit -m B-super-added + ) +' + +test_expect_success 'after add: existence of info/alternates' ' + test_line_count = 1 super/.git/modules/sub/objects/info/alternates +' + +test_expect_success 'that reference gets used with add' ' + ( + cd super/sub && + echo "0 objects, 0 kilobytes" >expected && + git count-objects >current && + diff expected current + ) +' + +test_expect_success 'cloning superproject' ' + git clone super super-clone +' + +test_expect_success 'update with reference' ' + cd super-clone && git submodule update --init --reference ../B +' + +test_expect_success 'after update: existence of info/alternates' ' + test_line_count = 1 super-clone/.git/modules/sub/objects/info/alternates +' + +test_expect_success 'that reference gets used with update' ' + cd super-clone/sub && + echo "0 objects, 0 kilobytes" >expected && + git count-objects >current && + diff expected current +' test_done -- 2.9.2.737.g4a14654 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 00/12] Update git revisions
This has grown like topsy from a little two patch series that tried to name the 2-dots notation [1] into this extended set of tweaks. As documentation can be rather personal, I've split out each small change so each can be individually justified. Since V4, I've confirmed that the format breaking issue is that we cannot quote code in headings in the man page layout - ultimately it's a docbook decision, and follows the line of analysis Peff identified (see commentary in the patch). In addition the multi-parent notations have been clarified and extended. Thus the old patch 4 has been split into three. The first three patches are unchanged. The following 4 patches are also unchanged. Finally, at the end an extra 2 patches are added to build up the examples by including details of the notation expansions. This updates po/range-doc (2016-07-20) 8 commits. Hopefully my updated workflow will get the right patches to the right people. [1] https://public-inbox.org/git/0648000B273C412AB7140AE959EBC99A%40PhilipOakley/ Philip Oakley (12): doc: use 'symmetric difference' consistently doc: revisions - name the left and right sides doc: show the actual left, right, and boundary marks doc: revisions: give headings for the two and three dot notations doc: revisions: extra clarification of ^! notation effects doc: revisions: single vs multi-parent notation comparison doc: gitrevisions - use 'reachable' in page description doc: gitrevisions - clarify 'latter case' is revision walk doc: revisions - define `reachable` doc: revisions - clarify reachability examples doc: revisions: show revision expansion in examples doc: revisions: sort examples and fix alignment of the unchanged Documentation/gitk.txt | 2 +- Documentation/gitrevisions.txt | 6 +- Documentation/pretty-formats.txt | 2 +- Documentation/rev-list-options.txt | 4 +- Documentation/revisions.txt| 121 +++-- 5 files changed, 84 insertions(+), 51 deletions(-) -- 2.9.0.windows.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 01/12] doc: use 'symmetric difference' consistently
Signed-off-by: Philip Oakley --- unchanged --- Documentation/gitk.txt | 2 +- Documentation/rev-list-options.txt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt index 6ade002..6c3eb15 100644 --- a/Documentation/gitk.txt +++ b/Documentation/gitk.txt @@ -70,7 +70,7 @@ linkgit:git-rev-list[1] for a complete list. --left-right:: - Mark which side of a symmetric diff a commit is reachable + Mark which side of a symmetric difference a commit is reachable from. Commits from the left side are prefixed with a `<` symbol and those from the right with a `>` symbol. diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 4f009d4..6dc0bb0 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -225,7 +225,7 @@ excluded from the output. --left-only:: --right-only:: - List only commits on the respective side of a symmetric range, + List only commits on the respective side of a symmetric difference, i.e. only those which would be marked `<` resp. `>` by `--left-right`. + @@ -766,7 +766,7 @@ ifdef::git-rev-list[] endif::git-rev-list[] --left-right:: - Mark which side of a symmetric diff a commit is reachable from. + Mark which side of a symmetric difference a commit is reachable from. Commits from the left side are prefixed with `<` and those from the right with `>`. If combined with `--boundary`, those commits are prefixed with `-`. -- 2.9.0.windows.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 00/12] Update git revisions
I'm having trouble with my ISP is not playing ball. only cover and patch 1 were sent. Will retry with alternate service. Philip - Original Message - From: "Philip Oakley" Sent: Thursday, August 11, 2016 10:50 PM Subject: [PATCH v5 00/12] Update git revisions This has grown like topsy from a little two patch series that tried to name the 2-dots notation [1] into this extended set of tweaks. As documentation can be rather personal, I've split out each small change so each can be individually justified. Since V4, I've confirmed that the format breaking issue is that we cannot quote code in headings in the man page layout - ultimately it's a docbook decision, and follows the line of analysis Peff identified (see commentary in the patch). In addition the multi-parent notations have been clarified and extended. Thus the old patch 4 has been split into three. The first three patches are unchanged. The following 4 patches are also unchanged. Finally, at the end an extra 2 patches are added to build up the examples by including details of the notation expansions. This updates po/range-doc (2016-07-20) 8 commits. Hopefully my updated workflow will get the right patches to the right people. [1] https://public-inbox.org/git/0648000B273C412AB7140AE959EBC99A%40PhilipOakley/ Philip Oakley (12): doc: use 'symmetric difference' consistently doc: revisions - name the left and right sides doc: show the actual left, right, and boundary marks doc: revisions: give headings for the two and three dot notations doc: revisions: extra clarification of ^! notation effects doc: revisions: single vs multi-parent notation comparison doc: gitrevisions - use 'reachable' in page description doc: gitrevisions - clarify 'latter case' is revision walk doc: revisions - define `reachable` doc: revisions - clarify reachability examples doc: revisions: show revision expansion in examples doc: revisions: sort examples and fix alignment of the unchanged Documentation/gitk.txt | 2 +- Documentation/gitrevisions.txt | 6 +- Documentation/pretty-formats.txt | 2 +- Documentation/rev-list-options.txt | 4 +- Documentation/revisions.txt| 121 +++-- 5 files changed, 84 insertions(+), 51 deletions(-) -- 2.9.0.windows.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/3] diff: add --diff-line-prefix option for passing in a prefix
From: Jacob Keller Add an option to pass additional prefix to be displayed before diff output. This feature will be used in a following patch to output correct --graph prefix when using a child_process/run_command interface for submodules. The prefix shall come first prior to any other prefix associated with the --graph option or other source. Add tests for the same. Signed-off-by: Jacob Keller --- - v5 * Changed name to --diff-line-prefix since --line-prefix might indicate for other commands such as log, when it only modifies diff output Documentation/diff-options.txt | 3 +++ diff.c | 16 ++-- diff.h | 1 + t/t4013-diff-various.sh| 6 + ...diff_--diff-line-prefix=-->_master_master^_side | 29 ++ .../diff.diff_--diff-line-prefix_--cached_--_file0 | 15 +++ 6 files changed, 68 insertions(+), 2 deletions(-) create mode 100644 t/t4013/diff.diff_--diff-line-prefix=-->_master_master^_side create mode 100644 t/t4013/diff.diff_--diff-line-prefix_--cached_--_file0 diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index 705a87394200..f924f57d4f62 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -569,5 +569,8 @@ endif::git-format-patch[] --no-prefix:: Do not show any source or destination prefix. +--diff-line-prefix=:: + Prepend an additional prefix to every line of diff output. + For more detailed explanation on these common options, see also linkgit:gitdiffcore[7]. diff --git a/diff.c b/diff.c index ae069c303077..73dda58c440c 100644 --- a/diff.c +++ b/diff.c @@ -1167,10 +1167,18 @@ const char *diff_get_color(int diff_use_color, enum color_diff ix) const char *diff_line_prefix(struct diff_options *opt) { struct strbuf *msgbuf; - if (!opt->output_prefix) - return ""; + + if (!opt->output_prefix) { + if (opt->line_prefix) + return opt->line_prefix; + else + return ""; + } msgbuf = opt->output_prefix(opt, opt->output_prefix_data); + /* line prefix must be printed before the output_prefix() */ + if (opt->line_prefix) + strbuf_insert(msgbuf, 0, opt->line_prefix, strlen(opt->line_prefix)); return msgbuf->buf; } @@ -3966,6 +3974,10 @@ int diff_opt_parse(struct diff_options *options, options->a_prefix = optarg; return argcount; } + else if ((argcount = parse_long_opt("diff-line-prefix", av, &optarg))) { + options->line_prefix = optarg; + return argcount; + } else if ((argcount = parse_long_opt("dst-prefix", av, &optarg))) { options->b_prefix = optarg; return argcount; diff --git a/diff.h b/diff.h index 49e4aaafb2da..83d0b1ae8580 100644 --- a/diff.h +++ b/diff.h @@ -115,6 +115,7 @@ struct diff_options { const char *pickaxe; const char *single_follow; const char *a_prefix, *b_prefix; + const char *line_prefix; unsigned flags; unsigned touched_flags; diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh index 94ef5000e787..5204645eb92b 100755 --- a/t/t4013-diff-various.sh +++ b/t/t4013-diff-various.sh @@ -306,6 +306,8 @@ diff --no-index --name-status dir2 dir diff --no-index --name-status -- dir2 dir diff --no-index dir dir3 diff master master^ side +# Can't use spaces... +diff --diff-line-prefix=--> master master^ side diff --dirstat master~1 master~2 diff --dirstat initial rearrange diff --dirstat-by-file initial rearrange @@ -325,6 +327,10 @@ test_expect_success 'diff --cached -- file on unborn branch' ' git diff --cached -- file0 >result && test_cmp "$TEST_DIRECTORY/t4013/diff.diff_--cached_--_file0" result ' +test_expect_success 'diff --diff-line-prefix with spaces' ' + git diff --diff-line-prefix="| | | " --cached -- file0 >result && + test_cmp "$TEST_DIRECTORY/t4013/diff.diff_--diff-line-prefix_--cached_--_file0" result +' test_expect_success 'diff-tree --stdin with log formatting' ' cat >expect <<-\EOF && diff --git a/t/t4013/diff.diff_--diff-line-prefix=-->_master_master^_side b/t/t4013/diff.diff_--diff-line-prefix=-->_master_master^_side new file mode 100644 index ..5cc90f27c2d9 --- /dev/null +++ b/t/t4013/diff.diff_--diff-line-prefix=-->_master_master^_side @@ -0,0 +1,29 @@ +$ git diff --diff-line-prefix=--> master master^ side +-->diff --cc dir/sub +-->index cead32e,7289e35..992913c +-->--- a/dir/sub +-->+++ b/dir/sub +-->@@@ -1,6 -1,4 +1,8 @@@ +--> A +--> B +--> +C +--> +D +--> +E +--> +F +-->+ 1 +-->+ 2 +-->diff --cc file0 +-->index b414108,f4615da..10a8a9f +-->--- a/file0 +-->+++ b/file0 +-->@@@ -1,6 -1,6 +1,9 @@@ +--> 1 +--> 2 +--> 3 +--> +4 +
[PATCH v5 3/3] diff: add SUBMODULE_DIFF format to display submodule diff
From: Jacob Keller Teach git-diff and friends a new format for displaying the difference of a submodule using git-diff inside the submodule project. This allows users to easily see exactly what source changed in a given commit that updates the submodule pointer. To do this, remove DIFF_SUBMODULE_LOG bit from the diff options, and instead add a new enum type for these formats. Add some tests for the new format and option. Signed-off-by: Jacob Keller --- Major changes in v5 after adding tests, I will try to give an overview. Unfortunately an interdiff is not easy as I do not remember which reflog entry was v4 and it's been many edits, so I haven't bothered to dig enough yet. - v5 * reword "can be tweaked" text in the diff options * remove unused variables * print the submodule contains untracked or modified header piece * print (new submodule) and (deleted submodule) * handle case where the submodule is not initialized at all, and thus has no .git/modules/xyz, making a diff impossible * handle the git directory better, plus comments explaining * handle modified files better. Untracked unfortunately is more difficult. Suggestions for this welcome. * handle null sha1 correctly (replace with empty tree) I also added tests, mostly for diff output. These should be viewed carefully since it's possible I didn't check the diff output very carefully. Also, I haven't yet added a test for --graph -p mode yet, as I haven't found a suitable place for it to be located. Documentation/diff-config.txt| 3 +- Documentation/diff-options.txt | 7 +- diff.c | 41 +- diff.h | 9 +- submodule.c | 130 + submodule.h | 6 + t/t4059-diff-submodule-option-diff-format.sh | 738 +++ 7 files changed, 915 insertions(+), 19 deletions(-) create mode 100755 t/t4059-diff-submodule-option-diff-format.sh diff --git a/Documentation/diff-config.txt b/Documentation/diff-config.txt index d5a5b17d5088..f5d693afad6c 100644 --- a/Documentation/diff-config.txt +++ b/Documentation/diff-config.txt @@ -123,7 +123,8 @@ diff.suppressBlankEmpty:: diff.submodule:: Specify the format in which differences in submodules are shown. The "log" format lists the commits in the range like - linkgit:git-submodule[1] `summary` does. The "short" format + linkgit:git-submodule[1] `summary` does. The "diff" format shows the + diff between the beginning and end of the range. The "short" format format just shows the names of the commits at the beginning and end of the range. Defaults to short. diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index f924f57d4f62..9a8e86ba1477 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -215,8 +215,11 @@ any of those replacements occurred. the commits in the range like linkgit:git-submodule[1] `summary` does. Omitting the `--submodule` option or specifying `--submodule=short`, uses the 'short' format. This format just shows the names of the commits - at the beginning and end of the range. Can be tweaked via the - `diff.submodule` configuration variable. + at the beginning and end of the range. When `--submodule=diff` is + given, the 'diff' format is used. This format shows the diff between + the old and new submodule commmit from the perspective of the + submodule. Defaults to `diff.submodule` or 'short' if the config + option is unset. --color[=]:: Show colored diff. diff --git a/diff.c b/diff.c index 73dda58c440c..660fdb4cfe46 100644 --- a/diff.c +++ b/diff.c @@ -131,9 +131,11 @@ static int parse_dirstat_params(struct diff_options *options, const char *params static int parse_submodule_params(struct diff_options *options, const char *value) { if (!strcmp(value, "log")) - DIFF_OPT_SET(options, SUBMODULE_LOG); + options->submodule_format = DIFF_SUBMODULE_LOG; + else if (!strcmp(value, "diff")) + options->submodule_format = DIFF_SUBMODULE_DIFF; else if (!strcmp(value, "short")) - DIFF_OPT_CLR(options, SUBMODULE_LOG); + options->submodule_format = DIFF_SUBMODULE_SHORT; else return -1; return 0; @@ -2307,9 +2309,18 @@ static void builtin_diff(const char *name_a, struct strbuf header = STRBUF_INIT; const char *line_prefix = diff_line_prefix(o); - if (DIFF_OPT_TST(o, SUBMODULE_LOG) && - (!one->mode || S_ISGITLINK(one->mode)) && - (!two->mode || S_ISGITLINK(two->mode))) { + diff_set_mnemonic_prefix(o, "a/", "b/"); + if (DIFF_OPT_TST(o, REVERSE_DIFF)) { + a_prefix = o->b_prefix; + b_prefix = o->a_pre
[PATCH v5 1/3] diff.c: remove output_prefix_length field
From: Junio C Hamano "diff/log --stat" has a logic that determines the display columns available for the diffstat part of the output and apportions it for pathnames and diffstat graph automatically. 5e71a84a (Add output_prefix_length to diff_options, 2012-04-16) added the output_prefix_length field to diff_options structure to allow this logic subtract the display columns used for display the history graph part from the total "terminal width"; this matters when the "git log --graph -p" option is in use. The field be set to the number of display columns needed to show the output from the output_prefix() callback. Any new output_prefix() callback must also update the field accordingly, which is error prone. As there is only one user of the field, and the user has the actual value of the prefix string, let's get rid of the field and have the user count the display width itself. Signed-off-by: Junio C Hamano --- This is the same as what Junio submitted already, just thought I'd add it for clarity. diff.c | 2 +- diff.h | 1 - graph.c | 2 -- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/diff.c b/diff.c index b43d3dd2ecb7..ae069c303077 100644 --- a/diff.c +++ b/diff.c @@ -1625,7 +1625,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) */ if (options->stat_width == -1) - width = term_columns() - options->output_prefix_length; + width = term_columns() - strlen(line_prefix); else width = options->stat_width ? options->stat_width : 80; number_width = decimal_width(max_change) > number_width ? diff --git a/diff.h b/diff.h index 125447be09eb..49e4aaafb2da 100644 --- a/diff.h +++ b/diff.h @@ -174,7 +174,6 @@ struct diff_options { diff_format_fn_t format_callback; void *format_callback_data; diff_prefix_fn_t output_prefix; - int output_prefix_length; void *output_prefix_data; int diff_path_counter; diff --git a/graph.c b/graph.c index dd1720148dc5..a46803840511 100644 --- a/graph.c +++ b/graph.c @@ -197,7 +197,6 @@ static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void assert(opt); assert(graph); - opt->output_prefix_length = graph->width; strbuf_reset(&msgbuf); graph_padding_line(graph, &msgbuf); return &msgbuf; @@ -245,7 +244,6 @@ struct git_graph *graph_init(struct rev_info *opt) */ opt->diffopt.output_prefix = diff_output_prefix_callback; opt->diffopt.output_prefix_data = graph; - opt->diffopt.output_prefix_length = 0; return graph; } -- 2.9.2.872.g367ebef.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Issue with global config defaults "user.useConfigOnly = true" + "pull.rebase = preserve" - "user.email"
Earlier, Peff sent this patch (slightly buried in a discussion) on "rebase -i" in <20160729223134.ga22...@sigill.intra.peff.net>. > Subject: rebase-interactive: drop early check for valid ident > > Since the very inception of interactive-rebase in 1b1dce4 > (Teach rebase an interactive mode, 2007-06-25), there has > been a preemptive check, before looking at any commits, to > see whether the user has a valid name/email combination. > > This is convenient, because it means that we abort the > operation before even beginning (rather than just > complaining that we are unable to pick a particular commit). > > However, it does the wrong thing when the rebase does not > actually need to generate any new commits (e.g., a > fast-forward with no commits to pick, or one where the base > stays the same, and we just pick the same commits without > rewriting anything). In this case it may complain about the > lack of ident, even though one would not be needed to > complete the operation. > > This may seem like mere nit-picking, but because interactive > rebase underlies the "preserve-merges" rebase, somebody who > has set "pull.rebase" to "preserve" cannot make even a > fast-forward pull without a valid ident, as we bail before > even realizing the fast-forward nature. > > This commit drops the extra ident check entirely. This means > we rely on individual commands that generate commit objects > to complain. So we will continue to notice and prevent cases > that actually do create commits, but with one important > difference: we fail while actually executing the "pick" > operations, and leave the rebase in a conflicted, half-done > state. > > In some ways this is less convenient, but in some ways it is > more so; the user can then manually commit or even "git > rebase --continue" after setting up their ident (or > providing it as a one-off on the command line). > > Reported-by: Dakota Hawkins > Signed-off-by: Jeff King > --- To which, I responded (referring to the last paragraph): Yup, that is the controvercial bit, and I suspect Dscho's original was siding for the "set up ident first, as you will need it anyway eventually", so I'll let others with viewpoints different from us to chime in first before picking it up. Do you have a preference either way to help us decide if we want to take this change or not? Thanks. > git-rebase--interactive.sh | 3 --- > t/t7517-per-repo-email.sh | 47 > ++ > 2 files changed, 47 insertions(+), 3 deletions(-) > > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > index ded4595..f0f4777 100644 > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh > @@ -1180,9 +1180,6 @@ To continue rebase after editing, run: > ;; > esac > > -git var GIT_COMMITTER_IDENT >/dev/null || > - die "$(gettext "You need to set your committer info first")" > - > comment_for_reflog start > > if test ! -z "$switch_to" > diff --git a/t/t7517-per-repo-email.sh b/t/t7517-per-repo-email.sh > index 337e6e3..2a22fa7 100755 > --- a/t/t7517-per-repo-email.sh > +++ b/t/t7517-per-repo-email.sh > @@ -36,4 +36,51 @@ test_expect_success 'succeeds cloning if global email is > not set' ' > git clone . clone > ' > > +test_expect_success 'set up rebase scenarios' ' > + # temporarily enable an actual ident for this setup > + test_config user.email f...@example.com && > + test_commit new && > + git branch side-without-commit HEAD^ && > + git checkout -b side-with-commit HEAD^ && > + test_commit side > +' > + > +test_expect_success 'fast-forward rebase does not care about ident' ' > + git checkout -B tmp side-without-commit && > + git rebase master > +' > + > +test_expect_success 'non-fast-forward rebase refuses to write commits' ' > + test_when_finished "git rebase --abort || true" && > + git checkout -B tmp side-with-commit && > + test_must_fail git rebase master > +' > + > +test_expect_success 'fast-forward rebase does not care about ident > (interactive)' ' > + git checkout -B tmp side-without-commit && > + git rebase -i master > +' > + > +test_expect_success 'non-fast-forward rebase refuses to write commits > (interactive)' ' > + test_when_finished "git rebase --abort || true" && > + git checkout -B tmp side-with-commit && > + test_must_fail git rebase -i master > +' > + > +test_expect_success 'noop interactive rebase does not care about ident' ' > + git checkout -B tmp side-with-commit && > + git rebase -i HEAD^ > +' > + > +test_expect_success 'fast-forward rebase does not care about ident > (preserve)' ' > + git checkout -B tmp side-without-commit && > + git rebase -p master > +' > + > +test_expect_success 'non-fast-forward rebase refuses to write commits > (preserve)' ' > + test_when_finished "git rebase --abort || true" && > + git checkout -B tmp side-with-commit && > + test_must_fail git rebase -p
Re: public-inbox.org/git updates
Junio C Hamano wrote: > Eric Wong writes: > > > There'll be over 5000K injected messages from 2006 I missed from > > the initial import :x > > > > I noticed this while adding gmane: mapping support to the > > search engine: > > https://public-inbox.org/git/20160811002819.GA8311@starla/T/#u > > > > There will still be some missing messages because some are spam. > > news.gmane.org remains up if you want to check my work > > (please do, because I am careless) > > Thanks for doing this. No problem! > I wanted to try out your NTTP service, but it took me a while to dig > in my inbox to find your announcement of news.public-inbox.org that > hosts inbox.comp.version-control.git "newsgroup". Is it possible to > make this a bit more discoverable, or there is not enough NNTP > audience these days to warrant such an addition? I first went to > http://public-inbox.org/git as I expected there may be some pointers > to other instances of the service, where you list the "git clone" > URL of the archive. Oops, I keep forgetting to do that :x Should be easier to do now since I cleaned up the footer generation a few weeks back. > By the way, it felt quite strange to see messages from 8 years ago > mixed with more recent messages when I gold Gnus to fetch the most > recent 333 messages (and of course that fetches 333 messages with > largest message numbers, not sorted by "Date:"). I am assuming that > this is an artifact of "over 5k injected messages" bundle that was > added out of order. Yes, definitely an artifact of fixing that screwup. For future inbox imports (maybe LKML), I may leave gaps in the sequence along the way, but I'm not sure how big the gaps should or could be. I suppose one could assign NNTP article numbers like line numbers in BASIC... -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Mapping old gmane numbers to existing amil servers?
Philip Oakley wrote: > The raw download works. My home ISP is currently blocking your email for > some reason, though I do see it at my work - my iee.org alias is via my > professional body which duplicates the email when it relays it. Hmm, do other emails from other users get blocked? I wonder if it's lack of DKIM (which gets invalidated by the list sig vger appends) or something else... > On thing I did note on the web display of the threads is that it would be > good to have a leader of " . . . . . `" so that one can count the level of > indent, and see the alignments via the columns of dots. > > When looking at > https://public-inbox.org/git/0648000B273C412AB7140AE959EBC99A%40PhilipOakley/ > I had difficulty working out which email the last 4 were replying to. https://public-inbox.org/git/0648000B273C412AB7140AE959EBC99A%40PhilipOakley/ So, right now, it's: 2016-07-20 21:10 ` [PATCH v4 8/8] doc: revisions - clarify reachability examples Philip Oakley 2016-07-20 22:22 ` Junio C Hamano 2016-08-11 21:50 ` [PATCH v5 00/12] Update git revisions Philip Oakley 2016-08-11 21:50 ` [PATCH v5 01/12] doc: use 'symmetric difference' consistently Philip Oakley 2016-06-25 19:49 ` Junio C Hamano 2016-06-27 13:37 ` Philip Oakley 2016-06-27 15:08 ` Junio C Hamano 2016-06-27 15:39 ` Philip Oakley And you would rather see something like: 2016-07-20 21:10 . ` [PATCH v4 8/8] doc: revisions - clarify reachability examples Philip Oakley 2016-07-20 22:22 . ` Junio C Hamano 2016-08-11 21:50 . ` [PATCH v5 00/12] Update git revisions Philip Oakley 2016-08-11 21:50 . ` [PATCH v5 01/12] doc: use 'symmetric difference' consistently Philip Oakley 2016-06-25 19:49 ` Junio C Hamano 2016-06-27 13:37 ` Philip Oakley 2016-06-27 15:08 ` Junio C Hamano 2016-06-27 15:39 ` Philip Oakley ? Actually, my initial inclination was to use the '|' character which is what mutt does 2016-07-20 21:10 | } [PATCH v4 8/8] doc: revisions - clarify reachability examples Philip Oakley 2016-07-20 22:22 | } Junio C Hamano 2016-08-11 21:50 | ` [PATCH v5 00/12] Update git revisions Philip Oakley 2016-08-11 21:50 | ` [PATCH v5 01/12] doc: use 'symmetric difference' consistently Philip Oakley 2016-06-25 19:49 ` Junio C Hamano 2016-06-27 13:37 ` Philip Oakley 2016-06-27 15:08 ` Junio C Hamano 2016-06-27 15:39 ` Philip Oakley It should be doable, and the '`' immediately following the last '|' probably ought to be a link to the parent, too. I would also like to use '}' (as above) or '+' where mutt would use "├─>" or just '├', but I don't think I can introduce multibyte chars without causing problems for some users. Anyways, it's definitely on my ever-growing todo list. (Wow, it is refreshing to be a "web designer" who can live entirely in a terminal without relying on screenshots :D) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Aug 2016, #04; Thu, 11)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. The ones marked with '.' do not appear in any of the integration branches, but I am still holding onto them. The 'maint' branch has been accumulating enough material to make it the next maintenance release v2.9.3, which hopefully will happen very soon. 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 -- [Graduated to "master"] * cc/mailmap-tuxfamily (2016-08-08) 1 commit (merged to 'next' on 2016-08-10 at 5905fbf) + .mailmap: use Christian Couder's Tuxfamily address * jk/completion-diff-submodule (2016-08-09) 1 commit (merged to 'next' on 2016-08-10 at 146ca11) + completion: add completion for --submodule=* diff option * jk/push-force-with-lease-creation (2016-08-04) 4 commits (merged to 'next' on 2016-08-04 at e42ce85) + t5533: make it pass on case-sensitive filesystems (merged to 'next' on 2016-08-03 at 475c080) + push: allow pushing new branches with --force-with-lease + push: add shorthand for --force-with-lease branch creation + Documentation/git-push: fix placeholder formatting "git push --force-with-lease" already had enough logic to allow ensuring that such a push results in creation of a ref (i.e. the receiving end did not have another push from sideways that would be discarded by our force-pushing), but didn't expose this possibility to the users. It does so now. * jk/reset-ident-time-per-commit (2016-08-01) 1 commit (merged to 'next' on 2016-08-03 at 76d569c) + am: reset cached ident date for each patch Not-so-recent rewrite of "git am" that started making internal calls into the commit machinery had an unintended regression, in that no matter how many seconds it took to apply many patches, the resulting committer timestamp for the resulting commits were all the same. * js/am-3-merge-recursive-direct (2016-08-01) 16 commits (merged to 'next' on 2016-08-05 at dc1c9bb) + merge-recursive: flush output buffer even when erroring out + merge_trees(): ensure that the callers release output buffer + merge-recursive: offer an option to retain the output in 'obuf' + merge-recursive: write the commit title in one go + merge-recursive: flush output buffer before printing error messages + am -3: use merge_recursive() directly again + merge-recursive: switch to returning errors instead of dying + merge-recursive: handle return values indicating errors + merge-recursive: allow write_tree_from_memory() to error out + merge-recursive: avoid returning a wholesale struct + merge_recursive: abort properly upon errors + prepare the builtins for a libified merge_recursive() + merge-recursive: clarify code in was_tracked() + die(_("BUG")): avoid translating bug messages + die("bug"): report bugs consistently + t5520: verify that `pull --rebase` shows the helpful advice when failing "git am -3" calls "git merge-recursive" when it needs to fall back to a three-way merge; this call has been turned into an internal subroutine call instead of spawning a separate subprocess. * js/commit-slab-decl-fix (2016-08-09) 2 commits (merged to 'next' on 2016-08-10 at 6675402) + commit-slab.h: avoid duplicated global static variables + config.c: avoid duplicated global static variables * jt/format-patch-from-config (2016-08-01) 1 commit (merged to 'next' on 2016-08-05 at 897e986) + format-patch: format.from gives the default for --from "git format-patch" learned format.from configuration variable to specify the default settings for its "--from" option. * sb/submodule-update-dot-branch (2016-08-10) 8 commits (merged to 'next' on 2016-08-10 at 40ba945) + t7406: fix breakage on OSX (merged to 'next' on 2016-08-04 at 47bff41) + submodule update: allow '.' for branch value + submodule--helper: add remote-branch helper + submodule-config: keep configured branch around + submodule--helper: fix usage string for relative-path + submodule update: narrow scope of local variable + submodule update: respect depth in subsequent fetches + t7406: future proof tests with hard coded depth A few updates to "git submodule update". -- [New Topics] * sb/submodule-clone-retry (2016-08-09) 1 commit (merged to 'next' on 2016-08-11 at 4600b20) + submodule--helper: use parallel processor correctly Fix-up to an error codepath in a topic already in 'master'. Will merge to 'master'. * vs/completion-branch-fully-spelled-d-m-r (2016-08-09) 1 commit (merged to 'next' on 2016-08-11 at a6f189c) + completion: complete --delete, --move, and --remotes for git branch Will merge to 'master'. * vs/typofix (2016-08-11) 1 commit (merged to 'next' on 2016-08-11 at 435c418) + Spelling fixes Will merge to
[PATCH v5 01/12] doc: use 'symmetric difference' consistently
Signed-off-by: Philip Oakley --- unchanged --- Documentation/gitk.txt | 2 +- Documentation/rev-list-options.txt | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Documentation/gitk.txt b/Documentation/gitk.txt index 6ade002..6c3eb15 100644 --- a/Documentation/gitk.txt +++ b/Documentation/gitk.txt @@ -70,7 +70,7 @@ linkgit:git-rev-list[1] for a complete list. --left-right:: - Mark which side of a symmetric diff a commit is reachable + Mark which side of a symmetric difference a commit is reachable from. Commits from the left side are prefixed with a `<` symbol and those from the right with a `>` symbol. diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt index 4f009d4..6dc0bb0 100644 --- a/Documentation/rev-list-options.txt +++ b/Documentation/rev-list-options.txt @@ -225,7 +225,7 @@ excluded from the output. --left-only:: --right-only:: - List only commits on the respective side of a symmetric range, + List only commits on the respective side of a symmetric difference, i.e. only those which would be marked `<` resp. `>` by `--left-right`. + @@ -766,7 +766,7 @@ ifdef::git-rev-list[] endif::git-rev-list[] --left-right:: - Mark which side of a symmetric diff a commit is reachable from. + Mark which side of a symmetric difference a commit is reachable from. Commits from the left side are prefixed with `<` and those from the right with `>`. If combined with `--boundary`, those commits are prefixed with `-`. -- 2.9.0.windows.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 00/12] Update git revisions
This has grown like topsy from a little two patch series that tried to name the 2-dots notation [1] into this extended set of tweaks. As documentation can be rather personal, I've split out each small change so each can be individually justified. Since V4, I've confirmed that the format breaking issue is that we cannot quote code in headings in the man page layout - ultimately it's a docbook decision, and follows the line of analysis Peff identified (see commentary in the patch). In addition the multi-parent notations have been clarified and extended. Thus the old patch 4 has been split into three. The first three patches are unchanged. The following 4 patches are also unchanged. Finally, at the end an extra 2 patches are added to build up the examples by including details of the notation expansions. This updates po/range-doc (2016-07-20) 8 commits. Hopefully my updated workflow will get the right patches to the right people. [1] https://public-inbox.org/git/0648000B273C412AB7140AE959EBC99A%40PhilipOakley/ Philip Oakley (12): doc: use 'symmetric difference' consistently doc: revisions - name the left and right sides doc: show the actual left, right, and boundary marks doc: revisions: give headings for the two and three dot notations doc: revisions: extra clarification of ^! notation effects doc: revisions: single vs multi-parent notation comparison doc: gitrevisions - use 'reachable' in page description doc: gitrevisions - clarify 'latter case' is revision walk doc: revisions - define `reachable` doc: revisions - clarify reachability examples doc: revisions: show revision expansion in examples doc: revisions: sort examples and fix alignment of the unchanged Documentation/gitk.txt | 2 +- Documentation/gitrevisions.txt | 6 +- Documentation/pretty-formats.txt | 2 +- Documentation/rev-list-options.txt | 4 +- Documentation/revisions.txt| 121 +++-- 5 files changed, 84 insertions(+), 51 deletions(-) -- 2.9.0.windows.1 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Mapping old gmane numbers to existing amil servers?
From: "Eric Wong" Sent: Thursday, August 11, 2016 8:56 AM Philip Oakley wrote: Is there an accessible mapping from the old gmane message numbers to one of the remaining email list servers for the git list? Yes, I just posted about this after posting an initial mapping a few weeks ago: https://public-inbox.org/git/20160811002819.GA8311@starla/T/#u Using "gmane:$NUM" in the search bar should work, now. (there's a few missing messages which might've been due to off-by-one errors, will fill them in soon) I've seen discussions about the public-inbox, but no mention of any mapping for old message references. I had a gzipped text file published originally if you look upthread there. Thanks. The raw download works. My home ISP is currently blocking your email for some reason, though I do see it at my work - my iee.org alias is via my professional body which duplicates the email when it relays it. On thing I did note on the web display of the threads is that it would be good to have a leader of " . . . . . `" so that one can count the level of indent, and see the alignments via the columns of dots. When looking at https://public-inbox.org/git/0648000B273C412AB7140AE959EBC99A%40PhilipOakley/ I had difficulty working out which email the last 4 were replying to. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v7 0/9] status: V2 porcelain status
Jeff Hostetler writes: > From: Jeff Hostetler > > This patch series adds porcelain V2 format to status. > This provides detailed information about file changes > and about the current branch. > > The new output is accessed via: > git status --porcelain=v2 [--branch] > > This v7 patch series address the most recent feedback > on the unit tests. > > This series has been rebased onto a more current master > branch (to get the EMPTY_BLOB changes in the test suite). Thanks for a note. I've been queuing the series on top of f8f7adc, so I am assuming I won't see any changes in 1-8. Unfortunately I cannot be taking new rerolls or new topics for the rest of the day as I need to start today's integration cycle and I'll have to delay taking a look at this round, but from a quick glance you still seem to have a lot of dirt in the new test and I cannot quite tell if they are intended. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 6/9] status: print branch info with --porcelain=v2 --branch
From: Jeff Hostetler Expand porcelain v2 output to include branch and tracking branch information. This includes the commit id, the branch, the upstream branch, and the ahead and behind counts. Signed-off-by: Jeff Hostetler --- builtin/commit.c | 5 wt-status.c | 90 wt-status.h | 1 + 3 files changed, 96 insertions(+) diff --git a/builtin/commit.c b/builtin/commit.c index c50faf9..bb9f79b 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -510,6 +510,8 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int s->fp = fp; s->nowarn = nowarn; s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0; + if (!s->is_initial) + hashcpy(s->sha1_commit, sha1); s->status_format = status_format; s->ignore_submodule_arg = ignore_submodule_arg; @@ -1378,6 +1380,9 @@ int cmd_status(int argc, const char **argv, const char *prefix) fd = hold_locked_index(&index_lock, 0); s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0; + if (!s.is_initial) + hashcpy(s.sha1_commit, sha1); + s.ignore_submodule_arg = ignore_submodule_arg; s.status_format = status_format; s.verbose = verbose; diff --git a/wt-status.c b/wt-status.c index 163b453..539aac1 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1816,6 +1816,92 @@ static void wt_porcelain_print(struct wt_status *s) } /* + * Print branch information for porcelain v2 output. These lines + * are printed when the '--branch' parameter is given. + * + *# branch.oid + *# branch.head + * [# branch.upstream + * [# branch.ab + -]] + * + * ::= the current commit hash or the the literal + * "(initial)" to indicate an initialized repo + * with no commits. + * + * ::= the current branch name or + * "(detached)" literal when detached head or + * "(unknown)" when something is wrong. + * + * ::= the upstream branch name, when set. + * + *::= integer ahead value, when upstream set + * and the commit is present (not gone). + * + * ::= integer behind value, when upstream set + * and commit is present. + * + * + * The end-of-line is defined by the -z flag. + * + * ::= NUL when -z, + * LF when NOT -z. + * + */ +static void wt_porcelain_v2_print_tracking(struct wt_status *s) +{ + struct branch *branch; + const char *base; + const char *branch_name; + struct wt_status_state state; + int ab_info, nr_ahead, nr_behind; + char eol = s->null_termination ? '\0' : '\n'; + + memset(&state, 0, sizeof(state)); + wt_status_get_state(&state, s->branch && !strcmp(s->branch, "HEAD")); + + fprintf(s->fp, "# branch.oid %s%c", + (s->is_initial ? "(initial)" : sha1_to_hex(s->sha1_commit)), + eol); + + if (!s->branch) + fprintf(s->fp, "# branch.head %s%c", "(unknown)", eol); + else { + if (!strcmp(s->branch, "HEAD")) { + fprintf(s->fp, "# branch.head %s%c", "(detached)", eol); + + if (state.rebase_in_progress || state.rebase_interactive_in_progress) + branch_name = state.onto; + else if (state.detached_from) + branch_name = state.detached_from; + else + branch_name = ""; + } else { + branch_name = NULL; + skip_prefix(s->branch, "refs/heads/", &branch_name); + + fprintf(s->fp, "# branch.head %s%c", branch_name, eol); + } + + /* Lookup stats on the upstream tracking branch, if set. */ + branch = branch_get(branch_name); + base = NULL; + ab_info = (stat_tracking_info(branch, &nr_ahead, &nr_behind, &base) == 0); + if (base) { + base = shorten_unambiguous_ref(base, 0); + fprintf(s->fp, "# branch.upstream %s%c", base, eol); + free((char *)base); + + if (ab_info) + fprintf(s->fp, "# branch.ab +%d -%d%c", nr_ahead, nr_behind, eol); + } + } + + free(state.branch); + free(state.onto); + free(state.detached_from); +} + +/* * Convert various submodule status values into a * fixed-length string of characters in the buffer provided. */ @@ -2061,6 +2147,7 @@ static void wt_porcelain_v2_print_other( /* * Print porcelain V2 status. * + * [] * []* * []* * []* @@ -2073,6 +2160,9 @@ static void wt_porcelain_v2_print(struct wt_status *s) struct string_list_item *it;
[PATCH v7 5/9] status: print per-file porcelain v2 status data
From: Jeff Hostetler Print per-file information in porcelain v2 format. Signed-off-by: Jeff Hostetler --- wt-status.c | 285 +++- 1 file changed, 284 insertions(+), 1 deletion(-) diff --git a/wt-status.c b/wt-status.c index aa804b5..163b453 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1815,6 +1815,289 @@ static void wt_porcelain_print(struct wt_status *s) wt_shortstatus_print(s); } +/* + * Convert various submodule status values into a + * fixed-length string of characters in the buffer provided. + */ +static void wt_porcelain_v2_submodule_state( + struct wt_status_change_data *d, + char sub[5]) +{ + if (S_ISGITLINK(d->mode_head) || + S_ISGITLINK(d->mode_index) || + S_ISGITLINK(d->mode_worktree)) { + sub[0] = 'S'; + sub[1] = d->new_submodule_commits ? 'C' : '.'; + sub[2] = (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED) ? 'M' : '.'; + sub[3] = (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) ? 'U' : '.'; + } else { + sub[0] = 'N'; + sub[1] = '.'; + sub[2] = '.'; + sub[3] = '.'; + } + sub[4] = 0; +} + +/* + * Fix-up changed entries before we print them. + */ +static void wt_porcelain_v2_fix_up_changed( + struct string_list_item *it, + struct wt_status *s) +{ + struct wt_status_change_data *d = it->util; + + if (!d->index_status) { + /* +* This entry is unchanged in the index (relative to the head). +* Therefore, the collect_updated_cb was never called for this +* entry (during the head-vs-index scan) and so the head column +* fields were never set. +* +* We must have data for the index column (from the +* index-vs-worktree scan (otherwise, this entry should not be +* in the list of changes)). +* +* Copy index column fields to the head column, so that our +* output looks complete. +*/ + assert(d->mode_head == 0); + d->mode_head = d->mode_index; + oidcpy(&d->oid_head, &d->oid_index); + } + + if (!d->worktree_status) { + /* +* This entry is unchanged in the worktree (relative to the index). +* Therefore, the collect_changed_cb was never called for this entry +* (during the index-vs-worktree scan) and so the worktree column +* fields were never set. +* +* We must have data for the index column (from the head-vs-index +* scan). +* +* Copy the index column fields to the worktree column so that +* our output looks complete. +* +* Note that we only have a mode field in the worktree column +* because the scan code tries really hard to not have to compute it. +*/ + assert(d->mode_worktree == 0); + d->mode_worktree = d->mode_index; + } +} + +/* + * Print porcelain v2 info for tracked entries with changes. + */ +static void wt_porcelain_v2_print_changed_entry( + struct string_list_item *it, + struct wt_status *s) +{ + struct wt_status_change_data *d = it->util; + struct strbuf buf_index = STRBUF_INIT; + struct strbuf buf_head = STRBUF_INIT; + const char *path_index = NULL; + const char *path_head = NULL; + char key[3]; + char submodule_token[5]; + char sep_char, eol_char; + + wt_porcelain_v2_fix_up_changed(it, s); + wt_porcelain_v2_submodule_state(d, submodule_token); + + key[0] = d->index_status ? d->index_status : '.'; + key[1] = d->worktree_status ? d->worktree_status : '.'; + key[2] = 0; + + if (s->null_termination) { + /* +* In -z mode, we DO NOT C-quote pathnames. Current path is ALWAYS first. +* A single NUL character separates them. +*/ + sep_char = '\0'; + eol_char = '\0'; + path_index = it->string; + path_head = d->head_path; + } else { + /* +* Path(s) are C-quoted if necessary. Current path is ALWAYS first. +* The source path is only present when necessary. +* A single TAB separates them (because paths can contain spaces +* which are not escaped and C-quoting does escape TAB characters). +*/ + sep_char = '\t'; + eol_char = '\n'; + path_index = quote_path(it->string, s->prefix, &buf_index); + if (d->head_path) + path_head
Re: [PATCH v4 1/2] diff: add --line-prefix option for passing in a prefix
Subject: SQUASH??? clarify the if/{if/else} nesting Otherwise GCC helpfully warns you. Signed-off-by: Junio C Hamano --- diff.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/diff.c b/diff.c index bfc0a6b..d128b9d 100644 --- a/diff.c +++ b/diff.c @@ -1170,11 +1170,12 @@ const char *diff_line_prefix(struct diff_options *opt) { struct strbuf *msgbuf; - if (!opt->output_prefix) + if (!opt->output_prefix) { if (opt->line_prefix) return opt->line_prefix; else return ""; + } msgbuf = opt->output_prefix(opt, opt->output_prefix_data); /* line prefix must be printed before the output_prefix() */ -- 2.9.2-863-g71ed253 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 3/9] status: support --porcelain[=]
From: Jeff Hostetler Update --porcelain argument to take optional version parameter to allow multiple porcelain formats to be supported in the future. The token "v1" is the default value and indicates the traditional porcelain format. (The token "1" is an alias for that.) Signed-off-by: Jeff Hostetler --- Documentation/git-status.txt | 7 +-- builtin/commit.c | 21 ++--- t/t7060-wtstatus.sh | 21 + 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt index e1e8f57..6b1454b 100644 --- a/Documentation/git-status.txt +++ b/Documentation/git-status.txt @@ -32,11 +32,14 @@ OPTIONS --branch:: Show the branch and tracking info even in short-format. ---porcelain:: +--porcelain[=]:: Give the output in an easy-to-parse format for scripts. This is similar to the short output, but will remain stable across Git versions and regardless of user configuration. See below for details. ++ +The version parameter is used to specify the format version. +This is optional and defaults to the original version 'v1' format. --long:: Give the output in the long-format. This is the default. @@ -96,7 +99,7 @@ configuration variable documented in linkgit:git-config[1]. -z:: Terminate entries with NUL, instead of LF. This implies - the `--porcelain` output format if no other format is given. + the `--porcelain=v1` output format if no other format is given. --column[=]:: --no-column:: diff --git a/builtin/commit.c b/builtin/commit.c index c5e0173..c9d24d5 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -144,6 +144,21 @@ static struct strbuf message = STRBUF_INIT; static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED; +static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset) +{ + enum wt_status_format *value = (enum wt_status_format *)opt->value; + if (unset) + *value = STATUS_FORMAT_NONE; + else if (!arg) + *value = STATUS_FORMAT_PORCELAIN; + else if (!strcmp(arg, "v1") || !strcmp(arg, "1")) + *value = STATUS_FORMAT_PORCELAIN; + else + die("unsupported porcelain version '%s'", arg); + + return 0; +} + static int opt_parse_m(const struct option *opt, const char *arg, int unset) { struct strbuf *buf = opt->value; @@ -1316,9 +1331,9 @@ int cmd_status(int argc, const char **argv, const char *prefix) N_("show status concisely"), STATUS_FORMAT_SHORT), OPT_BOOL('b', "branch", &s.show_branch, N_("show branch information")), - OPT_SET_INT(0, "porcelain", &status_format, - N_("machine-readable output"), - STATUS_FORMAT_PORCELAIN), + { OPTION_CALLBACK, 0, "porcelain", &status_format, + N_("version"), N_("machine-readable output"), + PARSE_OPT_OPTARG, opt_parse_porcelain }, OPT_SET_INT(0, "long", &status_format, N_("show status in long format (default)"), STATUS_FORMAT_LONG), diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh index 4d17363..53cf42f 100755 --- a/t/t7060-wtstatus.sh +++ b/t/t7060-wtstatus.sh @@ -232,4 +232,25 @@ test_expect_success 'status --branch with detached HEAD' ' test_i18ncmp expected actual ' +## Duplicate the above test and verify --porcelain=v1 arg parsing. +test_expect_success 'status --porcelain=v1 --branch with detached HEAD' ' + git reset --hard && + git checkout master^0 && + git status --branch --porcelain=v1 >actual && + cat >expected <<-EOF && + ## HEAD (no branch) + ?? .gitconfig + ?? actual + ?? expect + ?? expected + ?? mdconflict/ + EOF + test_i18ncmp expected actual +' + +## Verify parser error on invalid --porcelain argument. +test_expect_success 'status --porcelain=bogus' ' + test_must_fail git status --porcelain=bogus +' + test_done -- 2.8.0.rc4.17.gac42084.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 1/9] status: rename long-format print routines
From: Jeff Hostetler Rename the various wt_status_print*() routines to be wt_longstatus_print*() to make it clear that these routines are only concerned with the normal/long status output and reduce developer confusion as other status formats are added in the future. Signed-off-by: Jeff Hostetler --- builtin/commit.c | 4 +- wt-status.c | 110 +++ wt-status.h | 2 +- 3 files changed, 58 insertions(+), 58 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 77e3dc8..ecfc965 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -515,7 +515,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int break; case STATUS_FORMAT_NONE: case STATUS_FORMAT_LONG: - wt_status_print(s); + wt_longstatus_print(s); break; } @@ -1403,7 +1403,7 @@ int cmd_status(int argc, const char **argv, const char *prefix) case STATUS_FORMAT_LONG: s.verbose = verbose; s.ignore_submodule_arg = ignore_submodule_arg; - wt_status_print(&s); + wt_longstatus_print(&s); break; } return 0; diff --git a/wt-status.c b/wt-status.c index 6225a2d..bae9507 100644 --- a/wt-status.c +++ b/wt-status.c @@ -139,7 +139,7 @@ void wt_status_prepare(struct wt_status *s) s->display_comment_prefix = 0; } -static void wt_status_print_unmerged_header(struct wt_status *s) +static void wt_longstatus_print_unmerged_header(struct wt_status *s) { int i; int del_mod_conflict = 0; @@ -191,7 +191,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s) status_printf_ln(s, c, "%s", ""); } -static void wt_status_print_cached_header(struct wt_status *s) +static void wt_longstatus_print_cached_header(struct wt_status *s) { const char *c = color(WT_STATUS_HEADER, s); @@ -207,9 +207,9 @@ static void wt_status_print_cached_header(struct wt_status *s) status_printf_ln(s, c, "%s", ""); } -static void wt_status_print_dirty_header(struct wt_status *s, -int has_deleted, -int has_dirty_submodules) +static void wt_longstatus_print_dirty_header(struct wt_status *s, +int has_deleted, +int has_dirty_submodules) { const char *c = color(WT_STATUS_HEADER, s); @@ -226,9 +226,9 @@ static void wt_status_print_dirty_header(struct wt_status *s, status_printf_ln(s, c, "%s", ""); } -static void wt_status_print_other_header(struct wt_status *s, -const char *what, -const char *how) +static void wt_longstatus_print_other_header(struct wt_status *s, +const char *what, +const char *how) { const char *c = color(WT_STATUS_HEADER, s); status_printf_ln(s, c, "%s:", what); @@ -238,7 +238,7 @@ static void wt_status_print_other_header(struct wt_status *s, status_printf_ln(s, c, "%s", ""); } -static void wt_status_print_trailer(struct wt_status *s) +static void wt_longstatus_print_trailer(struct wt_status *s) { status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", ""); } @@ -304,8 +304,8 @@ static int maxwidth(const char *(*label)(int), int minval, int maxval) return result; } -static void wt_status_print_unmerged_data(struct wt_status *s, - struct string_list_item *it) +static void wt_longstatus_print_unmerged_data(struct wt_status *s, + struct string_list_item *it) { const char *c = color(WT_STATUS_UNMERGED, s); struct wt_status_change_data *d = it->util; @@ -331,9 +331,9 @@ static void wt_status_print_unmerged_data(struct wt_status *s, strbuf_release(&onebuf); } -static void wt_status_print_change_data(struct wt_status *s, - int change_type, - struct string_list_item *it) +static void wt_longstatus_print_change_data(struct wt_status *s, + int change_type, + struct string_list_item *it) { struct wt_status_change_data *d = it->util; const char *c = color(change_type, s); @@ -378,7 +378,7 @@ static void wt_status_print_change_data(struct wt_status *s, status = d->worktree_status; break; default: - die("BUG: unhandled change_type %d in wt_status_print_change_data", + die("BUG: unhandled change_type %d in wt_longstatus_print_change_data", change_type); } @@
Re: [PATCH v6 9/9] status: unit tests for --porcelain=v2
On 08/11/2016 02:36 PM, Junio C Hamano wrote: Jeff Hostetler writes: From: Jeff Hostetler +. ./test-lib.sh + +OID_EMPTY=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 It seems that test-lib.sh these days has EMPTY_BLOB defined for your use. You can remove this and replace its use (just two lines) with $EMPTY_BLOB down in the "add -N" test. That's a recent addition to test-lib.sh. I'll rebase my series onto a more recent master to get that. +test_expect_success setup ' + test_tick && + git config --local core.autocrlf false && I'd suggest removing "--local". Existing use of "git config" in the test suite, unless their use is about testing "git config" itself to validate the operation of the --local/--global/--system options, do not seem to explicitly say "--local", which is the default anyway. ok. just being cautious, but if that's the convention, that's fine. +test_expect_success 'after first commit, make dirt, confirm unstaged changes' ' Did you mean s/dirt/dirty/? "make and confirm unstaged changes" would be sufficient. Because "confirming" is implicit (as these are all tests), "after the first commit, modify working tree files" might even be better, perhaps? + echo x >>file_x && + OID_X1=$(git hash-object -t blob -- file_x) && + rm file_z && + H0=$(git rev-parse HEAD) && + ... got it. +test_expect_success 'after first commit, stage dirt, confirm staged changes' ' What you "git add" is meant to be good changes, so they are no longer dirt ;-) More importantly, because I never heard of "dirt" used in Git context, it is unclear if it is an untracked file, a modification that is not meant to be committed immediately, or what. "after the first commit, fully add changes to the index"? reworded and simplified. + git add file_x && + git rm file_z && + H0=$(git rev-parse HEAD) && + + cat >expect <<-EOF && + # branch.oid $H0 + # branch.head master + 1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x + 1 D. N... 100644 00 00 $OID_Z $_z40 file_z + ? actual + ? expect + EOF +test_expect_success 'after first commit, also stage rename, confirm 2 path line format' ' + git mv file_y renamed_y && + H0=$(git rev-parse HEAD) && + + q_to_tab >expect <<-EOF && + # branch.oid $H0 + # branch.head master + 1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x + 1 D. N... 100644 00 00 $OID_Z $_z40 file_z + 2 R. N... 100644 100644 100644 $OID_Y $OID_Y R100 renamed_yQfile_y + ? actual + ? expect + EOF + + git status --porcelain=v2 --branch --untracked-files=all >actual && + test_cmp expect actual +' Do we want to test -z format on this, too? Sure. I'll send these up in a v7 series in a few minutes. Thanks Jeff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 2/9] status: cleanup API to wt_status_print
From: Jeff Hostetler Refactor the API between builtin/commit.c and wt-status.[ch]. Hide the details of the various wt_*status_print() routines inside wt-status.c behind a single (new) wt_status_print() routine. Eliminate the switch statements from builtin/commit.c. Allow details of new status formats to be isolated within wt-status.c Signed-off-by: Jeff Hostetler --- builtin/commit.c | 51 +-- wt-status.c | 25 ++--- wt-status.h | 16 3 files changed, 43 insertions(+), 49 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index ecfc965..c5e0173 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -142,14 +142,7 @@ static int show_ignored_in_status, have_option_m; static const char *only_include_assumed; static struct strbuf message = STRBUF_INIT; -static enum status_format { - STATUS_FORMAT_NONE = 0, - STATUS_FORMAT_LONG, - STATUS_FORMAT_SHORT, - STATUS_FORMAT_PORCELAIN, - - STATUS_FORMAT_UNSPECIFIED -} status_format = STATUS_FORMAT_UNSPECIFIED; +static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED; static int opt_parse_m(const struct option *opt, const char *arg, int unset) { @@ -500,24 +493,11 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int s->fp = fp; s->nowarn = nowarn; s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0; + s->status_format = status_format; + s->ignore_submodule_arg = ignore_submodule_arg; wt_status_collect(s); - - switch (status_format) { - case STATUS_FORMAT_SHORT: - wt_shortstatus_print(s); - break; - case STATUS_FORMAT_PORCELAIN: - wt_porcelain_print(s); - break; - case STATUS_FORMAT_UNSPECIFIED: - die("BUG: finalize_deferred_config() should have been called"); - break; - case STATUS_FORMAT_NONE: - case STATUS_FORMAT_LONG: - wt_longstatus_print(s); - break; - } + wt_status_print(s); return s->commitable; } @@ -1099,7 +1079,7 @@ static const char *read_commit_message(const char *name) * is not in effect here. */ static struct status_deferred_config { - enum status_format status_format; + enum wt_status_format status_format; int show_branch; } status_deferred_config = { STATUS_FORMAT_UNSPECIFIED, @@ -1381,6 +1361,9 @@ int cmd_status(int argc, const char **argv, const char *prefix) s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0; s.ignore_submodule_arg = ignore_submodule_arg; + s.status_format = status_format; + s.verbose = verbose; + wt_status_collect(&s); if (0 <= fd) @@ -1389,23 +1372,7 @@ int cmd_status(int argc, const char **argv, const char *prefix) if (s.relative_paths) s.prefix = prefix; - switch (status_format) { - case STATUS_FORMAT_SHORT: - wt_shortstatus_print(&s); - break; - case STATUS_FORMAT_PORCELAIN: - wt_porcelain_print(&s); - break; - case STATUS_FORMAT_UNSPECIFIED: - die("BUG: finalize_deferred_config() should have been called"); - break; - case STATUS_FORMAT_NONE: - case STATUS_FORMAT_LONG: - s.verbose = verbose; - s.ignore_submodule_arg = ignore_submodule_arg; - wt_longstatus_print(&s); - break; - } + wt_status_print(&s); return 0; } diff --git a/wt-status.c b/wt-status.c index bae9507..59bfb0b 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1450,7 +1450,7 @@ static void wt_longstatus_print_state(struct wt_status *s, show_bisect_in_progress(s, state, state_color); } -void wt_longstatus_print(struct wt_status *s) +static void wt_longstatus_print(struct wt_status *s) { const char *branch_color = color(WT_STATUS_ONBRANCH, s); const char *branch_status_color = color(WT_STATUS_HEADER, s); @@ -1717,7 +1717,7 @@ static void wt_shortstatus_print_tracking(struct wt_status *s) fputc(s->null_termination ? '\0' : '\n', s->fp); } -void wt_shortstatus_print(struct wt_status *s) +static void wt_shortstatus_print(struct wt_status *s) { int i; @@ -1749,7 +1749,7 @@ void wt_shortstatus_print(struct wt_status *s) } } -void wt_porcelain_print(struct wt_status *s) +static void wt_porcelain_print(struct wt_status *s) { s->use_color = 0; s->relative_paths = 0; @@ -1757,3 +1757,22 @@ void wt_porcelain_print(struct wt_status *s) s->no_gettext = 1; wt_shortstatus_print(s); } + +void wt_status_print(struct wt_status *s) +{ + switch (s->status_format) { + case STATUS_FORMAT_SHORT: + wt_shortstatus_print(s); + break; + ca
[PATCH v7 9/9] status: unit tests for --porcelain=v2
From: Jeff Hostetler Test porcelain v2 status format. Signed-off-by: Jeff Hostetler --- t/t7064-wtstatus-pv2.sh | 592 1 file changed, 592 insertions(+) create mode 100755 t/t7064-wtstatus-pv2.sh diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh new file mode 100755 index 000..34d25b5 --- /dev/null +++ b/t/t7064-wtstatus-pv2.sh @@ -0,0 +1,592 @@ +#!/bin/sh + +test_description='git status --porcelain=v2 + +This test exercises porcelain V2 output for git status.' + + +. ./test-lib.sh + + +test_expect_success setup ' + test_tick && + git config core.autocrlf false && + echo x >file_x && + echo y >file_y && + echo z >file_z && + mkdir dir1 && + echo a >dir1/file_a && + echo b >dir1/file_b +' + +test_expect_success 'before initial commit, nothing added, only untracked' ' + cat >expect <<-EOF && + # branch.oid (initial) + # branch.head master + ? actual + ? dir1/ + ? expect + ? file_x + ? file_y + ? file_z + EOF + + git status --porcelain=v2 --branch --untracked-files=normal >actual && + test_cmp expect actual +' + +test_expect_success 'before initial commit, things added' ' + git add file_x file_y file_z dir1 && + OID_A=$(git hash-object -t blob -- dir1/file_a) && + OID_B=$(git hash-object -t blob -- dir1/file_b) && + OID_X=$(git hash-object -t blob -- file_x) && + OID_Y=$(git hash-object -t blob -- file_y) && + OID_Z=$(git hash-object -t blob -- file_z) && + + cat >expect <<-EOF && + # branch.oid (initial) + # branch.head master + 1 A. N... 00 100644 100644 $_z40 $OID_A dir1/file_a + 1 A. N... 00 100644 100644 $_z40 $OID_B dir1/file_b + 1 A. N... 00 100644 100644 $_z40 $OID_X file_x + 1 A. N... 00 100644 100644 $_z40 $OID_Y file_y + 1 A. N... 00 100644 100644 $_z40 $OID_Z file_z + ? actual + ? expect + EOF + + git status --porcelain=v2 --branch --untracked-files=all >actual && + test_cmp expect actual +' + +test_expect_success 'before initial commit, things added (-z)' ' + lf_to_nul >expect <<-EOF && + # branch.oid (initial) + # branch.head master + 1 A. N... 00 100644 100644 $_z40 $OID_A dir1/file_a + 1 A. N... 00 100644 100644 $_z40 $OID_B dir1/file_b + 1 A. N... 00 100644 100644 $_z40 $OID_X file_x + 1 A. N... 00 100644 100644 $_z40 $OID_Y file_y + 1 A. N... 00 100644 100644 $_z40 $OID_Z file_z + ? actual + ? expect + EOF + + git status -z --porcelain=v2 --branch --untracked-files=all >actual && + test_cmp expect actual +' + +test_expect_success 'make first commit, comfirm HEAD oid and branch' ' + git commit -m initial && + H0=$(git rev-parse HEAD) && + cat >expect <<-EOF && + # branch.oid $H0 + # branch.head master + ? actual + ? expect + EOF + + git status --porcelain=v2 --branch --untracked-files=all >actual && + test_cmp expect actual +' + +test_expect_success 'after first commit, create unstaged changes' ' + echo x >>file_x && + OID_X1=$(git hash-object -t blob -- file_x) && + rm file_z && + H0=$(git rev-parse HEAD) && + + cat >expect <<-EOF && + # branch.oid $H0 + # branch.head master + 1 .M N... 100644 100644 100644 $OID_X $OID_X file_x + 1 .D N... 100644 100644 00 $OID_Z $OID_Z file_z + ? actual + ? expect + EOF + + git status --porcelain=v2 --branch --untracked-files=all >actual && + test_cmp expect actual +' + +test_expect_success 'hide untracked files' ' + cat >expect <<-EOF && + 1 .M N... 100644 100644 100644 $OID_X $OID_X file_x + 1 .D N... 100644 100644 00 $OID_Z $OID_Z file_z + EOF + + git status --porcelain=v2 --untracked-files=no >actual && + test_cmp expect actual +' + +test_expect_success 'after first commit, stage changes' ' + git add file_x && + git rm file_z && + H0=$(git rev-parse HEAD) && + + cat >expect <<-EOF && + # branch.oid $H0 + # branch.head master + 1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x + 1 D. N... 100644 00 00 $OID_Z $_z40 file_z + ? actual + ? expect + EOF + + git status --porcelain=v2 --branch --untracked-files=all >actual && + test_cmp expect actual +' + +test_expect_success 'rename causes 2 path lines' ' + git mv file_y renamed_y && + H0=$(git rev-parse HEAD) && + + q_to_tab >expect <<-EOF && + # branch.oid $H0 + # branch.head master + 1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x + 1 D. N... 100644 00 00 $OID_Z $_z40 file_z + 2 R. N... 100644 100644 100644 $OID_Y $OID_Y R100 renamed_yQfile_y + ? actual
[PATCH v7 4/9] status: collect per-file data for --porcelain=v2
From: Jeff Hostetler Collect extra per-file data for porcelain V2 format. The output of `git status --porcelain` leaves out many details about the current status that clients might like to have. This can force them to be less efficient as they may need to launch secondary commands (and try to match the logic within git) to accumulate this extra information. For example, a GUI IDE might want the file mode to display the correct icon for a changed item (without having to stat it afterwards). Signed-off-by: Jeff Hostetler --- builtin/commit.c | 3 +++ wt-status.c | 64 ++-- wt-status.h | 4 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index c9d24d5..c50faf9 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -153,6 +153,8 @@ static int opt_parse_porcelain(const struct option *opt, const char *arg, int un *value = STATUS_FORMAT_PORCELAIN; else if (!strcmp(arg, "v1") || !strcmp(arg, "1")) *value = STATUS_FORMAT_PORCELAIN; + else if (!strcmp(arg, "v2") || !strcmp(arg, "2")) + *value = STATUS_FORMAT_PORCELAIN_V2; else die("unsupported porcelain version '%s'", arg); @@ -1104,6 +1106,7 @@ static struct status_deferred_config { static void finalize_deferred_config(struct wt_status *s) { int use_deferred_config = (status_format != STATUS_FORMAT_PORCELAIN && + status_format != STATUS_FORMAT_PORCELAIN_V2 && !s->null_termination); if (s->null_termination) { diff --git a/wt-status.c b/wt-status.c index 59bfb0b..aa804b5 100644 --- a/wt-status.c +++ b/wt-status.c @@ -434,6 +434,31 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q, if (S_ISGITLINK(p->two->mode)) d->new_submodule_commits = !!oidcmp(&p->one->oid, &p->two->oid); + + switch (p->status) { + case DIFF_STATUS_ADDED: + die("BUG: worktree status add???"); + break; + + case DIFF_STATUS_DELETED: + d->mode_index = p->one->mode; + oidcpy(&d->oid_index, &p->one->oid); + /* mode_worktree is zero for a delete. */ + break; + + case DIFF_STATUS_MODIFIED: + case DIFF_STATUS_TYPE_CHANGED: + case DIFF_STATUS_UNMERGED: + d->mode_index = p->one->mode; + d->mode_worktree = p->two->mode; + oidcpy(&d->oid_index, &p->one->oid); + break; + + case DIFF_STATUS_UNKNOWN: + die("BUG: worktree status unknown???"); + break; + } + } } @@ -479,12 +504,36 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q, if (!d->index_status) d->index_status = p->status; switch (p->status) { + case DIFF_STATUS_ADDED: + /* Leave {mode,oid}_head zero for an add. */ + d->mode_index = p->two->mode; + oidcpy(&d->oid_index, &p->two->oid); + break; + case DIFF_STATUS_DELETED: + d->mode_head = p->one->mode; + oidcpy(&d->oid_head, &p->one->oid); + /* Leave {mode,oid}_index zero for a delete. */ + break; + case DIFF_STATUS_COPIED: case DIFF_STATUS_RENAMED: d->head_path = xstrdup(p->one->path); + d->score = p->score * 100 / MAX_SCORE; + /* fallthru */ + case DIFF_STATUS_MODIFIED: + case DIFF_STATUS_TYPE_CHANGED: + d->mode_head = p->one->mode; + d->mode_index = p->two->mode; + oidcpy(&d->oid_head, &p->one->oid); + oidcpy(&d->oid_index, &p->two->oid); break; case DIFF_STATUS_UNMERGED: d->stagemask = unmerged_mask(p->two->path); + /* +* Don't bother setting {mode,oid}_{head,index} since the print +* code will output the stage values directly and not use the +* values in these fields. +*/ break; } } @@ -565,9 +614,17 @@ static void wt_status_collect_changes_initial(struct wt_status *s) if (ce_stage(ce)) { d->index_status = DIFF_STATUS_UNMERGED;
[PATCH v7 7/9] git-status.txt: describe --porcelain=v2 format
From: Jeff Hostetler Update status manpage to include information about porcelain v2 format. Signed-off-by: Jeff Hostetler --- Documentation/git-status.txt | 126 +-- 1 file changed, 122 insertions(+), 4 deletions(-) diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt index 6b1454b..a58973b 100644 --- a/Documentation/git-status.txt +++ b/Documentation/git-status.txt @@ -183,12 +183,12 @@ in which case `XY` are `!!`. If -b is used the short-format status is preceded by a line -## branchname tracking info +## branchname tracking info -Porcelain Format - +Porcelain Format Version 1 +~~ -The porcelain format is similar to the short format, but is guaranteed +Version 1 porcelain format is similar to the short format, but is guaranteed not to change in a backwards-incompatible way between Git versions or based on user configuration. This makes it ideal for parsing by scripts. The description of the short format above also describes the porcelain @@ -210,6 +210,124 @@ field from the first filename). Third, filenames containing special characters are not specially formatted; no quoting or backslash-escaping is performed. +Porcelain Format Version 2 +~~ + +Version 2 format adds more detailed information about the state of +the worktree and changed items. Version 2 also defines an extensible +set of easy to parse optional headers. + +Header lines start with "#" and are added in response to specific +command line arguments. Parsers should ignore headers they +don't recognize. + +### Branch Headers + +If `--branch` is given, a series of header lines are printed with +information about the current branch. + +Line Notes + +# branch.oid | (initial)Current commit. +# branch.head | (detached) Current branch. +# branch.upstream If upstream is set. +# branch.ab + - If upstream is set and + the commit is present. + + +### Changed Tracked Entries + +Following the headers, a series of lines are printed for tracked +entries. One of three different line formats may be used to describe +an entry depending on the type of change. Tracked entries are printed +in an undefined order; parsers should allow for a mixture of the 3 +line types in any order. + +Ordinary changed entries have the following format: + +1 + +Renamed or copied entries have the following format: + +2 + +Field Meaning + +A 2 character field containing the staged and +unstaged XY values described in the short format, +with unchanged indicated by a "." rather than +a space. + A 4 character field describing the submodule state. +"N..." when the entry is not a submodule. +"S" when the entry is a submodule. + is "C" if the commit changed; otherwise ".". + is "M" if it has tracked changes; otherwise ".". + is "U" if there are untracked changes; otherwise ".". +The octal file mode in HEAD. +The octal file mode in the index. +The octal file mode in the worktree. +The object name in HEAD. +The object name in the index. + The rename or copy score (denoting the percentage +of similarity between the source and target of the +move or copy). For example "R100" or "C75". + The pathname. In a renamed/copied entry, this +is the path in the index and in the working tree. + When the `-z` option is used, the 2 pathnames are separated +with a NUL (ASCII 0x00) byte; otherwise, a tab (ASCII 0x09) +byte separates them. + The pathname in the commit at HEAD. This is only +present in a renamed/copied entry, and tells +where the renamed/copied contents came from. + + +Unmerged entries have the following format; the first character is +a "u" to distinguish from ordinary changed entries. + +u + +Field Meaning + +A 2 character field describing the conflict type +as described in the short format. + A 4 character field describing the submodule state +as described above. +The octal file mode in stage 1. +The octal file mode in stage 2. +The octal file mode in stage 3. +The octal file mode in the worktree. +
[PATCH v7 8/9] test-lib-functions.sh: Add lf_to_nul
From: Jeff Hostetler Add lf_to_nul() function to test-lib-functions. Signed-off-by: Jeff Hostetler --- t/test-lib-functions.sh | 4 1 file changed, 4 insertions(+) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 4f7eadb..fdaeb3a 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -81,6 +81,10 @@ test_decode_color () { ' } +lf_to_nul () { + perl -pe 'y/\012/\000/' +} + nul_to_q () { perl -pe 'y/\000/Q/' } -- 2.8.0.rc4.17.gac42084.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v7 0/9] status: V2 porcelain status
From: Jeff Hostetler This patch series adds porcelain V2 format to status. This provides detailed information about file changes and about the current branch. The new output is accessed via: git status --porcelain=v2 [--branch] This v7 patch series address the most recent feedback on the unit tests. This series has been rebased onto a more current master branch (to get the EMPTY_BLOB changes in the test suite). Jeff Hostetler (9): status: rename long-format print routines status: cleanup API to wt_status_print status: support --porcelain[=] status: collect per-file data for --porcelain=v2 status: print per-file porcelain v2 status data status: print branch info with --porcelain=v2 --branch git-status.txt: describe --porcelain=v2 format test-lib-functions.sh: Add lf_to_nul status: unit tests for --porcelain=v2 Documentation/git-status.txt | 133 +- builtin/commit.c | 78 +++--- t/t7060-wtstatus.sh | 21 ++ t/t7064-wtstatus-pv2.sh | 592 +++ t/test-lib-functions.sh | 4 + wt-status.c | 570 - wt-status.h | 19 +- 7 files changed, 1305 insertions(+), 112 deletions(-) create mode 100755 t/t7064-wtstatus-pv2.sh -- 2.8.0.rc4.17.gac42084.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Warning: You tried to send an email with blocked content
The UCL E-Mail Virus Protection System has been triggered by a message you sent. One or more of the original e-mail attachments have been removed and replaced with this message. * The attachment may have contained a virus or malware * The attachment may have an extension of a type unacceptable for UCL Consider renaming the file extension (eg rename file.ext to file.ex_) to avoid this constraint. The recipient will be required to rename the file back to its original extension. -- UCL E-mail Virus Protection System serviced...@ucl.ac.uk Phone: +44 (0)20 767925000 Internal phone: 25000 --- Additional Information ---: Subject: Sender: git@vger.kernel.org Time received: 8/11/2016 8:20:16 PM Message ID:<000101d1f440$0e128336$c0a80...@cyberturbo.com.br> Detections found: 059015008745095s.guillas.zip JS/Nemucod.OB -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/2] diff: add SUBMODULE_DIFF format to display submodule diff
Jacob Keller writes: > diff --git a/submodule.c b/submodule.c > index 1b5cdfb7e784..36f8fd00c3ce 100644 > --- a/submodule.c > +++ b/submodule.c > @@ -333,6 +333,73 @@ static void print_submodule_summary(struct rev_info > *rev, FILE *f, > strbuf_release(&sb); > } > > +void show_submodule_diff(FILE *f, const char *path, > + const char *line_prefix, > + unsigned char one[20], unsigned char two[20], > + unsigned dirty_submodule, const char *meta, > + const char *a_prefix, const char *b_prefix, > + const char *reset) > +{ > + struct child_process cp = CHILD_PROCESS_INIT; > + struct strbuf sb = STRBUF_INIT; > + struct strbuf submodule_git_dir = STRBUF_INIT; > + const char *git_dir, *message = NULL; > + int create_dirty_diff = 0; > + FILE *diff; > + char c; The variables message, diff and c are not used, are they? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: public-inbox.org/git updates
Eric Wong writes: > There'll be over 5000K injected messages from 2006 I missed from > the initial import :x > > I noticed this while adding gmane: mapping support to the > search engine: > https://public-inbox.org/git/20160811002819.GA8311@starla/T/#u > > There will still be some missing messages because some are spam. > news.gmane.org remains up if you want to check my work > (please do, because I am careless) Thanks for doing this. I wanted to try out your NTTP service, but it took me a while to dig in my inbox to find your announcement of news.public-inbox.org that hosts inbox.comp.version-control.git "newsgroup". Is it possible to make this a bit more discoverable, or there is not enough NNTP audience these days to warrant such an addition? I first went to http://public-inbox.org/git as I expected there may be some pointers to other instances of the service, where you list the "git clone" URL of the archive. By the way, it felt quite strange to see messages from 8 years ago mixed with more recent messages when I gold Gnus to fetch the most recent 333 messages (and of course that fetches 333 messages with largest message numbers, not sorted by "Date:"). I am assuming that this is an artifact of "over 5k injected messages" bundle that was added out of order. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v12 12/13] apply: learn to use a different index file
Christian Couder writes: > Sometimes we want to apply in a different index file. > > Before the apply functionality was libified it was possible to > use the GIT_INDEX_FILE environment variable, for this purpose. > > But now, as the apply functionality has been libified, it should > be possible to do that in a libified way. > > Signed-off-by: Christian Couder > --- > apply.c | 10 -- > apply.h | 1 + > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/apply.c b/apply.c > index 2ec2a8a..7e561a4 100644 > --- a/apply.c > +++ b/apply.c > @@ -4674,8 +4674,14 @@ static int apply_patch(struct apply_state *state, > state->apply = 0; > > state->update_index = state->check_index && state->apply; > - if (state->update_index && state->newfd < 0) > - state->newfd = hold_locked_index(state->lock_file, 1); > + if (state->update_index && state->newfd < 0) { > + if (state->index_file) > + state->newfd = > hold_lock_file_for_update(state->lock_file, > + > state->index_file, > + > LOCK_DIE_ON_ERROR); > + else > + state->newfd = hold_locked_index(state->lock_file, 1); > + } > > if (state->check_index && read_cache() < 0) { > error(_("unable to read index file")); Here is a call to read_cache() that reads the default index file on the filesystem into the default in-core index "the_index". Shouldn't it be reading from state->index_file instead? If we limit the review only to the context of your series, I think fall_back_threeway() -> build_fake_ancestor() -- uses index_path to use custom index -> discard_cache() -> read_cache_from(index_path) -- reads back the fake ancestor -> write_index_as_tree() -- writes the fake_ancestor -> run_apply(index_path) -> apply_all_patches() -> apply_patch() is the only codepath that uses a custom index file, so when the control reaches this function with a custom index file, the in-core index is already populated, making read_cache() a no-op, and that is the only thing that makes the resulting code avoid triggering this bug, but as part of a general "libified" codepath, I think it should be made to read from state->index_file using read_cache_from(). I only noticed this call to read_cache(), but there may be others lurking nearby. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] submodule: mark a file-local symbol as static
On Thu, Aug 11, 2016 at 12:57 PM, Ramsay Jones wrote: > > Signed-off-by: Ramsay Jones > --- > > Hi Stefan, > > If you need to re-roll your 'sb/submodule-clone-rr' branch, could > you please squash this into the relevant patch (commit 336c21d, > "submodule: try alternates when superproject has an alternate", > 08-08-2016). Not just reroll but rethink and rewrite. ;) Thanks for catching! Stefan > > Thanks! > > ATB, > Ramsay Jones > > builtin/submodule--helper.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c > index 4c765e1..4c7d03c 100644 > --- a/builtin/submodule--helper.c > +++ b/builtin/submodule--helper.c > @@ -641,7 +641,7 @@ struct submodule_alternate_setup { > struct strbuf *out; > }; > > -int add_possible_reference(struct alternate_object_database *alt, void > *sas_cb) > +static int add_possible_reference(struct alternate_object_database *alt, > void *sas_cb) > { > struct submodule_alternate_setup *sas = sas_cb; > > -- > 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] submodule: mark a file-local symbol as static
Signed-off-by: Ramsay Jones --- Hi Stefan, If you need to re-roll your 'sb/submodule-clone-rr' branch, could you please squash this into the relevant patch (commit 336c21d, "submodule: try alternates when superproject has an alternate", 08-08-2016). Thanks! ATB, Ramsay Jones builtin/submodule--helper.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index 4c765e1..4c7d03c 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -641,7 +641,7 @@ struct submodule_alternate_setup { struct strbuf *out; }; -int add_possible_reference(struct alternate_object_database *alt, void *sas_cb) +static int add_possible_reference(struct alternate_object_database *alt, void *sas_cb) { struct submodule_alternate_setup *sas = sas_cb; -- 2.9.0 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
public-inbox.org/git updates
This mainly affects the folks following the top-level Atom feed at https://public-inbox.org/git/new.atom or over NNTP. There'll be over 5000K injected messages from 2006 I missed from the initial import :x I noticed this while adding gmane: mapping support to the search engine: https://public-inbox.org/git/20160811002819.GA8311@starla/T/#u There will still be some missing messages because some are spam. news.gmane.org remains up if you want to check my work (please do, because I am careless) Also, the following two .onions are geographically separate so it should remain up if the main server at http://ou63pmih66umazou.onion/git/ goes down. These two are on better hardware than the main one: http://czquwvybam4bgbro.onion/git/ http://hjrcffqmbrq6wope.onion/git/ Users without tor installed may use one of the MITM proxies at run by www.tor2web.org, too. And yes, I break stuff all the time and often run barely-tested development code on my server(s) :> Finally, HTTPS termination for public-inbox.org is provided by yahns, a Ruby/Rack server: git clone git://yhbt.net/yahns (more barely-tested code running off ruby/trunk) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: t0027 racy?
Torsten Bögershausen writes: > Good ideas, I will work on a series that fixes bugs first, and then we > can see if there is room for optimization. > > What do you think about this as a starting point, more things will > follow. > I like to here comments about the commit msg first ;-) Throughout t0027 there is no mention on what NNO stands for. Are they about operations that result in un-normalized index entries? > commit 3754404d3d1ea4a0cbbed4986cc4ac1b5fe6b66e > Author: Torsten Bögershausen > Date: Thu Aug 11 18:47:29 2016 +0200 > > t0027: Correct raciness in NNO test > > When a non-reversible CRLF conversion is done in "git add", > a warning is printed on stderr. > > The commit_chk_wrnNNO() function in t0027 was written to test this, > but did the wrong thing: Instead of looking at the warning > from "git add", it looked at the warning from "git commit". > > Correct this and replace the commit for each and every file with a commit > of all files in one go. > > The function commit_chk_wrnNNO() will to be renamed in a separate commit. > Thanks to Jeff King for analizing t0027. > Reporyed-By: Johannes Schindelin Reporyed? An obligatory "comments-about-the-commit-msg";-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 33/40] environment: add set_index_file()
Christian Couder writes: > Yeah, it is feasible and perhaps even simpler using > hold_lock_file_for_update() than with set_index_file(), so I > dropped the set_index_file() patch and added a new one that uses > hold_lock_file_for_update(). I wasn't paying too close an attention while reading the changes, but anyway that is a great news. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Can cc's be included in patches sent by send-email
From: "Junio C Hamano" Jacob Keller writes: On Thu, Aug 11, 2016 at 12:58 AM, Jacob Keller wrote: On Thu, Aug 11, 2016 at 12:32 AM, Philip Oakley wrote: While 'git send-email' can have multiple --cc="addressee" options on the command line, is it possible for the "cc:addressee" to actually be included in the patches that are to be sent, so that different patches can have different addressee? The fortmat-patch can include appropriate from lines, so it feels as if the sender should be able to add cc: lines at the same place. Maybe its just part of th docs I've missed. Yes, just put them in the body as tags below the signed-off-by. It should be on by default unless you change supresscc or signedoffbycc configuration. Thanks, Jake See --suppress-cc or --signed-off-by-cc help in git help send-email. Also, those who do not want to see Cc: in headers (like me) can instead edit the header part of the format-patch output to add Cc: lines and they should be picked up. -- When done via git gui, it looks like the Cc: is also included in the commit message (first initial tries) I'd probably place them after a --- break so that they don't get into the applied commit message, but will carry around during my rebasing. I've still to test if it works though. Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10 33/40] environment: add set_index_file()
On Wed, Aug 10, 2016 at 7:34 PM, Junio C Hamano wrote: > Christian Couder writes: > >>> Isn't the mention on NO_THE_INDEX_COMPATIBILITY_MACROS in the added >>> comments (there are two) pure red-herring? >> >> Yeah, true. >> >> So do you want me to refactor the code to use >> hold_lock_file_for_update() instead of hold_locked_index() and to >> avoid the set_index_file() hack? > > I somehow thought that we already agreed to accept the technical > debt, taking your "it is too much work" assessment at the face > value. If you now think it is feasible within the scope of the > series, of course we'd prefer it done "right" ;-) Yeah, it is feasible and perhaps even simpler using hold_lock_file_for_update() than with set_index_file(), so I dropped the set_index_file() patch and added a new one that uses hold_lock_file_for_update(). Sorry to have understood only recently what you said in some previous emails and thanks for the explanations. > Is the problematic hold_locked_index() something you do yourself, or > buried deep inside the callchain of a helper function you use? It is something done by the apply code, so we can easily modify that. > I am > sensing that it is the latter (otherwise you wouldn't be doing the > hack or at least will trivially fixing it up in a later "now we did > a faithful conversion from the previous code using GIT_INDEX_FILE > enviornment variable in an earlier step. Let's clean it up by being > in full control of what file we read from and write to" step in the > series). It was more a misunderstanding of how the index related functions are working. > Do you also have an issue on the reading side (i.e. you write it out > to temporary file and then later re-read the in-core cache from that > temporary file, for example)? Or is a single "write to a temporary > index" the only thing you need to work around? It looks like it is the latter. Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/9] status: V2 porcelain status
Jeff Hostetler writes: > From: Jeff Hostetler > > This patch series adds porcelain V2 format to status. > This provides detailed information about file changes > and about the current branch. Everything up to 8/9 I didn't have anything to comment on; I didn't see anything that smelled fishy, or anything I'd want to improve. I did have a few minor nits in 9/9, though. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: t0027 racy?
[] > FWIW I would strongly prefer to use the warning of `git add` and not even > bother with `git commit`. What we are interested in is the warning > message, generated by convert_to_git(). The commit is needed, because we check the content of the commit later. > Not using the first one and [] > > On that matter, I wonder whether there would be a chance to revamp t0027 > in a major way, with the following goals: > > - to make it very obvious to the casual reader what is being tested > > - to combine Git invocations when possible, e.g. running one big `git add` > on a couple of files and then verify the relevant parts of the output > > - dramatically decreasing the time required to run the test, without > sacrificing correctness (I would wager a bet that not only a few of > those 1388 test cases essentially exercise identical code paths) Good ideas, I will work on a series that fixes bugs first, and then we can see if there is room for optimization. What do you think about this as a starting point, more things will follow. I like to here comments about the commit msg first ;-) commit 3754404d3d1ea4a0cbbed4986cc4ac1b5fe6b66e Author: Torsten Bögershausen Date: Thu Aug 11 18:47:29 2016 +0200 t0027: Correct raciness in NNO test When a non-reversible CRLF conversion is done in "git add", a warning is printed on stderr. The commit_chk_wrnNNO() function in t0027 was written to test this, but did the wrong thing: Instead of looking at the warning from "git add", it looked at the warning from "git commit". Correct this and replace the commit for each and every file with a commit of all files in one go. The function commit_chk_wrnNNO() will to be renamed in a separate commit. Thanks to Jeff King for analizing t0027. Reporyed-By: Johannes Schindelin diff --git a/t/t0027-auto-crlf.sh b/t/t0027-auto-crlf.sh index 2860d2d..6e44382 100755 --- a/t/t0027-auto-crlf.sh +++ b/t/t0027-auto-crlf.sh @@ -119,8 +119,7 @@ commit_chk_wrnNNO () { fname=${pfx}_$f.txt && cp $f $fname && printf Z >>"$fname" && - git -c core.autocrlf=$crlf add $fname 2>/dev/null && - git -c core.autocrlf=$crlf commit -m "commit_$fname" $fname >"${pfx}_$f.err" 2>&1 + git -c core.autocrlf=$crlf add $fname 2>"${pfx}_$f.err" done test_expect_success "commit NNO files crlf=$crlf attr=$attr LF" ' @@ -394,11 +393,10 @@ test_expect_success 'commit files attr=crlf' ' # attrLFCRLF CRLFmixLF LF_mix_CR CRLFNUL commit_chk_wrnNNO "" "" false """""" "" "" -commit_chk_wrnNNO "" "" trueLF_CRLF """" "" "" +commit_chk_wrnNNO "" "" true"""""" "" "" commit_chk_wrnNNO "" "" input """""" "" "" - -commit_chk_wrnNNO "auto" "" false "$WILC" """" "" "" -commit_chk_wrnNNO "auto" "" trueLF_CRLF """" "" "" +commit_chk_wrnNNO "auto" "" false """""" "" "" +commit_chk_wrnNNO "auto" "" true"""""" "" "" commit_chk_wrnNNO "auto" "" input """""" "" "" for crlf in true false input do @@ -408,7 +406,7 @@ do commit_chk_wrnNNO ""lf $crlf "" CRLF_LFCRLF_LF "" CRLF_LF commit_chk_wrnNNO ""crlf$crlf LF_CRLF ""LF_CRLF LF_CRLF "" commit_chk_wrnNNO auto lf $crlf """""" "" "" - commit_chk_wrnNNO auto crlf$crlf LF_CRLF """" "" "" + commit_chk_wrnNNO auto crlf$crlf """""" "" "" commit_chk_wrnNNO text lf $crlf "" CRLF_LFCRLF_LF "" CRLF_LF commit_chk_wrnNNO text crlf$crlf LF_CRLF ""LF_CRLF LF_CRLF "" done @@ -417,7 +415,8 @@ commit_chk_wrnNNO "text" "" false "$WILC" "$WICL" "$WAMIX""$WILC commit_chk_wrnNNO "text" "" trueLF_CRLF ""LF_CRLF LF_CRLF "" commit_chk_wrnNNO "text" "" input ""CRLF_LF CRLF_LF "" CRLF_LF -test_expect_success 'create files cleanup' ' +test_expect_success 'commit NNO and cleanup' ' + git commit -m "commit files on top of NNO" && rm -f *.txt && git -c core.autocrlf=false reset --hard ' -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Can cc's be included in patches sent by send-email
From: "Jacob Keller" On Thu, Aug 11, 2016 at 12:58 AM, Jacob Keller wrote: On Thu, Aug 11, 2016 at 12:32 AM, Philip Oakley wrote: While 'git send-email' can have multiple --cc="addressee" options on the command line, is it possible for the "cc:addressee" to actually be included in the patches that are to be sent, so that different patches can have different addressee? The fortmat-patch can include appropriate from lines, so it feels as if the sender should be able to add cc: lines at the same place. Maybe its just part of th docs I've missed. Yes, just put them in the body as tags below the signed-off-by. It should be on by default unless you change supresscc or signedoffbycc configuration. Thanks, Jake See --suppress-cc or --signed-off-by-cc help in git help send-email. Thanks, Jake Thanks, I'll look at that. It wasn't immediately obvious what to do. Philip -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gc: default aggressive depth to 50
Jeff King writes: > On Thu, Aug 11, 2016 at 12:13:09PM -0400, Jeff King wrote: > >> Here are the numbers for linux.git: >> >>depth | size | %| rev-list | % | log -Sfoo | % >> ---+---+---+--++---+--- >> 250 | 967MB | n/a | 48.159s | n/a | 378.088 | n/a >> 100 | 971MB | +0.4% | 41.471s | -13.9% | 342.060 | -9.5% >> 50 | 979MB | +1.2% | 37.778s | -21.6% | 311.040s | -17.7% >> 10 | 1.1GB | +6.6% | 32.518s | -32.5% | 279.890s | -25.9% >> [...] >> >> You can see that that the CPU savings for regular operations improves as we >> decrease the depth. The savings are less for "rev-list" on a smaller >> repository >> than they are for blob-accessing operations, or even rev-list on a larger >> repository. This may mean that a larger delta cache would help (though >> setting >> core.deltaBaseCacheLimit by itself doesn't). > > The problem with deltaBaseCacheLimit is that it only changes the memory > parameter, but there are a fixed number of slots in the data structure. > Bumping it like this: > > diff --git a/sha1_file.c b/sha1_file.c > index 02940f1..ca79703 100644 > --- a/sha1_file.c > +++ b/sha1_file.c > @@ -2073,7 +2073,7 @@ static void *unpack_compressed_entry(struct packed_git > *p, > return buffer; > } > > -#define MAX_DELTA_CACHE (256) > +#define MAX_DELTA_CACHE (1024) > > static size_t delta_base_cached; > > along with the cache size does help (this was discussed a year or two > ago, but nobody ever followed up with numbers or patches). Yeah, and I also think Linus's "--depth=250 is just a sample; it will not perform well" already cited the number of delta-cache entries being the limiting factor. > I don't think bumping MAX_DELTA_CACHE naively is a good idea, though. I > seem to recall that it has scaling problems as it grows, so we may want > a better data structure (but I haven't looked at it recently enough to > say anything intelligent). Me neither. In any case, I do think reducing the aggressive depth down to 50 is a very sensible move. I also suspect that window size may want to be a bit increased (or even made dynamic; the first time we need the window size determined is after to_pack.objects[] array is fully populated, so we could use the number of commits as one of the hint, for example), but that can be treated as a separate topic. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 13/13] builtin/am: use apply API in run_apply()
This replaces run_apply() implementation with a new one that uses the apply API that has been previously prepared in apply.c and apply.h. This shoud improve performance a lot in certain cases. As the previous implementation was creating a new `git apply` process to apply each patch, it could be slow on systems like Windows where it is costly to create new processes. Also the new `git apply` process had to read the index from disk, and when the process was done the calling process discarded its own index and read back from disk the new index that had been created by the `git apply` process. This could be very inefficient with big repositories that have big index files, especially when the system decided that it was a good idea to run the `git apply` processes on a different processor core. Also eliminating index reads enables further performance improvements by using: `git update-index --split-index` For example here is a benchmark of a multi hundred commit rebase on the Linux kernel on a Debian laptop with SSD: command: git rebase --onto 1993b17 52bef0c 29dde7c Vanilla "next" without split index:1m54.953s Vanilla "next" with split index: 1m22.476s This series on top of "next" without split index: 1m12.034s This series on top of "next" with split index: 0m15.678s (using branch "next" from mid April 2016.) Benchmarked-by: Ævar Arnfjörð Bjarmason Signed-off-by: Christian Couder --- builtin/am.c | 65 1 file changed, 43 insertions(+), 22 deletions(-) diff --git a/builtin/am.c b/builtin/am.c index 739b34d..0e5d384 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -28,6 +28,7 @@ #include "rerere.h" #include "prompt.h" #include "mailinfo.h" +#include "apply.h" /** * Returns 1 if the file is empty or does not exist, 0 otherwise. @@ -1522,39 +1523,59 @@ static int parse_mail_rebase(struct am_state *state, const char *mail) */ static int run_apply(const struct am_state *state, const char *index_file) { - struct child_process cp = CHILD_PROCESS_INIT; - - cp.git_cmd = 1; - - if (index_file) - argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s", index_file); + struct argv_array apply_paths = ARGV_ARRAY_INIT; + struct argv_array apply_opts = ARGV_ARRAY_INIT; + struct apply_state apply_state; + int res, opts_left; + static struct lock_file lock_file; + int force_apply = 0; + int options = 0; + + if (init_apply_state(&apply_state, NULL, &lock_file)) + die("BUG: init_apply_state() failed"); + + argv_array_push(&apply_opts, "apply"); + argv_array_pushv(&apply_opts, state->git_apply_opts.argv); + + opts_left = apply_parse_options(apply_opts.argc, apply_opts.argv, + &apply_state, &force_apply, &options, + NULL); + + if (opts_left != 0) + die("unknown option passed thru to git apply"); + + if (index_file) { + apply_state.index_file = index_file; + apply_state.cached = 1; + } else + apply_state.check_index = 1; /* * If we are allowed to fall back on 3-way merge, don't give false * errors during the initial attempt. */ - if (state->threeway && !index_file) { - cp.no_stdout = 1; - cp.no_stderr = 1; - } + if (state->threeway && !index_file) + apply_state.apply_verbosity = verbosity_silent; - argv_array_push(&cp.args, "apply"); + if (check_apply_state(&apply_state, force_apply)) + die("BUG: check_apply_state() failed"); - argv_array_pushv(&cp.args, state->git_apply_opts.argv); + argv_array_push(&apply_paths, am_path(state, "patch")); - if (index_file) - argv_array_push(&cp.args, "--cached"); - else - argv_array_push(&cp.args, "--index"); + res = apply_all_patches(&apply_state, apply_paths.argc, apply_paths.argv, options); - argv_array_push(&cp.args, am_path(state, "patch")); + argv_array_clear(&apply_paths); + argv_array_clear(&apply_opts); + clear_apply_state(&apply_state); - if (run_command(&cp)) - return -1; + if (res) + return res; - /* Reload index as git-apply will have modified it. */ - discard_cache(); - read_cache_from(index_file ? index_file : get_index_file()); + if (index_file) { + /* Reload index as apply_all_patches() will have modified it. */ + discard_cache(); + read_cache_from(index_file); + } return 0; } -- 2.9.2.769.gc0f0333 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majord
[PATCH v12 10/13] apply: change error_routine when silent
To avoid printing anything when applying with `state->apply_verbosity == verbosity_silent`, let's save the existing warn and error routines before applying, and let's replace them with a routine that does nothing. Then after applying, let's restore the saved routines. Helped-by: Stefan Beller Signed-off-by: Christian Couder --- apply.c | 21 - apply.h | 8 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/apply.c b/apply.c index ddbb0a2..bf81b70 100644 --- a/apply.c +++ b/apply.c @@ -112,6 +112,11 @@ void clear_apply_state(struct apply_state *state) /* &state->fn_table is cleared at the end of apply_patch() */ } +static void mute_routine(const char *bla, va_list params) +{ + /* do nothing */ +} + int check_apply_state(struct apply_state *state, int force_apply) { int is_not_gitdir = !startup_info->have_repository; @@ -144,6 +149,13 @@ int check_apply_state(struct apply_state *state, int force_apply) if (!state->lock_file) return error("BUG: state->lock_file should not be NULL"); + if (state->apply_verbosity <= verbosity_silent) { + state->saved_error_routine = get_error_routine(); + state->saved_warn_routine = get_warn_routine(); + set_error_routine(mute_routine); + set_warn_routine(mute_routine); + } + return 0; } @@ -4864,7 +4876,7 @@ int apply_all_patches(struct apply_state *state, state->newfd = -1; } - return !!errs; + res = !!errs; end: if (state->newfd >= 0) { @@ -4872,5 +4884,12 @@ int apply_all_patches(struct apply_state *state, state->newfd = -1; } + if (state->apply_verbosity <= verbosity_silent) { + set_error_routine(state->saved_error_routine); + set_warn_routine(state->saved_warn_routine); + } + + if (res > -1) + return res; return (res == -1 ? 1 : 128); } diff --git a/apply.h b/apply.h index bd4eb6d..5cde641 100644 --- a/apply.h +++ b/apply.h @@ -94,6 +94,14 @@ struct apply_state { */ struct string_list fn_table; + /* +* This is to save reporting routines before using +* set_error_routine() or set_warn_routine() to install muting +* routines when in verbosity_silent mode. +*/ + void (*saved_error_routine)(const char *err, va_list params); + void (*saved_warn_routine)(const char *warn, va_list params); + /* These control whitespace errors */ enum apply_ws_error_action ws_error_action; enum apply_ws_ignore ws_ignore_action; -- 2.9.2.769.gc0f0333 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 08/13] usage: add set_warn_routine()
There are already set_die_routine() and set_error_routine(), so let's add set_warn_routine() as this will be needed in a following commit. Signed-off-by: Christian Couder --- git-compat-util.h | 1 + usage.c | 5 + 2 files changed, 6 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index 590bfdd..c7a51f8 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -440,6 +440,7 @@ static inline int const_error(void) extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params)); extern void set_error_routine(void (*routine)(const char *err, va_list params)); +extern void set_warn_routine(void (*routine)(const char *warn, va_list params)); extern void set_die_is_recursing_routine(int (*routine)(void)); extern void set_error_handle(FILE *); diff --git a/usage.c b/usage.c index 1dad03f..67e5526 100644 --- a/usage.c +++ b/usage.c @@ -70,6 +70,11 @@ void set_error_routine(void (*routine)(const char *err, va_list params)) error_routine = routine; } +void set_warn_routine(void (*routine)(const char *warn, va_list params)) +{ + warn_routine = routine; +} + void set_die_is_recursing_routine(int (*routine)(void)) { die_is_recursing = routine; -- 2.9.2.769.gc0f0333 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 09/13] usage: add get_error_routine() and get_warn_routine()
Let's make it possible to get the current error_routine and warn_routine, so that we can store them before using set_error_routine() or set_warn_routine() to use new ones. This way we will be able put back the original routines, when we are done with using new ones. Signed-off-by: Christian Couder --- git-compat-util.h | 2 ++ usage.c | 10 ++ 2 files changed, 12 insertions(+) diff --git a/git-compat-util.h b/git-compat-util.h index c7a51f8..de04df1 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -440,7 +440,9 @@ static inline int const_error(void) extern void set_die_routine(NORETURN_PTR void (*routine)(const char *err, va_list params)); extern void set_error_routine(void (*routine)(const char *err, va_list params)); +extern void (*get_error_routine(void))(const char *err, va_list params); extern void set_warn_routine(void (*routine)(const char *warn, va_list params)); +extern void (*get_warn_routine(void))(const char *warn, va_list params); extern void set_die_is_recursing_routine(int (*routine)(void)); extern void set_error_handle(FILE *); diff --git a/usage.c b/usage.c index 67e5526..2fd3045 100644 --- a/usage.c +++ b/usage.c @@ -70,11 +70,21 @@ void set_error_routine(void (*routine)(const char *err, va_list params)) error_routine = routine; } +void (*get_error_routine(void))(const char *err, va_list params) +{ + return error_routine; +} + void set_warn_routine(void (*routine)(const char *warn, va_list params)) { warn_routine = routine; } +void (*get_warn_routine(void))(const char *warn, va_list params) +{ + return warn_routine; +} + void set_die_is_recursing_routine(int (*routine)(void)) { die_is_recursing = routine; -- 2.9.2.769.gc0f0333 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 11/13] apply: refactor `git apply` option parsing
Parsing `git apply` options can be useful to other commands that want to call the libified apply functionality, because this way they can easily pass some options from their own command line to the libified apply functionality. This will be used by `git am` in a following patch. To make this possible, let's refactor the `git apply` option parsing code into a new libified apply_parse_options() function. Doing that makes it possible to remove some functions definitions from "apply.h" and make them static in "apply.c". Helped-by: Ramsay Jones Signed-off-by: Christian Couder --- apply.c | 103 +--- apply.h | 18 +++--- builtin/apply.c | 74 ++-- 3 files changed, 97 insertions(+), 98 deletions(-) diff --git a/apply.c b/apply.c index bf81b70..2ec2a8a 100644 --- a/apply.c +++ b/apply.c @@ -4730,16 +4730,16 @@ static int apply_patch(struct apply_state *state, return res; } -int apply_option_parse_exclude(const struct option *opt, - const char *arg, int unset) +static int apply_option_parse_exclude(const struct option *opt, + const char *arg, int unset) { struct apply_state *state = opt->value; add_name_limit(state, arg, 1); return 0; } -int apply_option_parse_include(const struct option *opt, - const char *arg, int unset) +static int apply_option_parse_include(const struct option *opt, + const char *arg, int unset) { struct apply_state *state = opt->value; add_name_limit(state, arg, 0); @@ -4747,9 +4747,9 @@ int apply_option_parse_include(const struct option *opt, return 0; } -int apply_option_parse_p(const struct option *opt, -const char *arg, -int unset) +static int apply_option_parse_p(const struct option *opt, + const char *arg, + int unset) { struct apply_state *state = opt->value; state->p_value = atoi(arg); @@ -4757,8 +4757,8 @@ int apply_option_parse_p(const struct option *opt, return 0; } -int apply_option_parse_space_change(const struct option *opt, - const char *arg, int unset) +static int apply_option_parse_space_change(const struct option *opt, + const char *arg, int unset) { struct apply_state *state = opt->value; if (unset) @@ -4768,8 +4768,8 @@ int apply_option_parse_space_change(const struct option *opt, return 0; } -int apply_option_parse_whitespace(const struct option *opt, - const char *arg, int unset) +static int apply_option_parse_whitespace(const struct option *opt, +const char *arg, int unset) { struct apply_state *state = opt->value; state->whitespace_option = arg; @@ -4778,8 +4778,8 @@ int apply_option_parse_whitespace(const struct option *opt, return 0; } -int apply_option_parse_directory(const struct option *opt, -const char *arg, int unset) +static int apply_option_parse_directory(const struct option *opt, + const char *arg, int unset) { struct apply_state *state = opt->value; strbuf_reset(&state->root); @@ -4893,3 +4893,80 @@ int apply_all_patches(struct apply_state *state, return res; return (res == -1 ? 1 : 128); } + +int apply_parse_options(int argc, const char **argv, + struct apply_state *state, + int *force_apply, int *options, + const char * const *apply_usage) +{ + struct option builtin_apply_options[] = { + { OPTION_CALLBACK, 0, "exclude", state, N_("path"), + N_("don't apply changes matching the given path"), + 0, apply_option_parse_exclude }, + { OPTION_CALLBACK, 0, "include", state, N_("path"), + N_("apply changes matching the given path"), + 0, apply_option_parse_include }, + { OPTION_CALLBACK, 'p', NULL, state, N_("num"), + N_("remove leading slashes from traditional diff paths"), + 0, apply_option_parse_p }, + OPT_BOOL(0, "no-add", &state->no_add, + N_("ignore additions made by the patch")), + OPT_BOOL(0, "stat", &state->diffstat, + N_("instead of applying the patch, output diffstat for the input")), + OPT_NOOP_NOARG(0, "allow-binary-replacement"), + OPT_NOOP_NOARG(0, "binary"), + OPT_BOOL(0, "numstat", &state->numstat, + N_("show number o
[PATCH v12 12/13] apply: learn to use a different index file
Sometimes we want to apply in a different index file. Before the apply functionality was libified it was possible to use the GIT_INDEX_FILE environment variable, for this purpose. But now, as the apply functionality has been libified, it should be possible to do that in a libified way. Signed-off-by: Christian Couder --- apply.c | 10 -- apply.h | 1 + 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/apply.c b/apply.c index 2ec2a8a..7e561a4 100644 --- a/apply.c +++ b/apply.c @@ -4674,8 +4674,14 @@ static int apply_patch(struct apply_state *state, state->apply = 0; state->update_index = state->check_index && state->apply; - if (state->update_index && state->newfd < 0) - state->newfd = hold_locked_index(state->lock_file, 1); + if (state->update_index && state->newfd < 0) { + if (state->index_file) + state->newfd = hold_lock_file_for_update(state->lock_file, + state->index_file, + LOCK_DIE_ON_ERROR); + else + state->newfd = hold_locked_index(state->lock_file, 1); + } if (state->check_index && read_cache() < 0) { error(_("unable to read index file")); diff --git a/apply.h b/apply.h index e2b89e8..1ba4f8d 100644 --- a/apply.h +++ b/apply.h @@ -63,6 +63,7 @@ struct apply_state { int unsafe_paths; /* Other non boolean parameters */ + const char *index_file; enum apply_verbosity apply_verbosity; const char *fake_ancestor; const char *patch_input_file; -- 2.9.2.769.gc0f0333 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 06/13] apply: make it possible to silently apply
This changes 'int apply_verbosely' into 'enum apply_verbosity', and changes the possible values of the variable from a bool to a tristate. The previous 'false' state is changed into 'verbosity_normal'. The previous 'true' state is changed into 'verbosity_verbose'. The new added state is 'verbosity_silent'. It should prevent anything to be printed on both stderr and stdout. This is needed because `git am` wants to first call apply functionality silently, if it can then fall back on 3-way merge in case of error. Printing on stdout, and calls to warning() or error() are not taken care of in this patch, as that will be done in following patches. Signed-off-by: Christian Couder --- apply.c | 62 + apply.h | 8 +++- builtin/apply.c | 2 +- 3 files changed, 48 insertions(+), 24 deletions(-) diff --git a/apply.c b/apply.c index 41a33d3..df85cbc 100644 --- a/apply.c +++ b/apply.c @@ -125,8 +125,11 @@ int check_apply_state(struct apply_state *state, int force_apply) return error(_("--3way outside a repository")); state->check_index = 1; } - if (state->apply_with_reject) - state->apply = state->apply_verbosely = 1; + if (state->apply_with_reject) { + state->apply = 1; + if (state->apply_verbosity == verbosity_normal) + state->apply_verbosity = verbosity_verbose; + } if (!force_apply && (state->diffstat || state->numstat || state->summary || state->check || state->fake_ancestor)) state->apply = 0; if (state->check_index && is_not_gitdir) @@ -1620,8 +1623,9 @@ static void record_ws_error(struct apply_state *state, return; err = whitespace_error_string(result); - fprintf(stderr, "%s:%d: %s.\n%.*s\n", - state->patch_input_file, linenr, err, len, line); + if (state->apply_verbosity > verbosity_silent) + fprintf(stderr, "%s:%d: %s.\n%.*s\n", + state->patch_input_file, linenr, err, len, line); free(err); } @@ -1816,7 +1820,7 @@ static int parse_single_patch(struct apply_state *state, return error(_("new file %s depends on old contents"), patch->new_name); if (0 < patch->is_delete && newlines) return error(_("deleted file %s still has contents"), patch->old_name); - if (!patch->is_delete && !newlines && context) + if (!patch->is_delete && !newlines && context && state->apply_verbosity > verbosity_silent) fprintf_ln(stderr, _("** warning: " "file %s becomes empty but is not deleted"), @@ -2911,7 +2915,7 @@ static int apply_one_fragment(struct apply_state *state, /* Ignore it, we already handled it */ break; default: - if (state->apply_verbosely) + if (state->apply_verbosity > verbosity_normal) error(_("invalid start of line: '%c'"), first); applied_pos = -1; goto out; @@ -3026,7 +3030,7 @@ static int apply_one_fragment(struct apply_state *state, state->apply = 0; } - if (state->apply_verbosely && applied_pos != pos) { + if (state->apply_verbosity > verbosity_normal && applied_pos != pos) { int offset = applied_pos - pos; if (state->apply_in_reverse) offset = 0 - offset; @@ -3041,14 +3045,14 @@ static int apply_one_fragment(struct apply_state *state, * Warn if it was necessary to reduce the number * of context lines. */ - if ((leading != frag->leading) || - (trailing != frag->trailing)) + if ((leading != frag->leading || +trailing != frag->trailing) && state->apply_verbosity > verbosity_silent) fprintf_ln(stderr, _("Context reduced to (%ld/%ld)" " to apply fragment at %d"), leading, trailing, applied_pos+1); update_image(state, img, applied_pos, &preimage, &postimage); } else { - if (state->apply_verbosely) + if (state->apply_verbosity > verbosity_normal) error(_("while searching for:\n%.*s"), (int)(old - oldlines), oldlines); } @@ -3539,7 +3543,8 @@ static int try_threeway(struct apply_state *state, read_blob_object(&buf, pre_sha1, patch->old_mode)) return error("repository lacks the necessary blob to fall back on 3-way merge."); - fprintf(stderr,
[PATCH v12 01/13] builtin/apply: rename option parsing functions
As these functions are going to be part of the libified apply API, let's give them a name that is more specific to the apply API. Signed-off-by: Christian Couder --- builtin/apply.c | 40 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/builtin/apply.c b/builtin/apply.c index ad403f8..429fe44 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -4588,16 +4588,16 @@ static int apply_patch(struct apply_state *state, return res; } -static int option_parse_exclude(const struct option *opt, - const char *arg, int unset) +static int apply_option_parse_exclude(const struct option *opt, + const char *arg, int unset) { struct apply_state *state = opt->value; add_name_limit(state, arg, 1); return 0; } -static int option_parse_include(const struct option *opt, - const char *arg, int unset) +static int apply_option_parse_include(const struct option *opt, + const char *arg, int unset) { struct apply_state *state = opt->value; add_name_limit(state, arg, 0); @@ -4605,9 +4605,9 @@ static int option_parse_include(const struct option *opt, return 0; } -static int option_parse_p(const struct option *opt, - const char *arg, - int unset) +static int apply_option_parse_p(const struct option *opt, + const char *arg, + int unset) { struct apply_state *state = opt->value; state->p_value = atoi(arg); @@ -4615,8 +4615,8 @@ static int option_parse_p(const struct option *opt, return 0; } -static int option_parse_space_change(const struct option *opt, -const char *arg, int unset) +static int apply_option_parse_space_change(const struct option *opt, + const char *arg, int unset) { struct apply_state *state = opt->value; if (unset) @@ -4626,8 +4626,8 @@ static int option_parse_space_change(const struct option *opt, return 0; } -static int option_parse_whitespace(const struct option *opt, - const char *arg, int unset) +static int apply_option_parse_whitespace(const struct option *opt, +const char *arg, int unset) { struct apply_state *state = opt->value; state->whitespace_option = arg; @@ -4636,8 +4636,8 @@ static int option_parse_whitespace(const struct option *opt, return 0; } -static int option_parse_directory(const struct option *opt, - const char *arg, int unset) +static int apply_option_parse_directory(const struct option *opt, + const char *arg, int unset) { struct apply_state *state = opt->value; strbuf_reset(&state->root); @@ -4755,13 +4755,13 @@ int cmd_apply(int argc, const char **argv, const char *prefix) struct option builtin_apply_options[] = { { OPTION_CALLBACK, 0, "exclude", &state, N_("path"), N_("don't apply changes matching the given path"), - 0, option_parse_exclude }, + 0, apply_option_parse_exclude }, { OPTION_CALLBACK, 0, "include", &state, N_("path"), N_("apply changes matching the given path"), - 0, option_parse_include }, + 0, apply_option_parse_include }, { OPTION_CALLBACK, 'p', NULL, &state, N_("num"), N_("remove leading slashes from traditional diff paths"), - 0, option_parse_p }, + 0, apply_option_parse_p }, OPT_BOOL(0, "no-add", &state.no_add, N_("ignore additions made by the patch")), OPT_BOOL(0, "stat", &state.diffstat, @@ -4793,13 +4793,13 @@ int cmd_apply(int argc, const char **argv, const char *prefix) N_("ensure at least lines of context match")), { OPTION_CALLBACK, 0, "whitespace", &state, N_("action"), N_("detect new or modified lines that have whitespace errors"), - 0, option_parse_whitespace }, + 0, apply_option_parse_whitespace }, { OPTION_CALLBACK, 0, "ignore-space-change", &state, NULL, N_("ignore changes in whitespace when finding context"), - PARSE_OPT_NOARG, option_parse_space_change }, + PARSE_OPT_NOARG, apply_option_parse_space_change }, { OPTION_CALLBACK, 0, "ignore-whitespace", &state, NULL, N_("ignore changes in whitespace when finding context"), -
[PATCH v12 02/13] apply: rename and move opt constants to apply.h
The constants for the "inaccurate-eof" and the "recount" options will be used in both "apply.c" and "builtin/apply.c", so they need to go into "apply.h", and therefore they need a name that is more specific to the API they belong to. Signed-off-by: Christian Couder --- apply.h | 3 +++ builtin/apply.c | 11 --- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/apply.h b/apply.h index 53f09b5..ca1dcee 100644 --- a/apply.h +++ b/apply.h @@ -108,4 +108,7 @@ extern int init_apply_state(struct apply_state *state, extern void clear_apply_state(struct apply_state *state); extern int check_apply_state(struct apply_state *state, int force_apply); +#define APPLY_OPT_INACCURATE_EOF (1<<0) +#define APPLY_OPT_RECOUNT (1<<1) + #endif diff --git a/builtin/apply.c b/builtin/apply.c index 429fe44..9c396bb 100644 --- a/builtin/apply.c +++ b/builtin/apply.c @@ -4463,9 +4463,6 @@ static int write_out_results(struct apply_state *state, struct patch *list) static struct lock_file lock_file; -#define INACCURATE_EOF (1<<0) -#define RECOUNT(1<<1) - /* * Try to apply a patch. * @@ -4495,8 +4492,8 @@ static int apply_patch(struct apply_state *state, int nr; patch = xcalloc(1, sizeof(*patch)); - patch->inaccurate_eof = !!(options & INACCURATE_EOF); - patch->recount = !!(options & RECOUNT); + patch->inaccurate_eof = !!(options & APPLY_OPT_INACCURATE_EOF); + patch->recount = !!(options & APPLY_OPT_RECOUNT); nr = parse_chunk(state, buf.buf + offset, buf.len - offset, patch); if (nr < 0) { free_patch(patch); @@ -4811,10 +4808,10 @@ int cmd_apply(int argc, const char **argv, const char *prefix) OPT__VERBOSE(&state.apply_verbosely, N_("be verbose")), OPT_BIT(0, "inaccurate-eof", &options, N_("tolerate incorrectly detected missing new-line at the end of file"), - INACCURATE_EOF), + APPLY_OPT_INACCURATE_EOF), OPT_BIT(0, "recount", &options, N_("do not trust the line counts in the hunk headers"), - RECOUNT), + APPLY_OPT_RECOUNT), { OPTION_CALLBACK, 0, "directory", &state, N_("root"), N_("prepend to all filenames"), 0, apply_option_parse_directory }, -- 2.9.2.769.gc0f0333 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 00/13] libify apply and use lib in am, part 3
Goal This is a patch series about libifying `git apply` functionality, and using this libified functionality in `git am`, so that no 'git apply' process is spawn anymore. This makes `git am` significantly faster, so `git rebase`, when it uses the am backend, is also significantly faster. Previous discussions and patches series ~~~ This has initially been discussed in the following thread: http://thread.gmane.org/gmane.comp.version-control.git/287236/ Then the following patch series were sent: RFC: http://thread.gmane.org/gmane.comp.version-control.git/288489/ v1: http://thread.gmane.org/gmane.comp.version-control.git/292324/ v2: http://thread.gmane.org/gmane.comp.version-control.git/294248/ v3: http://thread.gmane.org/gmane.comp.version-control.git/295429/ v4: http://thread.gmane.org/gmane.comp.version-control.git/296350/ v5: http://thread.gmane.org/gmane.comp.version-control.git/296490/ v6: http://thread.gmane.org/gmane.comp.version-control.git/297024/ v7: http://thread.gmane.org/gmane.comp.version-control.git/297193/ v8: https://public-inbox.org/git/20160627182429.31550-1-chriscool%40tuxfamily.org/ v9: https://public-inbox.org/git/20160730172509.22939-1-chriscool%40tuxfamily.org/ v10: https://public-inbox.org/git/20160808210337.5038-1-chriscool%40tuxfamily.org/ v11: https://public-inbox.org/git/20160811085229.19017-1-chriscool%40tuxfamily.org/ Highlevel view of the patches in the series ~~~ This is "part 3" of the full patch series. I am resending only the last 13 patches of the full series as "part 3", because I don't want to resend the first 27 patches of v10 nearly as is. So "part 2" is made of patch 01/40 from v11 and patches from 02/40 to 27/40 from v10. The "part 1" is in "master" for some time now. - Patch 01/13 was v1, v2, v6, v7, v8, v9, v10 and v11. Since v10 and v11 only the commit message has been changed. As suggested by Stefan Naewe, 'api' as been replaced with 'API'. - Patches 02/13 to 04/13 were in v1, v2, v6, v7, v8, v9 and v10. They haven't changed since v10. Along with 01/13 they finish libifying the apply functionality that was in builtin/apply.c and move it into apply.{c,h}, but the libified functionality is not yet used in `git am`. - Patch 05/13 was in v6, v7, v8, v9 and v10, and hasn't changed. It replaces some calls to error() with calls to error_errno(). - Patches 06/13 to 10/13 were in v2, v6, v7, v8, v9 and v10. They haven't changed since v10. They implement a way to make the libified apply code silent by changing the bool `apply_verbosely` into a tristate enum called `apply_verbosity`, that can be one of `verbosity_verbose`, `verbosity_normal` or `verbosity_silent`. This is because "git am", since it was a shell script, has been silencing the apply functionality by redirecting file descriptors to /dev/null, but this is not acceptable in C. - Patch 11/13 was in v9 and v10, and hasn't changed. It refactors `git apply` option parsing to make it possible for `git am` to easily pass some command line options to the libified applied code as suggested by Junio. - Patch 12/13 is new. It replaces patch 33/40 in v10 (environment: add set_index_file()) that was a hack to make it possible for `git am` to use the libified apply functionality on a different index file. Now for this purpose, in this new patch, we add "const char *index_file" into "struct apply_state", and when "index_file" is set, we use hold_lock_file_for_update(), passing it "state->index_file", instead of using hold_locked_index(), as it is not possible to pass an index filename in hold_locked_index(). - Patch 13/13 was in v1, v2, v6, v7, v8, v9 and v10. This patch makes `git am` use the libified functionality. It now uses "index_file" in "struct apply_state" added by 12/13 to pass the file we want the index written into to the libified apply functionality. General comments Now this patch series is shorter than previously. Hopefully also the early part of this series until 05/13 or 11/13 will be ready soon to be moved to next and master, and I may only need to resend the last 2 patches if anything. I will send a diff between this version and v10 as a reply to this email. (Note that in v11 only commit messages changed, so there is no difference between v10 and v11.) The benefits are not just related to not creating new processes. When `git am` launched a `git apply` process, this new process had to read the index from disk. Then after the `git apply`process had terminated, `git am` dropped its index and read the index from disk to get the index that had been modified by the `git apply`process. This was inefficient and also prevented the split-index mechanism to provide many performance benefits. By the way, current work is ongoing to make it possible to use split-index more easily by adding a config variable, see: https://public-inbox.org/git/20160711172254.13439-1-chriscoo
[PATCH v12 04/13] apply: make some parsing functions static again
Some parsing functions that were used in both "apply.c" and "builtin/apply.c" are now only used in the former, so they can be made static to "apply.c". Signed-off-by: Christian Couder --- apply.c | 6 +++--- apply.h | 5 - 2 files changed, 3 insertions(+), 8 deletions(-) diff --git a/apply.c b/apply.c index 7b96130..c0cb3f5 100644 --- a/apply.c +++ b/apply.c @@ -27,7 +27,7 @@ static void git_apply_config(void) git_config(git_default_config, NULL); } -int parse_whitespace_option(struct apply_state *state, const char *option) +static int parse_whitespace_option(struct apply_state *state, const char *option) { if (!option) { state->ws_error_action = warn_on_ws_error; @@ -57,8 +57,8 @@ int parse_whitespace_option(struct apply_state *state, const char *option) return error(_("unrecognized whitespace option '%s'"), option); } -int parse_ignorewhitespace_option(struct apply_state *state, - const char *option) +static int parse_ignorewhitespace_option(struct apply_state *state, +const char *option) { if (!option || !strcmp(option, "no") || !strcmp(option, "false") || !strcmp(option, "never") || diff --git a/apply.h b/apply.h index 5ec022c..df44b51 100644 --- a/apply.h +++ b/apply.h @@ -97,11 +97,6 @@ struct apply_state { int applied_after_fixing_ws; }; -extern int parse_whitespace_option(struct apply_state *state, - const char *option); -extern int parse_ignorewhitespace_option(struct apply_state *state, -const char *option); - extern int apply_option_parse_exclude(const struct option *opt, const char *arg, int unset); extern int apply_option_parse_include(const struct option *opt, -- 2.9.2.769.gc0f0333 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 05/13] apply: use error_errno() where possible
To avoid possible mistakes and to uniformly show the errno related messages, let's use error_errno() where possible. Signed-off-by: Christian Couder --- apply.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/apply.c b/apply.c index c0cb3f5..41a33d3 100644 --- a/apply.c +++ b/apply.c @@ -3497,7 +3497,7 @@ static int load_current(struct apply_state *state, ce = active_cache[pos]; if (lstat(name, &st)) { if (errno != ENOENT) - return error(_("%s: %s"), name, strerror(errno)); + return error_errno("%s", name); if (checkout_target(&the_index, ce, &st)) return -1; } @@ -3647,7 +3647,7 @@ static int check_preimage(struct apply_state *state, } else if (!state->cached) { stat_ret = lstat(old_name, st); if (stat_ret && errno != ENOENT) - return error(_("%s: %s"), old_name, strerror(errno)); + return error_errno("%s", old_name); } if (state->check_index && !previous) { @@ -3669,7 +3669,7 @@ static int check_preimage(struct apply_state *state, } else if (stat_ret < 0) { if (patch->is_new < 0) goto is_new; - return error(_("%s: %s"), old_name, strerror(errno)); + return error_errno("%s", old_name); } if (!state->cached && !previous) @@ -3728,7 +3728,7 @@ static int check_to_create(struct apply_state *state, return EXISTS_IN_WORKTREE; } else if ((errno != ENOENT) && (errno != ENOTDIR)) { - return error("%s: %s", new_name, strerror(errno)); + return error_errno("%s", new_name); } return 0; } @@ -4247,9 +4247,9 @@ static int add_index_file(struct apply_state *state, if (!state->cached) { if (lstat(path, &st) < 0) { free(ce); - return error(_("unable to stat newly " - "created file '%s': %s"), -path, strerror(errno)); + return error_errno(_("unable to stat newly " +"created file '%s'"), + path); } fill_stat_cache_info(ce, &st); } @@ -4306,7 +4306,7 @@ static int try_create_file(const char *path, unsigned int mode, const char *buf, strbuf_release(&nbuf); if (close(fd) < 0 && !res) - return error(_("closing file '%s': %s"), path, strerror(errno)); + return error_errno(_("closing file '%s'"), path); return res ? -1 : 0; } @@ -4503,7 +4503,7 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch) rej = fopen(namebuf, "w"); if (!rej) - return error(_("cannot open %s: %s"), namebuf, strerror(errno)); + return error_errno(_("cannot open %s"), namebuf); /* Normal git tools never deal with .rej, so do not pretend * this is a git patch by saying --git or giving extended -- 2.9.2.769.gc0f0333 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v12 07/13] apply: don't print on stdout in verbosity_silent mode
When apply_verbosity is set to verbosity_silent nothing should be printed on both stderr and stdout. To avoid printing on stdout, we can just skip calling the following functions: - stat_patch_list(), - numstat_patch_list(), - summary_patch_list(). It is safe to do that because the above functions have no side effects other than printing: - stat_patch_list() only computes some local values and then call show_stats() and print_stat_summary(), those two functions only compute local values and call printing functions, - numstat_patch_list() also only computes local values and calls printing functions, - summary_patch_list() calls show_file_mode_name(), printf(), show_rename_copy(), show_mode_change() that are only printing. Signed-off-by: Christian Couder --- apply.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/apply.c b/apply.c index df85cbc..ddbb0a2 100644 --- a/apply.c +++ b/apply.c @@ -4702,13 +4702,13 @@ static int apply_patch(struct apply_state *state, goto end; } - if (state->diffstat) + if (state->diffstat && state->apply_verbosity > verbosity_silent) stat_patch_list(state, list); - if (state->numstat) + if (state->numstat && state->apply_verbosity > verbosity_silent) numstat_patch_list(state, list); - if (state->summary) + if (state->summary && state->apply_verbosity > verbosity_silent) summary_patch_list(list); end: -- 2.9.2.769.gc0f0333 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 9/9] status: unit tests for --porcelain=v2
Jeff Hostetler writes: > From: Jeff Hostetler > > Test porcelain v2 status format. > > Signed-off-by: Jeff Hostetler > --- > t/t7064-wtstatus-pv2.sh | 576 > > 1 file changed, 576 insertions(+) > create mode 100755 t/t7064-wtstatus-pv2.sh > > diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh > new file mode 100755 > index 000..44a8671 > --- /dev/null > +++ b/t/t7064-wtstatus-pv2.sh > @@ -0,0 +1,576 @@ > +#!/bin/sh > + > +test_description='git status --porcelain=v2 > + > +This test exercises porcelain V2 output for git status.' A general comment on the titles; with retitling of individual tests, the result has become a lot easier to understand. I know coming up with a short and to-the-point description for them is hard, but that is effort and time well spent and it shows in the result. Thanks. > +. ./test-lib.sh > + > +OID_EMPTY=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 It seems that test-lib.sh these days has EMPTY_BLOB defined for your use. You can remove this and replace its use (just two lines) with $EMPTY_BLOB down in the "add -N" test. > +test_expect_success setup ' > + test_tick && > + git config --local core.autocrlf false && I'd suggest removing "--local". Existing use of "git config" in the test suite, unless their use is about testing "git config" itself to validate the operation of the --local/--global/--system options, do not seem to explicitly say "--local", which is the default anyway. > +test_expect_success 'after first commit, make dirt, confirm unstaged > changes' ' Did you mean s/dirt/dirty/? "make and confirm unstaged changes" would be sufficient. Because "confirming" is implicit (as these are all tests), "after the first commit, modify working tree files" might even be better, perhaps? > + echo x >>file_x && > + OID_X1=$(git hash-object -t blob -- file_x) && > + rm file_z && > + H0=$(git rev-parse HEAD) && > + ... > +test_expect_success 'after first commit, stage dirt, confirm staged changes' > ' What you "git add" is meant to be good changes, so they are no longer dirt ;-) More importantly, because I never heard of "dirt" used in Git context, it is unclear if it is an untracked file, a modification that is not meant to be committed immediately, or what. "after the first commit, fully add changes to the index"? > + git add file_x && > + git rm file_z && > + H0=$(git rev-parse HEAD) && > + > + cat >expect <<-EOF && > + # branch.oid $H0 > + # branch.head master > + 1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x > + 1 D. N... 100644 00 00 $OID_Z $_z40 file_z > + ? actual > + ? expect > + EOF > +test_expect_success 'after first commit, also stage rename, confirm 2 path > line format' ' > + git mv file_y renamed_y && > + H0=$(git rev-parse HEAD) && > + > + q_to_tab >expect <<-EOF && > + # branch.oid $H0 > + # branch.head master > + 1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x > + 1 D. N... 100644 00 00 $OID_Z $_z40 file_z > + 2 R. N... 100644 100644 100644 $OID_Y $OID_Y R100 renamed_yQfile_y > + ? actual > + ? expect > + EOF > + > + git status --porcelain=v2 --branch --untracked-files=all >actual && > + test_cmp expect actual > +' Do we want to test -z format on this, too? > ... > +test_expect_success 'create ignored files, confirm they are not printed' ' > +test_expect_success 'create ignored files, confirm --ignored prints them' ' > ... These are all good and readably titled. > +test_expect_success 'verify upstream fields in branch header' ' > + git checkout master && > + test_when_finished rm -rf sub_repo && Forgot to quote? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/2] diff: add SUBMODULE_DIFF format to display submodule diff
On Thu, Aug 11, 2016 at 10:53 AM, Junio C Hamano wrote: > Jacob Keller writes: > >> From: Jacob Keller >> >> Teach git-diff and friends a new format for displaying the difference of >> a submodule using git-diff inside the submodule project. This allows >> users to easily see exactly what source changed in a given commit that >> updates the submodule pointer. To do this, remove DIFF_SUBMODULE_LOG bit >> from the diff options, and instead add a new enum type for these >> formats. >> >> Signed-off-by: Jacob Keller >> --- >> Documentation/diff-config.txt | 3 +- >> Documentation/diff-options.txt | 6 ++-- >> diff.c | 41 -- >> diff.h | 9 +- >> submodule.c| 67 >> ++ >> submodule.h| 6 >> 6 files changed, 113 insertions(+), 19 deletions(-) > > This looks good. > > You'd want some tests to make sure that the "--submodule" and the > "--submodule=" command line options and the diff.submodule > configuration variables are parsed correctly (and when combined, the > command line option overrides the configured default), and of course > the machinery does the right thing, with and without "--graph" when > used in "git log". Yes, I am adding tests now, but I ran into some interesting corner cases for this, that still need some work. There's a bunch of issues with cases involving adding a submodule that isn't stored in .git/modules/etc, which the current tests for --submodule=log do. There's also the case of empty trees, which I believe I have resolved now. Hopefully I can sort these cases correctly. > >> diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt >> index e7b729f3644f..988068225463 100644 >> --- a/Documentation/diff-options.txt >> +++ b/Documentation/diff-options.txt >> @@ -215,8 +215,10 @@ any of those replacements occurred. >> the commits in the range like linkgit:git-submodule[1] `summary` does. >> Omitting the `--submodule` option or specifying `--submodule=short`, >> uses the 'short' format. This format just shows the names of the >> commits >> - at the beginning and end of the range. Can be tweaked via the >> - `diff.submodule` configuration variable. >> + at the beginning and end of the range. When `--submodule=diff` is >> + given, the 'diff' format is used. This format shows the diff between >> + the old and new submodule commmit from the perspective of the >> + submodule. Can be tweaked via the `diff.submodule` configuration >> variable. > > This is carried over from the existing text, but "Can be tweaked > via" sounds a bit wasteful (and strange); "Defaults to" (or "The > default is taken from") is the phrase we seem to use more often. Probably worth fixing here. WIll do. Thanks, Jake > > Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] diff: add --line-prefix option for passing in a prefix
On Thu, Aug 11, 2016 at 10:22 AM, Junio C Hamano wrote: > -- >8 -- > Subject: diff.c: remove output_prefix_length field > > "diff/log --stat" has a logic that determines the display columns > available for the diffstat part of the output and apportions it for > pathnames and diffstat graph automatically. > > 5e71a84a (Add output_prefix_length to diff_options, 2012-04-16) > added the output_prefix_length field to diff_options structure to > allow this logic subtract the display columns used for display the > history graph part from the total "terminal width"; this matters > when the "git log --graph -p" option is in use. > > The field be set to the number of display columns needed to show the > output from the output_prefix() callback. Any new output_prefix() > callback must also update the field accordingly, which is error > prone. As there is only one user of the field, and the user has the > actual value of the prefix string, let's get rid of the field and > have the user count the display width itself. > Seems correct to me. Thanks, Jake -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] checkout: do not mention detach advice for explicit --detach option
Stefan Beller writes: > On Wed, Aug 10, 2016 at 10:32 AM, Jonathan Nieder wrote: >> Stefan Beller wrote: >> >>> When a user asked for a detached HEAD specifically with `--detach`, >>> we do not need to give advice on what a detached HEAD state entails as >>> we can assume they know what they're getting into as they asked for it. >> >> Example? Tests? > > There are no tests for the advice things IIUC. There seem to already be tests that explicitly sets advice.* to true like in t7201 and t7512, but even if there weren't any existing ones, it would be appropriate to make sure that an explicit --detach does not trigger the advice, even when advice.detachedHEAD left unconfigured (or set to true), given that doing so is the whole purpose of this patch, I would think. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Can cc's be included in patches sent by send-email
On Thu, Aug 11, 2016 at 10:57 AM, Junio C Hamano wrote: > > Also, those who do not want to see Cc: in headers (like me) can Stupid typo that ruins the whole message. I meant "in FOOTERS". > instead edit the header part of the format-patch output to add Cc: > lines and they should be picked up. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Can cc's be included in patches sent by send-email
Jacob Keller writes: > On Thu, Aug 11, 2016 at 12:58 AM, Jacob Keller wrote: >> On Thu, Aug 11, 2016 at 12:32 AM, Philip Oakley wrote: >>> While 'git send-email' can have multiple --cc="addressee" options on the >>> command line, is it possible for the "cc:addressee" to actually be >>> included in the patches that are to be sent, so that different patches can >>> have different addressee? >>> >>> The fortmat-patch can include appropriate from lines, so it feels as if the >>> sender should be able to add cc: lines at the same place. Maybe its just >>> part of th docs I've missed. >>> >> >> Yes, just put them in the body as tags below the signed-off-by. It >> should be on by default unless you change supresscc or signedoffbycc >> configuration. >> >> Thanks, >> Jake >> > > See --suppress-cc or --signed-off-by-cc help in git help send-email. Also, those who do not want to see Cc: in headers (like me) can instead edit the header part of the format-patch output to add Cc: lines and they should be picked up. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [ANNOUNCE] tig-2.2
Jonas Fonseca writes: > I've just released the 35th release of Tig. It brings several search > improvements such as highlighting and wrap around, and machinery for future > support of typeahead search. This release also gives more choice over how the > user configuration file is loaded either at built-time or at runtime through > support of the XDG basedir spec. Among fixes several segfaults and invalid > reads have been addressed and the tests are now run with Valgrind and > AddressSanitizer by Travis on each push. There are several breaking changes so > ensure you read the section on incompatibilities in the release notes before > upgrading. Nice. Thanks for your continued effort on this little Gem. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 2/2] diff: add SUBMODULE_DIFF format to display submodule diff
Jacob Keller writes: > From: Jacob Keller > > Teach git-diff and friends a new format for displaying the difference of > a submodule using git-diff inside the submodule project. This allows > users to easily see exactly what source changed in a given commit that > updates the submodule pointer. To do this, remove DIFF_SUBMODULE_LOG bit > from the diff options, and instead add a new enum type for these > formats. > > Signed-off-by: Jacob Keller > --- > Documentation/diff-config.txt | 3 +- > Documentation/diff-options.txt | 6 ++-- > diff.c | 41 -- > diff.h | 9 +- > submodule.c| 67 > ++ > submodule.h| 6 > 6 files changed, 113 insertions(+), 19 deletions(-) This looks good. You'd want some tests to make sure that the "--submodule" and the "--submodule=" command line options and the diff.submodule configuration variables are parsed correctly (and when combined, the command line option overrides the configured default), and of course the machinery does the right thing, with and without "--graph" when used in "git log". > diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt > index e7b729f3644f..988068225463 100644 > --- a/Documentation/diff-options.txt > +++ b/Documentation/diff-options.txt > @@ -215,8 +215,10 @@ any of those replacements occurred. > the commits in the range like linkgit:git-submodule[1] `summary` does. > Omitting the `--submodule` option or specifying `--submodule=short`, > uses the 'short' format. This format just shows the names of the commits > - at the beginning and end of the range. Can be tweaked via the > - `diff.submodule` configuration variable. > + at the beginning and end of the range. When `--submodule=diff` is > + given, the 'diff' format is used. This format shows the diff between > + the old and new submodule commmit from the perspective of the > + submodule. Can be tweaked via the `diff.submodule` configuration > variable. This is carried over from the existing text, but "Can be tweaked via" sounds a bit wasteful (and strange); "Defaults to" (or "The default is taken from") is the phrase we seem to use more often. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 28/40] builtin/apply: rename option parsing functions
On Thu, Aug 11, 2016 at 10:58 AM, wrote: > Am 11.08.2016 um 10:52 schrieb Christian Couder: >> As these functions are going to be part of the libified >> apply api, let's give them a name that is more specific > > s/api/API/ > > ;-) Ooops. Anyway as this is patch 28/40 and the other problem you found is in 40/40, I will just resend patches from 28/40 to 40/40 in v12 (so only the last 13 patches, that I will call part 3 of the whole series). Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 1/2] diff: add --line-prefix option for passing in a prefix
Jacob Keller writes: > const char *diff_line_prefix(struct diff_options *opt) > { > struct strbuf *msgbuf; > + > if (!opt->output_prefix) > - return ""; > + if (opt->line_prefix) > + return opt->line_prefix; > + else > + return ""; > > msgbuf = opt->output_prefix(opt, opt->output_prefix_data); > + /* line prefix must be printed before the output_prefix() */ > + if (opt->line_prefix) > + strbuf_insert(msgbuf, 0, opt->line_prefix, > strlen(opt->line_prefix)); > return msgbuf->buf; > } The result of applying this change leaves the semantics of the line_prefix field, the output_prefix() callback, and the output_prefix_length field in the diff_options structure a bit tricky to reason about. The code pretends as if the remainder of the system does not even care about the presence of line_prefix (i.e. output_prefix_length is not updated), but its only user of output_prefix_length cares, I think. It is in diff.c::show_stats() where it auto-computes the allowed display width for the stat portion: if (options->stat_width == -1) width = term_columns() - options->output_prefix_length; The output_prefix_length is initialized to be 0, but when --graph is in effect, it is set to the width of the graph portion of the output in the output_prefix callback, diff_output_prefix_callback(). So the above change is clearly wrong in that it needs to add the number of display columns needed to show opt->line_prefix to the output_prefix_length, but I wonder if a better "fix" to this is to get rid of output_prefix_length field from diff_options struct as a preparatory step. That would make the bug in this patch disappear. Perhaps like this. I do not know if Lucian is still interested in, or remembers what did for, Git in 2012, but this updates his code and replaces it with what I hope is an equivalent, so I added him to the Cc: line. -- >8 -- Subject: diff.c: remove output_prefix_length field "diff/log --stat" has a logic that determines the display columns available for the diffstat part of the output and apportions it for pathnames and diffstat graph automatically. 5e71a84a (Add output_prefix_length to diff_options, 2012-04-16) added the output_prefix_length field to diff_options structure to allow this logic subtract the display columns used for display the history graph part from the total "terminal width"; this matters when the "git log --graph -p" option is in use. The field be set to the number of display columns needed to show the output from the output_prefix() callback. Any new output_prefix() callback must also update the field accordingly, which is error prone. As there is only one user of the field, and the user has the actual value of the prefix string, let's get rid of the field and have the user count the display width itself. Signed-off-by: Junio C Hamano --- diff.c | 2 +- diff.h | 1 - graph.c | 2 -- 3 files changed, 1 insertion(+), 4 deletions(-) diff --git a/diff.c b/diff.c index b43d3dd..ae069c3 100644 --- a/diff.c +++ b/diff.c @@ -1625,7 +1625,7 @@ static void show_stats(struct diffstat_t *data, struct diff_options *options) */ if (options->stat_width == -1) - width = term_columns() - options->output_prefix_length; + width = term_columns() - strlen(line_prefix); else width = options->stat_width ? options->stat_width : 80; number_width = decimal_width(max_change) > number_width ? diff --git a/diff.h b/diff.h index 125447b..49e4aaa 100644 --- a/diff.h +++ b/diff.h @@ -174,7 +174,6 @@ struct diff_options { diff_format_fn_t format_callback; void *format_callback_data; diff_prefix_fn_t output_prefix; - int output_prefix_length; void *output_prefix_data; int diff_path_counter; diff --git a/graph.c b/graph.c index dd17201..a468038 100644 --- a/graph.c +++ b/graph.c @@ -197,7 +197,6 @@ static struct strbuf *diff_output_prefix_callback(struct diff_options *opt, void assert(opt); assert(graph); - opt->output_prefix_length = graph->width; strbuf_reset(&msgbuf); graph_padding_line(graph, &msgbuf); return &msgbuf; @@ -245,7 +244,6 @@ struct git_graph *graph_init(struct rev_info *opt) */ opt->diffopt.output_prefix = diff_output_prefix_callback; opt->diffopt.output_prefix_data = graph; - opt->diffopt.output_prefix_length = 0; return graph; } -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] gc: default aggressive depth to 50
On Thu, Aug 11, 2016 at 12:13:09PM -0400, Jeff King wrote: > Here are the numbers for linux.git: > >depth | size | %| rev-list | % | log -Sfoo | % > ---+---+---+--++---+--- > 250 | 967MB | n/a | 48.159s | n/a | 378.088 | n/a > 100 | 971MB | +0.4% | 41.471s | -13.9% | 342.060 | -9.5% > 50 | 979MB | +1.2% | 37.778s | -21.6% | 311.040s | -17.7% > 10 | 1.1GB | +6.6% | 32.518s | -32.5% | 279.890s | -25.9% > [...] > > You can see that that the CPU savings for regular operations improves as we > decrease the depth. The savings are less for "rev-list" on a smaller > repository > than they are for blob-accessing operations, or even rev-list on a larger > repository. This may mean that a larger delta cache would help (though setting > core.deltaBaseCacheLimit by itself doesn't). The problem with deltaBaseCacheLimit is that it only changes the memory parameter, but there are a fixed number of slots in the data structure. Bumping it like this: diff --git a/sha1_file.c b/sha1_file.c index 02940f1..ca79703 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -2073,7 +2073,7 @@ static void *unpack_compressed_entry(struct packed_git *p, return buffer; } -#define MAX_DELTA_CACHE (256) +#define MAX_DELTA_CACHE (1024) static size_t delta_base_cached; along with the cache size does help (this was discussed a year or two ago, but nobody ever followed up with numbers or patches). Here are best-of-3 numbers for rev-list on linux.git at each depth with that patch and "-c core.deltabasecachelimit=256m": depth | time 250 | 36.524s 100 | 33.200s 50 | 31.065s 10 | 28.266s So there's clearly a lot of room for improvement on the reading side in general. And doing so clearly lessens the impact of the delta chains. But you still get a 15% improvement moving to depth=50, versus only a 1.2% storage increase. So I don't think it fundamentally changes the conclusion of the "depth=50" patch I'm responding to. I don't think bumping MAX_DELTA_CACHE naively is a good idea, though. I seem to recall that it has scaling problems as it grows, so we may want a better data structure (but I haven't looked at it recently enough to say anything intelligent). I might work on it in the near future, but if anybody is interested, please be my guest. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] pack-objects mru
On Thu, Aug 11, 2016 at 08:11:33AM -0700, Junio C Hamano wrote: > Jeff King writes: > > > So considering "--depth" as a space-saving measure for --aggressive does > > not seem that effective. But it feels weird to quietly drop actions > > people might have done with previous aggressive runs. > > That argument cuts both ways, doesn't it? > > If the user explicitly asks to use lower "--depth" from the command > line when the second repack runs, the intention is clear: the > existing pack may use delta chains that are too long and is > detrimental to the run-time performance, and the user wants to > correct it by repacking with shorter delta chain. > > Should the act of letting "gc --auto" use lower "--depth", by not > configuring to always use deeper chain, be interpreted the same way? > I am not sure. The old packing with large --depth is something the > user did long time ago, and the decision the user made not to use > large depth always is also something the user did long time ago. I > do not think it is so cut-and-dried which one of the two conflicting > wishes we should honor when running the second repack, especially > when it is run unattended like "gc --auto" does. Good points. Explicitly saying "repack --depth=..." carries a lot more weight to me than "git gc --auto" randomly kicking in, as far as knowing that what the user actually wants. My patch doesn't differentiate, of course, but I think it could. The other problem with my patch is the fact that we don't do a good job of finding new, in-limit deltas for the ones we discard. If you want to do that, you really need to "git repack -f" (at least with the current code). At which point we do not reuse the on-disk deltas at all, and the problem is moot (you could also interpret the fact that the user did _not_ pass "-f" as "you want to reuse deltas, which means you want to reuse even long chains", but as you've argued above, you can make a lot of guesses about the user's intention from what they did or did not say). So if we were to go this route, I don't think my patch is quite sufficient; we'd want something else on top to do a better job of finding replacement deltas. Regarding my "does not seem that effective" above, I think we should drop the aggressive depth to 50, and I just posted a patch with reasoning and numbers: http://public-inbox.org/git/20160811161309.ecmebaafcz6rk...@sigill.intra.peff.net/ That's maybe orthogonal, but it does remove the weird "gc --aggressive followed by gc --auto produces a bad pack" issue, because unless you are doing something clever, the depth will always be 50 (modulo people who did an aggressive pack with an older version of git :-/ ). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gc: default aggressive depth to 50
This commit message is long and has lots of background and numbers. The summary is: the current default of 250 doesn't save much space, and costs CPU. It's not a good tradeoff. Read on for details. The "--aggressive" flag to git-gc does three things: 1. use "-f" to throw out existing deltas and recompute from scratch 2. use "--window=250" to look harder for deltas 3. use "--depth=250" to make longer delta chains Items (1) and (2) are good matches for an "aggressive" repack. They ask the repack to do more computation work in the hopes of getting a better pack. You pay the costs during the repack, and other operations see only the benefit. Item (3) is not so clear. Allowing longer chains means fewer restrictions on the deltas, which means potentially finding better ones and saving some space. But it also means that operations which access the deltas have to follow longer chains, which affects their performance. So it's a tradeoff, and it's not clear that the tradeoff is even a good one. The existing "250" numbers for "--aggressive" come originally from this thread: http://public-inbox.org/git/alpine.lfd.0..0712060803430.13...@woody.linux-foundation.org/ where Linus says: So when I said "--depth=250 --window=250", I chose those numbers more as an example of extremely aggressive packing, and I'm not at all sure that the end result is necessarily wonderfully usable. It's going to save disk space (and network bandwidth - the delta's will be re-used for the network protocol too!), but there are definitely downsides too, and using long delta chains may simply not be worth it in practice. There are some numbers in that thread, but they're mostly focused on the improved window size, and measure the improvement from --depth=250 and --window=250 together. E.g.: http://public-inbox.org/git/9e4733910712062006l651571f3w7f76ce64c6650...@mail.gmail.com/ talks about the improved run-time of "git-blame", which comes from the reduced pack size. But most of that reduction is coming from --window=250, whereas most of the extra costs come from --depth=250. There's a link in that thread showing that increasing the depth beyond 50 doesn't seem to help much with the size: https://vcscompare.blogspot.com/2008/06/git-repack-parameters.html but again, no discussion of the timing impact. In an earlier thread from Ted Ts'o which discussed setting the non-aggressive default (from 10 to 50): http://public-inbox.org/git/20070509134958.GA21489%40thunk.org/ we have more numbers, with the conclusion that going past 50 does not help size much, and hurts the speed of normal operations. So from that, we might guess that 50 is actually a sweet spot, even for aggressive, if we interpret aggressive to "spend time now to make a better pack". It is not clear that "--depth=250" is actually a better pack. It may be slightly _smaller_, but it carries a run-time penalty. Here are some more recent timings I did to verify that. They show three things: - the size of the resulting pack (so disk saved to store, bandwidth saved on clones/fetches) - the cost of "rev-list --objects --all", which shows the effect of the delta chains on trees (commits typically don't delta, and the command doesn't touch the blobs at all) - the cost of "log -Sfoo", which will additionally access each blob All cases were repacked with "git repack -adf --depth=$d --window=250" (so basically, what would happen if we tweaked the "gc --aggressive" default depth). The timings are all wall-clock best-of-3. The machine itself has plenty of RAM compared to the repositories (which is probably typical of most workstations these days), so we're really measuring CPU usage, as the whole thing will be in disk cache after the first run. The core.deltaBaseCacheLimit is at its default of 96MiB. It's possible that tweaking it would have some impact on the tests, as some of them (especially "log -S" on a large repo) are likely to overflow that. But bumping that carries a run-time memory cost, so for these tests, I focused on what we could do just with the on-disk pack tradeoffs. Each test is done for four depths: 250 (the current value), 50 (the current default that tested well previously), 100 (to show something on the larger side, which previous tests showed was not a good tradeoff), and 10 (the very old default, which previous tests showed was worse than 50). Here are the numbers for linux.git: depth | size | %| rev-list | % | log -Sfoo | % ---+---+---+--++---+--- 250 | 967MB | n/a | 48.159s | n/a | 378.088 | n/a 100 | 971MB | +0.4% | 41.471s | -13.9% | 342.060 | -9.5% 50 | 979MB | +1.2% | 37.778s | -21.6% | 311.040s | -17.7% 10 | 1.1GB | +6.6% | 32.518s | -32.5% | 279.890s | -25.9% and for git.git: depth | size | %| rev-list | % | log -Sfoo | % ---+---+---+--+
Re: [PATCH v5] pack-objects mru
Jeff King writes: > So considering "--depth" as a space-saving measure for --aggressive does > not seem that effective. But it feels weird to quietly drop actions > people might have done with previous aggressive runs. That argument cuts both ways, doesn't it? If the user explicitly asks to use lower "--depth" from the command line when the second repack runs, the intention is clear: the existing pack may use delta chains that are too long and is detrimental to the run-time performance, and the user wants to correct it by repacking with shorter delta chain. Should the act of letting "gc --auto" use lower "--depth", by not configuring to always use deeper chain, be interpreted the same way? I am not sure. The old packing with large --depth is something the user did long time ago, and the decision the user made not to use large depth always is also something the user did long time ago. I do not think it is so cut-and-dried which one of the two conflicting wishes we should honor when running the second repack, especially when it is run unattended like "gc --auto" does. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 8/9] test-lib-functions.sh: Add lf_to_nul
From: Jeff Hostetler Add lf_to_nul() function to test-lib-functions. Signed-off-by: Jeff Hostetler --- t/test-lib-functions.sh | 4 1 file changed, 4 insertions(+) diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 4f7eadb..fdaeb3a 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -81,6 +81,10 @@ test_decode_color () { ' } +lf_to_nul () { + perl -pe 'y/\012/\000/' +} + nul_to_q () { perl -pe 'y/\000/Q/' } -- 2.8.0.rc4.17.gac42084.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 7/9] git-status.txt: describe --porcelain=v2 format
From: Jeff Hostetler Update status manpage to include information about porcelain v2 format. Signed-off-by: Jeff Hostetler --- Documentation/git-status.txt | 126 +-- 1 file changed, 122 insertions(+), 4 deletions(-) diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt index 6b1454b..a58973b 100644 --- a/Documentation/git-status.txt +++ b/Documentation/git-status.txt @@ -183,12 +183,12 @@ in which case `XY` are `!!`. If -b is used the short-format status is preceded by a line -## branchname tracking info +## branchname tracking info -Porcelain Format - +Porcelain Format Version 1 +~~ -The porcelain format is similar to the short format, but is guaranteed +Version 1 porcelain format is similar to the short format, but is guaranteed not to change in a backwards-incompatible way between Git versions or based on user configuration. This makes it ideal for parsing by scripts. The description of the short format above also describes the porcelain @@ -210,6 +210,124 @@ field from the first filename). Third, filenames containing special characters are not specially formatted; no quoting or backslash-escaping is performed. +Porcelain Format Version 2 +~~ + +Version 2 format adds more detailed information about the state of +the worktree and changed items. Version 2 also defines an extensible +set of easy to parse optional headers. + +Header lines start with "#" and are added in response to specific +command line arguments. Parsers should ignore headers they +don't recognize. + +### Branch Headers + +If `--branch` is given, a series of header lines are printed with +information about the current branch. + +Line Notes + +# branch.oid | (initial)Current commit. +# branch.head | (detached) Current branch. +# branch.upstream If upstream is set. +# branch.ab + - If upstream is set and + the commit is present. + + +### Changed Tracked Entries + +Following the headers, a series of lines are printed for tracked +entries. One of three different line formats may be used to describe +an entry depending on the type of change. Tracked entries are printed +in an undefined order; parsers should allow for a mixture of the 3 +line types in any order. + +Ordinary changed entries have the following format: + +1 + +Renamed or copied entries have the following format: + +2 + +Field Meaning + +A 2 character field containing the staged and +unstaged XY values described in the short format, +with unchanged indicated by a "." rather than +a space. + A 4 character field describing the submodule state. +"N..." when the entry is not a submodule. +"S" when the entry is a submodule. + is "C" if the commit changed; otherwise ".". + is "M" if it has tracked changes; otherwise ".". + is "U" if there are untracked changes; otherwise ".". +The octal file mode in HEAD. +The octal file mode in the index. +The octal file mode in the worktree. +The object name in HEAD. +The object name in the index. + The rename or copy score (denoting the percentage +of similarity between the source and target of the +move or copy). For example "R100" or "C75". + The pathname. In a renamed/copied entry, this +is the path in the index and in the working tree. + When the `-z` option is used, the 2 pathnames are separated +with a NUL (ASCII 0x00) byte; otherwise, a tab (ASCII 0x09) +byte separates them. + The pathname in the commit at HEAD. This is only +present in a renamed/copied entry, and tells +where the renamed/copied contents came from. + + +Unmerged entries have the following format; the first character is +a "u" to distinguish from ordinary changed entries. + +u + +Field Meaning + +A 2 character field describing the conflict type +as described in the short format. + A 4 character field describing the submodule state +as described above. +The octal file mode in stage 1. +The octal file mode in stage 2. +The octal file mode in stage 3. +The octal file mode in the worktree. +
[PATCH v6 5/9] status: print per-file porcelain v2 status data
From: Jeff Hostetler Print per-file information in porcelain v2 format. Signed-off-by: Jeff Hostetler --- wt-status.c | 285 +++- 1 file changed, 284 insertions(+), 1 deletion(-) diff --git a/wt-status.c b/wt-status.c index 904b5c2..03c8e23 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1812,6 +1812,289 @@ static void wt_porcelain_print(struct wt_status *s) wt_shortstatus_print(s); } +/* + * Convert various submodule status values into a + * fixed-length string of characters in the buffer provided. + */ +static void wt_porcelain_v2_submodule_state( + struct wt_status_change_data *d, + char sub[5]) +{ + if (S_ISGITLINK(d->mode_head) || + S_ISGITLINK(d->mode_index) || + S_ISGITLINK(d->mode_worktree)) { + sub[0] = 'S'; + sub[1] = d->new_submodule_commits ? 'C' : '.'; + sub[2] = (d->dirty_submodule & DIRTY_SUBMODULE_MODIFIED) ? 'M' : '.'; + sub[3] = (d->dirty_submodule & DIRTY_SUBMODULE_UNTRACKED) ? 'U' : '.'; + } else { + sub[0] = 'N'; + sub[1] = '.'; + sub[2] = '.'; + sub[3] = '.'; + } + sub[4] = 0; +} + +/* + * Fix-up changed entries before we print them. + */ +static void wt_porcelain_v2_fix_up_changed( + struct string_list_item *it, + struct wt_status *s) +{ + struct wt_status_change_data *d = it->util; + + if (!d->index_status) { + /* +* This entry is unchanged in the index (relative to the head). +* Therefore, the collect_updated_cb was never called for this +* entry (during the head-vs-index scan) and so the head column +* fields were never set. +* +* We must have data for the index column (from the +* index-vs-worktree scan (otherwise, this entry should not be +* in the list of changes)). +* +* Copy index column fields to the head column, so that our +* output looks complete. +*/ + assert(d->mode_head == 0); + d->mode_head = d->mode_index; + oidcpy(&d->oid_head, &d->oid_index); + } + + if (!d->worktree_status) { + /* +* This entry is unchanged in the worktree (relative to the index). +* Therefore, the collect_changed_cb was never called for this entry +* (during the index-vs-worktree scan) and so the worktree column +* fields were never set. +* +* We must have data for the index column (from the head-vs-index +* scan). +* +* Copy the index column fields to the worktree column so that +* our output looks complete. +* +* Note that we only have a mode field in the worktree column +* because the scan code tries really hard to not have to compute it. +*/ + assert(d->mode_worktree == 0); + d->mode_worktree = d->mode_index; + } +} + +/* + * Print porcelain v2 info for tracked entries with changes. + */ +static void wt_porcelain_v2_print_changed_entry( + struct string_list_item *it, + struct wt_status *s) +{ + struct wt_status_change_data *d = it->util; + struct strbuf buf_index = STRBUF_INIT; + struct strbuf buf_head = STRBUF_INIT; + const char *path_index = NULL; + const char *path_head = NULL; + char key[3]; + char submodule_token[5]; + char sep_char, eol_char; + + wt_porcelain_v2_fix_up_changed(it, s); + wt_porcelain_v2_submodule_state(d, submodule_token); + + key[0] = d->index_status ? d->index_status : '.'; + key[1] = d->worktree_status ? d->worktree_status : '.'; + key[2] = 0; + + if (s->null_termination) { + /* +* In -z mode, we DO NOT C-quote pathnames. Current path is ALWAYS first. +* A single NUL character separates them. +*/ + sep_char = '\0'; + eol_char = '\0'; + path_index = it->string; + path_head = d->head_path; + } else { + /* +* Path(s) are C-quoted if necessary. Current path is ALWAYS first. +* The source path is only present when necessary. +* A single TAB separates them (because paths can contain spaces +* which are not escaped and C-quoting does escape TAB characters). +*/ + sep_char = '\t'; + eol_char = '\n'; + path_index = quote_path(it->string, s->prefix, &buf_index); + if (d->head_path) + path_head
[PATCH v6 9/9] status: unit tests for --porcelain=v2
From: Jeff Hostetler Test porcelain v2 status format. Signed-off-by: Jeff Hostetler --- t/t7064-wtstatus-pv2.sh | 576 1 file changed, 576 insertions(+) create mode 100755 t/t7064-wtstatus-pv2.sh diff --git a/t/t7064-wtstatus-pv2.sh b/t/t7064-wtstatus-pv2.sh new file mode 100755 index 000..44a8671 --- /dev/null +++ b/t/t7064-wtstatus-pv2.sh @@ -0,0 +1,576 @@ +#!/bin/sh + +test_description='git status --porcelain=v2 + +This test exercises porcelain V2 output for git status.' + + +. ./test-lib.sh + +OID_EMPTY=e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 + +test_expect_success setup ' + test_tick && + git config --local core.autocrlf false && + echo x >file_x && + echo y >file_y && + echo z >file_z && + mkdir dir1 && + echo a >dir1/file_a && + echo b >dir1/file_b +' + +test_expect_success 'before initial commit, nothing added, only untracked' ' + cat >expect <<-EOF && + # branch.oid (initial) + # branch.head master + ? actual + ? dir1/ + ? expect + ? file_x + ? file_y + ? file_z + EOF + + git status --porcelain=v2 --branch --untracked-files=normal >actual && + test_cmp expect actual +' + +test_expect_success 'before initial commit, things added' ' + git add file_x file_y file_z dir1 && + OID_A=$(git hash-object -t blob -- dir1/file_a) && + OID_B=$(git hash-object -t blob -- dir1/file_b) && + OID_X=$(git hash-object -t blob -- file_x) && + OID_Y=$(git hash-object -t blob -- file_y) && + OID_Z=$(git hash-object -t blob -- file_z) && + + cat >expect <<-EOF && + # branch.oid (initial) + # branch.head master + 1 A. N... 00 100644 100644 $_z40 $OID_A dir1/file_a + 1 A. N... 00 100644 100644 $_z40 $OID_B dir1/file_b + 1 A. N... 00 100644 100644 $_z40 $OID_X file_x + 1 A. N... 00 100644 100644 $_z40 $OID_Y file_y + 1 A. N... 00 100644 100644 $_z40 $OID_Z file_z + ? actual + ? expect + EOF + + git status --porcelain=v2 --branch --untracked-files=all >actual && + test_cmp expect actual +' + +test_expect_success 'before initial commit, things added (-z)' ' + lf_to_nul >expect <<-EOF && + # branch.oid (initial) + # branch.head master + 1 A. N... 00 100644 100644 $_z40 $OID_A dir1/file_a + 1 A. N... 00 100644 100644 $_z40 $OID_B dir1/file_b + 1 A. N... 00 100644 100644 $_z40 $OID_X file_x + 1 A. N... 00 100644 100644 $_z40 $OID_Y file_y + 1 A. N... 00 100644 100644 $_z40 $OID_Z file_z + ? actual + ? expect + EOF + + git status -z --porcelain=v2 --branch --untracked-files=all >actual && + test_cmp expect actual +' + +test_expect_success 'make first commit, comfirm HEAD oid and branch' ' + git commit -m initial && + H0=$(git rev-parse HEAD) && + cat >expect <<-EOF && + # branch.oid $H0 + # branch.head master + ? actual + ? expect + EOF + + git status --porcelain=v2 --branch --untracked-files=all >actual && + test_cmp expect actual +' + +test_expect_success 'after first commit, make dirt, confirm unstaged changes' ' + echo x >>file_x && + OID_X1=$(git hash-object -t blob -- file_x) && + rm file_z && + H0=$(git rev-parse HEAD) && + + cat >expect <<-EOF && + # branch.oid $H0 + # branch.head master + 1 .M N... 100644 100644 100644 $OID_X $OID_X file_x + 1 .D N... 100644 100644 00 $OID_Z $OID_Z file_z + ? actual + ? expect + EOF + + git status --porcelain=v2 --branch --untracked-files=all >actual && + test_cmp expect actual +' + +test_expect_success 'confirm again unstaged changes, hiding untracked files' ' + cat >expect <<-EOF && + 1 .M N... 100644 100644 100644 $OID_X $OID_X file_x + 1 .D N... 100644 100644 00 $OID_Z $OID_Z file_z + EOF + + git status --porcelain=v2 --untracked-files=no >actual && + test_cmp expect actual +' + +test_expect_success 'after first commit, stage dirt, confirm staged changes' ' + git add file_x && + git rm file_z && + H0=$(git rev-parse HEAD) && + + cat >expect <<-EOF && + # branch.oid $H0 + # branch.head master + 1 M. N... 100644 100644 100644 $OID_X $OID_X1 file_x + 1 D. N... 100644 00 00 $OID_Z $_z40 file_z + ? actual + ? expect + EOF + + git status --porcelain=v2 --branch --untracked-files=all >actual && + test_cmp expect actual +' + +test_expect_success 'after first commit, also stage rename, confirm 2 path line format' ' + git mv file_y renamed_y && + H0=$(git rev-parse HEAD) && + + q_to_tab >expect <<-EOF && + # branch.oid $H0 + # branch.head master + 1 M. N... 100644 100644 100644 $OID_
[PATCH v6 4/9] status: collect per-file data for --porcelain=v2
From: Jeff Hostetler Collect extra per-file data for porcelain V2 format. The output of `git status --porcelain` leaves out many details about the current status that clients might like to have. This can force them to be less efficient as they may need to launch secondary commands (and try to match the logic within git) to accumulate this extra information. For example, a GUI IDE might want the file mode to display the correct icon for a changed item (without having to stat it afterwards). Signed-off-by: Jeff Hostetler --- builtin/commit.c | 3 +++ wt-status.c | 64 ++-- wt-status.h | 4 3 files changed, 69 insertions(+), 2 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 185ac35..3d222d3 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -153,6 +153,8 @@ static int opt_parse_porcelain(const struct option *opt, const char *arg, int un *value = STATUS_FORMAT_PORCELAIN; else if (!strcmp(arg, "v1") || !strcmp(arg, "1")) *value = STATUS_FORMAT_PORCELAIN; + else if (!strcmp(arg, "v2") || !strcmp(arg, "2")) + *value = STATUS_FORMAT_PORCELAIN_V2; else die("unsupported porcelain version '%s'", arg); @@ -1104,6 +1106,7 @@ static struct status_deferred_config { static void finalize_deferred_config(struct wt_status *s) { int use_deferred_config = (status_format != STATUS_FORMAT_PORCELAIN && + status_format != STATUS_FORMAT_PORCELAIN_V2 && !s->null_termination); if (s->null_termination) { diff --git a/wt-status.c b/wt-status.c index a9031e4..904b5c2 100644 --- a/wt-status.c +++ b/wt-status.c @@ -434,6 +434,31 @@ static void wt_status_collect_changed_cb(struct diff_queue_struct *q, if (S_ISGITLINK(p->two->mode)) d->new_submodule_commits = !!oidcmp(&p->one->oid, &p->two->oid); + + switch (p->status) { + case DIFF_STATUS_ADDED: + die("BUG: worktree status add???"); + break; + + case DIFF_STATUS_DELETED: + d->mode_index = p->one->mode; + oidcpy(&d->oid_index, &p->one->oid); + /* mode_worktree is zero for a delete. */ + break; + + case DIFF_STATUS_MODIFIED: + case DIFF_STATUS_TYPE_CHANGED: + case DIFF_STATUS_UNMERGED: + d->mode_index = p->one->mode; + d->mode_worktree = p->two->mode; + oidcpy(&d->oid_index, &p->one->oid); + break; + + case DIFF_STATUS_UNKNOWN: + die("BUG: worktree status unknown???"); + break; + } + } } @@ -479,12 +504,36 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q, if (!d->index_status) d->index_status = p->status; switch (p->status) { + case DIFF_STATUS_ADDED: + /* Leave {mode,oid}_head zero for an add. */ + d->mode_index = p->two->mode; + oidcpy(&d->oid_index, &p->two->oid); + break; + case DIFF_STATUS_DELETED: + d->mode_head = p->one->mode; + oidcpy(&d->oid_head, &p->one->oid); + /* Leave {mode,oid}_index zero for a delete. */ + break; + case DIFF_STATUS_COPIED: case DIFF_STATUS_RENAMED: d->head_path = xstrdup(p->one->path); + d->score = p->score * 100 / MAX_SCORE; + /* fallthru */ + case DIFF_STATUS_MODIFIED: + case DIFF_STATUS_TYPE_CHANGED: + d->mode_head = p->one->mode; + d->mode_index = p->two->mode; + oidcpy(&d->oid_head, &p->one->oid); + oidcpy(&d->oid_index, &p->two->oid); break; case DIFF_STATUS_UNMERGED: d->stagemask = unmerged_mask(p->two->path); + /* +* Don't bother setting {mode,oid}_{head,index} since the print +* code will output the stage values directly and not use the +* values in these fields. +*/ break; } } @@ -565,9 +614,17 @@ static void wt_status_collect_changes_initial(struct wt_status *s) if (ce_stage(ce)) { d->index_status = DIFF_STATUS_UNMERGED;
[PATCH v6 1/9] status: rename long-format print routines
From: Jeff Hostetler Rename the various wt_status_print*() routines to be wt_longstatus_print*() to make it clear that these routines are only concerned with the normal/long status output and reduce developer confusion as other status formats are added in the future. Signed-off-by: Jeff Hostetler --- builtin/commit.c | 4 +- wt-status.c | 110 +++ wt-status.h | 2 +- 3 files changed, 58 insertions(+), 58 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 1f6dbcd..b80273b 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -515,7 +515,7 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int break; case STATUS_FORMAT_NONE: case STATUS_FORMAT_LONG: - wt_status_print(s); + wt_longstatus_print(s); break; } @@ -1403,7 +1403,7 @@ int cmd_status(int argc, const char **argv, const char *prefix) case STATUS_FORMAT_LONG: s.verbose = verbose; s.ignore_submodule_arg = ignore_submodule_arg; - wt_status_print(&s); + wt_longstatus_print(&s); break; } return 0; diff --git a/wt-status.c b/wt-status.c index de62ab2..b9a58fd 100644 --- a/wt-status.c +++ b/wt-status.c @@ -139,7 +139,7 @@ void wt_status_prepare(struct wt_status *s) s->display_comment_prefix = 0; } -static void wt_status_print_unmerged_header(struct wt_status *s) +static void wt_longstatus_print_unmerged_header(struct wt_status *s) { int i; int del_mod_conflict = 0; @@ -191,7 +191,7 @@ static void wt_status_print_unmerged_header(struct wt_status *s) status_printf_ln(s, c, "%s", ""); } -static void wt_status_print_cached_header(struct wt_status *s) +static void wt_longstatus_print_cached_header(struct wt_status *s) { const char *c = color(WT_STATUS_HEADER, s); @@ -207,9 +207,9 @@ static void wt_status_print_cached_header(struct wt_status *s) status_printf_ln(s, c, "%s", ""); } -static void wt_status_print_dirty_header(struct wt_status *s, -int has_deleted, -int has_dirty_submodules) +static void wt_longstatus_print_dirty_header(struct wt_status *s, +int has_deleted, +int has_dirty_submodules) { const char *c = color(WT_STATUS_HEADER, s); @@ -226,9 +226,9 @@ static void wt_status_print_dirty_header(struct wt_status *s, status_printf_ln(s, c, "%s", ""); } -static void wt_status_print_other_header(struct wt_status *s, -const char *what, -const char *how) +static void wt_longstatus_print_other_header(struct wt_status *s, +const char *what, +const char *how) { const char *c = color(WT_STATUS_HEADER, s); status_printf_ln(s, c, "%s:", what); @@ -238,7 +238,7 @@ static void wt_status_print_other_header(struct wt_status *s, status_printf_ln(s, c, "%s", ""); } -static void wt_status_print_trailer(struct wt_status *s) +static void wt_longstatus_print_trailer(struct wt_status *s) { status_printf_ln(s, color(WT_STATUS_HEADER, s), "%s", ""); } @@ -304,8 +304,8 @@ static int maxwidth(const char *(*label)(int), int minval, int maxval) return result; } -static void wt_status_print_unmerged_data(struct wt_status *s, - struct string_list_item *it) +static void wt_longstatus_print_unmerged_data(struct wt_status *s, + struct string_list_item *it) { const char *c = color(WT_STATUS_UNMERGED, s); struct wt_status_change_data *d = it->util; @@ -331,9 +331,9 @@ static void wt_status_print_unmerged_data(struct wt_status *s, strbuf_release(&onebuf); } -static void wt_status_print_change_data(struct wt_status *s, - int change_type, - struct string_list_item *it) +static void wt_longstatus_print_change_data(struct wt_status *s, + int change_type, + struct string_list_item *it) { struct wt_status_change_data *d = it->util; const char *c = color(change_type, s); @@ -378,7 +378,7 @@ static void wt_status_print_change_data(struct wt_status *s, status = d->worktree_status; break; default: - die("BUG: unhandled change_type %d in wt_status_print_change_data", + die("BUG: unhandled change_type %d in wt_longstatus_print_change_data", change_type); } @@
[PATCH v6 6/9] status: print branch info with --porcelain=v2 --branch
From: Jeff Hostetler Expand porcelain v2 output to include branch and tracking branch information. This includes the commit id, the branch, the upstream branch, and the ahead and behind counts. Signed-off-by: Jeff Hostetler --- builtin/commit.c | 5 wt-status.c | 90 wt-status.h | 1 + 3 files changed, 96 insertions(+) diff --git a/builtin/commit.c b/builtin/commit.c index 3d222d3..5504afe 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -510,6 +510,8 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int s->fp = fp; s->nowarn = nowarn; s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0; + if (!s->is_initial) + hashcpy(s->sha1_commit, sha1); s->status_format = status_format; s->ignore_submodule_arg = ignore_submodule_arg; @@ -1378,6 +1380,9 @@ int cmd_status(int argc, const char **argv, const char *prefix) fd = hold_locked_index(&index_lock, 0); s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0; + if (!s.is_initial) + hashcpy(s.sha1_commit, sha1); + s.ignore_submodule_arg = ignore_submodule_arg; s.status_format = status_format; s.verbose = verbose; diff --git a/wt-status.c b/wt-status.c index 03c8e23..3609531 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1813,6 +1813,92 @@ static void wt_porcelain_print(struct wt_status *s) } /* + * Print branch information for porcelain v2 output. These lines + * are printed when the '--branch' parameter is given. + * + *# branch.oid + *# branch.head + * [# branch.upstream + * [# branch.ab + -]] + * + * ::= the current commit hash or the the literal + * "(initial)" to indicate an initialized repo + * with no commits. + * + * ::= the current branch name or + * "(detached)" literal when detached head or + * "(unknown)" when something is wrong. + * + * ::= the upstream branch name, when set. + * + *::= integer ahead value, when upstream set + * and the commit is present (not gone). + * + * ::= integer behind value, when upstream set + * and commit is present. + * + * + * The end-of-line is defined by the -z flag. + * + * ::= NUL when -z, + * LF when NOT -z. + * + */ +static void wt_porcelain_v2_print_tracking(struct wt_status *s) +{ + struct branch *branch; + const char *base; + const char *branch_name; + struct wt_status_state state; + int ab_info, nr_ahead, nr_behind; + char eol = s->null_termination ? '\0' : '\n'; + + memset(&state, 0, sizeof(state)); + wt_status_get_state(&state, s->branch && !strcmp(s->branch, "HEAD")); + + fprintf(s->fp, "# branch.oid %s%c", + (s->is_initial ? "(initial)" : sha1_to_hex(s->sha1_commit)), + eol); + + if (!s->branch) + fprintf(s->fp, "# branch.head %s%c", "(unknown)", eol); + else { + if (!strcmp(s->branch, "HEAD")) { + fprintf(s->fp, "# branch.head %s%c", "(detached)", eol); + + if (state.rebase_in_progress || state.rebase_interactive_in_progress) + branch_name = state.onto; + else if (state.detached_from) + branch_name = state.detached_from; + else + branch_name = ""; + } else { + branch_name = NULL; + skip_prefix(s->branch, "refs/heads/", &branch_name); + + fprintf(s->fp, "# branch.head %s%c", branch_name, eol); + } + + /* Lookup stats on the upstream tracking branch, if set. */ + branch = branch_get(branch_name); + base = NULL; + ab_info = (stat_tracking_info(branch, &nr_ahead, &nr_behind, &base) == 0); + if (base) { + base = shorten_unambiguous_ref(base, 0); + fprintf(s->fp, "# branch.upstream %s%c", base, eol); + free((char *)base); + + if (ab_info) + fprintf(s->fp, "# branch.ab +%d -%d%c", nr_ahead, nr_behind, eol); + } + } + + free(state.branch); + free(state.onto); + free(state.detached_from); +} + +/* * Convert various submodule status values into a * fixed-length string of characters in the buffer provided. */ @@ -2058,6 +2144,7 @@ static void wt_porcelain_v2_print_other( /* * Print porcelain V2 status. * + * [] * []* * []* * []* @@ -2070,6 +2157,9 @@ static void wt_porcelain_v2_print(struct wt_status *s) struct string_list_item *it;
[PATCH v6 2/9] status: cleanup API to wt_status_print
From: Jeff Hostetler Refactor the API between builtin/commit.c and wt-status.[ch]. Hide the details of the various wt_*status_print() routines inside wt-status.c behind a single (new) wt_status_print() routine. Eliminate the switch statements from builtin/commit.c. Allow details of new status formats to be isolated within wt-status.c Signed-off-by: Jeff Hostetler --- builtin/commit.c | 51 +-- wt-status.c | 25 ++--- wt-status.h | 16 3 files changed, 43 insertions(+), 49 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index b80273b..a792deb 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -142,14 +142,7 @@ static int show_ignored_in_status, have_option_m; static const char *only_include_assumed; static struct strbuf message = STRBUF_INIT; -static enum status_format { - STATUS_FORMAT_NONE = 0, - STATUS_FORMAT_LONG, - STATUS_FORMAT_SHORT, - STATUS_FORMAT_PORCELAIN, - - STATUS_FORMAT_UNSPECIFIED -} status_format = STATUS_FORMAT_UNSPECIFIED; +static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED; static int opt_parse_m(const struct option *opt, const char *arg, int unset) { @@ -500,24 +493,11 @@ static int run_status(FILE *fp, const char *index_file, const char *prefix, int s->fp = fp; s->nowarn = nowarn; s->is_initial = get_sha1(s->reference, sha1) ? 1 : 0; + s->status_format = status_format; + s->ignore_submodule_arg = ignore_submodule_arg; wt_status_collect(s); - - switch (status_format) { - case STATUS_FORMAT_SHORT: - wt_shortstatus_print(s); - break; - case STATUS_FORMAT_PORCELAIN: - wt_porcelain_print(s); - break; - case STATUS_FORMAT_UNSPECIFIED: - die("BUG: finalize_deferred_config() should have been called"); - break; - case STATUS_FORMAT_NONE: - case STATUS_FORMAT_LONG: - wt_longstatus_print(s); - break; - } + wt_status_print(s); return s->commitable; } @@ -1099,7 +1079,7 @@ static const char *read_commit_message(const char *name) * is not in effect here. */ static struct status_deferred_config { - enum status_format status_format; + enum wt_status_format status_format; int show_branch; } status_deferred_config = { STATUS_FORMAT_UNSPECIFIED, @@ -1381,6 +1361,9 @@ int cmd_status(int argc, const char **argv, const char *prefix) s.is_initial = get_sha1(s.reference, sha1) ? 1 : 0; s.ignore_submodule_arg = ignore_submodule_arg; + s.status_format = status_format; + s.verbose = verbose; + wt_status_collect(&s); if (0 <= fd) @@ -1389,23 +1372,7 @@ int cmd_status(int argc, const char **argv, const char *prefix) if (s.relative_paths) s.prefix = prefix; - switch (status_format) { - case STATUS_FORMAT_SHORT: - wt_shortstatus_print(&s); - break; - case STATUS_FORMAT_PORCELAIN: - wt_porcelain_print(&s); - break; - case STATUS_FORMAT_UNSPECIFIED: - die("BUG: finalize_deferred_config() should have been called"); - break; - case STATUS_FORMAT_NONE: - case STATUS_FORMAT_LONG: - s.verbose = verbose; - s.ignore_submodule_arg = ignore_submodule_arg; - wt_longstatus_print(&s); - break; - } + wt_status_print(&s); return 0; } diff --git a/wt-status.c b/wt-status.c index b9a58fd..a9031e4 100644 --- a/wt-status.c +++ b/wt-status.c @@ -1447,7 +1447,7 @@ static void wt_longstatus_print_state(struct wt_status *s, show_bisect_in_progress(s, state, state_color); } -void wt_longstatus_print(struct wt_status *s) +static void wt_longstatus_print(struct wt_status *s) { const char *branch_color = color(WT_STATUS_ONBRANCH, s); const char *branch_status_color = color(WT_STATUS_HEADER, s); @@ -1714,7 +1714,7 @@ static void wt_shortstatus_print_tracking(struct wt_status *s) fputc(s->null_termination ? '\0' : '\n', s->fp); } -void wt_shortstatus_print(struct wt_status *s) +static void wt_shortstatus_print(struct wt_status *s) { int i; @@ -1746,7 +1746,7 @@ void wt_shortstatus_print(struct wt_status *s) } } -void wt_porcelain_print(struct wt_status *s) +static void wt_porcelain_print(struct wt_status *s) { s->use_color = 0; s->relative_paths = 0; @@ -1754,3 +1754,22 @@ void wt_porcelain_print(struct wt_status *s) s->no_gettext = 1; wt_shortstatus_print(s); } + +void wt_status_print(struct wt_status *s) +{ + switch (s->status_format) { + case STATUS_FORMAT_SHORT: + wt_shortstatus_print(s); + break; + ca
[PATCH v6 0/9] status: V2 porcelain status
From: Jeff Hostetler This patch series adds porcelain V2 format to status. This provides detailed information about file changes and about the current branch. The new output is accessed via: git status --porcelain=v2 [--branch] This v6 patch series addresses the unit test discussion from the previous series. I also moved the "else {" onto the previous line as was discussed in a previous comment. Jeff Hostetler (9): status: rename long-format print routines status: cleanup API to wt_status_print status: support --porcelain[=] status: collect per-file data for --porcelain=v2 status: print per-file porcelain v2 status data status: print branch info with --porcelain=v2 --branch git-status.txt: describe --porcelain=v2 format test-lib-functions.sh: Add lf_to_nul status: unit tests for --porcelain=v2 Documentation/git-status.txt | 133 +- builtin/commit.c | 78 +++--- t/t7060-wtstatus.sh | 21 ++ t/t7064-wtstatus-pv2.sh | 576 +++ t/test-lib-functions.sh | 4 + wt-status.c | 570 +- wt-status.h | 19 +- 7 files changed, 1289 insertions(+), 112 deletions(-) create mode 100755 t/t7064-wtstatus-pv2.sh -- 2.8.0.rc4.17.gac42084.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 3/9] status: support --porcelain[=]
From: Jeff Hostetler Update --porcelain argument to take optional version parameter to allow multiple porcelain formats to be supported in the future. The token "v1" is the default value and indicates the traditional porcelain format. (The token "1" is an alias for that.) Signed-off-by: Jeff Hostetler --- Documentation/git-status.txt | 7 +-- builtin/commit.c | 21 ++--- t/t7060-wtstatus.sh | 21 + 3 files changed, 44 insertions(+), 5 deletions(-) diff --git a/Documentation/git-status.txt b/Documentation/git-status.txt index e1e8f57..6b1454b 100644 --- a/Documentation/git-status.txt +++ b/Documentation/git-status.txt @@ -32,11 +32,14 @@ OPTIONS --branch:: Show the branch and tracking info even in short-format. ---porcelain:: +--porcelain[=]:: Give the output in an easy-to-parse format for scripts. This is similar to the short output, but will remain stable across Git versions and regardless of user configuration. See below for details. ++ +The version parameter is used to specify the format version. +This is optional and defaults to the original version 'v1' format. --long:: Give the output in the long-format. This is the default. @@ -96,7 +99,7 @@ configuration variable documented in linkgit:git-config[1]. -z:: Terminate entries with NUL, instead of LF. This implies - the `--porcelain` output format if no other format is given. + the `--porcelain=v1` output format if no other format is given. --column[=]:: --no-column:: diff --git a/builtin/commit.c b/builtin/commit.c index a792deb..185ac35 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -144,6 +144,21 @@ static struct strbuf message = STRBUF_INIT; static enum wt_status_format status_format = STATUS_FORMAT_UNSPECIFIED; +static int opt_parse_porcelain(const struct option *opt, const char *arg, int unset) +{ + enum wt_status_format *value = (enum wt_status_format *)opt->value; + if (unset) + *value = STATUS_FORMAT_NONE; + else if (!arg) + *value = STATUS_FORMAT_PORCELAIN; + else if (!strcmp(arg, "v1") || !strcmp(arg, "1")) + *value = STATUS_FORMAT_PORCELAIN; + else + die("unsupported porcelain version '%s'", arg); + + return 0; +} + static int opt_parse_m(const struct option *opt, const char *arg, int unset) { struct strbuf *buf = opt->value; @@ -1316,9 +1331,9 @@ int cmd_status(int argc, const char **argv, const char *prefix) N_("show status concisely"), STATUS_FORMAT_SHORT), OPT_BOOL('b', "branch", &s.show_branch, N_("show branch information")), - OPT_SET_INT(0, "porcelain", &status_format, - N_("machine-readable output"), - STATUS_FORMAT_PORCELAIN), + { OPTION_CALLBACK, 0, "porcelain", &status_format, + N_("version"), N_("machine-readable output"), + PARSE_OPT_OPTARG, opt_parse_porcelain }, OPT_SET_INT(0, "long", &status_format, N_("show status in long format (default)"), STATUS_FORMAT_LONG), diff --git a/t/t7060-wtstatus.sh b/t/t7060-wtstatus.sh index 44bf1d8..00e0ceb 100755 --- a/t/t7060-wtstatus.sh +++ b/t/t7060-wtstatus.sh @@ -228,4 +228,25 @@ test_expect_success 'status --branch with detached HEAD' ' test_i18ncmp expected actual ' +## Duplicate the above test and verify --porcelain=v1 arg parsing. +test_expect_success 'status --porcelain=v1 --branch with detached HEAD' ' + git reset --hard && + git checkout master^0 && + git status --branch --porcelain=v1 >actual && + cat >expected <<-EOF && + ## HEAD (no branch) + ?? .gitconfig + ?? actual + ?? expect + ?? expected + ?? mdconflict/ + EOF + test_i18ncmp expected actual +' + +## Verify parser error on invalid --porcelain argument. +test_expect_success 'status --porcelain=bogus' ' + test_must_fail git status --porcelain=bogus +' + test_done -- 2.8.0.rc4.17.gac42084.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] guilt: update reflog with annotations of guilt-command being run
On Sat, Jul 09, 2016 at 18:16:05 -0400, Theodore Ts'o wrote: > Many of the updates made by guilt use git update-ref, which means that > the output of "git reflog" is extremely unedifying, e.g: This has been an annoyance for me as well. Thanks for fixing it. I'll give it a test run, and likely push it out later today. Jeff. > ff0031d HEAD@{177}: reset: moving to ff0031d848a0cd7002606f9feef958de8d5edf19 > 90f4305 HEAD@{178}: > a638d43 HEAD@{179}: > ff0031d HEAD@{180}: > 079788d HEAD@{181}: > 87a6280 HEAD@{182}: > 5b9554d HEAD@{183}: > de9e918 HEAD@{184}: reset: moving to de9e9181bc066d63d78b768e95b5d949e2a8673a > 5b9554d HEAD@{185}: > > So teach guilt to use the "set_reflog_action" helper, and since > git-update-ref doesn't respect the GIT_REFLOG_ACTION environment > variable, use its -m option so that "git reflog" can look like this > instead: > > 1eaa566 HEAD@{11}: guilt-push: track-more-dependencies-on-transaction-commit > ab714af HEAD@{12}: guilt-push: move-lockdep-tracking-to-journal_s > 7a4b188 HEAD@{13}: guilt-push: move-lockdep-instrumentation-for-jbd2-handles > 78d9625 HEAD@{14}: guilt-push: > respect-nobarrier-mount-option-in-nojournal-mode > d08854f HEAD@{15}: guilt-pop: updating HEAD > d08854f HEAD@{16}: guilt-pop: updating HEAD > d08854f HEAD@{17}: guilt-push: > optimize-ext4_should_retry_alloc-to-improve-ENOSPC-performance > > Signed-off-by: Theodore Ts'o > Cc: Josef 'Jeff' Sipek > --- > guilt | 6 -- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/guilt b/guilt > index 35177b9..38d426b 100755 > --- a/guilt > +++ b/guilt > @@ -114,6 +114,7 @@ if [ $# -ne 0 ]; then > disp "" >&2 > exit 1 > fi > + set_reflog_action "guilt-$CMDNAME" > > shift > else > @@ -640,7 +641,7 @@ commit() > commitish=`git commit-tree $treeish -p $2 < "$TMP_MSG"` > if $old_style_prefix || git rev-parse --verify --quiet > refs/heads/$GUILT_PREFIX$branch >/dev/null > then > - git update-ref HEAD $commitish > + git update-ref -m "$GIT_REFLOG_ACTION" HEAD $commitish > else > git branch $GUILT_PREFIX$branch $commitish > git symbolic-ref HEAD refs/heads/$GUILT_PREFIX$branch > @@ -687,7 +688,8 @@ push_patch() > fi > fi > > - commit "$pname" HEAD > + GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: $pname" \ > + commit "$pname" HEAD > > echo "$pname" >> "$applied" > > -- > 2.5.0 > -- Real Programmers consider "what you see is what you get" to be just as bad a concept in Text Editors as it is in women. No, the Real Programmer wants a "you asked for it, you got it" text editor -- complicated, cryptic, powerful, unforgiving, dangerous. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
*RE: Deposit|Charitable Gesture
Hello Sir/Madam, Your 3.7 million USD deposit from Susanne Klatten 50M Philanthropic Donation is ready. Contact: susannecharityfoundat...@aol.de -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5] pack-objects mru
On Thu, Aug 11, 2016 at 05:20:30AM -0400, Jeff King wrote: > Here it is. It ended up needing a few preparatory patches. > > [1/4]: provide an initializer for "struct object_info" > [2/4]: sha1_file: make packed_object_info public > [3/4]: pack-objects: break delta cycles before delta-search phase > [4/4]: pack-objects: use mru list when iterating over packs > > I had originally intended to include an extra patch to handle the > --depth limits better. But after writing it, I'm not sure it's actually > a good idea. I'll post it as an addendum with more discussion. And here's the depth patch. It does work, as you can see by running the snippet at the bottom of the commit message. But I began to wonder if it's actually a desirable thing. For instance, if you do: git gc --aggressive ... time passes ... git gc the first gc may generate chains up to 250 objects. When the second gc runs (and you may not even run it yourself; it might be "gc --auto"!), it will generally reuse most of your existing deltas, even though the default depth is only 50. But with the patch below, it will drop deltas from the middle of those long chains, undoing the prior --aggressive results. Worse, we don't find new deltas for those objects, because it falls afoul of the "they are in the same pack, so don't bother looking for a delta" rule. An --aggressive repack of my git.git is 52MB. Repacking that with the patch below and "--depth=50" bumps it to 55MB. Dumping the "do not bother" condition in try_delta() drops that to 54MB. So it _is_ worse for the space to drop those high-depth deltas. Even if we fixed the "do not bother" (e.g., by recording a bit that says "even though these are in the same pack, try anyway, because we had to break the delta for other reasons"), it's still a loss. OTOH, I am not altogether convinced that the tradeoff of a giant --depth is worth. I'm looking only at the space here, but deeper delta chains cost more CPU to access (especially if they start to exceed the delta cache size). And the space savings aren't that amazing. Doing a "git repack -adf --depth=50 --window=250" (i.e., if aggressive had just tweaked the window size and not the depth in the first place), the result is only 53MB. So considering "--depth" as a space-saving measure for --aggressive does not seem that effective. But it feels weird to quietly drop actions people might have done with previous aggressive runs. -- >8 -- Subject: [PATCH] pack-objects: enforce --depth limit in reused deltas Since 898b14c (pack-objects: rework check_delta_limit usage, 2007-04-16), we check the delta depth limit only when figuring out whether we should make a new delta. We don't consider it at all when reusing deltas, which means that packing once with --depth=50, and then against with --depth=10, the second pack my still contain chains larger than 10. This is probably not a huge deal, as it is limited to whatever chains you happened to create in a previous run. But as we start allowing cross-pack delta reuse in a future commit, this maximum will rise to the number of packs times the per-pack depth (in the worst case; on average, it will likely be much smaller). We can easily detect this as part of the existing search for cycles, since we visit every node in a depth-first way. That lets us compute the depth of any node based on the depth of its base, because we know the base is DFS_DONE by the time we look at it (modulo any cycles in the graph, but we know there cannot be any because we break them as we see them). There is some subtlety worth mentioning, though. We record the depth of each object as we compute it. It might seem like we could save the per-object storage space by just keeping track of the depth of our traversal (i.e., have break_delta_chains() report how deep it went). But we may visit an object through multiple delta paths, and on subsequent paths we want to know its depth immediately, without having to walk back down to its final base (doing so would make our graph walk quadratic rather than linear). Likewise, one could try to record the depth not from the base, but from our starting point (i.e., start recursion_depth at 0, and pass "recursion_depth + 1" to each invocation of break_delta_chains()). And then when recursion_depth gets too big, we know that we must cut the delta chain. But that technique is wrong if we do not visit the nodes in topological order. In a chain A->B->C, it if we visit "C", then "B", then "A", we will never recurse deeper than 1 link (because we see at each node that we have already visited it). Unfortunately there is no automated test, because it's hard to convince pack-objects to reliably produce delta chains. Naively, it would seem that a sequence of ever-increasing blobs would work. E.g., something like: for i in 1 2 3 4 5; do test-genrandom $i 4096 >>file git add file git commit -m $i done where a reasonable set of deltas would use "1:file" as the b
[PATCH v5 4/4] pack-objects: use mru list when iterating over packs
In the original implementation of want_object_in_pack(), we always looked for the object in every pack, so the order did not matter for performance. As of the last few patches, however, we can now often break out of the loop early after finding the first instance, and avoid looking in the other packs at all. In this case, pack order can make a big difference, because we'd like to find the objects by looking at as few packs as possible. This patch switches us to the same packed_git_mru list that is now used by normal object lookups. Here are timings for p5303 on linux.git: Test HEAD^HEAD 5303.3: rev-list (1) 31.31(31.07+0.23)31.28(31.00+0.27) -0.1% 5303.4: repack (1)40.35(38.84+2.60)40.53(39.31+2.32) +0.4% 5303.6: rev-list (50) 31.37(31.15+0.21)31.41(31.16+0.24) +0.1% 5303.7: repack (50) 58.25(68.54+2.03)47.28(57.66+1.89) -18.8% 5303.9: rev-list (1000) 31.91(31.57+0.33)31.93(31.64+0.28) +0.1% 5303.10: repack (1000)304.80(376.00+3.92) 87.21(159.54+2.84) -71.4% The rev-list numbers are unchanged, which makes sense (they are not exercising this code at all). The 50- and 1000-pack repack cases show considerable improvement. The single-pack repack case doesn't, of course; there's nothing to improve. In fact, it gives us a baseline for how fast we could possibly go. You can see that though rev-list can approach the single-pack case even with 1000 packs, repack doesn't. The reason is simple: the loop we are optimizing is only part of what the repack is doing. After the "counting" phase, we do delta compression, which is much more expensive when there are multiple packs, because we have fewer deltas we can reuse (you can also see that these numbers come from a multicore machine; the CPU times are much higher than the wall-clock times due to the delta phase). So the good news is that in cases with many packs, we used to be dominated by the "counting" phase, and now we are dominated by the delta compression (which is faster, and which we have already parallelized). Here are similar numbers for git.git: Test HEAD^ HEAD - 5303.3: rev-list (1) 1.55(1.51+0.02) 1.54(1.53+0.00) -0.6% 5303.4: repack (1)1.82(1.80+0.08) 1.82(1.78+0.09) +0.0% 5303.6: rev-list (50) 1.58(1.57+0.00) 1.58(1.56+0.01) +0.0% 5303.7: repack (50) 2.50(3.12+0.07) 2.31(2.95+0.06) -7.6% 5303.9: rev-list (1000) 2.22(2.20+0.02) 2.23(2.19+0.03) +0.5% 5303.10: repack (1000)10.47(16.78+0.22) 7.50(13.76+0.22) -28.4% Not as impressive in terms of percentage, but still measurable wins. If you look at the wall-clock time improvements in the 1000-pack case, you can see that linux improved by roughly 10x as many seconds as git. That's because it has roughly 10x as many objects, and we'd expect this improvement to scale linearly with the number of objects (since the number of packs is kept constant). It's just that the "counting" phase is a smaller percentage of the total time spent for a git.git repack, and hence the percentage win is smaller. The implementation itself is a straightforward use of the MRU code. We only bother marking a pack as used when we know that we are able to break early out of the loop, for two reasons: 1. If we can't break out early, it does no good; we have to visit each pack anyway, so we might as well avoid even the minor overhead of managing the cache order. 2. The mru_mark() function reorders the list, which would screw up our traversal. So it is only safe to mark when we are about to break out of the loop. We could record the found pack and mark it after the loop finishes, of course, but that's more complicated and it doesn't buy us anything due to (1). Note that this reordering does have a potential impact on the final pack, as we store only a single "found" pack for each object, even if it is present in multiple packs. In principle, any copy is acceptable, as they all refer to the same content. But in practice, they may differ in whether they are stored as deltas, against which base, etc. This may have an impact on delta reuse, and even the delta search (since we skip pairs that were already in the same pack). It's not clear whether this change of order would hurt or even help average cases, though. The most likely reason to have duplicate objects is from the completion of thin packs (e.g., you have some objects in a "base" pack, then receive several pushes; the packs you receive may be thin on the wire, with deltas that refer to bases outside the pack, but we complete them with duplicate base objects when indexing them). In such a case the current code would always find the thin duplicates (because we currently walk the packs in reverse chronological order). Whereas with
[PATCH v5 3/4] pack-objects: break delta cycles before delta-search phase
We do not allow cycles in the delta graph of a pack (i.e., A is a delta of B which is a delta of A) for the obvious reason that you cannot actually access any of the objects in such a case. There's a last-ditch attempt to notice cycles during the write phase, during which we issue a warning to the user and write one of the objects out in full. However, this is "last-ditch" for two reasons: 1. By this time, it's too late to find another delta for the object, so the resulting pack is larger than it otherwise could be. 2. The warning is there because this is something that _shouldn't_ ever happen. If it does, then either: a. a pack we are reusing deltas from had its own cycle b. we are reusing deltas from multiple packs, and we found a cycle among them (i.e., A is a delta of B in one pack, but B is a delta of A in another, and we choose to use both deltas). c. there is a bug in the delta-search code So this code serves as a final check that none of these things has happened, warns the user, and prevents us from writing a bogus pack. Right now, (2b) should never happen because of the static ordering of packs in want_object_in_pack(). If two objects have a delta relationship, then they must be in the same pack, and therefore we will find them from that same pack. However, a future patch would like to change that static ordering, which will make (2b) a common occurrence. In preparation, we should be able to handle those kinds of cycles better. This patch does by introducing a cycle-breaking step during the get_object_details() phase, when we are deciding which deltas can be reused. That gives us the chance to feed the objects into the delta search as if the cycle did not exist. We'll leave the detection and warning in the write_object() phase in place, as it still serves as a check for case (2c). This does mean we will stop warning for (2a). That case is caused by bogus input packs, and we ideally would warn the user about it. However, since those cycles show up after picking reusable deltas, they look the same as (2b) to us; our new code will break the cycles early and the last-ditch check will never see them. We could do analysis on any cycles that we find to distinguish the two cases (i.e., it is a bogus pack if and only if every delta in the cycle is in the same pack), but we don't need to. If there is a cycle inside a pack, we'll run into problems not only reusing the delta, but accessing the object data at all. So when we try to dig up the actual size of the object, we'll hit that same cycle and kick in our usual complain-and-try-another-source code. Signed-off-by: Jeff King --- This has the DONE and printf fixes from v4, along with using packed_object_info() to fill in the correct type when drop a delta. builtin/pack-objects.c | 84 + pack-objects.h | 9 t/t5314-pack-cycle-detection.sh | 113 3 files changed, 206 insertions(+) create mode 100755 t/t5314-pack-cycle-detection.sh diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index 4a63398..32c1dba 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -1495,6 +1495,83 @@ static int pack_offset_sort(const void *_a, const void *_b) (a->in_pack_offset > b->in_pack_offset); } +/* + * Drop an on-disk delta we were planning to reuse. Naively, this would + * just involve blanking out the "delta" field, but we have to deal + * with some extra book-keeping: + * + * 1. Removing ourselves from the delta_sibling linked list. + * + * 2. Updating our size/type to the non-delta representation. These were + * either not recorded initially (size) or overwritten with the delta type + * (type) when check_object() decided to reuse the delta. + */ +static void drop_reused_delta(struct object_entry *entry) +{ + struct object_entry **p = &entry->delta->delta_child; + struct object_info oi = OBJECT_INFO_INIT; + + while (*p) { + if (*p == entry) + *p = (*p)->delta_sibling; + else + p = &(*p)->delta_sibling; + } + entry->delta = NULL; + + oi.sizep = &entry->size; + oi.typep = &entry->type; + if (packed_object_info(entry->in_pack, entry->in_pack_offset, &oi) < 0) { + /* +* We failed to get the info from this pack for some reason; +* fall back to sha1_object_info, which may find another copy. +* And if that fails, the error will be recorded in entry->type +* and dealt with in prepare_pack(). +*/ + entry->type = sha1_object_info(entry->idx.sha1, &entry->size); + } +} + +/* + * Follow the chain of deltas from this entry onward, throwing away any links + * that cause us to hit
[PATCH v5 2/4] sha1_file: make packed_object_info public
Some code may have a pack/offset pair for an object, but would like to look up more information. Using sha1_object_info() is too heavy-weight; it starts from the sha1 and has to find the pack again (so not only does it waste time, it might not even find the same instance). In some cases, this problem is solved by helpers like get_size_from_delta(), which is used by pack-objects to take a shortcut for objects whose packed representation has already been found. But there's no similar function for getting the object type, for instance. Rather than introduce one, let's just make the whole packed_object_info() available. It is smart enough to spend effort only on the items the caller wants. Signed-off-by: Jeff King --- cache.h | 1 + sha1_file.c | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cache.h b/cache.h index b2e4969..f23968e 100644 --- a/cache.h +++ b/cache.h @@ -1558,6 +1558,7 @@ struct object_info { #define OBJECT_INFO_INIT {NULL} extern int sha1_object_info_extended(const unsigned char *, struct object_info *, unsigned flags); +extern int packed_object_info(struct packed_git *pack, off_t offset, struct object_info *); /* Dumb servers support */ extern int update_server_info(int); diff --git a/sha1_file.c b/sha1_file.c index b8cc76b..5ca0ffa 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1970,8 +1970,8 @@ static enum object_type packed_to_object_type(struct packed_git *p, goto out; } -static int packed_object_info(struct packed_git *p, off_t obj_offset, - struct object_info *oi) +int packed_object_info(struct packed_git *p, off_t obj_offset, + struct object_info *oi) { struct pack_window *w_curs = NULL; unsigned long size; -- 2.9.2.790.gaa5bc72 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 1/4] provide an initializer for "struct object_info"
An all-zero initializer is fine for this struct, but because the first element is a pointer, call sites need to know to use "NULL" instead of "0". Otherwise some static checkers like "sparse" will complain; see d099b71 (Fix some sparse warnings, 2013-07-18) for example. So let's provide an initializer to make this easier to get right. But let's also comment that memset() to zero is explicitly OK[1]. One of the callers embeds object_info in another struct which is initialized via memset (expand_data in builtin/cat-file.c). Since our subset of C doesn't allow assignment from a compound literal, handling this in any other way is awkward, so we'd like to keep the ability to initialize by memset(). By documenting this property, it should make anybody who wants to change the initializer think twice before doing so. There's one other caller of interest. In parse_sha1_header(), we did not initialize the struct fully in the first place. This turned out not to be a bug because the sub-function it calls does not look at any other fields except the ones we did initialize. But that assumption might not hold in the future, so it's a dangerous construct. This patch switches it to initializing the whole struct, which protects us against unexpected reads of the other fields. [1] Obviously using memset() to initialize a pointer violates the C standard, but we long ago decided that it was an acceptable tradeoff in the real world. Signed-off-by: Jeff King --- builtin/cat-file.c | 5 ++--- cache.h| 7 +++ sha1_file.c| 6 ++ streaming.c| 2 +- 4 files changed, 12 insertions(+), 8 deletions(-) diff --git a/builtin/cat-file.c b/builtin/cat-file.c index 2dfe626..2cbc31e 100644 --- a/builtin/cat-file.c +++ b/builtin/cat-file.c @@ -28,7 +28,7 @@ static int cat_one_file(int opt, const char *exp_type, const char *obj_name, char *buf; unsigned long size; struct object_context obj_context; - struct object_info oi = {NULL}; + struct object_info oi = OBJECT_INFO_INIT; struct strbuf sb = STRBUF_INIT; unsigned flags = LOOKUP_REPLACE_OBJECT; @@ -378,8 +378,7 @@ static int batch_objects(struct batch_options *opt) data.mark_query = 0; if (opt->all_objects) { - struct object_info empty; - memset(&empty, 0, sizeof(empty)); + struct object_info empty = OBJECT_INFO_INIT; if (!memcmp(&data.info, &empty, sizeof(empty))) data.skip_object_info = 1; } diff --git a/cache.h b/cache.h index 95a0bd3..b2e4969 100644 --- a/cache.h +++ b/cache.h @@ -1550,6 +1550,13 @@ struct object_info { } packed; } u; }; + +/* + * Initializer for a "struct object_info" that wants no items. You may + * also memset() the memory to all-zeroes. + */ +#define OBJECT_INFO_INIT {NULL} + extern int sha1_object_info_extended(const unsigned char *, struct object_info *, unsigned flags); /* Dumb servers support */ diff --git a/sha1_file.c b/sha1_file.c index 02940f1..b8cc76b 100644 --- a/sha1_file.c +++ b/sha1_file.c @@ -1730,11 +1730,9 @@ static int parse_sha1_header_extended(const char *hdr, struct object_info *oi, int parse_sha1_header(const char *hdr, unsigned long *sizep) { - struct object_info oi; + struct object_info oi = OBJECT_INFO_INIT; oi.sizep = sizep; - oi.typename = NULL; - oi.typep = NULL; return parse_sha1_header_extended(hdr, &oi, LOOKUP_REPLACE_OBJECT); } @@ -2738,7 +2736,7 @@ int sha1_object_info_extended(const unsigned char *sha1, struct object_info *oi, int sha1_object_info(const unsigned char *sha1, unsigned long *sizep) { enum object_type type; - struct object_info oi = {NULL}; + struct object_info oi = OBJECT_INFO_INIT; oi.typep = &type; oi.sizep = sizep; diff --git a/streaming.c b/streaming.c index 811fcc2..431254b 100644 --- a/streaming.c +++ b/streaming.c @@ -135,7 +135,7 @@ struct git_istream *open_istream(const unsigned char *sha1, struct stream_filter *filter) { struct git_istream *st; - struct object_info oi = {NULL}; + struct object_info oi = OBJECT_INFO_INIT; const unsigned char *real = lookup_replace_object(sha1); enum input_source src = istream_source(real, type, &oi); -- 2.9.2.790.gaa5bc72 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5] pack-objects mru
On Thu, Aug 11, 2016 at 02:57:51AM -0400, Jeff King wrote: > > One thing to be careful of is that there are more things this > > drop_reused_delta() should be doing. But I looked through the rest of > > check_object() and could not find anything else. > > Argh, I spoke too soon. > > It is true that the size lookup is the only part of check_object() > we skip if we are reusing the delta. But what I didn't notice is that > when we choose to reuse a delta, we overwrite entry->type (the real > type!) with the in_pack_type (OFS_DELTA, etc). We need to undo that so > that later stages see the real type. > > I'm not sure how my existing tests worked (I confirmed that they do > indeed break the delta). It may be that only some code paths actually > care about the real type. But when playing with the depth-limit (which > uses the same drop_reused_delta() helper), I managed to make some pretty > broken packs. > > So please disregard the v4 patch I just sent; I haven't triggered it, > but I imagine it has the same problem, and I just didn't manage to > trigger it. > > I'll clean that up and send out a new series. Here it is. It ended up needing a few preparatory patches. [1/4]: provide an initializer for "struct object_info" [2/4]: sha1_file: make packed_object_info public [3/4]: pack-objects: break delta cycles before delta-search phase [4/4]: pack-objects: use mru list when iterating over packs I had originally intended to include an extra patch to handle the --depth limits better. But after writing it, I'm not sure it's actually a good idea. I'll post it as an addendum with more discussion. -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 28/40] builtin/apply: rename option parsing functions
Am 11.08.2016 um 10:52 schrieb Christian Couder: > As these functions are going to be part of the libified > apply api, let's give them a name that is more specific s/api/API/ ;-) > to the apply API. > > Signed-off-by: Christian Couder > --- > builtin/apply.c | 40 > 1 file changed, 20 insertions(+), 20 deletions(-) > > diff --git a/builtin/apply.c b/builtin/apply.c > index ad403f8..429fe44 100644 > --- a/builtin/apply.c > +++ b/builtin/apply.c > @@ -4588,16 +4588,16 @@ static int apply_patch(struct apply_state *state, > return res; > } > > -static int option_parse_exclude(const struct option *opt, > - const char *arg, int unset) > +static int apply_option_parse_exclude(const struct option *opt, > + const char *arg, int unset) > { > struct apply_state *state = opt->value; > add_name_limit(state, arg, 1); > return 0; > } > > -static int option_parse_include(const struct option *opt, > - const char *arg, int unset) > +static int apply_option_parse_include(const struct option *opt, > + const char *arg, int unset) > { > struct apply_state *state = opt->value; > add_name_limit(state, arg, 0); > @@ -4605,9 +4605,9 @@ static int option_parse_include(const struct option > *opt, > return 0; > } > > -static int option_parse_p(const struct option *opt, > - const char *arg, > - int unset) > +static int apply_option_parse_p(const struct option *opt, > + const char *arg, > + int unset) > { > struct apply_state *state = opt->value; > state->p_value = atoi(arg); > @@ -4615,8 +4615,8 @@ static int option_parse_p(const struct option *opt, > return 0; > } > > -static int option_parse_space_change(const struct option *opt, > - const char *arg, int unset) > +static int apply_option_parse_space_change(const struct option *opt, > +const char *arg, int unset) > { > struct apply_state *state = opt->value; > if (unset) > @@ -4626,8 +4626,8 @@ static int option_parse_space_change(const struct > option *opt, > return 0; > } > > -static int option_parse_whitespace(const struct option *opt, > -const char *arg, int unset) > +static int apply_option_parse_whitespace(const struct option *opt, > + const char *arg, int unset) > { > struct apply_state *state = opt->value; > state->whitespace_option = arg; > @@ -4636,8 +4636,8 @@ static int option_parse_whitespace(const struct option > *opt, > return 0; > } > > -static int option_parse_directory(const struct option *opt, > - const char *arg, int unset) > +static int apply_option_parse_directory(const struct option *opt, > + const char *arg, int unset) > { > struct apply_state *state = opt->value; > strbuf_reset(&state->root); > @@ -4755,13 +4755,13 @@ int cmd_apply(int argc, const char **argv, const char > *prefix) > struct option builtin_apply_options[] = { > { OPTION_CALLBACK, 0, "exclude", &state, N_("path"), > N_("don't apply changes matching the given path"), > - 0, option_parse_exclude }, > + 0, apply_option_parse_exclude }, > { OPTION_CALLBACK, 0, "include", &state, N_("path"), > N_("apply changes matching the given path"), > - 0, option_parse_include }, > + 0, apply_option_parse_include }, > { OPTION_CALLBACK, 'p', NULL, &state, N_("num"), > N_("remove leading slashes from traditional diff > paths"), > - 0, option_parse_p }, > + 0, apply_option_parse_p }, > OPT_BOOL(0, "no-add", &state.no_add, > N_("ignore additions made by the patch")), > OPT_BOOL(0, "stat", &state.diffstat, > @@ -4793,13 +4793,13 @@ int cmd_apply(int argc, const char **argv, const char > *prefix) > N_("ensure at least lines of context > match")), > { OPTION_CALLBACK, 0, "whitespace", &state, N_("action"), > N_("detect new or modified lines that have whitespace > errors"), > - 0, option_parse_whitespace }, > + 0, apply_option_parse_whitespace }, > { OPTION_CALLBACK, 0, "ignore-space-change", &state, NULL, > N_("ignore changes in whitespace when finding context"), > - PARSE_OPT_NOARG, option_parse_space_change }, > + PARSE_OPT_NOARG, apply_option_pars
Re: [PATCH v10 01/40] apply: make some names more specific
Am 11.08.2016 um 10:40 schrieb Christian Couder: > On Tue, Aug 9, 2016 at 4:51 PM, wrote: >> Am 08.08.2016 um 23:02 schrieb Christian Couder: >>> To prepare for some structs and constants being moved from >>> builtin/apply.c to apply.h, we should give them some more >>> specific names to avoid possible name collisions in th global >> >> s/th/the/ >> >>> namespace. > > Thanks. I will send an update with only a new version of this patch > and of 28/40. > There's another lowercase 'api' in patch 40 Stefan -- /dev/random says: Tis better to have loved and lost than just to have lost. python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')" GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9 9666 829B 49C5 9221 27AF
Re: [PATCH v10 01/40] apply: make some names more specific
On Tue, Aug 9, 2016 at 4:51 PM, wrote: > Am 08.08.2016 um 23:02 schrieb Christian Couder: >> To prepare for some structs and constants being moved from >> builtin/apply.c to apply.h, we should give them some more >> specific names to avoid possible name collisions in th global > > s/th/the/ > >> namespace. Thanks. I will send an update with only a new version of this patch and of 28/40. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html