Re: [RFC/PATCH] reset --hard: make use of the pretty machinery
On Thu, Feb 01, 2018 at 08:57:21PM +, Thomas Gummerer wrote: > It is a slight change of the output if the second line of the commit > message is not a blank line, i.e. if the commit message is > > foo > bar > > previously we would print "HEAD is now at 00 foo", while after > this change we print "HEAD is now at 00 foo bar", same as 'git log > --oneline' shows "00 foo bar". > > So this does make the output more consistent with other commands, and > 'reset' is a porcelain command, so nobody should be parsing the output > in scripts. Yeah, I'd say it's definitely fine to change the (already translated!) stderr output of git-reset. I agree that it's an improvement to be consistent with other commands here. > The current behaviour dates back to 0e5a7faa3a ("Make "git reset" a > builtin.", 2007-09-11), so I assume (without digging into the old > codebase too much) that the logic was implemented because there was > no convenience function such as 'pp_commit_easy' that would do this > already. Yes, there used to be quite a lot of ad hoc parsing of commit objects, but these days we have safer and more readable alternatives. Even without the visible behavior change, I think it would be worth doing this patch just as a code cleanup. > + struct strbuf buf = STRBUF_INIT; > + > + printf(_("HEAD is now at %s"), > + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV)); > + > + pp_commit_easy(CMIT_FMT_ONELINE, commit, ); > + if (buf.len > 0) > + printf(" %s", buf.buf); > + putchar('\n'); > + strbuf_release(); I was disappointed you still had to call find_unique_abbrev(). It seems like we ought to be able to do this in a single formatting call. But CMIT_FMT_ONELINE is just about the subject, and doesn't include the hash at all. There's no equivalent without turning to the user-format code. You can do: struct pretty_print_context ctx = {0, DEFAULT_ABBREV}; format_commit_message(commit, _("HEAD is now at %H %s"), , ); puts(buf.buf); but I was annoyed at having to say "DEFAULT_ABBREV". It seems like that ought to be the default. Anyway, I'm fine with your patch as-is. I just needed a little formatting code-golf in my day. -Peff
Re: how to ignore whitespace changes with --color-moved (git diff move detection)?
On Thu, Feb 01, 2018 at 06:13:52PM -0800, Timothee Cour wrote: > this PR from october 2017 was discussing a patch that'd introduce > `--color-moved-[no-]ignore-space-change` > https://public-inbox.org/git/20171025224620.27657-3-sbel...@google.com/ > > however not sure what happened since then as I can't find in `git help > diff` options even after `brew install --HEAD git` > > it's a really useful feature as it's a common use case (ppl move > blocks and reformat in same PR) I think you can still do "--color-moved -w" to ignore whitespace. It's just that the defaults did not end up getting flipped. -Peff
[PATCH] rebase: add --allow-empty-message option
--> Motivation commit 4bee95847 ("cherry-pick: add --allow-empty-message option", 2012-08-02) started doing this work, but it was never completed. For more discussion on why this approach was chosen, see the thread beginning here: https://public-inbox.org/git/2012080658.ga21...@arachsys.com/ https://stackoverflow.com/q/8542304 also shows this as a desirable feature, and that the workaround is non-trivial to get right. --> Implementation Add a new --allow-empty-message flag. Propagate it to all calls of 'git commit', 'git cherry-pick', and 'git rebase--helper' within the rebase scripts. Signed-off-by: Genki Sky--- Documentation/git-rebase.txt| 5 +++ builtin/rebase--helper.c| 2 ++ git-rebase--am.sh | 1 + git-rebase--interactive.sh | 25 +++-- git-rebase--merge.sh| 3 +- git-rebase.sh | 5 +++ t/t3430-rebase-empty-message.sh | 79 + 7 files changed, 109 insertions(+), 11 deletions(-) create mode 100755 t/t3430-rebase-empty-message.sh diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 8a861c1e0..d713951b8 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -244,6 +244,11 @@ leave out at most one of A and B, in which case it defaults to HEAD. Keep the commits that do not change anything from its parents in the result. +--allow-empty-message:: + By default, rebasing commits with an empty message will fail. + This option overrides that behavior, allowing commits with empty + messages to be rebased. + --skip:: Restart the rebasing process by skipping the current patch. diff --git a/builtin/rebase--helper.c b/builtin/rebase--helper.c index 7daee544b..2090114e9 100644 --- a/builtin/rebase--helper.c +++ b/builtin/rebase--helper.c @@ -22,6 +22,8 @@ int cmd_rebase__helper(int argc, const char **argv, const char *prefix) struct option options[] = { OPT_BOOL(0, "ff", _ff, N_("allow fast-forward")), OPT_BOOL(0, "keep-empty", _empty, N_("keep empty commits")), + OPT_BOOL(0, "allow-empty-message", _empty_message, + N_("allow commits with empty messages")), OPT_CMDMODE(0, "continue", , N_("continue rebase"), CONTINUE), OPT_CMDMODE(0, "abort", , N_("abort rebase"), diff --git a/git-rebase--am.sh b/git-rebase--am.sh index 14c50782e..0f7805179 100644 --- a/git-rebase--am.sh +++ b/git-rebase--am.sh @@ -46,6 +46,7 @@ then # makes this easy git cherry-pick ${gpg_sign_opt:+"$gpg_sign_opt"} --allow-empty \ $allow_rerere_autoupdate --right-only "$revisions" \ + $allow_empty_message \ ${restrict_revision+^$restrict_revision} ret=$? else diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index d47bd2959..81c5b4287 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -281,7 +281,7 @@ pick_one () { test -d "$rewritten" && pick_one_preserving_merges "$@" && return - output eval git cherry-pick $allow_rerere_autoupdate \ + output eval git cherry-pick $allow_rerere_autoupdate $allow_empty_message \ ${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")} \ "$strategy_args" $empty_args $ff "$@" @@ -406,6 +406,7 @@ pick_one_preserving_merges () { ;; *) output eval git cherry-pick $allow_rerere_autoupdate \ + $allow_empty_message \ ${gpg_sign_opt:+$(git rev-parse --sq-quote "$gpg_sign_opt")} \ "$strategy_args" "$@" || die_with_patch $sha1 "$(eval_gettext "Could not pick \$sha1")" @@ -559,7 +560,8 @@ do_next () { mark_action_done do_pick $sha1 "$rest" - git commit --amend --no-post-rewrite ${gpg_sign_opt:+"$gpg_sign_opt"} || { + git commit --amend --no-post-rewrite ${gpg_sign_opt:+"$gpg_sign_opt"} \ + $allow_empty_message || { warn "$(eval_gettext "\ Could not amend commit after successfully picking \$sha1... \$rest This is most likely due to an empty commit message, or the pre-commit hook @@ -607,7 +609,7 @@ you are able to reword the commit.")" # This is an intermediate commit; its message will only be # used in case of trouble. So use the long version: do_with_author output git commit --amend --no-verify -F "$squash_msg" \ - ${gpg_sign_opt:+"$gpg_sign_opt"} || + ${gpg_sign_opt:+"$gpg_sign_opt"} $allow_empty_message ||
Re: [PATCH v1 0/5] Incremental rewrite of git-submodules: git-foreach
Since due to some reason, the previous patch-series list was unavailable on the mailing list, I have re-posted the series. It is available at: https://public-inbox.org/git/20180202045745.5076-1-pc44...@gmail.com/ Thanks, Prathamesh Chavan
[PATCH v1 5/5] submodule: port submodule subcommand 'foreach' from shell to C
This aims to make git-submodule foreach a builtin. This is the very first step taken in this direction. Hence, 'foreach' is ported to submodule--helper, and submodule--helper is called from git-submodule.sh. The code is split up to have one function to obtain all the list of submodules. This function acts as the front-end of git-submodule foreach subcommand. It calls the function for_each_listed_submodule(), which basically loops through the list and calls function fn, which in this case is runcommand_in_submodule_cb(). This third function is a callback function that calls runcommand_in_submodule() with the appropriate parameters and then takes care of running the command in that submodule, and recursively performing the same when --recursive is flagged. The first function module_foreach first parses the options present in argv, and then with the help of module_list_compute(), generates the list of submodules present in the current working tree. The second function for_each_listed_submodule() traverses through the list, and calls function fn (which in case of submodule subcommand foreach is runcommand_in_submodule_cb()) is called for each entry. The third function runcommand_in_submodule_cb() calls the function runcommand_in_submodule() after passing appropraite parameters. The fourth function runcommand_in_submodule(), generates a submodule struct sub for $name, value and then later prepends name=sub->name; and other value assignment to the env argv_array structure of a child_process. Also the of submodule-foreach is push to args argv_array structure and finally, using run_command the commands are executed using a shell. The fourth function also takes care of the recursive flag, by creating a separate child_process structure and prepending "--super-prefix displaypath", to the args argv_array structure. Other required arguments and the input of submodule-foreach is also appended to this argv_array. Helped-by: Brandon WilliamsMentored-by: Christian Couder Mentored-by: Stefan Beller Signed-off-by: Prathamesh Chavan --- builtin/submodule--helper.c | 151 git-submodule.sh| 39 +--- 2 files changed, 152 insertions(+), 38 deletions(-) diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c index a5c4a8a69..46dee6bf5 100644 --- a/builtin/submodule--helper.c +++ b/builtin/submodule--helper.c @@ -718,6 +718,156 @@ static int module_name(int argc, const char **argv, const char *prefix) return 0; } +struct cb_foreach { + int argc; + const char **argv; + const char *prefix; + unsigned int flags; +}; +#define CB_FOREACH_INIT { 0, NULL, NULL, 0 } + +static void runcommand_in_submodule(const char *path, const struct object_id *ce_oid, + int argc, const char **argv, const char *prefix, + unsigned int flags) +{ + const struct submodule *sub; + struct child_process cp = CHILD_PROCESS_INIT; + char *displaypath; + + displaypath = get_submodule_displaypath(path, prefix); + + sub = submodule_from_path(_oid, path); + + if (!sub) + die(_("No url found for submodule path '%s' in .gitmodules"), + displaypath); + + if (!is_submodule_populated_gently(path, NULL)) + goto cleanup; + + prepare_submodule_repo_env(_array); + + /* +* For the purpose of executing in the submodule, +* separate shell is used for the purpose of running the +* child process. +*/ + cp.use_shell = 1; + cp.dir = path; + + /* +* NEEDSWORK: the command currently has access to the variables $name, +* $sm_path, $displaypath, $sha1 and $toplevel only when the command +* contains a single argument. This is done for maintianing a faithful +* translation from shell script. +*/ + if (argc == 1) { + char *toplevel = xgetcwd(); + + argv_array_pushf(_array, "name=%s", sub->name); + argv_array_pushf(_array, "sm_path=%s", path); + argv_array_pushf(_array, "displaypath=%s", displaypath); + argv_array_pushf(_array, "sha1=%s", +oid_to_hex(ce_oid)); + argv_array_pushf(_array, "toplevel=%s", toplevel); + + /* +* Since the path variable was accessible from the script +* before porting, it is also made available after porting. +* The environment variable "PATH" has a very special purpose +* on windows. And since environment variables are +* case-insensitive in windows, it interferes with the +* existing PATH variable. Hence, to avoid that, we expose +* path via the
[PATCH v1 2/5] submodule foreach: document '$sm_path' instead of '$path'
As using a variable '$path' may be harmful to users due to capitalization issues, see 64394e3ae9 (git-submodule.sh: Don't use $path variable in eval_gettext string, 2012-04-17). Adjust the documentation to advocate for using $sm_path, which contains the same value. We still make the 'path' variable available and document it as a deprecated synonym of 'sm_path'. Discussed-with: Ramsay JonesSigned-off-by: Stefan Beller Signed-off-by: Prathamesh Chavan --- Documentation/git-submodule.txt | 10 ++ 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index ff612001d..a23baef62 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -183,12 +183,14 @@ information too. foreach [--recursive] :: Evaluates an arbitrary shell command in each checked out submodule. - The command has access to the variables $name, $path, $sha1 and + The command has access to the variables $name, $sm_path, $sha1 and $toplevel: $name is the name of the relevant submodule section in `.gitmodules`, - $path is the name of the submodule directory relative to the - superproject, $sha1 is the commit as recorded in the superproject, - and $toplevel is the absolute path to the top-level of the superproject. + $sm_path is the path of the submodule as recorded in the superproject, + $sha1 is the commit as recorded in the superproject, and + $toplevel is the absolute path to the top-level of the superproject. + Note that to avoid conflicts with '$PATH' on Windows, the '$path' + variable is now a deprecated synonym of '$sm_path' variable. Any submodules defined in the superproject but not checked out are ignored by this command. Unless given `--quiet`, foreach prints the name of each submodule before evaluating the command. -- 2.15.1
[PATCH v1 4/5] submodule foreach: document variable '$displaypath'
It was observed that the variable '$displaypath' was accessible but undocumented. Hence, document it. Discussed-with: Ramsay JonesSigned-off-by: Stefan Beller Signed-off-by: Prathamesh Chavan --- Documentation/git-submodule.txt | 6 -- t/t7407-submodule-foreach.sh| 22 +++--- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index 8e7930ebc..0cca702cb 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -183,10 +183,12 @@ information too. foreach [--recursive] :: Evaluates an arbitrary shell command in each checked out submodule. - The command has access to the variables $name, $sm_path, $sha1 and - $toplevel: + The command has access to the variables $name, $sm_path, $displaypath, + $sha1 and $toplevel: $name is the name of the relevant submodule section in `.gitmodules`, $sm_path is the path of the submodule as recorded in the superproject, + $displaypath contains the relative path from the current working + directory to the submodules root directory, $sha1 is the commit as recorded in the superproject, and $toplevel is the absolute path to its superproject, such that $toplevel/$sm_path is the absolute path of the submodule. diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 0663622a4..6ad57e061 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -82,16 +82,16 @@ test_expect_success 'test basic "submodule foreach" usage' ' cat >expect <../../actual + git submodule foreach "echo \$toplevel-\$name-\$sm_path-\$displaypath-\$sha1" >../../actual ) && test_i18ncmp expect actual ' @@ -206,25 +206,25 @@ submodulesha1=$(cd clone2/nested1/nested2/nested3/submodule && git rev-parse HEA cat >expect <../../actual + git submodule foreach --recursive "echo \$toplevel-\$name-\$sm_path-\$displaypath-\$sha1" >../../actual ) && test_i18ncmp expect actual ' -- 2.15.1
[PATCH v1 3/5] submodule foreach: clarify the '$toplevel' variable documentation
It does not contain the topmost superproject as the author assumed, but the direct superproject, such that $toplevel/$sm_path is the actual absolute path of the submodule. Discussed-with: Ramsay JonesSigned-off-by: Stefan Beller Signed-off-by: Prathamesh Chavan --- Documentation/git-submodule.txt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Documentation/git-submodule.txt b/Documentation/git-submodule.txt index a23baef62..8e7930ebc 100644 --- a/Documentation/git-submodule.txt +++ b/Documentation/git-submodule.txt @@ -188,7 +188,8 @@ foreach [--recursive] :: $name is the name of the relevant submodule section in `.gitmodules`, $sm_path is the path of the submodule as recorded in the superproject, $sha1 is the commit as recorded in the superproject, and - $toplevel is the absolute path to the top-level of the superproject. + $toplevel is the absolute path to its superproject, such that + $toplevel/$sm_path is the absolute path of the submodule. Note that to avoid conflicts with '$PATH' on Windows, the '$path' variable is now a deprecated synonym of '$sm_path' variable. Any submodules defined in the superproject but not checked out are -- 2.15.1
[PATCH v1 0/5] Incremental rewrite of git-submodules
Following series of patches focuses on porting submodule subcommand git-foreach from shell to C. An initial attempt for porting was introduced about 9 months back, and since then then patches have undergone many changes. Some of the notable discussion thread which I would like to point out is: [1] The previous version of this patch series which was floated is available at: [2]. The following changes were made to that: * As it was observed in other submodule subcommand's ported function that the number of params increased a lot, the variables quiet and recursive, were replaced in the cb_foreach struct with a single unsigned integer variable called flags. * To accomodate the possiblity of a direct call to the functions runcommand_in_submodule(), callback function runcommand_in_submodule_cb() was introduced. [1]: https://public-inbox.org/git/20170419170513.16475-1-pc44...@gmail.com/T/#u [2]: https://public-inbox.org/git/20170807211900.15001-14-pc44...@gmail.com/ As before you can find this series at: https://github.com/pratham-pc/git/commits/patch-series-3 And its build report is available at: https://travis-ci.org/pratham-pc/git/builds/ Branch: patch-series-3 Build #202 Prathamesh Chavan (5): submodule foreach: correct '$path' in nested submodules from a subdirectory submodule foreach: document '$sm_path' instead of '$path' submodule foreach: clarify the '$toplevel' variable documentation submodule foreach: document variable '$displaypath' submodule: port submodule subcommand 'foreach' from shell to C Documentation/git-submodule.txt | 15 ++-- builtin/submodule--helper.c | 151 git-submodule.sh| 40 +-- t/t7407-submodule-foreach.sh| 38 +- 4 files changed, 197 insertions(+), 47 deletions(-) -- 2.15.1
[PATCH v1 1/5] submodule foreach: correct '$path' in nested submodules from a subdirectory
When running 'git submodule foreach' from a subdirectory of your repository, nested submodules get a bogus value for $sm_path: For a submodule 'sub' that contains a nested submodule 'nested', running 'git -C dir submodule foreach echo $path' would report path='../nested' for the nested submodule. The first part '../' is derived from the logic computing the relative path from $pwd to the root of the superproject. The second part is the submodule path inside the submodule. This value is of little use and is hard to document. There are two different possible solutions that have more value: (a) The path value is documented as the path from the toplevel of the superproject to the mount point of the submodule. In this case we would want to have path='sub/nested'. (b) As Ramsay noticed the documented value is wrong. For the non-nested case the path is equal to the relative path from $pwd to the submodules working directory. When following this model, the expected value would be path='../sub/nested'. The behavior for (b) was introduced in 091a6eb0fe (submodule: drop the top-level requirement, 2013-06-16) the intent for $path seemed to be relative to $cwd to the submodule worktree, but that did not work for nested submodules, as the intermittent submodules were not included in the path. If we were to fix the meaning of the $path using (a) such that "path" is "the path from the toplevel of the superproject to the mount point of the submodule", we would break any existing submodule user that runs foreach from non-root of the superproject as the non-nested submodule '../sub' would change its path to 'sub'. If we would fix the meaning of the $path using (b), such that "path" is "the relative path from $pwd to the submodule", then we would break any user that uses nested submodules (even from the root directory) as the 'nested' would become 'sub/nested'. Both groups can be found in the wild. The author has no data if one group outweighs the other by large margin, and offending each one seems equally bad at first. However in the authors imagination it is better to go with (a) as running from a sub directory sounds like it is carried out by a human rather than by some automation task. With a human on the keyboard the feedback loop is short and the changed behavior can be adapted to quickly unlike some automation that can break silently. Discussed-with: Ramsay JonesSigned-off-by: Prathamesh Chavan Signed-off-by: Stefan Beller --- git-submodule.sh | 1 - t/t7407-submodule-foreach.sh | 36 ++-- 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/git-submodule.sh b/git-submodule.sh index 156255a9e..7305ee25f 100755 --- a/git-submodule.sh +++ b/git-submodule.sh @@ -345,7 +345,6 @@ cmd_foreach() prefix="$prefix$sm_path/" sanitize_submodule_env cd "$sm_path" && - sm_path=$(git submodule--helper relative-path "$sm_path" "$wt_prefix") && # we make $path available to scripts ... path=$sm_path && if test $# -eq 1 diff --git a/t/t7407-submodule-foreach.sh b/t/t7407-submodule-foreach.sh index 6ba5daf42..0663622a4 100755 --- a/t/t7407-submodule-foreach.sh +++ b/t/t7407-submodule-foreach.sh @@ -82,9 +82,9 @@ test_expect_success 'test basic "submodule foreach" usage' ' cat >expect expect <
Re: [PATCH v2 08/14] commit-graph: implement git-commit-graph --clear
> Teach Git to delete the current 'graph_head' file and the commit graph > it references. And will it leave other, non-important graph files behind? Looking at the code it indeed does. What is the use case for keeping the non-important graph files? > This is a good safety valve if somehow the file is > corrupted and needs to be recalculated. Since the commit graph is a > summary of contents already in the ODB, it can be regenerated. Wouldn't a simple 'git commit-graph --write --update-head' regenerate it on it's own, without cleaning first? It appears, after running a few tests, that a corrupt graph file can be recreated without cleaning, which is great. However, if graph-head is corrupt, then the command errors out with 'failed to read graph-head'. I don't understand the rationale behind this, it would be overwritten anyway, and its content is not necessary for recreating the graph. And indeed, after commenting out that get_graph_head_hash() call in cmd_commit_graph() it doesn't want to read my corrupted graph-head file, and recreates both the graph and graph-head files just fine. I think the requirement for explicitly cleaning a corrupt graph-head before re-writing it is just unnecessary complication. On second thought, what's the point of '--write' without '--update-head', when consumers (thinking 'log --topo-order...) will need the graph-head anyway? I think '--write' should create a graph-head without requiring an additional option. Hmph, another second thought: the word 'head' has a rather specific meaning in Git, although it's usually capitalized. Using this word in options and filenames may lead to confusion, especially the option '--update-head'. > Signed-off-by: Derrick Stolee> --- > Documentation/git-commit-graph.txt | 16 ++-- > builtin/commit-graph.c | 32 +++- > t/t5318-commit-graph.sh| 7 ++- > 3 files changed, 51 insertions(+), 4 deletions(-) > > diff --git a/Documentation/git-commit-graph.txt > b/Documentation/git-commit-graph.txt > index 99ced16ddc..33d6567f11 100644 > --- a/Documentation/git-commit-graph.txt > +++ b/Documentation/git-commit-graph.txt > @@ -11,6 +11,7 @@ SYNOPSIS > [verse] > 'git commit-graph' --write [--pack-dir ] > 'git commit-graph' --read [--pack-dir ] > +'git commit-graph' --clear [--pack-dir ] > > OPTIONS > --- > @@ -18,16 +19,21 @@ OPTIONS > Use given directory for the location of packfiles, graph-head, > and graph files. > > +--clear:: > + Delete the graph-head file and the graph file it references. > + (Cannot be combined with --read or --write.) > + > --read:: > Read a graph file given by the graph-head file and output basic > - details about the graph file. (Cannot be combined with --write.) > + details about the graph file. (Cannot be combined with --clear > + or --write.) > > --graph-id:: > When used with --read, consider the graph file graph-.graph. > > --write:: > Write a new graph file to the pack directory. (Cannot be combined > - with --read.) > + with --clear or --read.) All these "cannot be combined with --this and --that" remarks make subcommands more and more appealing. > > --update-head:: > When used with --write, update the graph-head file to point to > @@ -61,6 +67,12 @@ $ git commit-graph --write --update-head > $ git commit-graph --read --graph-hash= > > > +* Delete /graph-head and the file it references. > ++ > + > +$ git commit-graph --clear --pack-dir= > + > + > GIT > --- > Part of the linkgit:git[1] suite > diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c > index d73cbc907d..4970dec133 100644 > --- a/builtin/commit-graph.c > +++ b/builtin/commit-graph.c > @@ -10,6 +10,7 @@ > > static char const * const builtin_commit_graph_usage[] = { > N_("git commit-graph [--pack-dir ]"), > + N_("git commit-graph --clear [--pack-dir ]"), > N_("git commit-graph --read [--graph-hash=]"), > N_("git commit-graph --write [--pack-dir ] [--update-head]"), > NULL > @@ -17,6 +18,7 @@ static char const * const builtin_commit_graph_usage[] = { > > static struct opts_commit_graph { > const char *pack_dir; > + int clear; > int read; > const char *graph_hash; > int write; > @@ -25,6 +27,30 @@ static struct opts_commit_graph { > struct object_id old_graph_hash; > } opts; > > +static int graph_clear(void) > +{ > + struct strbuf head_path = STRBUF_INIT; > + char *old_path; > + > + if (!opts.has_existing) > + return 0; > + > + strbuf_addstr(_path, opts.pack_dir); > + strbuf_addstr(_path, "/"); > + strbuf_addstr(_path, "graph-head"); strbuf_addstr(_path, "/graph-head") Although, considering that this is
Re: [PATCH v2 07/14] commit-graph: implement git-commit-graph --update-head
> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh > index da565624e3..d1a23bcdaf 100755 > --- a/t/t5318-commit-graph.sh > +++ b/t/t5318-commit-graph.sh > @@ -107,6 +112,9 @@ test_expect_success 'setup bare repo' \ > test_expect_success 'write graph in bare repo' \ > 'graphbare=$(git commit-graph --write) && > test_path_is_file ${baredir}/graph-${graphbare}.graph && > + test_path_is_file ${baredir}/graph-head && This test and the one preceeding it are wrong. Note that 'git commit-graph --write' above is missing the '--update-head' option, so there should be no graph-head file written, yet this 'this test_path_is_file' doesn't fail the test. The devil lies in the previous test 'setup bare repo', where this bare repo is created by cloning from a local remote: a simple 'git clone --bare full bare' hardlinks all files under .git/objects, including all graph and graph-head files that exist in the remote repo. The previous test should run 'git clone --bare --no-local full bare' instead, and then this test would fail because of the missing graph-head file, as it should. Specifying '--update-head' will make it work again. > + echo ${graphbare} >expect && > + cmp -n 40 expect ${baredir}/graph-head && > git commit-graph --read --graph-hash=${graphbare} >output && > _graph_read_expect "18" "${baredir}" && > cmp expect output'
how to ignore whitespace changes with --color-moved (git diff move detection)?
this PR from october 2017 was discussing a patch that'd introduce `--color-moved-[no-]ignore-space-change` https://public-inbox.org/git/20171025224620.27657-3-sbel...@google.com/ however not sure what happened since then as I can't find in `git help diff` options even after `brew install --HEAD git` it's a really useful feature as it's a common use case (ppl move blocks and reformat in same PR) If it's not merged in git repo yet is there an easy way to try out this feature? (even if experimental)
Re: [PATCH v2 11/14] commit: integrate commit graph with commit parsing
On Tue, 30 Jan 2018 16:39:40 -0500 Derrick Stoleewrote: > +/* global storage */ > +struct commit_graph *commit_graph = 0; NULL, not 0. > +static int bsearch_graph(struct commit_graph *g, struct object_id *oid, > uint32_t *pos) > +{ > + uint32_t last, first = 0; > + > + if (oid->hash[0]) > + first = ntohl(*(uint32_t*)(g->chunk_oid_fanout + 4 * > (oid->hash[0] - 1))); > + last = ntohl(*(uint32_t*)(g->chunk_oid_fanout + 4 * oid->hash[0])); > + > + while (first < last) { > + uint32_t mid = first + (last - first) / 2; > + const unsigned char *current; > + int cmp; > + > + current = g->chunk_oid_lookup + g->hdr->hash_len * mid; > + cmp = hashcmp(oid->hash, current); > + if (!cmp) { > + *pos = mid; > + return 1; > + } > + if (cmp > 0) { > + first = mid + 1; > + continue; > + } > + last = mid; > + } > + > + *pos = first; > + return 0; > +} This would be better in sha1-lookup.c, something like the reverse of commit f1068efefe6d ("sha1_file: drop experimental GIT_USE_LOOKUP search", 2017-08-09), except that it can be done using a simple binary search. > +static int full_parse_commit(struct commit *item, struct commit_graph *g, > + uint32_t pos, const unsigned char *commit_data) > +{ > + struct object_id oid; > + struct commit *new_parent; > + uint32_t new_parent_pos; > + uint32_t *parent_data_ptr; > + uint64_t date_low, date_high; > + struct commit_list **pptr; > + > + item->object.parsed = 1; > + item->graph_pos = pos; > + > + hashcpy(oid.hash, commit_data); > + item->tree = lookup_tree(); > + > + date_high = ntohl(*(uint32_t*)(commit_data + g->hdr->hash_len + 8)) & > 0x3; > + date_low = ntohl(*(uint32_t*)(commit_data + g->hdr->hash_len + 12)); > + item->date = (timestamp_t)((date_high << 32) | date_low); > + > + pptr = >parents; > + > + new_parent_pos = ntohl(*(uint32_t*)(commit_data + g->hdr->hash_len)); > + if (new_parent_pos == GRAPH_PARENT_NONE) > + return 1; > + get_nth_commit_oid(g, new_parent_pos, ); > + new_parent = lookup_commit(); > + if (new_parent) { > + new_parent->graph_pos = new_parent_pos; > + pptr = _list_insert(new_parent, pptr)->next; > + } else { > + die("could not find commit %s", oid_to_hex()); > + } > + > + new_parent_pos = ntohl(*(uint32_t*)(commit_data + g->hdr->hash_len + > 4)); > + if (new_parent_pos == GRAPH_PARENT_NONE) > + return 1; > + if (!(new_parent_pos & GRAPH_LARGE_EDGES_NEEDED)) { > + get_nth_commit_oid(g, new_parent_pos, ); > + new_parent = lookup_commit(); > + if (new_parent) { > + new_parent->graph_pos = new_parent_pos; > + pptr = _list_insert(new_parent, pptr)->next; > + } else > + die("could not find commit %s", oid_to_hex()); > + return 1; > + } > + > + parent_data_ptr = (uint32_t*)(g->chunk_large_edges + 4 * > (new_parent_pos ^ GRAPH_LARGE_EDGES_NEEDED)); > + do { > + new_parent_pos = ntohl(*parent_data_ptr); > + > + get_nth_commit_oid(g, new_parent_pos & GRAPH_EDGE_LAST_MASK, > ); > + new_parent = lookup_commit(); > + if (new_parent) { > + new_parent->graph_pos = new_parent_pos & > GRAPH_EDGE_LAST_MASK; > + pptr = _list_insert(new_parent, pptr)->next; > + } else > + die("could not find commit %s", oid_to_hex()); > + parent_data_ptr++; > + } while (!(new_parent_pos & GRAPH_LAST_EDGE)); > + > + return 1; > +} The part that converts into seems to be duplicated 3 times. Refactor into a function? > +/** > + * Fill 'item' to contain all information that would be parsed by > parse_commit_buffer. > + */ > +static int fill_commit_in_graph(struct commit *item, struct commit_graph *g, > uint32_t pos) > +{ > + uint32_t new_parent_pos; > + uint32_t *parent_data_ptr; > + const unsigned char *commit_data = g->chunk_commit_data + > (g->hdr->hash_len + 16) * pos; > + > + new_parent_pos = ntohl(*(uint32_t*)(commit_data + g->hdr->hash_len)); > + > + if (new_parent_pos == GRAPH_PARENT_MISSING) > + return 0; > + > + if (new_parent_pos == GRAPH_PARENT_NONE) > + return full_parse_commit(item, g, pos, commit_data); > + > + new_parent_pos = ntohl(*(uint32_t*)(commit_data + g->hdr->hash_len + > 4)); > + > + if (new_parent_pos == GRAPH_PARENT_MISSING) > + return 0; > + if (!(new_parent_pos & GRAPH_LARGE_EDGES_NEEDED)) > + return full_parse_commit(item, g, pos, commit_data); > + > +
Re: [PATCH v2 05/14] commit-graph: implement git-commit-graph --write
> diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh > new file mode 100755 > index 00..6bcd1cc264 > --- /dev/null > +++ b/t/t5318-commit-graph.sh > @@ -0,0 +1,96 @@ > +#!/bin/sh > + > +test_description='commit graph' > +. ./test-lib.sh > + > +test_expect_success 'setup full repo' \ > +'rm -rf .git && > + mkdir full && > + cd full && > + git init && > + git config core.commitgraph true && This config variable is unknown at this point. I think the test shouldn't set it before it's introduced in patch 10. > + git config pack.threads 1 && > + packdir=".git/objects/pack"' > +test_expect_success 'setup bare repo' \ > +'cd .. && > + git clone --bare full bare && > + cd bare && > + git config core.graph true && Likewise, and its name should be updated as well. > + git config pack.threads 1 && > + baredir="objects/pack"'
Re: [PATCH v2 07/14] commit-graph: implement git-commit-graph --update-head
> It is possible to have multiple commit graph files in a pack directory, > but only one is important at a time. Use a 'graph_head' file to point > to the important file. This implies that all those other files are ignored, right? > Teach git-commit-graph to write 'graph_head' upon > writing a new commit graph file. > > Signed-off-by: Derrick Stolee> --- > Documentation/git-commit-graph.txt | 34 ++ > builtin/commit-graph.c | 38 > +++--- > commit-graph.c | 25 + > commit-graph.h | 2 ++ > t/t5318-commit-graph.sh| 12 ++-- > 5 files changed, 106 insertions(+), 5 deletions(-) > > diff --git a/Documentation/git-commit-graph.txt > b/Documentation/git-commit-graph.txt > index 09aeaf6c82..99ced16ddc 100644 > --- a/Documentation/git-commit-graph.txt > +++ b/Documentation/git-commit-graph.txt > @@ -12,15 +12,49 @@ SYNOPSIS > 'git commit-graph' --write [--pack-dir ] > 'git commit-graph' --read [--pack-dir ] > > +OPTIONS > +--- Oh, look, the 'OPTIONS' section I missed in the previous patches! ;) This should be split up and squashed into the previous patches where the individual --options are first mentioned. > +--pack-dir:: > + Use given directory for the location of packfiles, graph-head, > + and graph files. > + > +--read:: > + Read a graph file given by the graph-head file and output basic > + details about the graph file. (Cannot be combined with --write.) >From the output of 'git commit-graph --read' it seems that it's not a generally useful option to the user. Perhaps it should be mentioned that it's only intended as a debugging aid? Or maybe it doesn't really matter, because eventually this command will become irrelevant, as other commands (clone, fetch, gc) will invoke it automagically... > +--graph-id:: > + When used with --read, consider the graph file graph-.graph. > + > +--write:: > + Write a new graph file to the pack directory. (Cannot be combined > + with --read.) I think this should also mention that it prints the generated graph file's checksum. > + > +--update-head:: > + When used with --write, update the graph-head file to point to > + the written graph file. So it should be used with '--write', noted. > EXAMPLES > > > +* Output the hash of the graph file pointed to by /graph-head. > ++ > + > +$ git commit-graph --pack-dir= > + > + > * Write a commit graph file for the packed commits in your local .git folder. > + > > $ git commit-graph --write > > > +* Write a graph file for the packed commits in your local .git folder, > +* and update graph-head. > ++ > + > +$ git commit-graph --write --update-head > + > + > * Read basic information from a graph file. > + > > diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c > index 218740b1f8..d73cbc907d 100644 > --- a/builtin/commit-graph.c > +++ b/builtin/commit-graph.c > @@ -11,7 +11,7 @@ > static char const * const builtin_commit_graph_usage[] = { > N_("git commit-graph [--pack-dir ]"), > N_("git commit-graph --read [--graph-hash=]"), > - N_("git commit-graph --write [--pack-dir ]"), > + N_("git commit-graph --write [--pack-dir ] [--update-head]"), > NULL > }; > > @@ -20,6 +20,9 @@ static struct opts_commit_graph { > int read; > const char *graph_hash; > int write; > + int update_head; > + int has_existing; > + struct object_id old_graph_hash; > } opts; > > static int graph_read(void) > @@ -30,8 +33,8 @@ static int graph_read(void) > > if (opts.graph_hash && strlen(opts.graph_hash) == GIT_MAX_HEXSZ) > get_oid_hex(opts.graph_hash, _hash); > - else > - die("no graph hash specified"); > + else if (!get_graph_head_hash(opts.pack_dir, _hash)) > + die("no graph-head exists"); > > graph_file = get_commit_graph_filename_hash(opts.pack_dir, _hash); > graph = load_commit_graph_one(graph_file, opts.pack_dir); > @@ -62,10 +65,33 @@ static int graph_read(void) > return 0; > } > > +static void update_head_file(const char *pack_dir, const struct object_id > *graph_hash) > +{ > + struct strbuf head_path = STRBUF_INIT; > + int fd; > + struct lock_file lk = LOCK_INIT; > + > + strbuf_addstr(_path, pack_dir); > + strbuf_addstr(_path, "/"); > + strbuf_addstr(_path, "graph-head"); strbuf_addstr(_path, "/graph-head"); ? > + > + fd = hold_lock_file_for_update(, head_path.buf, LOCK_DIE_ON_ERROR); > +
Re: [PATCHv2] tag: add --edit option
On Thu, Feb 1, 2018 at 12:21 PM, Nicolas Morey-Chaisemartinwrote: > Add a --edit option whichs allows modifying the messages provided by -m or -F, > the same way git commit --edit does. > > Signed-off-by: Nicolas Morey-Chaisemartin > --- > Changes since v1: > - Fix usage string > - Use write_script to generate editor > - Rename editor to fakeeditor to match the other tests in the testsuite Thanks for explaining what changed since the previous attempt. It is also helpful for reviewers if you include a reference to the previous iteration, like this: https://public-inbox.org/git/450140f4-d410-4f1a-e5c1-c56d345a7...@suse.com/T/#u Cc:'ing reviewers of previous iterations is also good etiquette when submitting a new version. > - I'll post another series to fix the misleading messages in both commit.c > and tag.c when launch_editor fails Typically, it's easier on Junio, from a patch management standpoint, if you submit all these related patches as a single series. Alternately, if you do want to submit those changes separately, before the current patch lands in "master", be sure to mention atop which patch (this one) the additional patch(es) should live. Thanks. > diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt > @@ -167,6 +167,12 @@ This option is only applicable when listing tags without > annotation lines. > +-e:: > +--edit:: > + The message taken from file with `-F` and command line with > + `-m` are usually used as the tag message unmodified. > + This option lets you further edit the message taken from these > sources. You probably ought to add this new option to the command synopsis. In the git-commit man page, the synopsis mentions only '-e' (not --edit), so perhaps this man page could mirror that one. (Sorry for not noticing this earlier.) > diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh > @@ -452,6 +452,21 @@ test_expect_success \ > +get_tag_header annotated-tag-edit $commit commit $time >expect > +echo "An edited message" >>expect Modern practice is to perform these "expect" setup actions (and all other actions) within tests themselves rather than outside of tests. However, consistency also has value, and since this test script is filled with this sort of stylized "expect" setup already, this may be fine, and probably not worth a re-roll. (A "modernization" patch by someone can come later if warranted.) > +test_expect_success 'set up editor' ' > + write_script fakeeditor <<-\EOF > + sed -e "s/A message/An edited message/g" <"$1" >"$1-" > + mv "$1-" "$1" > + EOF > +'
Re: [PATCH v2 03/14] commit-graph: create git-commit-graph builtin
> diff --git a/Documentation/git-commit-graph.txt > b/Documentation/git-commit-graph.txt > new file mode 100644 > index 00..c8ea548dfb > --- /dev/null > +++ b/Documentation/git-commit-graph.txt > @@ -0,0 +1,7 @@ > +git-commit-graph(1) > + Here the length of the '' must match the length of the title line above, or AsciiDoc will complain about missing document title.
Re: 2 conflicts referencing the same path?
Hi, On Thu, Feb 1, 2018 at 3:00 PM, Bryan Turnerwrote: > While investigating an issue with rendering conflicts on a pull > request, I noticed that the merge was producing this output (sanitized > for paths) > > $ git merge --no-ff --log -m "Test" 190a25b6e0f32c7b8ccddf8c31e054149dece8b7 > CONFLICT (rename/add): Rename A->B in HEAD. B added in > 190a25b6e0f32c7b8ccddf8c31e054149dece8b7 > Adding as B~190a25b6e0f32c7b8ccddf8c31e054149dece8b7 instead > ... > Auto-merging B > CONFLICT (content): Merge conflicts in B > > (There are several other conflicts listed "between" the two I'm > showing here, various rename/add, add/add and content conflicts, but > I'm trimming those out to focus on the lines that I think are relevant > to my question.) > > This merge produces 2 (or is it 3?) conflicts for the same B path: > - Rename A to B in HEAD, add B in 190a25b > - Content conflicts in B Right, so the merge-base has just one (relevant) file, A. For sake of argument, let's say it's contents is "hello\nworld". One side of history, leading to HEAD, also has one (relevant) file, which was a rename of A->B which also changed its contents to say "hello\nbeautiful\nworld" The other side of history, leading to commit 190a25 has two files. The original A, whose contents has changed to say "hello\namazing\nworld", and a new file called B. When you merge the two, the "hello world" file has been modified differently on the two sides as well as having been renamed from A->B, AND there was a separate file also placed at B on the other side of history which gets in the way. Git resolves the two-files-getting-in-the-way-of-each-other (the rename/add) by moving one of the two out of the way (though it really ought to move both out of the way, but that's a tangent). And it resolves the conflicting content changes in the other B by doing a content merge with conflict markers, giving a file with contents of the form: """ hello << beautiful == amazing >> world """ and it treats that B (from the "rename") as more important than other (the "add") which it shows by recording it at B. > I'm still trying to produce a set of steps that will allow a minimal > reproduction, but I thought I'd post to the list just to see if anyone > had any thoughts on how it could happen. Is it a "normal" (albeit > rare) case? Or could it represent some sort of issue in Git's 3-way > merge algorithm (in its behavior or perhaps in how the merge conflicts > are logged)? It is "normal" to get this, and functioning as intended, albeit fairly rare. rename/rename(1to2) and rename/rename(2to1) conflicts could provide very similar situations. rename/add conflicts have three issues I know about[1], but that didn't include the output messages being suboptimal. That might just mean my mind has been warped by reading merge-recursive.c or that I'm too familiar with it, so I don't notice how much it could be improved. If you can think of how to improve the messages, I'm happy to listen, especially since I'm trying to find time to continue my rewrite of merge-recursive. It'd have to apply to other rename cases, as noted above (and, in particular, each side of a rename/rename(1to2) can further be involved in other collisions, such as rename/add or rename/rename(2to1), so it could get hairy quick if the solution isn't simple enough.) [1] https://public-inbox.org/git/20171120221944.15431-6-new...@gmail.com/; that patch and the others in the series are waiting for the directory rename detection series to progress before I resubmit. Elijah
Re: [PATCH v2 06/14] commit-graph: implement git-commit-graph --read
On Tue, 30 Jan 2018 16:39:35 -0500 Derrick Stoleewrote: > Teach git-commit-graph to read commit graph files and summarize their > contents. One overall question - is the "read" command meant to be a command used by the end user, or is it here just to test that some aspects of reading works? If the former, I'm not sure how useful it is. And if the latter, I think that it is more useful to just implementing something that reads it, then make the 11/14 change (modifying parse_commit_gently) and include a perf test to show that your commit graph reading is both correct and (performance-)effective.
Re: [PATCH v2 06/14] commit-graph: implement git-commit-graph --read
> Teach git-commit-graph to read commit graph files and summarize their > contents. > > Use the --read option to verify the contents of a commit graph file in the > tests. > > Signed-off-by: Derrick Stolee> --- > Documentation/git-commit-graph.txt | 7 ++ > builtin/commit-graph.c | 55 +++ > commit-graph.c | 138 > - > commit-graph.h | 25 +++ > t/t5318-commit-graph.sh| 28 ++-- > 5 files changed, 247 insertions(+), 6 deletions(-) > > diff --git a/Documentation/git-commit-graph.txt > b/Documentation/git-commit-graph.txt > index 3f3790d9a8..09aeaf6c82 100644 > --- a/Documentation/git-commit-graph.txt > +++ b/Documentation/git-commit-graph.txt > @@ -10,6 +10,7 @@ SYNOPSIS > > [verse] > 'git commit-graph' --write [--pack-dir ] > +'git commit-graph' --read [--pack-dir ] Again, what does this option do? > EXAMPLES > > @@ -20,6 +21,12 @@ EXAMPLES > $ git commit-graph --write > > > +* Read basic information from a graph file. > ++ > + > +$ git commit-graph --read --graph-hash= > + > + > GIT > --- > Part of the linkgit:git[1] suite > diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c > index 7affd512f1..218740b1f8 100644 > --- a/builtin/commit-graph.c > +++ b/builtin/commit-graph.c > +int close_commit_graph(struct commit_graph *g) static, perhaps? I see it's declared as extern in the headeer file below, but I don't see it called outside of this source file by the end of the patch series. > +{ > + if (g->graph_fd < 0) > + return 0; > + > + munmap((void *)g->data, g->data_len); > + g->data = 0; > + > + close(g->graph_fd); > + g->graph_fd = -1; > + > + return 1; > +} > + > +static void free_commit_graph(struct commit_graph **g) > +{ > + if (!g || !*g) > + return; > + > + close_commit_graph(*g); > + > + free(*g); > + *g = NULL; > +} > + > +struct commit_graph *load_commit_graph_one(const char *graph_file, const > char *pack_dir) > +{ > + void *graph_map; > + const unsigned char *data; > + struct commit_graph_header *hdr; > + size_t graph_size; > + struct stat st; > + uint32_t i; > + struct commit_graph *graph; > + int fd = git_open(graph_file); > + uint64_t last_chunk_offset; > + uint32_t last_chunk_id; > + > + if (fd < 0) > + return 0; > + if (fstat(fd, )) { > + close(fd); > + return 0; > + } > + graph_size = xsize_t(st.st_size); > + > + if (graph_size < GRAPH_MIN_SIZE) { > + close(fd); > + die("graph file %s is too small", graph_file); > + } > + graph_map = xmmap(NULL, graph_size, PROT_READ, MAP_PRIVATE, fd, 0); > + data = (const unsigned char *)graph_map; > + > + hdr = graph_map; > + if (ntohl(hdr->graph_signature) != GRAPH_SIGNATURE) { > + uint32_t signature = ntohl(hdr->graph_signature); > + munmap(graph_map, graph_size); > + close(fd); > + die("graph signature %X does not match signature %X", > + signature, GRAPH_SIGNATURE); > + } > + if (hdr->graph_version != GRAPH_VERSION) { > + unsigned char version = hdr->graph_version; > + munmap(graph_map, graph_size); > + close(fd); > + die("graph version %X does not match version %X", > + version, GRAPH_VERSION); > + } > + > + graph = alloc_commit_graph(strlen(pack_dir) + 1); > + > + graph->hdr = hdr; > + graph->graph_fd = fd; > + graph->data = graph_map; > + graph->data_len = graph_size; > + > + last_chunk_id = 0; > + last_chunk_offset = (uint64_t)sizeof(*hdr); > + for (i = 0; i < hdr->num_chunks; i++) { > + uint32_t chunk_id = ntohl(*(uint32_t*)(data + sizeof(*hdr) + 12 > * i)); > + uint64_t chunk_offset1 = ntohl(*(uint32_t*)(data + sizeof(*hdr) > + 12 * i + 4)); > + uint32_t chunk_offset2 = ntohl(*(uint32_t*)(data + sizeof(*hdr) > + 12 * i + 8)); There are a lot of magic number in these three lines, but at least they are all multiples of 4. > + uint64_t chunk_offset = (chunk_offset1 << 32) | chunk_offset2; > + > + if (chunk_offset > graph_size - GIT_MAX_RAWSZ) > + die("improper chunk offset %08x%08x", > (uint32_t)(chunk_offset >> 32), > + (uint32_t)chunk_offset); > + > + switch (chunk_id) { > + case GRAPH_CHUNKID_OIDFANOUT: > + graph->chunk_oid_fanout = data + chunk_offset; > + break; > + > + case GRAPH_CHUNKID_OIDLOOKUP: > +
Re: [PATCH v2 05/14] commit-graph: implement git-commit-graph --write
> Teach git-commit-graph to write graph files. Create new test script to verify > this command succeeds without failure. > > Signed-off-by: Derrick Stolee> --- > Documentation/git-commit-graph.txt | 18 +++ > builtin/commit-graph.c | 30 > t/t5318-commit-graph.sh| 96 > ++ > 3 files changed, 144 insertions(+) > create mode 100755 t/t5318-commit-graph.sh > > diff --git a/Documentation/git-commit-graph.txt > b/Documentation/git-commit-graph.txt > index c8ea548dfb..3f3790d9a8 100644 > --- a/Documentation/git-commit-graph.txt > +++ b/Documentation/git-commit-graph.txt > @@ -5,3 +5,21 @@ NAME > > git-commit-graph - Write and verify Git commit graphs (.graph files) > > + > +SYNOPSIS > + > +[verse] > +'git commit-graph' --write [--pack-dir ] > + What do these options do and what is the command's output? IOW, an 'OPTIONS' section would be nice. > +EXAMPLES > + > + > +* Write a commit graph file for the packed commits in your local .git folder. > ++ > + > +$ git commit-graph --write > + > + > +GIT > +--- > +Part of the linkgit:git[1] suite > diff --git a/t/t5318-commit-graph.sh b/t/t5318-commit-graph.sh > new file mode 100755 > index 00..6bcd1cc264 > --- /dev/null > +++ b/t/t5318-commit-graph.sh > @@ -0,0 +1,96 @@ > +#!/bin/sh > + > +test_description='commit graph' > +. ./test-lib.sh > + > +test_expect_success 'setup full repo' \ > +'rm -rf .git && > + mkdir full && > + cd full && > + git init && > + git config core.commitgraph true && > + git config pack.threads 1 && Does this pack.threads=1 make a difference? > + packdir=".git/objects/pack"' We tend to put single quotes around tests like this: test_expect_success 'setup full repo' ' do-this && check-that ' This is not a mere style nit: those newlines before and after the test block make the test's output with '--verbose-log' slightly more readable. Furthermore, we prefer tabs for indentation. Finally, 'cd'-ing around such that it affects subsequent tests is usually frowned upon. However, in this particular case (going into one repo, doing a bunch of tests there, then going into another repo, and doing another bunch of tests) I think it's better than changing directory in a subshell in every single test. > + > +test_expect_success 'write graph with no packs' \ > +'git commit-graph --write --pack-dir .' > + > +test_expect_success 'create commits and repack' \ > +'for i in $(test_seq 5) > + do > +echo $i >$i.txt && > +git add $i.txt && > +git commit -m "commit $i" && > +git branch commits/$i > + done && > + git repack' > + > +test_expect_success 'write graph' \ > +'graph1=$(git commit-graph --write) && > + test_path_is_file ${packdir}/graph-${graph1}.graph' Style nit: those {} around the variable names are unnecessary, but I see you use them a lot. > + > +t_expect_success 'Add more commits' \ This must be test_expect_success. > +'git reset --hard commits/3 && > + for i in $(test_seq 6 10) > + do > +echo $i >$i.txt && > +git add $i.txt && > +git commit -m "commit $i" && > +git branch commits/$i > + done && > + git reset --hard commits/3 && > + for i in $(test_seq 11 15) > + do > +echo $i >$i.txt && > +git add $i.txt && > +git commit -m "commit $i" && > +git branch commits/$i > + done && > + git reset --hard commits/7 && > + git merge commits/11 && > + git branch merge/1 && > + git reset --hard commits/8 && > + git merge commits/12 && > + git branch merge/2 && > + git reset --hard commits/5 && > + git merge commits/10 commits/15 && > + git branch merge/3 && > + git repack' > + > +# Current graph structure: > +# > +# M3 > +# / |\_ > +#/ 10 15 > +# / | | > +# /9 M2 14 > +# | |/ \ | > +# | 8 M1 | 13 > +# | |/ | \_| > +# 5 7 | 12 > +# | | \__| > +# 4 6 11 > +# |/__/ > +# 3 > +# | > +# 2 > +# | > +# 1 > + > +test_expect_success 'write graph with merges' \ > +'graph2=$(git commit-graph --write) && > + test_path_is_file ${packdir}/graph-${graph2}.graph' > + > +test_expect_success 'setup bare repo' \ > +'cd .. && > + git clone --bare full bare && > + cd bare && > + git config core.graph true && > + git config pack.threads 1 && > + baredir="objects/pack"' > + > +test_expect_success 'write graph in bare repo' \ > +'graphbare=$(git commit-graph --write) && > + test_path_is_file ${baredir}/graph-${graphbare}.graph' > + > +test_done > -- > 2.16.0.15.g9c3cf44.dirty
Re: [PATCH v2 04/14] commit-graph: implement construct_commit_graph()
> Teach Git to write a commit graph file by checking all packed objects > to see if they are commits, then store the file in the given pack > directory. > > Signed-off-by: Derrick Stolee> --- > Makefile | 1 + > commit-graph.c | 376 > + > commit-graph.h | 20 +++ > 3 files changed, 397 insertions(+) > create mode 100644 commit-graph.c > create mode 100644 commit-graph.h > diff --git a/commit-graph.c b/commit-graph.c > new file mode 100644 > index 00..db2b7390c7 > --- /dev/null > +++ b/commit-graph.c > +struct packed_commit_list { > + struct commit **list; > + int num; > + int size; > +}; > + > +struct packed_oid_list { > + struct object_id **list; > + int num; > + int size; > +}; When we manage the memory allocation of an array with the ALLOC_GROW macro, then we tend to call the helper fields as 'alloc' and 'nr'. > +static int if_packed_commit_add_to_list(const struct object_id *oid, > + struct packed_git *pack, > + uint32_t pos, > + void *data) > +{ > + struct packed_oid_list *list = (struct packed_oid_list*)data; > + enum object_type type; > + unsigned long size; > + void *inner_data; > + off_t offset = nth_packed_object_offset(pack, pos); > + inner_data = unpack_entry(pack, offset, , ); > + > + if (inner_data) > + free(inner_data); > + > + if (type != OBJ_COMMIT) > + return 0; > + > + ALLOC_GROW(list->list, list->num + 1, list->size); > + list->list[list->num] = (struct object_id *)malloc(sizeof(struct > object_id)); > + oidcpy(list->list[list->num], oid); > + (list->num)++; > + > + return 0; > +} > + > +struct object_id *construct_commit_graph(const char *pack_dir) > +{ > + struct packed_oid_list oids; > + struct packed_commit_list commits; > + struct commit_graph_header hdr; > + struct sha1file *f; > + int i, count_distinct = 0; > + struct strbuf tmp_file = STRBUF_INIT; > + unsigned char final_hash[GIT_MAX_RAWSZ]; > + char *graph_name; > + int fd; > + uint32_t chunk_ids[5]; > + uint64_t chunk_offsets[5]; > + int num_long_edges; > + struct object_id *f_hash; > + char *fname; > + struct commit_list *parent; > + > + oids.num = 0; > + oids.size = 1024; > + ALLOC_ARRAY(oids.list, oids.size); > + for_each_packed_object(if_packed_commit_add_to_list, , 0); > + QSORT(oids.list, oids.num, commit_compare); > + > + count_distinct = 1; > + for (i = 1; i < oids.num; i++) { > + if (oidcmp(oids.list[i-1], oids.list[i])) > + count_distinct++; > + } > + > + commits.num = 0; > + commits.size = count_distinct; > + ALLOC_ARRAY(commits.list, commits.size); > + > + num_long_edges = 0; > + for (i = 0; i < oids.num; i++) { > + int num_parents = 0; > + if (i > 0 && !oidcmp(oids.list[i-1], oids.list[i])) > + continue; > + > + commits.list[commits.num] = lookup_commit(oids.list[i]); > + parse_commit(commits.list[commits.num]); > + > + for (parent = commits.list[commits.num]->parents; > + parent; parent = parent->next) > + num_parents++; > + > + if (num_parents > 2) > + num_long_edges += num_parents - 1; > + > + commits.num++; > + } > + > + strbuf_addstr(_file, pack_dir); > + strbuf_addstr(_file, "/tmp_graph_XX"); > + > + fd = git_mkstemp_mode(tmp_file.buf, 0444); > + if (fd < 0) > + die_errno("unable to create '%s'", tmp_file.buf); > + > + graph_name = strbuf_detach(_file, NULL); > + f = sha1fd(fd, graph_name); > + > + hdr.graph_signature = htonl(GRAPH_SIGNATURE); > + hdr.graph_version = GRAPH_VERSION; > + hdr.hash_version = GRAPH_OID_VERSION; > + hdr.hash_len = GRAPH_OID_LEN; > + hdr.num_chunks = 4; > + > + assert(sizeof(hdr) == 8); > + sha1write(f, , sizeof(hdr)); > + > + chunk_ids[0] = GRAPH_CHUNKID_OIDFANOUT; > + chunk_ids[1] = GRAPH_CHUNKID_OIDLOOKUP; > + chunk_ids[2] = GRAPH_CHUNKID_DATA; > + chunk_ids[3] = GRAPH_CHUNKID_LARGEEDGES; > + chunk_ids[4] = 0; > + > + chunk_offsets[0] = sizeof(hdr) + GRAPH_CHUNKLOOKUP_SIZE; > + chunk_offsets[1] = chunk_offsets[0] + GRAPH_FANOUT_SIZE; > + chunk_offsets[2] = chunk_offsets[1] + GRAPH_OID_LEN * commits.num; > + chunk_offsets[3] = chunk_offsets[2] + (GRAPH_OID_LEN + 16) * > commits.num; > + chunk_offsets[4] = chunk_offsets[3] + 4 * num_long_edges; > + > + for (i = 0; i <= hdr.num_chunks; i++) { > + uint32_t chunk_write[3]; > + > + chunk_write[0] = htonl(chunk_ids[i]); > + chunk_write[1] = htonl(chunk_offsets[i] >> 32); >
Re: [PATCH v2 05/14] commit-graph: implement git-commit-graph --write
On Tue, 30 Jan 2018 16:39:34 -0500 Derrick Stoleewrote: > diff --git a/Documentation/git-commit-graph.txt > b/Documentation/git-commit-graph.txt > index c8ea548dfb..3f3790d9a8 100644 > --- a/Documentation/git-commit-graph.txt > +++ b/Documentation/git-commit-graph.txt > @@ -5,3 +5,21 @@ NAME > > git-commit-graph - Write and verify Git commit graphs (.graph files) > > + > +SYNOPSIS > + > +[verse] > +'git commit-graph' --write [--pack-dir ] Subcommands (like those in git submodule) generally don't take "--", as far as I know. > +static int graph_write(void) > +{ > + struct object_id *graph_hash = construct_commit_graph(opts.pack_dir); I should have noticed this when I was reviewing the previous patch, but this is probably better named write_commit_graph. > +test_expect_success 'create commits and repack' \ > +'for i in $(test_seq 5) > + do > +echo $i >$i.txt && > +git add $i.txt && > +git commit -m "commit $i" && You can generate commits more easily with test_commit. Also, the final character of the test_expect_success line should be the apostrophe that starts the big text block, like in other files. (That also means that the backslash is unnecessary.) > +# Current graph structure: > +# > +# M3 > +# / |\_ > +#/ 10 15 > +# / | | > +# /9 M2 14 > +# | |/ \ | > +# | 8 M1 | 13 > +# | |/ | \_| > +# 5 7 | 12 > +# | | \__| > +# 4 6 11 > +# |/__/ > +# 3 > +# | > +# 2 > +# | > +# 1 I don't think we need such a complicated graph structure - maybe it's sufficient to have one 2-way merge and one 3-way merge. E.g. 6 |\-. 5 \ \ |\ \ \ 1 2 3 4 Also, I wonder if it is possible to test the output to a greater extent. We don't want anything that relies on the ordering of commits (especially since we plan on transitioning away from using SHA-1 as the hash function) but we could still test, for example, that a 3-way merge results in an edge list of the form "EDGE_..._..." (where the first _ does not have its high bit set, but the second one does). This could be done by using my hex_unpack() function as seen here [1] with grep (e.g. "45444745[0-7]...[8-9a-f]..."). [1] https://public-inbox.org/git/b9ea93edabc42754dc3643d6307c22a947eabaf3.1506714999.git.jonathanta...@google.com/
Re: Git on Windows maps creation time onto changed time
On Fri, Feb 2, 2018 at 12:10 AM, Keith Goldfarbwrote: > Dear git, > > While tracking down a problem with a filesystem shared by Windows and Ubuntu, > I came across the following code in compat/mingw.c (ming_fstat(), also in > do_lstat()): > > if (GetFileInformationByHandle(fh, )) { > buf->st_ino = 0; > buf->st_gid = 0; > buf->st_uid = 0; > buf->st_nlink = 1; > buf->st_mode = file_attr_to_st_mode(fdata.dwFileAttributes); > buf->st_size = fdata.nFileSizeLow | > (((off_t)fdata.nFileSizeHigh)<<32); > buf->st_dev = buf->st_rdev = 0; /* not used by Git */ > buf->st_atime = filetime_to_time_t(&(fdata.ftLastAccessTime)); > buf->st_mtime = filetime_to_time_t(&(fdata.ftLastWriteTime)); > buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime)); > return 0; > } > > The assignment of buf->st_ctime doesn’t seem right to me. I understand > there’s no good choice here, but I think a better choice would be to > duplicate the definition used for st_mtime. > > Background: When I do a git status on Windows and then later on Ubuntu (or > the other order), it is extremely slow, as the entire tree is being > traversed. I tracked it down to this difference in definition of c_time. Yes, > I know about the core.trustctime variable, but my problem aside this seems > like an unwise choice. > > Thanks for listening, Let's CC Marius Storm-Olsen who added this behavior (CC'd).
Git on Windows maps creation time onto changed time
Dear git, While tracking down a problem with a filesystem shared by Windows and Ubuntu, I came across the following code in compat/mingw.c (ming_fstat(), also in do_lstat()): if (GetFileInformationByHandle(fh, )) { buf->st_ino = 0; buf->st_gid = 0; buf->st_uid = 0; buf->st_nlink = 1; buf->st_mode = file_attr_to_st_mode(fdata.dwFileAttributes); buf->st_size = fdata.nFileSizeLow | (((off_t)fdata.nFileSizeHigh)<<32); buf->st_dev = buf->st_rdev = 0; /* not used by Git */ buf->st_atime = filetime_to_time_t(&(fdata.ftLastAccessTime)); buf->st_mtime = filetime_to_time_t(&(fdata.ftLastWriteTime)); buf->st_ctime = filetime_to_time_t(&(fdata.ftCreationTime)); return 0; } The assignment of buf->st_ctime doesn’t seem right to me. I understand there’s no good choice here, but I think a better choice would be to duplicate the definition used for st_mtime. Background: When I do a git status on Windows and then later on Ubuntu (or the other order), it is extremely slow, as the entire tree is being traversed. I tracked it down to this difference in definition of c_time. Yes, I know about the core.trustctime variable, but my problem aside this seems like an unwise choice. Thanks for listening, K.
2 conflicts referencing the same path?
While investigating an issue with rendering conflicts on a pull request, I noticed that the merge was producing this output (sanitized for paths) $ git merge --no-ff --log -m "Test" 190a25b6e0f32c7b8ccddf8c31e054149dece8b7 CONFLICT (rename/add): Rename A->B in HEAD. B added in 190a25b6e0f32c7b8ccddf8c31e054149dece8b7 Adding as B~190a25b6e0f32c7b8ccddf8c31e054149dece8b7 instead ... Auto-merging B CONFLICT (content): Merge conflicts in B (There are several other conflicts listed "between" the two I'm showing here, various rename/add, add/add and content conflicts, but I'm trimming those out to focus on the lines that I think are relevant to my question.) This merge produces 2 (or is it 3?) conflicts for the same B path: - Rename A to B in HEAD, add B in 190a25b - Content conflicts in B The version of B left in place _does_ contain conflict markers, after the merge processes completes. The version added as B~190a25b has no conflict markers; it just shows a small number of inserted lines. The merge in question produces 23 different CONFLICT lines, but aside from "composite" conflicts (rename/add, add/add) that can include the same path multiple times in their output, only this one path is mentioned in multiple CONFLICT lines. I'm still trying to produce a set of steps that will allow a minimal reproduction, but I thought I'd post to the list just to see if anyone had any thoughts on how it could happen. Is it a "normal" (albeit rare) case? Or could it represent some sort of issue in Git's 3-way merge algorithm (in its behavior or perhaps in how the merge conflicts are logged)? Any insights appreciated! Bryan Turner
Re: [PATCH 5/5] dir.c: stop ignoring opendir() error in open_cached_dir()
On Wed, Jan 24 2018, Nguyễn Thái Ngọc Duy jotted: > A follow-up to the recently fixed bugs in the untracked > invalidation. If opendir() fails it should show a warning, perhaps > this should die, but if this ever happens the error is probably > recoverable for the user, and dying would just make things worse. > > Signed-off-by: Nguyễn Thái Ngọc Duy> Signed-off-by: Ævar Arnfjörð Bjarmason > --- > dir.c | 7 ++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/dir.c b/dir.c > index 44b4dd2ec8..55736d3e2a 100644 > --- a/dir.c > +++ b/dir.c > @@ -1787,11 +1787,16 @@ static int open_cached_dir(struct cached_dir *cdir, > struct strbuf *path, > int check_only) > { > + const char *c_path; > + > memset(cdir, 0, sizeof(*cdir)); > cdir->untracked = untracked; > if (valid_cached_dir(dir, untracked, istate, path, check_only)) > return 0; > - cdir->fdir = opendir(path->len ? path->buf : "."); > + c_path = path->len ? path->buf : "."; > + cdir->fdir = opendir(c_path); > + if (!cdir->fdir) > + warning_errno(_("could not open directory '%s'"), c_path); > if (dir->untracked) { > invalidate_directory(dir->untracked, untracked); > dir->untracked->dir_opened++; I haven't found the root cause yet, but we should not release a 2.17 with this patch. I tried deploying a 2.16.1 + various patches (including this) internally today, and on a test set of 236 machines with existing checkouts (already using untracked cache) 79 would spew "warning: could not open directory" warnings, warning about a lot of directories that did not exist at the currently checked-out commit. The warnings go away on a one-off: git -c core.untrackedCache=false status So there's some issue where an existing index with stale UC will be used by a newer version of git, and will start very verbosely warning about every directory referenced that can't be found anymore.
Re: [PATCH v2 04/14] commit-graph: implement construct_commit_graph()
On Tue, 30 Jan 2018 16:39:33 -0500 Derrick Stoleewrote: > +#define GRAPH_SIGNATURE 0x43475048 /* "CGPH" */ > +#define GRAPH_CHUNKID_OIDFANOUT 0x4f494446 /* "OIDF" */ > +#define GRAPH_CHUNKID_OIDLOOKUP 0x4f49444c /* "OIDL" */ > +#define GRAPH_CHUNKID_DATA 0x43444154 /* "CDAT" */ > +#define GRAPH_CHUNKID_LARGEEDGES 0x45444745 /* "EDGE" */ Could all these just be string constants? sha1write can handle them well enough. > +static void write_graph_chunk_fanout(struct sha1file *f, > + struct commit **commits, > + int nr_commits) > +{ > + uint32_t i, count = 0; > + struct commit **list = commits; > + struct commit **last = commits + nr_commits; > + > + /* > + * Write the first-level table (the list is sorted, > + * but we use a 256-entry lookup to be able to avoid > + * having to do eight extra binary search iterations). > + */ > + for (i = 0; i < 256; i++) { > + uint32_t swap_count; > + > + while (list < last) { > + if ((*list)->object.oid.hash[0] != i) > + break; > + count++; > + list++; > + } > + > + swap_count = htonl(count); > + sha1write(f, _count, 4); You can use sha1write_be32() instead of swapping. > +static void write_graph_chunk_large_edges(struct sha1file *f, > + struct commit **commits, > + int nr_commits) > +{ > + struct commit **list = commits; > + struct commit **last = commits + nr_commits; > + struct commit_list *parent; > + > + while (list < last) { > + int num_parents = 0; > + for (parent = (*list)->parents; num_parents < 3 && parent; > + parent = parent->next) > + num_parents++; > + > + if (num_parents <= 2) { > + list++; > + continue; > + } > + > + for (parent = (*list)->parents; parent; parent = parent->next) { > + uint32_t int_id, swap_int_id; > + uint32_t last_edge = 0; > + > + if (parent == (*list)->parents) > + continue; Probably better to just initialize "parent = (*list)->parents->next". Also probably best to add a comment describing why you are doing this (e.g. "The first parent is already in the main commit table; the large edges table only contains the second parent onwards"). > +struct packed_commit_list { > + struct commit **list; > + int num; > + int size; > +}; > + > +struct packed_oid_list { > + struct object_id **list; > + int num; > + int size; > +}; What are num and size? If they're nr and alloc, maybe use those names instead. > +static int if_packed_commit_add_to_list(const struct object_id *oid, > + struct packed_git *pack, > + uint32_t pos, > + void *data) > +{ > + struct packed_oid_list *list = (struct packed_oid_list*)data; > + enum object_type type; > + unsigned long size; > + void *inner_data; > + off_t offset = nth_packed_object_offset(pack, pos); > + inner_data = unpack_entry(pack, offset, , ); > + > + if (inner_data) > + free(inner_data); > + > + if (type != OBJ_COMMIT) > + return 0; > + > + ALLOC_GROW(list->list, list->num + 1, list->size); > + list->list[list->num] = (struct object_id *)malloc(sizeof(struct > object_id)); No need to cast return value of malloc. Also, use xmalloc? > +struct object_id *construct_commit_graph(const char *pack_dir) > +{ > + struct packed_oid_list oids; > + struct packed_commit_list commits; > + struct commit_graph_header hdr; > + struct sha1file *f; > + int i, count_distinct = 0; > + struct strbuf tmp_file = STRBUF_INIT; > + unsigned char final_hash[GIT_MAX_RAWSZ]; > + char *graph_name; > + int fd; > + uint32_t chunk_ids[5]; > + uint64_t chunk_offsets[5]; > + int num_long_edges; > + struct object_id *f_hash; > + char *fname; > + struct commit_list *parent; > + > + oids.num = 0; > + oids.size = 1024; > + ALLOC_ARRAY(oids.list, oids.size); > + for_each_packed_object(if_packed_commit_add_to_list, , 0); > + QSORT(oids.list, oids.num, commit_compare); > + > + count_distinct = 1; > + for (i = 1; i < oids.num; i++) { > + if (oidcmp(oids.list[i-1], oids.list[i])) > + count_distinct++; > + } > + > + commits.num = 0; > + commits.size = count_distinct; > + ALLOC_ARRAY(commits.list, commits.size); > + > + num_long_edges = 0; > + for (i = 0; i < oids.num; i++) { > + int num_parents = 0; > + if (i
Re: [PATCH v2 00/12] object_id part 11 (the_hash_algo)
On Wed, Jan 31, 2018 at 6:18 PM, brian m. carlsonwrote: > This series includes various changes to adopt the use of the_hash_algo > for abstracting hash algorithms away. This series looks good to me. > > Changes from v1: > * Fix comments referring to SHA-1. > * Switch hash function wrappers to take git_hash_ctx *. > * Use a const int variable for a constant value. > * Wrap an overly long line. > > brian m. carlson (12): > hash: move SHA-1 macros to hash.h > hash: create union for hash context allocation > builtin/index-pack: improve hash function abstraction > builtin/unpack-objects: switch uses of SHA-1 to the_hash_algo > sha1_file: switch uses of SHA-1 to the_hash_algo > fast-import: switch various uses of SHA-1 to the_hash_algo > pack-check: convert various uses of SHA-1 to abstract forms > pack-write: switch various SHA-1 values to abstract forms > read-cache: abstract away uses of SHA-1 > csum-file: rename sha1file to hashfile > csum-file: abstract uses of SHA-1 > bulk-checkin: abstract SHA-1 usage > > builtin/index-pack.c | 108 > +++ > builtin/pack-objects.c | 52 +++ > builtin/unpack-objects.c | 18 > bulk-checkin.c | 28 ++-- > cache.h | 25 --- > csum-file.c | 46 ++-- > csum-file.h | 38 - > fast-import.c| 70 +++--- > hash.h | 40 +++--- > pack-bitmap-write.c | 30 ++--- > pack-check.c | 32 +++--- > pack-write.c | 77 - > pack.h | 4 +- > read-cache.c | 58 - > sha1_file.c | 72 +++ > 15 files changed, 351 insertions(+), 347 deletions(-) >
Re: [PATCH v2 01/14] commit-graph: add format document
On Tue, 30 Jan 2018 16:39:30 -0500 Derrick Stoleewrote: > + Commit Data (ID: {'C', 'G', 'E', 'T' }) (N * (H + 16) bytes) > +* The first H bytes are for the OID of the root tree. > +* The next 8 bytes are for the int-ids of the first two parents > + of the ith commit. Stores value 0x if no parent in that > + position. If there are more than two parents, the second value > + has its most-significant bit on and the other bits store an array > + position into the Large Edge List chunk. [snip] > + Large Edge List (ID: {'E', 'D', 'G', 'E'}) > + This list of 4-byte values store the second through nth parents for > + all octopus merges. The second parent value in the commit data is a > + negative number pointing into this list. Looking at the paragraph which I quoted before the [snip], this sentence is confusing (in particular, the second parent value is not interpreted as the normal two's-complement negative value, and the semantics of negative values indexing into the list is unclear). Maybe it's better to omit it entirely.
[RFC/PATCH] reset --hard: make use of the pretty machinery
reset --hard currently uses its own logic for printing the first line of the commit message in its output. Instead of just using the first line, use the pretty machinery to create the output. In addition to the easier to follow code, this makes the output more consistent with other commands that print the title of the commit, such as 'git commit --oneline' or 'git checkout', which both use 'pp_commit_easy()' with the CMIT_FMT_ONELINE modifier. It is a slight change of the output if the second line of the commit message is not a blank line, i.e. if the commit message is foo bar previously we would print "HEAD is now at 00 foo", while after this change we print "HEAD is now at 00 foo bar", same as 'git log --oneline' shows "00 foo bar". So this does make the output more consistent with other commands, and 'reset' is a porcelain command, so nobody should be parsing the output in scripts. The current behaviour dates back to 0e5a7faa3a ("Make "git reset" a builtin.", 2007-09-11), so I assume (without digging into the old codebase too much) that the logic was implemented because there was no convenience function such as 'pp_commit_easy' that would do this already. Signed-off-by: Thomas Gummerer--- Sending this as RFC/PATCH, as I'm not 100% sure this change in behaviour is acceptable, and that I'm not missing some other edge case, but I noticed this while trying to find out how this message gets constructed, and just using the pretty machinery seems much simpler, and more consisent builtin/reset.c | 28 ++-- 1 file changed, 10 insertions(+), 18 deletions(-) diff --git a/builtin/reset.c b/builtin/reset.c index e15f595799..5da0f75de9 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -106,24 +106,16 @@ static int reset_index(const struct object_id *oid, int reset_type, int quiet) static void print_new_head_line(struct commit *commit) { - const char *hex, *body; - const char *msg; - - hex = find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV); - printf(_("HEAD is now at %s"), hex); - msg = logmsg_reencode(commit, NULL, get_log_output_encoding()); - body = strstr(msg, "\n\n"); - if (body) { - const char *eol; - size_t len; - body = skip_blank_lines(body + 2); - eol = strchr(body, '\n'); - len = eol ? eol - body : strlen(body); - printf(" %.*s\n", (int) len, body); - } - else - printf("\n"); - unuse_commit_buffer(commit, msg); + struct strbuf buf = STRBUF_INIT; + + printf(_("HEAD is now at %s"), + find_unique_abbrev(commit->object.oid.hash, DEFAULT_ABBREV)); + + pp_commit_easy(CMIT_FMT_ONELINE, commit, ); + if (buf.len > 0) + printf(" %s", buf.buf); + putchar('\n'); + strbuf_release(); } static void update_index_from_diff(struct diff_queue_struct *q, -- 2.16.1.101.gde0f0111ea
Re: [PATCH 11/26] serve: introduce git-serve
On Thu, Feb 1, 2018 at 12:37 PM, Randall S. Beckerwrote: >> I think that it would be too much of a change to up to 1MB lines at the >> moment so I'm planning on leaving it right where it is :) > > In for a kilo, in for a tonne. Once we're way up there, it's not a problem > or much of a difference. :) What benefit does a larger buffer have? I outlined the negatives above (large static buffer, issues with progress meter). And it seems to me that Brandon wants to keep this series as small as possible w.r.t. bait for endless discussions and only deliver innovation, that solves the immediate needs. Are there issues with too small buffers? (Can you link to the performance measurements or an analysis?)
Good Morning !
Good Morning , I have a project i would like to bring to you and i want you to help me discuss it. please let me know if you will be available for this project. Thanks
RE: [PATCH 11/26] serve: introduce git-serve
On February 1, 2018 3:08 PM, Brandon Williams wrote: > On 02/01, Randall S. Becker wrote: > > On February 1, 2018 1:58 PM, Stefan Beller wrote: > > > On Thu, Feb 1, 2018 at 10:48 AM, Jeff Hostetler > > >> > > wrote: > > > > > > > > > > > > On 1/2/2018 7:18 PM, Brandon Williams wrote: > > > >> > > > >> Introduce git-serve, the base server for protocol version 2. > > > >> > > > >> Protocol version 2 is intended to be a replacement for Git's > > > >> current wire protocol. The intention is that it will be a > > > >> simpler, less wasteful protocol which can evolve over time. > > > >> > > > >> Protocol version 2 improves upon version 1 by eliminating the > > > >> initial ref advertisement. In its place a server will export a > > > >> list of capabilities and commands which it supports in a > > > >> capability advertisement. A client can then request that a > > > >> particular command be executed by providing a number of > > > >> capabilities and command specific parameters. At the completion > > > >> of a command, a client can request that another command be > > > >> executed or can terminate the connection by sending a flush packet. > > > >> > > > >> Signed-off-by: Brandon Williams > > > >> --- > > > >> .gitignore | 1 + > > > >> Documentation/technical/protocol-v2.txt | 91 > > > >> Makefile| 2 + > > > >> builtin.h | 1 + > > > >> builtin/serve.c | 30 > > > >> git.c | 1 + > > > >> serve.c | 239 > > > >> > > > >> serve.h | 15 ++ > > > >> 8 files changed, 380 insertions(+) > > > >> create mode 100644 Documentation/technical/protocol-v2.txt > > > >> create mode 100644 builtin/serve.c > > > >> create mode 100644 serve.c > > > >> create mode 100644 serve.h > > > >> > > > >> diff --git a/.gitignore b/.gitignore index 833ef3b0b..2d0450c26 > > > >> 100644 > > > >> --- a/.gitignore > > > >> +++ b/.gitignore > > > >> @@ -140,6 +140,7 @@ > > > >> /git-rm > > > >> /git-send-email > > > >> /git-send-pack > > > >> +/git-serve > > > >> /git-sh-i18n > > > >> /git-sh-i18n--envsubst > > > >> /git-sh-setup > > > >> diff --git a/Documentation/technical/protocol-v2.txt > > > >> b/Documentation/technical/protocol-v2.txt > > > >> new file mode 100644 > > > >> index 0..b87ba3816 > > > >> --- /dev/null > > > >> +++ b/Documentation/technical/protocol-v2.txt > > > >> @@ -0,0 +1,91 @@ > > > >> + Git Wire Protocol, Version 2 > > > >> +== > > > >> + > > > >> +This document presents a specification for a version 2 of Git's > > > >> +wire protocol. Protocol v2 will improve upon v1 in the following > ways: > > > >> + > > > >> + * Instead of multiple service names, multiple commands will be > > > >> +supported by a single service. > > > >> + * Easily extendable as capabilities are moved into their own section > > > >> +of the protocol, no longer being hidden behind a NUL byte and > > > >> +limited by the size of a pkt-line (as there will be a single > > > >> +capability per pkt-line). > > > >> + * Separate out other information hidden behind NUL bytes (e.g. > agent > > > >> +string as a capability and symrefs can be requested using > > > >> + 'ls-refs') > > > >> + * Reference advertisement will be omitted unless explicitly > > > >> + requested > > > >> + * ls-refs command to explicitly request some refs > > > >> + > > > >> + Detailed Design > > > >> += > > > >> + > > > >> +A client can request to speak protocol v2 by sending `version=2` > > > >> +in the side-channel `GIT_PROTOCOL` in the initial request to the > server. > > > >> + > > > >> +In protocol v2 communication is command oriented. When first > > > >> +contacting > > > >> a > > > >> +server a list of capabilities will advertised. Some of these > > > >> capabilities > > > >> +will be commands which a client can request be executed. Once a > > > >> +command has completed, a client can reuse the connection and > > > >> +request that other commands be executed. > > > >> + > > > >> + Special Packets > > > >> +- > > > >> + > > > >> +In protocol v2 these special packets will have the following > semantics: > > > >> + > > > >> + * '' Flush Packet (flush-pkt) - indicates the end of a > > > >> + message > > > >> + * '0001' Delimiter Packet (delim-pkt) - separates sections of > > > >> + a message > > > > > > > > > > > > Previously, a 0001 pkt-line meant that there was 1 byte of data > > > > following, right? > > > > > > No, the length was including the length field, so 0005 would > > > indicate that there is one byte following, (+4 bytes of "0005" > > > included) > > > > > > > Does this change that and/or prevent 1 byte packets? (Not sure
Re: [PATCHv3 0/4] blame: (dim rep. metadata lines or fields, decay date coloring)
On Thu, Feb 1, 2018 at 11:29 AM, Ævar Arnfjörð Bjarmasonwrote: > > On Thu, Jan 04 2018, Stefan Beller jotted: > >> Stefan Beller (4): >> color.h: document and modernize header >> builtin/blame: dim uninteresting metadata >> builtin/blame: add option to color metadata fields separately >> builtin/blame: highlight recently changed lines > > I like this feature in principle, but I can't get it to work. Building > pu and: Thanks for testing the feature! Please use the flag `--color-lines`, `--color-fields` or `--heated-lines` for experimentation. In the future we may decide that one of them becomes the default (which one?) and is triggered by the color.ui=always setting as well. > ./git -c color.ui=always --exec-path=$PWD blame Makefile > > Shows no colors. Neither does: > > ./git -c color.ui=always --exec-path=$PWD -c > color.blame.highlightRecent="red,12 month ago,blue" blame Makefile > > And there's a bug, it segfaults on any custom value to the other config > option: > > ./git -c color.ui=always --exec-path=$PWD -c color.blame.repeatedMeta=red > blame Makefile > > 0x004c312b in color_parse_mem (value=0x8f6520 "red", value_len=3, > dst=0x1 ) at color.c:272 > 272 OUT('\033'); > > The repeated_meta_color variable is NULL when passed to > color_parse_mem(). Didn't dig further. Thanks for noticing. Fix below (white space mangled) --- i/builtin/blame.c +++ w/builtin/blame.c @@ -48,7 +48,7 @@ static int xdl_opts; static int abbrev = -1; static int no_whole_file_rename; static int show_progress; -static char *repeated_meta_color; +static char repeated_meta_color[COLOR_MAXLEN]; static struct date_mode blame_date_mode = { DATE_ISO8601 }; static size_t blame_date_width; @@ -1099,9 +1099,9 @@ int cmd_blame(int argc, const char **argv, const char *prefix) if (!(output_option & OUTPUT_PORCELAIN)) { find_alignment(, _option); - if (!repeated_meta_color && + if (!*repeated_meta_color && (output_option & (OUTPUT_COLOR_LINE | OUTPUT_COLOR_FIELDS))) - repeated_meta_color = GIT_COLOR_DARK; + strcpy(repeated_meta_color, GIT_COLOR_DARK); }
Re: [PATCH 11/26] serve: introduce git-serve
On 02/01, Randall S. Becker wrote: > On February 1, 2018 1:58 PM, Stefan Beller wrote: > > On Thu, Feb 1, 2018 at 10:48 AM, Jeff Hostetler> > wrote: > > > > > > > > > On 1/2/2018 7:18 PM, Brandon Williams wrote: > > >> > > >> Introduce git-serve, the base server for protocol version 2. > > >> > > >> Protocol version 2 is intended to be a replacement for Git's current > > >> wire protocol. The intention is that it will be a simpler, less > > >> wasteful protocol which can evolve over time. > > >> > > >> Protocol version 2 improves upon version 1 by eliminating the initial > > >> ref advertisement. In its place a server will export a list of > > >> capabilities and commands which it supports in a capability > > >> advertisement. A client can then request that a particular command > > >> be executed by providing a number of capabilities and command > > >> specific parameters. At the completion of a command, a client can > > >> request that another command be executed or can terminate the > > >> connection by sending a flush packet. > > >> > > >> Signed-off-by: Brandon Williams > > >> --- > > >> .gitignore | 1 + > > >> Documentation/technical/protocol-v2.txt | 91 > > >> Makefile| 2 + > > >> builtin.h | 1 + > > >> builtin/serve.c | 30 > > >> git.c | 1 + > > >> serve.c | 239 > > >> > > >> serve.h | 15 ++ > > >> 8 files changed, 380 insertions(+) > > >> create mode 100644 Documentation/technical/protocol-v2.txt > > >> create mode 100644 builtin/serve.c > > >> create mode 100644 serve.c > > >> create mode 100644 serve.h > > >> > > >> diff --git a/.gitignore b/.gitignore > > >> index 833ef3b0b..2d0450c26 100644 > > >> --- a/.gitignore > > >> +++ b/.gitignore > > >> @@ -140,6 +140,7 @@ > > >> /git-rm > > >> /git-send-email > > >> /git-send-pack > > >> +/git-serve > > >> /git-sh-i18n > > >> /git-sh-i18n--envsubst > > >> /git-sh-setup > > >> diff --git a/Documentation/technical/protocol-v2.txt > > >> b/Documentation/technical/protocol-v2.txt > > >> new file mode 100644 > > >> index 0..b87ba3816 > > >> --- /dev/null > > >> +++ b/Documentation/technical/protocol-v2.txt > > >> @@ -0,0 +1,91 @@ > > >> + Git Wire Protocol, Version 2 > > >> +== > > >> + > > >> +This document presents a specification for a version 2 of Git's wire > > >> +protocol. Protocol v2 will improve upon v1 in the following ways: > > >> + > > >> + * Instead of multiple service names, multiple commands will be > > >> +supported by a single service. > > >> + * Easily extendable as capabilities are moved into their own section > > >> +of the protocol, no longer being hidden behind a NUL byte and > > >> +limited by the size of a pkt-line (as there will be a single > > >> +capability per pkt-line). > > >> + * Separate out other information hidden behind NUL bytes (e.g. agent > > >> +string as a capability and symrefs can be requested using > > >> + 'ls-refs') > > >> + * Reference advertisement will be omitted unless explicitly > > >> + requested > > >> + * ls-refs command to explicitly request some refs > > >> + > > >> + Detailed Design > > >> += > > >> + > > >> +A client can request to speak protocol v2 by sending `version=2` in > > >> +the side-channel `GIT_PROTOCOL` in the initial request to the server. > > >> + > > >> +In protocol v2 communication is command oriented. When first > > >> +contacting > > >> a > > >> +server a list of capabilities will advertised. Some of these > > >> capabilities > > >> +will be commands which a client can request be executed. Once a > > >> +command has completed, a client can reuse the connection and request > > >> +that other commands be executed. > > >> + > > >> + Special Packets > > >> +- > > >> + > > >> +In protocol v2 these special packets will have the following semantics: > > >> + > > >> + * '' Flush Packet (flush-pkt) - indicates the end of a message > > >> + * '0001' Delimiter Packet (delim-pkt) - separates sections of a > > >> + message > > > > > > > > > Previously, a 0001 pkt-line meant that there was 1 byte of data > > > following, right? > > > > No, the length was including the length field, so 0005 would indicate that > > there is one byte following, (+4 bytes of "0005" included) > > > > > Does this change that and/or prevent 1 byte packets? (Not sure if it > > > is likely, but the odd-tail of a packfile might get sent in a 0001 > > > line, right?) Or is it that 0001 is only special during the V2 > > > negotiation stuff, but not during the packfile transmission? > > > > 0001 is invalid in the current protocol v0. > > > > > > > > (I'm not
Good Morning !
Good Morning , I have a project i would like to bring to you and i want you to help me discuss it. please let me know if you will be available for this project. Thanks
Re: [PATCH 11/26] serve: introduce git-serve
On 02/01, Jeff Hostetler wrote: > > > On 2/1/2018 1:57 PM, Stefan Beller wrote: > > On Thu, Feb 1, 2018 at 10:48 AM, Jeff Hostetler> > wrote: > > > > > > > > > On 1/2/2018 7:18 PM, Brandon Williams wrote: > > > > > > > > Introduce git-serve, the base server for protocol version 2. > [...] > > > > + Special Packets > > > > +- > > > > + > > > > +In protocol v2 these special packets will have the following semantics: > > > > + > > > > + * '' Flush Packet (flush-pkt) - indicates the end of a message > > > > + * '0001' Delimiter Packet (delim-pkt) - separates sections of a > > > > message > > > > > > > > > Previously, a 0001 pkt-line meant that there was 1 byte of data > > > following, right? > > > > No, the length was including the length field, so 0005 would indicate that > > there is one byte following, (+4 bytes of "0005" included) > > d'oh. right. thanks! > > > > Should we also consider increasing the pkt-line limit to 5 hex-digits > > > while we're at it ? That would let us have 1MB buffers if that would > > > help with large packfiles. > > > > AFAICT there is a static allocation of one pkt-line (of maximum size), > > such that the code can read in a full packet and then process it. > > If we'd increase the packet size we'd need the static buffer to be 1MB, > > which sounds good for my developer machine. But I suspect it may be > > too much for people using git on embedded devices? > > I got burned by that static buffer once upon a time when I wanted > to have 2 streams going at the same time. Hopefully, we can move > that into the new reader structure at some point (if it isn't already). Yeah the reader struct could easily be extended to take in the buffer to read the data into. Because I'm not trying to do any of that atm I decided to have it default to using the static buffer, but it would be as simple as changing the reader->buffer variable to use a different buffer. > > > > > pack files larger than 64k are put into multiple pkt-lines, which is > > not a big deal, as the overhead of 4bytes per 64k is negligible. > > (also there is progress information in the side channel, which > > would come in as a special packet in between real packets, > > such that every 64k transmitted you can update your progress > > meter; Not sure I feel strongly on fewer progress updates) > > > > > Granted, we're throttled by the network, > > > so it might not matter. Would it be interesting to have a 5 digit > > > prefix with parts of the high bits of first digit being flags ? > > > Or is this too radical of a change? > > > > What would the flags be for? > > > > As an alternative we could put the channel number in one byte, > > such that we can have a side channel not just while streaming the > > pack but all the time. (Again, not sure if that buys a lot for us) > > > > Delimiters like the 0001 and the side channel are a couple of > ideas, but I was just thinking out loud. And right, I'm not sure > it gets us much right now. > > Jeff -- Brandon Williams
RE: [PATCH 11/26] serve: introduce git-serve
On February 1, 2018 1:58 PM, Stefan Beller wrote: > On Thu, Feb 1, 2018 at 10:48 AM, Jeff Hostetler> wrote: > > > > > > On 1/2/2018 7:18 PM, Brandon Williams wrote: > >> > >> Introduce git-serve, the base server for protocol version 2. > >> > >> Protocol version 2 is intended to be a replacement for Git's current > >> wire protocol. The intention is that it will be a simpler, less > >> wasteful protocol which can evolve over time. > >> > >> Protocol version 2 improves upon version 1 by eliminating the initial > >> ref advertisement. In its place a server will export a list of > >> capabilities and commands which it supports in a capability > >> advertisement. A client can then request that a particular command > >> be executed by providing a number of capabilities and command > >> specific parameters. At the completion of a command, a client can > >> request that another command be executed or can terminate the > >> connection by sending a flush packet. > >> > >> Signed-off-by: Brandon Williams > >> --- > >> .gitignore | 1 + > >> Documentation/technical/protocol-v2.txt | 91 > >> Makefile| 2 + > >> builtin.h | 1 + > >> builtin/serve.c | 30 > >> git.c | 1 + > >> serve.c | 239 > >> > >> serve.h | 15 ++ > >> 8 files changed, 380 insertions(+) > >> create mode 100644 Documentation/technical/protocol-v2.txt > >> create mode 100644 builtin/serve.c > >> create mode 100644 serve.c > >> create mode 100644 serve.h > >> > >> diff --git a/.gitignore b/.gitignore > >> index 833ef3b0b..2d0450c26 100644 > >> --- a/.gitignore > >> +++ b/.gitignore > >> @@ -140,6 +140,7 @@ > >> /git-rm > >> /git-send-email > >> /git-send-pack > >> +/git-serve > >> /git-sh-i18n > >> /git-sh-i18n--envsubst > >> /git-sh-setup > >> diff --git a/Documentation/technical/protocol-v2.txt > >> b/Documentation/technical/protocol-v2.txt > >> new file mode 100644 > >> index 0..b87ba3816 > >> --- /dev/null > >> +++ b/Documentation/technical/protocol-v2.txt > >> @@ -0,0 +1,91 @@ > >> + Git Wire Protocol, Version 2 > >> +== > >> + > >> +This document presents a specification for a version 2 of Git's wire > >> +protocol. Protocol v2 will improve upon v1 in the following ways: > >> + > >> + * Instead of multiple service names, multiple commands will be > >> +supported by a single service. > >> + * Easily extendable as capabilities are moved into their own section > >> +of the protocol, no longer being hidden behind a NUL byte and > >> +limited by the size of a pkt-line (as there will be a single > >> +capability per pkt-line). > >> + * Separate out other information hidden behind NUL bytes (e.g. agent > >> +string as a capability and symrefs can be requested using > >> + 'ls-refs') > >> + * Reference advertisement will be omitted unless explicitly > >> + requested > >> + * ls-refs command to explicitly request some refs > >> + > >> + Detailed Design > >> += > >> + > >> +A client can request to speak protocol v2 by sending `version=2` in > >> +the side-channel `GIT_PROTOCOL` in the initial request to the server. > >> + > >> +In protocol v2 communication is command oriented. When first > >> +contacting > >> a > >> +server a list of capabilities will advertised. Some of these > >> capabilities > >> +will be commands which a client can request be executed. Once a > >> +command has completed, a client can reuse the connection and request > >> +that other commands be executed. > >> + > >> + Special Packets > >> +- > >> + > >> +In protocol v2 these special packets will have the following semantics: > >> + > >> + * '' Flush Packet (flush-pkt) - indicates the end of a message > >> + * '0001' Delimiter Packet (delim-pkt) - separates sections of a > >> + message > > > > > > Previously, a 0001 pkt-line meant that there was 1 byte of data > > following, right? > > No, the length was including the length field, so 0005 would indicate that > there is one byte following, (+4 bytes of "0005" included) > > > Does this change that and/or prevent 1 byte packets? (Not sure if it > > is likely, but the odd-tail of a packfile might get sent in a 0001 > > line, right?) Or is it that 0001 is only special during the V2 > > negotiation stuff, but not during the packfile transmission? > > 0001 is invalid in the current protocol v0. > > > > > (I'm not against having this delimiter -- I think it is useful, but > > just curious if will cause problems elsewhere.) > > > > Should we also consider increasing the pkt-line limit to 5 hex-digits > > while we're at it ? That would let us have 1MB buffers if that would > > help
Re: [PATCH v2 00/27] protocol version 2
On 1/25/2018 6:58 PM, Brandon Williams wrote: Changes in v2: * Added documentation for fetch * changes #defines for state variables to be enums * couple code changes to pkt-line functions and documentation * Added unit tests for the git-serve binary as well as for ls-refs [...] This looks really nice. I'm eager to get this in so we can do some additional commands to help make partial clone more efficient. Thanks, Jeff
Re: [PATCHv3 0/4] blame: (dim rep. metadata lines or fields, decay date coloring)
On Thu, Jan 04 2018, Stefan Beller jotted: > Stefan Beller (4): > color.h: document and modernize header > builtin/blame: dim uninteresting metadata > builtin/blame: add option to color metadata fields separately > builtin/blame: highlight recently changed lines I like this feature in principle, but I can't get it to work. Building pu and: ./git -c color.ui=always --exec-path=$PWD blame Makefile Shows no colors. Neither does: ./git -c color.ui=always --exec-path=$PWD -c color.blame.highlightRecent="red,12 month ago,blue" blame Makefile And there's a bug, it segfaults on any custom value to the other config option: ./git -c color.ui=always --exec-path=$PWD -c color.blame.repeatedMeta=red blame Makefile 0x004c312b in color_parse_mem (value=0x8f6520 "red", value_len=3, dst=0x1 ) at color.c:272 272 OUT('\033'); The repeated_meta_color variable is NULL when passed to color_parse_mem(). Didn't dig further.
Re: [PATCH 12/26] ls-refs: introduce ls-refs server command
On 1/2/2018 7:18 PM, Brandon Williams wrote: Introduce the ls-refs server command. In protocol v2, the ls-refs command is used to request the ref advertisement from the server. Since it is a command which can be requested (as opposed to mandatory in v1), a client can sent a number of parameters in its request to limit the ref advertisement based on provided ref-patterns. Signed-off-by: Brandon Williams--- Documentation/technical/protocol-v2.txt | 26 + Makefile| 1 + ls-refs.c | 97 + ls-refs.h | 9 +++ serve.c | 2 + 5 files changed, 135 insertions(+) create mode 100644 ls-refs.c create mode 100644 ls-refs.h diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt index b87ba3816..5f4d0e719 100644 --- a/Documentation/technical/protocol-v2.txt +++ b/Documentation/technical/protocol-v2.txt @@ -89,3 +89,29 @@ terminate the connection. Commands are the core actions that a client wants to perform (fetch, push, etc). Each command will be provided with a list capabilities and arguments as requested by a client. + + Ls-refs +- + +Ls-refs is the command used to request a reference advertisement in v2. +Unlike the current reference advertisement, ls-refs takes in parameters +which can be used to limit the refs sent from the server. + +Ls-ref takes in the following parameters wraped in packet-lines: + + symrefs: In addition to the object pointed by it, show the underlying + ref pointed by it when showing a symbolic ref. + peel: Show peeled tags. + ref-pattern : When specified, only references matching the +given patterns are displayed. + +The output of ls-refs is as follows: + +output = *ref +flush-pkt +ref = PKT-LINE((tip | peeled) LF) +tip = obj-id SP refname (SP symref-target) +peeled = obj-id SP refname "^{}" + +symref = PKT-LINE("symref" SP symbolic-ref SP resolved-ref LF) +shallow = PKT-LINE("shallow" SP obj-id LF) Do you want to talk about ordering requirements on this? I think packed-refs has one, but I'm not sure it matters here where the client or server sorts it. Are there any provisions for compressing the renames, like in the reftable spec or in index-v4 ? It doesn't need to be in the initial version. Just asking. We could always add a "ls-refs-2" command that builds upon this. Thanks, Jeff
Re: [PATCH v2 01/41] parse-options: support --git-completion-helper
On Thu, Feb 1, 2018 at 5:21 AM, Duy Nguyenwrote: > On Thu, Feb 1, 2018 at 4:54 PM, Eric Sunshine wrote: >> I don't see that as convincing argument for two classes of "no >> complete". Since git-completion.bash already special-cases >> rebase/am/cherry-pick for --continue|--abort|--skip, it is not far >> fetched that that special-case treatment can be extended slightly to >> also filter out those three options from the list returned by >> --git-completion-helper. > > I agree that is possible, but it's a bit tricky to do the filtering > right in bash (all options are sent back as one line instead of one > per line, which is easier to process by command line tools). Perhaps I'm missing something, but wouldn't filtering out those options directly in Bash require only this? % x='--foo --bar --snoo' % echo ${x/--bar} --foo --snoo > On top of that, I think we want git-completion.bash to be fast, the > more commands we execute there, the unhappier Windows users are. It is > too possible to do this kind of filtering just once, before caching. > It does not fit well to how I designed __gitcomp_builtin so I need to > sit back and see how to sort that out. The filtering as illustrated above using builtin Bash functionality seems unlikely to be a source of noticeable slow down on Windows. And, if you're concerned about it not fitting the design of __gitcomp_builtin, it's almost certainly cheap enough to do the filtering each time its needed rather than worrying about caching the filtered list. (It's hard to imagine anyone noticing a slow down of three extra ${x/} substitutions.)
Re: [PATCH 11/26] serve: introduce git-serve
On 2/1/2018 1:57 PM, Stefan Beller wrote: On Thu, Feb 1, 2018 at 10:48 AM, Jeff Hostetlerwrote: On 1/2/2018 7:18 PM, Brandon Williams wrote: Introduce git-serve, the base server for protocol version 2. [...] + Special Packets +- + +In protocol v2 these special packets will have the following semantics: + + * '' Flush Packet (flush-pkt) - indicates the end of a message + * '0001' Delimiter Packet (delim-pkt) - separates sections of a message Previously, a 0001 pkt-line meant that there was 1 byte of data following, right? No, the length was including the length field, so 0005 would indicate that there is one byte following, (+4 bytes of "0005" included) d'oh. right. thanks! Should we also consider increasing the pkt-line limit to 5 hex-digits while we're at it ? That would let us have 1MB buffers if that would help with large packfiles. AFAICT there is a static allocation of one pkt-line (of maximum size), such that the code can read in a full packet and then process it. If we'd increase the packet size we'd need the static buffer to be 1MB, which sounds good for my developer machine. But I suspect it may be too much for people using git on embedded devices? I got burned by that static buffer once upon a time when I wanted to have 2 streams going at the same time. Hopefully, we can move that into the new reader structure at some point (if it isn't already). pack files larger than 64k are put into multiple pkt-lines, which is not a big deal, as the overhead of 4bytes per 64k is negligible. (also there is progress information in the side channel, which would come in as a special packet in between real packets, such that every 64k transmitted you can update your progress meter; Not sure I feel strongly on fewer progress updates) Granted, we're throttled by the network, so it might not matter. Would it be interesting to have a 5 digit prefix with parts of the high bits of first digit being flags ? Or is this too radical of a change? What would the flags be for? As an alternative we could put the channel number in one byte, such that we can have a side channel not just while streaming the pack but all the time. (Again, not sure if that buys a lot for us) Delimiters like the 0001 and the side channel are a couple of ideas, but I was just thinking out loud. And right, I'm not sure it gets us much right now. Jeff
Re: [PATCH 11/26] serve: introduce git-serve
On Thu, Feb 1, 2018 at 10:48 AM, Jeff Hostetlerwrote: > > > On 1/2/2018 7:18 PM, Brandon Williams wrote: >> >> Introduce git-serve, the base server for protocol version 2. >> >> Protocol version 2 is intended to be a replacement for Git's current >> wire protocol. The intention is that it will be a simpler, less >> wasteful protocol which can evolve over time. >> >> Protocol version 2 improves upon version 1 by eliminating the initial >> ref advertisement. In its place a server will export a list of >> capabilities and commands which it supports in a capability >> advertisement. A client can then request that a particular command be >> executed by providing a number of capabilities and command specific >> parameters. At the completion of a command, a client can request that >> another command be executed or can terminate the connection by sending a >> flush packet. >> >> Signed-off-by: Brandon Williams >> --- >> .gitignore | 1 + >> Documentation/technical/protocol-v2.txt | 91 >> Makefile| 2 + >> builtin.h | 1 + >> builtin/serve.c | 30 >> git.c | 1 + >> serve.c | 239 >> >> serve.h | 15 ++ >> 8 files changed, 380 insertions(+) >> create mode 100644 Documentation/technical/protocol-v2.txt >> create mode 100644 builtin/serve.c >> create mode 100644 serve.c >> create mode 100644 serve.h >> >> diff --git a/.gitignore b/.gitignore >> index 833ef3b0b..2d0450c26 100644 >> --- a/.gitignore >> +++ b/.gitignore >> @@ -140,6 +140,7 @@ >> /git-rm >> /git-send-email >> /git-send-pack >> +/git-serve >> /git-sh-i18n >> /git-sh-i18n--envsubst >> /git-sh-setup >> diff --git a/Documentation/technical/protocol-v2.txt >> b/Documentation/technical/protocol-v2.txt >> new file mode 100644 >> index 0..b87ba3816 >> --- /dev/null >> +++ b/Documentation/technical/protocol-v2.txt >> @@ -0,0 +1,91 @@ >> + Git Wire Protocol, Version 2 >> +== >> + >> +This document presents a specification for a version 2 of Git's wire >> +protocol. Protocol v2 will improve upon v1 in the following ways: >> + >> + * Instead of multiple service names, multiple commands will be >> +supported by a single service. >> + * Easily extendable as capabilities are moved into their own section >> +of the protocol, no longer being hidden behind a NUL byte and >> +limited by the size of a pkt-line (as there will be a single >> +capability per pkt-line). >> + * Separate out other information hidden behind NUL bytes (e.g. agent >> +string as a capability and symrefs can be requested using 'ls-refs') >> + * Reference advertisement will be omitted unless explicitly requested >> + * ls-refs command to explicitly request some refs >> + >> + Detailed Design >> += >> + >> +A client can request to speak protocol v2 by sending `version=2` in the >> +side-channel `GIT_PROTOCOL` in the initial request to the server. >> + >> +In protocol v2 communication is command oriented. When first contacting >> a >> +server a list of capabilities will advertised. Some of these >> capabilities >> +will be commands which a client can request be executed. Once a command >> +has completed, a client can reuse the connection and request that other >> +commands be executed. >> + >> + Special Packets >> +- >> + >> +In protocol v2 these special packets will have the following semantics: >> + >> + * '' Flush Packet (flush-pkt) - indicates the end of a message >> + * '0001' Delimiter Packet (delim-pkt) - separates sections of a message > > > Previously, a 0001 pkt-line meant that there was 1 byte of data > following, right? No, the length was including the length field, so 0005 would indicate that there is one byte following, (+4 bytes of "0005" included) > Does this change that and/or prevent 1 byte > packets? (Not sure if it is likely, but the odd-tail of a packfile > might get sent in a 0001 line, right?) Or is it that 0001 is only > special during the V2 negotiation stuff, but not during the packfile > transmission? 0001 is invalid in the current protocol v0. > > (I'm not against having this delimiter -- I think it is useful, but > just curious if will cause problems elsewhere.) > > Should we also consider increasing the pkt-line limit to 5 hex-digits > while we're at it ? That would let us have 1MB buffers if that would > help with large packfiles. AFAICT there is a static allocation of one pkt-line (of maximum size), such that the code can read in a full packet and then process it. If we'd increase the packet size we'd need the static buffer to be 1MB, which sounds good for my developer machine. But I suspect it may be too much
[PATCH] cocci: simplify check for trivial format strings
353d84c537 (coccicheck: make transformation for strbuf_addf(sb, "...") more precise) added a check to avoid transforming calls with format strings which contain percent signs, as that would change the result. It uses embedded Python code for that. Simplify this rule by using the regular expression matching operator instead. Signed-off-by: Rene Scharfe--- Inspired by the Coccinelle package in Debian experimental, which lost support for Python for some reason. Tested only with that version (1.0.6.deb-3) and Debian testing's 1.0.4.deb-3+b3. contrib/coccinelle/strbuf.cocci | 17 + 1 file changed, 1 insertion(+), 16 deletions(-) diff --git a/contrib/coccinelle/strbuf.cocci b/contrib/coccinelle/strbuf.cocci index 6fe8727421..e34eada1ad 100644 --- a/contrib/coccinelle/strbuf.cocci +++ b/contrib/coccinelle/strbuf.cocci @@ -1,21 +1,6 @@ @ strbuf_addf_with_format_only @ expression E; -constant fmt; -@@ - strbuf_addf(E, -( - fmt -| - _(fmt) -) - ); - -@ script:python @ -fmt << strbuf_addf_with_format_only.fmt; -@@ -cocci.include_match("%" not in fmt) - -@ extends strbuf_addf_with_format_only @ +constant fmt !~ "%"; @@ - strbuf_addf + strbuf_addstr -- 2.16.1
Re: [PATCH 11/26] serve: introduce git-serve
On 1/2/2018 7:18 PM, Brandon Williams wrote: Introduce git-serve, the base server for protocol version 2. Protocol version 2 is intended to be a replacement for Git's current wire protocol. The intention is that it will be a simpler, less wasteful protocol which can evolve over time. Protocol version 2 improves upon version 1 by eliminating the initial ref advertisement. In its place a server will export a list of capabilities and commands which it supports in a capability advertisement. A client can then request that a particular command be executed by providing a number of capabilities and command specific parameters. At the completion of a command, a client can request that another command be executed or can terminate the connection by sending a flush packet. Signed-off-by: Brandon Williams--- .gitignore | 1 + Documentation/technical/protocol-v2.txt | 91 Makefile| 2 + builtin.h | 1 + builtin/serve.c | 30 git.c | 1 + serve.c | 239 serve.h | 15 ++ 8 files changed, 380 insertions(+) create mode 100644 Documentation/technical/protocol-v2.txt create mode 100644 builtin/serve.c create mode 100644 serve.c create mode 100644 serve.h diff --git a/.gitignore b/.gitignore index 833ef3b0b..2d0450c26 100644 --- a/.gitignore +++ b/.gitignore @@ -140,6 +140,7 @@ /git-rm /git-send-email /git-send-pack +/git-serve /git-sh-i18n /git-sh-i18n--envsubst /git-sh-setup diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt new file mode 100644 index 0..b87ba3816 --- /dev/null +++ b/Documentation/technical/protocol-v2.txt @@ -0,0 +1,91 @@ + Git Wire Protocol, Version 2 +== + +This document presents a specification for a version 2 of Git's wire +protocol. Protocol v2 will improve upon v1 in the following ways: + + * Instead of multiple service names, multiple commands will be +supported by a single service. + * Easily extendable as capabilities are moved into their own section +of the protocol, no longer being hidden behind a NUL byte and +limited by the size of a pkt-line (as there will be a single +capability per pkt-line). + * Separate out other information hidden behind NUL bytes (e.g. agent +string as a capability and symrefs can be requested using 'ls-refs') + * Reference advertisement will be omitted unless explicitly requested + * ls-refs command to explicitly request some refs + + Detailed Design += + +A client can request to speak protocol v2 by sending `version=2` in the +side-channel `GIT_PROTOCOL` in the initial request to the server. + +In protocol v2 communication is command oriented. When first contacting a +server a list of capabilities will advertised. Some of these capabilities +will be commands which a client can request be executed. Once a command +has completed, a client can reuse the connection and request that other +commands be executed. + + Special Packets +- + +In protocol v2 these special packets will have the following semantics: + + * '' Flush Packet (flush-pkt) - indicates the end of a message + * '0001' Delimiter Packet (delim-pkt) - separates sections of a message Previously, a 0001 pkt-line meant that there was 1 byte of data following, right? Does this change that and/or prevent 1 byte packets? (Not sure if it is likely, but the odd-tail of a packfile might get sent in a 0001 line, right?) Or is it that 0001 is only special during the V2 negotiation stuff, but not during the packfile transmission? (I'm not against having this delimiter -- I think it is useful, but just curious if will cause problems elsewhere.) Should we also consider increasing the pkt-line limit to 5 hex-digits while we're at it ? That would let us have 1MB buffers if that would help with large packfiles. Granted, we're throttled by the network, so it might not matter. Would it be interesting to have a 5 digit prefix with parts of the high bits of first digit being flags ? Or is this too radical of a change? + + Capability Advertisement +-- + +A server which decides to communicate (based on a request from a client) +using protocol version 2, notifies the client by sending a version string +in its initial response followed by an advertisement of its capabilities. +Each capability is a key with an optional value. Clients must ignore all +unknown keys. Semantics of unknown values are left to the definition of +each key. Some capabilities will describe commands which can be requested +to be executed by the client. + +capability-advertisement = protocol-version +
Re: [PATCH v2 08/27] connect: discover protocol version outside of get_remote_heads
On 01/31, Derrick Stolee wrote: > On 1/25/2018 6:58 PM, Brandon Williams wrote: > > In order to prepare for the addition of protocol_v2 push the protocol > > version discovery outside of 'get_remote_heads()'. This will allow for > > keeping the logic for processing the reference advertisement for > > protocol_v1 and protocol_v0 separate from the logic for protocol_v2. > > > > Signed-off-by: Brandon Williams> > --- > > builtin/fetch-pack.c | 16 +++- > > builtin/send-pack.c | 17 +++-- > > connect.c| 27 ++- > > connect.h| 3 +++ > > remote-curl.c| 20 ++-- > > remote.h | 5 +++-- > > transport.c | 24 +++- > > 7 files changed, 83 insertions(+), 29 deletions(-) > > > > diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c > > index 366b9d13f..85d4faf76 100644 > > --- a/builtin/fetch-pack.c > > +++ b/builtin/fetch-pack.c > > @@ -4,6 +4,7 @@ > > #include "remote.h" > > #include "connect.h" > > #include "sha1-array.h" > > +#include "protocol.h" > > static const char fetch_pack_usage[] = > > "git fetch-pack [--all] [--stdin] [--quiet | -q] [--keep | -k] [--thin] " > > @@ -52,6 +53,7 @@ int cmd_fetch_pack(int argc, const char **argv, const > > char *prefix) > > struct fetch_pack_args args; > > struct oid_array shallow = OID_ARRAY_INIT; > > struct string_list deepen_not = STRING_LIST_INIT_DUP; > > + struct packet_reader reader; > > packet_trace_identity("fetch-pack"); > > @@ -193,7 +195,19 @@ int cmd_fetch_pack(int argc, const char **argv, const > > char *prefix) > > if (!conn) > > return args.diag_url ? 0 : 1; > > } > > - get_remote_heads(fd[0], NULL, 0, , 0, NULL, ); > > + > > + packet_reader_init(, fd[0], NULL, 0, > > + PACKET_READ_CHOMP_NEWLINE | > > + PACKET_READ_GENTLE_ON_EOF); > > + > > + switch (discover_version()) { > > + case protocol_v1: > > + case protocol_v0: > > + get_remote_heads(, , 0, NULL, ); > > + break; > > + case protocol_unknown_version: > > + BUG("unknown protocol version"); > > Is this really a BUG in the client, or a bug/incompatibility in the server? > > Perhaps I'm misunderstanding, but it looks like discover_version() will > die() on an unknown version (the die() is in > protocol.c:determine_protocol_version_client()). So maybe that's why this is > a BUG()? > > If there is something to change here, this BUG() appears three more times. Yes, I have it labeled as a BUG because discover_version can't return an unknown protocol version. If the server actually returns an unknown protocol version then it should be handled in protocol.c:determine_protocol_version_client() as you mentioned. > > > + } > > ref = fetch_pack(, fd, conn, ref, dest, sought, nr_sought, > > , pack_lockfile_ptr); > > diff --git a/builtin/send-pack.c b/builtin/send-pack.c > > index fc4f0bb5f..83cb125a6 100644 > > --- a/builtin/send-pack.c > > +++ b/builtin/send-pack.c > > @@ -14,6 +14,7 @@ > > #include "sha1-array.h" > > #include "gpg-interface.h" > > #include "gettext.h" > > +#include "protocol.h" > > static const char * const send_pack_usage[] = { > > N_("git send-pack [--all | --mirror] [--dry-run] [--force] " > > @@ -154,6 +155,7 @@ int cmd_send_pack(int argc, const char **argv, const > > char *prefix) > > int progress = -1; > > int from_stdin = 0; > > struct push_cas_option cas = {0}; > > + struct packet_reader reader; > > struct option options[] = { > > OPT__VERBOSITY(), > > @@ -256,8 +258,19 @@ int cmd_send_pack(int argc, const char **argv, const > > char *prefix) > > args.verbose ? CONNECT_VERBOSE : 0); > > } > > - get_remote_heads(fd[0], NULL, 0, _refs, REF_NORMAL, > > -_have, ); > > + packet_reader_init(, fd[0], NULL, 0, > > + PACKET_READ_CHOMP_NEWLINE | > > + PACKET_READ_GENTLE_ON_EOF); > > + > > + switch (discover_version()) { > > + case protocol_v1: > > + case protocol_v0: > > + get_remote_heads(, _refs, REF_NORMAL, > > +_have, ); > > + break; > > + case protocol_unknown_version: > > + BUG("unknown protocol version"); > > + } > > transport_verify_remote_names(nr_refspecs, refspecs); > > diff --git a/connect.c b/connect.c > > index 00e90075c..db3c9d24c 100644 > > --- a/connect.c > > +++ b/connect.c > > @@ -62,7 +62,7 @@ static void die_initial_contact(int unexpected) > > "and the repository exists.")); > > } > > -static enum protocol_version discover_version(struct packet_reader *reader) > > +enum protocol_version discover_version(struct packet_reader *reader) > > { > > enum protocol_version version = protocol_unknown_version;
[PATCHv2] tag: add --edit option
Add a --edit option whichs allows modifying the messages provided by -m or -F, the same way git commit --edit does. Signed-off-by: Nicolas Morey-Chaisemartin--- Changes since v1: - Fix usage string - Use write_script to generate editor - Rename editor to fakeeditor to match the other tests in the testsuite - I'll post another series to fix the misleading messages in both commit.c and tag.c when launch_editor fails Documentation/git-tag.txt | 6 ++ builtin/tag.c | 11 +-- t/t7004-tag.sh| 30 ++ 3 files changed, 45 insertions(+), 2 deletions(-) diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index 956fc019f984..b9e5a993bea0 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -167,6 +167,12 @@ This option is only applicable when listing tags without annotation lines. Implies `-a` if none of `-a`, `-s`, or `-u ` is given. +-e:: +--edit:: + The message taken from file with `-F` and command line with + `-m` are usually used as the tag message unmodified. + This option lets you further edit the message taken from these sources. + --cleanup=:: This option sets how the tag message is cleaned up. The '' can be one of 'verbatim', 'whitespace' and 'strip'. The diff --git a/builtin/tag.c b/builtin/tag.c index a7e6a5b0f234..ce5cac3dd23f 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -194,6 +194,7 @@ static int build_tag_object(struct strbuf *buf, int sign, struct object_id *resu struct create_tag_options { unsigned int message_given:1; + unsigned int use_editor:1; unsigned int sign; enum { CLEANUP_NONE, @@ -224,7 +225,7 @@ static void create_tag(const struct object_id *object, const char *tag, tag, git_committer_info(IDENT_STRICT)); - if (!opt->message_given) { + if (!opt->message_given || opt->use_editor) { int fd; /* write the template message before editing: */ @@ -233,7 +234,10 @@ static void create_tag(const struct object_id *object, const char *tag, if (fd < 0) die_errno(_("could not create file '%s'"), path); - if (!is_null_oid(prev)) { + if (opt->message_given) { + write_or_die(fd, buf->buf, buf->len); + strbuf_reset(buf); + } else if (!is_null_oid(prev)) { write_tag_body(fd, prev); } else { struct strbuf buf = STRBUF_INIT; @@ -372,6 +376,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) static struct ref_sorting *sorting = NULL, **sorting_tail = struct ref_format format = REF_FORMAT_INIT; int icase = 0; + int edit_flag = 0; struct option options[] = { OPT_CMDMODE('l', "list", , N_("list tag names"), 'l'), { OPTION_INTEGER, 'n', NULL, , N_("n"), @@ -386,6 +391,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) OPT_CALLBACK('m', "message", , N_("message"), N_("tag message"), parse_msg_arg), OPT_FILENAME('F', "file", , N_("read message from file")), + OPT_BOOL('e', "edit", _flag, N_("force edit of tag message")), OPT_BOOL('s', "sign", , N_("annotated and GPG-signed tag")), OPT_STRING(0, "cleanup", _arg, N_("mode"), N_("how to strip spaces and #comments from message")), @@ -524,6 +530,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) die(_("tag '%s' already exists"), tag); opt.message_given = msg.given || msgfile; + opt.use_editor = edit_flag; if (!cleanup_arg || !strcmp(cleanup_arg, "strip")) opt.cleanup_mode = CLEANUP_ALL; diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index a9af2de9960b..063996ddc05c 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -452,6 +452,21 @@ test_expect_success \ test_cmp expect actual ' +get_tag_header annotated-tag-edit $commit commit $time >expect +echo "An edited message" >>expect +test_expect_success 'set up editor' ' + write_script fakeeditor <<-\EOF + sed -e "s/A message/An edited message/g" <"$1" >"$1-" + mv "$1-" "$1" + EOF +' +test_expect_success \ + 'creating an annotated tag with -m message --edit should succeed' ' + EDITOR=./fakeeditor git tag -m "A message" --edit annotated-tag-edit && + get_tag_msg annotated-tag-edit >actual && + test_cmp expect actual +' + cat >msgfile >expect +test_expect_success 'set up editor' ' + write_script fakeeditor <<-\EOF + sed -e "s/Another message/Another edited
Respond for details
Dear sir/ madam Do you need a loan to pay off bills ? To pay off your mortgage quickly ? To set up a new business or to Re- finance your existing business ? I can help you secure a private loan at 3% interest rate please respond for more details Thanks Allen
[no subject]
Guess you are fine? I have been trying to reach you!
Respond for details
Dear sir/ madam Do you need a loan to pay off bills ? To pay off your mortgage quickly ? To set up a new business or to Re- finance your existing business ? I can help you secure a private loan at 3% interest rate please respond for more details Thanks Allen
Re: [PATCH] tag: add --edit option
On 01/02/18 09:49, Nicolas Morey-Chaisemartin wrote: > Add a --edit option whichs allows modifying the messages provided by -m or -F, > the same way git commit --edit does. > > Signed-off-by: Nicolas Morey-Chaisemartin> --- > Documentation/git-tag.txt | 6 ++ > builtin/tag.c | 11 +-- > t/t7004-tag.sh| 34 ++ > 3 files changed, 49 insertions(+), 2 deletions(-) > > diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt > index 956fc019f984..b9e5a993bea0 100644 > --- a/Documentation/git-tag.txt > +++ b/Documentation/git-tag.txt > @@ -167,6 +167,12 @@ This option is only applicable when listing tags without > annotation lines. > Implies `-a` if none of `-a`, `-s`, or `-u ` > is given. > > +-e:: > +--edit:: > + The message taken from file with `-F` and command line with > + `-m` are usually used as the tag message unmodified. > + This option lets you further edit the message taken from these sources. > + > --cleanup=:: > This option sets how the tag message is cleaned up. > The '' can be one of 'verbatim', 'whitespace' and 'strip'. The > diff --git a/builtin/tag.c b/builtin/tag.c > index a7e6a5b0f234..91c60829d5f9 100644 > --- a/builtin/tag.c > +++ b/builtin/tag.c > @@ -194,6 +194,7 @@ static int build_tag_object(struct strbuf *buf, int sign, > struct object_id *resu > > struct create_tag_options { > unsigned int message_given:1; > + unsigned int use_editor:1; > unsigned int sign; > enum { > CLEANUP_NONE, > @@ -224,7 +225,7 @@ static void create_tag(const struct object_id *object, > const char *tag, > tag, > git_committer_info(IDENT_STRICT)); > > - if (!opt->message_given) { > + if (!opt->message_given || opt->use_editor) { > int fd; > > /* write the template message before editing: */ > @@ -233,7 +234,10 @@ static void create_tag(const struct object_id *object, > const char *tag, > if (fd < 0) > die_errno(_("could not create file '%s'"), path); > > - if (!is_null_oid(prev)) { > + if (opt->message_given) { > + write_or_die(fd, buf->buf, buf->len); > + strbuf_reset(buf); > + } else if (!is_null_oid(prev)) { > write_tag_body(fd, prev); > } else { > struct strbuf buf = STRBUF_INIT; > @@ -372,6 +376,7 @@ int cmd_tag(int argc, const char **argv, const char > *prefix) > static struct ref_sorting *sorting = NULL, **sorting_tail = > struct ref_format format = REF_FORMAT_INIT; > int icase = 0; > + int edit_flag = 0; > struct option options[] = { > OPT_CMDMODE('l', "list", , N_("list tag names"), 'l'), > { OPTION_INTEGER, 'n', NULL, , N_("n"), > @@ -386,6 +391,7 @@ int cmd_tag(int argc, const char **argv, const char > *prefix) > OPT_CALLBACK('m', "message", , N_("message"), >N_("tag message"), parse_msg_arg), > OPT_FILENAME('F', "file", , N_("read message from > file")), > + OPT_BOOL('e', "edit", _flag, N_("force edit of commit")), s/commit/tag message/ ? ATB, Ramsay Jones
Re: [PATCH] tag: add --edit option
Le 01/02/2018 à 11:56, Eric Sunshine a écrit : > On Thu, Feb 1, 2018 at 5:43 AM, Nicolas Morey-Chaisemartin >wrote: >> Le 01/02/2018 à 11:34, Nicolas Morey-Chaisemartin a écrit : >>> Le 01/02/2018 à 11:16, Eric Sunshine a écrit : A little below this change is where launch_editor() is actually invoked. If it fails for some reason, it prints: Please supply the message using either -m or -F option. which seems a bit counterintuitive if the user *did* specify one of those options along with --edit. I wonder if that message needs to be adjusted. >>> Yes I'll fix this. >> I just checked what commit.c does and it seems to behave as my patch: >> if (launch_editor(git_path_commit_editmsg(), NULL, env.argv)) { >> fprintf(stderr, >> _("Please supply the message using either -m or -F option.\n")); >> >> To be honest the message is not that clear either. >> If I'm reading launch_editor right most (or all) its falire are du to a >> failure to launch the editor or the editor crashed/exited with an error. >> In this case, I wouldn't advise the user to use -m or -F but to fix its >> editor. > Indeed, I also looked at the implementation of launch_editor(), and my > "wondering" about whether the message needed adjustment was just that. > The message seems somewhat counterintuitive in this case, but I didn't > necessarily have a better suggestion. A valid response, therefore, > might be to punt on it and leave that change for the future, or > perhaps take it on as a second patch which adjusts the message in both > commands. I don't have strong feelings about it at this time. It seems all the error paths from launch_editor have an error message. A simple "Editor failure, cancelling {commit, tag}" would probably be a better error message. I'll post another series for that.
t3404.6 breaks on master under GIT_FSMONITOR_TEST
The GIT_FSMONITOR_TEST variable allows you to roundtrip the fsmonitor codpath in the whole test suite. On both Debian & CentOS this breaks for me: (cd t && GIT_FSMONITOR_TEST=$PWD/t7519/fsmonitor-all ./t3404-rebase-interactive.sh -i) Whereas this works: (cd t && GIT_FSMONITOR_TEST=$PWD/t7519/fsmonitor-all GIT_SKIP_TESTS=t3404.6 ./t3404-rebase-interactive.sh -i) The entirety of the rest of the test suite still passes with GIT_FSMONITOR_TEST. This has been failing ever since GIT_FSMONITOR_TEST was introduced in 883e248b8a ("fsmonitor: teach git to optionally utilize a file system monitor to speed up detecting new or changed files.", 2017-09-22). Under -v -x -i: + echo test_must_fail: command succeeded: env FAKE_LINES=exec_echo_foo_>file1 1 git rebase -i HEAD^ test_must_fail: command succeeded: env FAKE_LINES=exec_echo_foo_>file1 1 git rebase -i HEAD^ + return 1 error: last command exited with $?=1 not ok 6 - rebase -i with the exec command checks tree cleanness # # git checkout master && # set_fake_editor && # test_must_fail env FAKE_LINES="exec_echo_foo_>file1 1" git rebase -i HEAD^ && Maybe once this is fixed running the test suite under GIT_FSMONITOR_TEST would be a useful Travis target, but I don't know the current status of adding new options to Travis.
[PATCH v3 2/2] diff: add --stat-with-summary
Certain information is currently shown with --summary, but when used in combination with --stat it's a bit hard to read since info of the same file is in two places (--stat and --summary). On top of that, commits that add or remove files double the number of display lines, which could be a lot if you add or remove a lot of files. --stat-with-summary embeds most of --summary back in --stat in the little space between the file name part and the graph line, e.g. with commit 0433d533f1: Documentation/merge-config.txt | 4 + builtin/merge.c| 2 + ...-pull-verify-signatures.sh (new +x) | 81 ++ t/t7612-merge-verify-signatures.sh | 45 4 files changed, 132 insertions(+) It helps both condensing information and saving some text space. What's new in diffstat is: - A new 0644 file is shown as (new) - A new 0755 file is shown as (new +x) - A new symlink is shown as (new +l) - A deleted file is shown as (gone) - A mode change adding executable bit is shown as (mode +x) - A mode change removing it is shown as (mode -x) Note that --stat-with-summary does not contain all the information --summary provides. Rewrite percentage is not shown but it could be added later, like R50% or C20%. Signed-off-by: Nguyễn Thái Ngọc Duy--- Documentation/diff-options.txt| 8 diff.c| 42 ++- diff.h| 1 + t/t4013-diff-various.sh | 5 +++ ...-pretty_--root_--stat-with-summary_initial | 12 ++ ...etty_-R_--root_--stat-with-summary_initial | 12 ++ ...diff-tree_--stat-with-summary_initial_mode | 4 ++ ...f-tree_-R_--stat-with-summary_initial_mode | 4 ++ 8 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 t/t4013/diff.diff-tree_--pretty_--root_--stat-with-summary_initial create mode 100644 t/t4013/diff.diff-tree_--pretty_-R_--root_--stat-with-summary_initial create mode 100644 t/t4013/diff.diff-tree_--stat-with-summary_initial_mode create mode 100644 t/t4013/diff.diff-tree_-R_--stat-with-summary_initial_mode diff --git a/Documentation/diff-options.txt b/Documentation/diff-options.txt index c330c01ff0..595e4cd548 100644 --- a/Documentation/diff-options.txt +++ b/Documentation/diff-options.txt @@ -128,6 +128,14 @@ have to use `--diff-algorithm=default` option. These parameters can also be set individually with `--stat-width=`, `--stat-name-width=` and `--stat-count=`. +--stat-with-summary:: + Output a condensed summary of extended header information such + as file creations or deletions ("new" or "gone", optionally "+l" + if it's a symlink) and mode changes ("+x" or "-x" for adding + or removing executable bit respectively) in diffstat. The + information is put betwen the filename part and the graph + part. Implies `--stat`. + --numstat:: Similar to `--stat`, but shows number of added and deleted lines in decimal notation and pathname without diff --git a/diff.c b/diff.c index 9d874a670f..6bf9867388 100644 --- a/diff.c +++ b/diff.c @@ -2129,6 +2129,7 @@ struct diffstat_t { char *from_name; char *name; char *print_name; + const char *comments; unsigned is_unmerged:1; unsigned is_binary:1; unsigned is_renamed:1; @@ -2205,6 +2206,9 @@ static void fill_print_name(struct diffstat_file *file) else quote_c_style(file->name, , NULL, 0); + if (file->comments) + strbuf_addf(, " (%s)", file->comments); + file->print_name = strbuf_detach(, NULL); } @@ -3239,6 +3243,32 @@ static void builtin_diff(const char *name_a, return; } +static char *get_compact_summary(const struct diff_filepair *p, int is_renamed) +{ + if (!is_renamed) { + if (p->status == DIFF_STATUS_ADDED) { + if (S_ISLNK(p->two->mode)) + return "new +l"; + else if ((p->two->mode & 0777) == 0755) + return "new +x"; + else + return "new"; + } else if (p->status == DIFF_STATUS_DELETED) + return "gone"; + } + if (S_ISLNK(p->one->mode) && !S_ISLNK(p->two->mode)) + return "mode -l"; + else if (!S_ISLNK(p->one->mode) && S_ISLNK(p->two->mode)) + return "mode +l"; + else if ((p->one->mode & 0777) == 0644 && +(p->two->mode & 0777) == 0755) + return "mode +x"; + else if ((p->one->mode & 0777) == 0755 && +(p->two->mode & 0777) == 0644) + return "mode -x"; + return NULL; +} + static void builtin_diffstat(const char *name_a, const char *name_b, struct
[PATCH v3 1/2] diff.c: refactor pprint_rename() to use strbuf
Instead of passing char* around, let function handle strbuf directly. All callers already use strbuf internally. This helps kill the "not free" exception in free_diffstat_info(). I don't think this code is so critical that we need to avoid some free() calls. The other benefit comes in the next patch, where we append something in pname before returning from fill_print_name(). With strbuf, it's very simple. With "char *" we may have to resort to explicit reallocation and stuff. Signed-off-by: Nguyễn Thái Ngọc Duy--- diff.c | 59 ++ 1 file changed, 26 insertions(+), 33 deletions(-) diff --git a/diff.c b/diff.c index 0a9a0cdf18..9d874a670f 100644 --- a/diff.c +++ b/diff.c @@ -2045,11 +2045,10 @@ static void fn_out_consume(void *priv, char *line, unsigned long len) } } -static char *pprint_rename(const char *a, const char *b) +static void pprint_rename(struct strbuf *name, const char *a, const char *b) { const char *old = a; const char *new = b; - struct strbuf name = STRBUF_INIT; int pfx_length, sfx_length; int pfx_adjust_for_slash; int len_a = strlen(a); @@ -2059,10 +2058,10 @@ static char *pprint_rename(const char *a, const char *b) int qlen_b = quote_c_style(b, NULL, NULL, 0); if (qlen_a || qlen_b) { - quote_c_style(a, , NULL, 0); - strbuf_addstr(, " => "); - quote_c_style(b, , NULL, 0); - return strbuf_detach(, NULL); + quote_c_style(a, name, NULL, 0); + strbuf_addstr(name, " => "); + quote_c_style(b, name, NULL, 0); + return; } /* Find common prefix */ @@ -2109,19 +2108,18 @@ static char *pprint_rename(const char *a, const char *b) if (b_midlen < 0) b_midlen = 0; - strbuf_grow(, pfx_length + a_midlen + b_midlen + sfx_length + 7); + strbuf_grow(name, pfx_length + a_midlen + b_midlen + sfx_length + 7); if (pfx_length + sfx_length) { - strbuf_add(, a, pfx_length); - strbuf_addch(, '{'); + strbuf_add(name, a, pfx_length); + strbuf_addch(name, '{'); } - strbuf_add(, a + pfx_length, a_midlen); - strbuf_addstr(, " => "); - strbuf_add(, b + pfx_length, b_midlen); + strbuf_add(name, a + pfx_length, a_midlen); + strbuf_addstr(name, " => "); + strbuf_add(name, b + pfx_length, b_midlen); if (pfx_length + sfx_length) { - strbuf_addch(, '}'); - strbuf_add(, a + len_a - sfx_length, sfx_length); + strbuf_addch(name, '}'); + strbuf_add(name, a + len_a - sfx_length, sfx_length); } - return strbuf_detach(, NULL); } struct diffstat_t { @@ -2197,23 +2195,17 @@ static void show_graph(struct strbuf *out, char ch, int cnt, static void fill_print_name(struct diffstat_file *file) { - char *pname; + struct strbuf pname = STRBUF_INIT; if (file->print_name) return; - if (!file->is_renamed) { - struct strbuf buf = STRBUF_INIT; - if (quote_c_style(file->name, , NULL, 0)) { - pname = strbuf_detach(, NULL); - } else { - pname = file->name; - strbuf_release(); - } - } else { - pname = pprint_rename(file->from_name, file->name); - } - file->print_name = pname; + if (file->is_renamed) + pprint_rename(, file->from_name, file->name); + else + quote_c_style(file->name, , NULL, 0); + + file->print_name = strbuf_detach(, NULL); } static void print_stat_summary_inserts_deletes(struct diff_options *options, @@ -2797,8 +2789,7 @@ static void free_diffstat_info(struct diffstat_t *diffstat) int i; for (i = 0; i < diffstat->nr; i++) { struct diffstat_file *f = diffstat->files[i]; - if (f->name != f->print_name) - free(f->print_name); + free(f->print_name); free(f->name); free(f->from_name); free(f); @@ -5241,10 +5232,12 @@ static void show_rename_copy(struct diff_options *opt, const char *renamecopy, struct diff_filepair *p) { struct strbuf sb = STRBUF_INIT; - char *names = pprint_rename(p->one->path, p->two->path); + struct strbuf names = STRBUF_INIT; + + pprint_rename(, p->one->path, p->two->path); strbuf_addf(, " %s %s (%d%%)\n", - renamecopy, names, similarity_index(p)); - free(names); + renamecopy, names.buf, similarity_index(p)); + strbuf_release(); emit_diff_symbol(opt, DIFF_SYMBOL_SUMMARY, sb.buf, sb.len, 0);
[PATCH v3 0/2] diff: add --stat-with-summary (was --compact-summary)
Changes since v2 [1]: - goes back to my original version (yay!) where the extra info is appended after the path name. More is described in 2/2 - --compact-summary is now renamed --stat-with-summary and implies --stat - 1/2 is just a cleanup patch to make it easier to add 2/2 [1] https://public-inbox.org/git/20180118100546.32251-1-pclo...@gmail.com/ Nguyễn Thái Ngọc Duy (2): diff.c: refactor pprint_rename() to use strbuf diff: add --stat-with-summary Documentation/diff-options.txt | 8 ++ diff.c | 101 ++--- diff.h | 1 + t/t4013-diff-various.sh| 5 + ...pretty_--root_--stat-with-summary_initial (new) | 12 +++ ...tty_-R_--root_--stat-with-summary_initial (new) | 12 +++ ...iff-tree_--stat-with-summary_initial_mode (new) | 4 + ...-tree_-R_--stat-with-summary_initial_mode (new) | 4 + 8 files changed, 113 insertions(+), 34 deletions(-) -- 2.16.1.75.ga05eb4
[PATCH v3 1/2] format-patch: keep cover-letter diffstat wrapped in 72 columns
We already wrap shortlog around 72 columns in cover letters. Do the same for diffstat (also in cover letters). Signed-off-by: Nguyễn Thái Ngọc Duy--- builtin/log.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/builtin/log.c b/builtin/log.c index 46b4ca13e5..96af897403 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -29,6 +29,8 @@ #include "gpg-interface.h" #include "progress.h" +#define MAIL_DEFAULT_WRAP 72 + /* Set a default date-time format for git log ("log.date" config variable) */ static const char *default_date_mode = NULL; @@ -1044,7 +1046,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, shortlog_init(); log.wrap_lines = 1; - log.wrap = 72; + log.wrap = MAIL_DEFAULT_WRAP; log.in1 = 2; log.in2 = 4; log.file = rev->diffopt.file; @@ -1061,6 +1063,7 @@ static void make_cover_letter(struct rev_info *rev, int use_stdout, memcpy(, >diffopt, sizeof(opts)); opts.output_format = DIFF_FORMAT_SUMMARY | DIFF_FORMAT_DIFFSTAT; + opts.stat_width = MAIL_DEFAULT_WRAP; diff_setup_done(); -- 2.16.1.205.g271f633410
[PATCH v3 2/2] format-patch: reduce patch diffstat width to 72
Patches generated by format-patch are meant to be exchanged as emails, most of the time. And since it's generally agreed that text in mails should be wrapped around 70 columns or so, make sure these diffstat follow the convention (especially when used with --cover-letter since we already defaults to wrapping 72 columns). The default can still be overriden with command line options. Signed-off-by: Nguyễn Thái Ngọc Duy--- builtin/log.c | 2 ++ t/t4052-stat-output.sh | 46 -- 2 files changed, 33 insertions(+), 15 deletions(-) diff --git a/builtin/log.c b/builtin/log.c index 96af897403..94ee177d56 100644 --- a/builtin/log.c +++ b/builtin/log.c @@ -1617,6 +1617,8 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix) (!rev.diffopt.output_format || rev.diffopt.output_format == DIFF_FORMAT_PATCH)) rev.diffopt.output_format = DIFF_FORMAT_DIFFSTAT | DIFF_FORMAT_SUMMARY; + if (!rev.diffopt.stat_width) + rev.diffopt.stat_width = MAIL_DEFAULT_WRAP; /* Always generate a patch */ rev.diffopt.output_format |= DIFF_FORMAT_PATCH; diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh index 9f563db20a..6e2cf933f7 100755 --- a/t/t4052-stat-output.sh +++ b/t/t4052-stat-output.sh @@ -19,17 +19,33 @@ test_expect_success 'preparation' ' git commit -m message "$name" ' +cat >expect72 <<-'EOF' + ...a | 1 + +EOF +test_expect_success "format-patch: small change with long name gives more space to the name" ' + git format-patch -1 --stdout >output && + grep " | " output >actual && + test_cmp expect72 actual +' + while read cmd args do - cat >expect <<-'EOF' + cat >expect80 <<-'EOF' ...a | 1 + EOF test_expect_success "$cmd: small change with long name gives more space to the name" ' git $cmd $args >output && grep " | " output >actual && - test_cmp expect actual + test_cmp expect80 actual ' +done <<\EOF +diff HEAD^ HEAD --stat +show --stat +log -1 --stat +EOF +while read cmd args +do cat >expect <<-'EOF' ...a | 1 + EOF @@ -79,11 +95,11 @@ test_expect_success 'preparation for big change tests' ' git commit -m message abcd ' -cat >expect80 <<'EOF' - abcd | 1000 ++ +cat >expect72 <<'EOF' + abcd | 1000 ++ EOF -cat >expect80-graph <<'EOF' -| abcd | 1000 ++ +cat >expect72-graph <<'EOF' +| abcd | 1000 ++ EOF cat >expect200 <<'EOF' abcd | 1000 ++ @@ -107,7 +123,7 @@ do test_cmp "$expect-graph" actual ' done <<\EOF -ignores expect80 format-patch -1 --stdout +ignores expect72 format-patch -1 --stdout respects expect200 diff HEAD^ HEAD --stat respects expect200 show --stat respects expect200 log -1 --stat @@ -135,7 +151,7 @@ do test_cmp "$expect-graph" actual ' done <<\EOF -ignores expect80 format-patch -1 --stdout +ignores expect72 format-patch -1 --stdout respects expect40 diff HEAD^ HEAD --stat respects expect40 show --stat respects expect40 log -1 --stat @@ -163,7 +179,7 @@ do test_cmp "$expect-graph" actual ' done <<\EOF -ignores expect80 format-patch -1 --stdout +ignores expect72 format-patch -1 --stdout respects expect40 diff HEAD^ HEAD --stat respects expect40 show --stat respects expect40 log -1 --stat @@ -250,11 +266,11 @@ show --stat log -1 --stat EOF -cat >expect80 <<'EOF' - ...aaa | 1000 +cat >expect72 <<'EOF' + ...aa | 1000 + EOF -cat >expect80-graph <<'EOF' -| ...aaa | 1000 +cat >expect72-graph <<'EOF' +| ...aa | 1000 + EOF cat >expect200 <<'EOF' aaa | 1000 +++ @@ -278,7 +294,7 @@ do test_cmp "$expect-graph" actual ' done <<\EOF -ignores expect80 format-patch -1 --stdout +ignores expect72 format-patch -1 --stdout respects expect200 diff HEAD^ HEAD --stat
[PATCH v3 0/2] wrap format-patch diffstats around 72 columns
v3 fixes tests in 2/2 that I overlooked (but Jeff didn't). Interdiff: diff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh index 1e62333b46..6e2cf933f7 100755 --- a/t/t4052-stat-output.sh +++ b/t/t4052-stat-output.sh @@ -19,17 +19,33 @@ test_expect_success 'preparation' ' git commit -m message "$name" ' +cat >expect72 <<-'EOF' + ...a | 1 + +EOF +test_expect_success "format-patch: small change with long name gives more space to the name" ' + git format-patch -1 --stdout >output && + grep " | " output >actual && + test_cmp expect72 actual +' + while read cmd args do - cat >expect <<-'EOF' + cat >expect80 <<-'EOF' ...a | 1 + EOF test_expect_success "$cmd: small change with long name gives more space to the name" ' git $cmd $args >output && grep " | " output >actual && - test_cmp expect actual + test_cmp expect80 actual ' +done <<\EOF +diff HEAD^ HEAD --stat +show --stat +log -1 --stat +EOF +while read cmd args +do cat >expect <<-'EOF' ...a | 1 + EOF @@ -60,7 +76,7 @@ do test_cmp expect actual ' done <<\EOF -format-patch --stat=80 -1 --stdout +format-patch -1 --stdout diff HEAD^ HEAD --stat show --stat log -1 --stat Nguyễn Thái Ngọc Duy (2): format-patch: keep cover-letter diffstat wrapped in 72 columns format-patch: reduce patch diffstat width to 72 builtin/log.c | 7 ++- t/t4052-stat-output.sh | 46 -- 2 files changed, 37 insertions(+), 16 deletions(-) -- 2.16.1.205.g271f633410
Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)
On Thu, Feb 01 2018, Junio C. Hamano jotted: > * nd/fix-untracked-cache-invalidation (2018-01-24) 5 commits > - dir.c: stop ignoring opendir() error in open_cached_dir() > - update-index doc: note a fixed bug in the untracked cache > - dir.c: fix missing dir invalidation in untracked code > - dir.c: avoid stat() in valid_cached_dir() > - status: add a failing test showing a core.untrackedCache bug > > Some bugs around "untracked cache" feature have been fixed. > > Will merge to 'next'. The "update-index doc: note a fixed bug in the untracked cache" needs to be amended so it doesn't say "Before 2.16, ": https://github.com/gitster/git/commit/b9fc38c9f90b8bf2c9147ad536813b66aa45220d#diff-01fe1588047a177245bfaf82336cdeaeR467 I'm not sure whether you're planning this for 2.16.2, or whether it'll be in 2.17.0, but the patch should be amended to mention either one of those. I can submit a replacement patch, but figured this was trivial enough (and you know more about the release plan) that you'd like to amend this locally.
Re: [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT
On 31/01/18 09:30, Nguyễn Thái Ngọc Duy wrote: > > The new command `git rebase --show-current-patch` is useful for seeing > the commit related to the current rebase state. Some however may find > the "git show" command behind it too limiting. You may want to > increase context lines, do a diff that ignores whitespaces... > > For these advanced use cases, the user can execute any command they > want with the new pseudo ref ORIG_COMMIT. > > This also helps show where the stopped commit is from, which is hard > to see from the previous patch which implements --show-current-patch. > > Helped-by: Tim Landscheidt> Signed-off-by: Nguyễn Thái Ngọc Duy > --- > Documentation/git-rebase.txt | 3 ++- > builtin/am.c | 4 > contrib/completion/git-completion.bash | 2 +- > git-rebase--interactive.sh | 5 - > git-rebase--merge.sh | 4 +++- > git-rebase.sh | 1 + > sequencer.c| 3 +++ > t/t3400-rebase.sh | 3 ++- > t/t3404-rebase-interactive.sh | 5 - > 9 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > index 7ef9577472..6da9296bf8 100644 > --- a/Documentation/git-rebase.txt > +++ b/Documentation/git-rebase.txt > @@ -252,7 +252,8 @@ leave out at most one of A and B, in which case it > defaults to HEAD. > > --show-current-patch:: > Show the current patch in an interactive rebase or when rebase > - is stopped because of conflicts. > + is stopped because of conflicts. This is the equivalent of > + `git show ORIG_COMMIT`. > > -m:: > --merge:: > diff --git a/builtin/am.c b/builtin/am.c > index caec50cba9..bf9b356340 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -1011,6 +1011,7 @@ static void am_setup(struct am_state *state, enum > patch_format patch_format, > > if (mkdir(state->dir, 0777) < 0 && errno != EEXIST) > die_errno(_("failed to create directory '%s'"), state->dir); > + delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF); > > if (split_mail(state, patch_format, paths, keep_cr) < 0) { > am_destroy(state); > @@ -1110,6 +,7 @@ static void am_next(struct am_state *state) > > oidclr(>orig_commit); > unlink(am_path(state, "original-commit")); > + delete_ref(NULL, "ORIG_COMMIT", NULL, REF_NO_DEREF); > > if (!get_oid("HEAD", )) > write_state_text(state, "abort-safety", oid_to_hex()); > @@ -1441,6 +1443,8 @@ static int parse_mail_rebase(struct am_state *state, > const char *mail) > > oidcpy(>orig_commit, _oid); > write_state_text(state, "original-commit", oid_to_hex(_oid)); > + update_ref("am", "ORIG_COMMIT", _oid, > +NULL, 0, UPDATE_REFS_DIE_ON_ERR); > > return 0; > } > diff --git a/contrib/completion/git-completion.bash > b/contrib/completion/git-completion.bash > index 2bd30d68cf..deea688e0e 100644 > --- a/contrib/completion/git-completion.bash > +++ b/contrib/completion/git-completion.bash > @@ -439,7 +439,7 @@ __git_refs () > track="" > ;; > *) > - for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD; do > + for i in HEAD FETCH_HEAD ORIG_HEAD MERGE_HEAD > ORIG_COMMIT; do > case "$i" in > $match*) > if [ -e "$dir/$i" ]; then > diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh > index 0c0f8abbf9..ef72bd5871 100644 > --- a/git-rebase--interactive.sh > +++ b/git-rebase--interactive.sh > @@ -199,12 +199,14 @@ make_patch () { > > die_with_patch () { > echo "$1" > "$state_dir"/stopped-sha > + git update-ref ORIG_COMMIT "$1" > make_patch "$1" > die "$2" > } > > exit_with_patch () { > echo "$1" > "$state_dir"/stopped-sha > + git update-ref ORIG_COMMIT "$1" > make_patch $1 > git rev-parse --verify HEAD > "$amend" > gpg_sign_opt_quoted=${gpg_sign_opt:+$(git rev-parse --sq-quote > "$gpg_sign_opt")} > @@ -841,7 +843,7 @@ To continue rebase after editing, run: > exit > ;; > show-current-patch) > - exec git show "$(cat "$state_dir/stopped-sha")" -- > + exec git show ORIG_COMMIT -- > ;; > esac > > @@ -858,6 +860,7 @@ fi > > orig_head=$(git rev-parse --verify HEAD) || die "$(gettext "No HEAD?")" > mkdir -p "$state_dir" || die "$(eval_gettext "Could not create temporary > \$state_dir")" > +rm -f "$(git rev-parse --git-path ORIG_COMMIT)" > > : > "$state_dir"/interactive || die "$(gettext "Could not mark as > interactive")" > write_basic_state > diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh > index 0a96dfae37..70966c32c3 100644 > --- a/git-rebase--merge.sh > +++
Re: What's cooking in git.git (Jan 2018, #04; Wed, 31)
On Thu, Feb 01 2018, Junio C. Hamano jotted: > * ab/wildmatch-tests (2018-01-30) 10 commits > - wildmatch test: mark test as EXPENSIVE_ON_WINDOWS > - test-lib: add an EXPENSIVE_ON_WINDOWS prerequisite > - wildmatch test: create & test files on disk in addition to in-memory > - wildmatch test: perform all tests under all wildmatch() modes > - wildmatch test: use test_must_fail, not ! for test-wildmatch > - wildmatch test: remove dead fnmatch() test code > - wildmatch test: use a paranoia pattern from nul_match() > - wildmatch test: don't try to vertically align our output > - wildmatch test: use more standard shell style > - wildmatch test: indent with tabs, not spaces > > More tests for wildmatch functions. > > Expecting an update. > cf. <87vaga9mgf@evledraar.gmail.com> The 2018-01-30 series is the update mentioned in 87vaga9mgf@evledraar.gmail.com. You probably noticed this / just didn't adjust the note since you queued in in pu already, but just in case: the known issues in it have been resolved, but hopefully Johannes Schindelin can test it on Windows & report. > * nd/fix-untracked-cache-invalidation (2018-01-24) 5 commits > - dir.c: stop ignoring opendir() error in open_cached_dir() > - update-index doc: note a fixed bug in the untracked cache > - dir.c: fix missing dir invalidation in untracked code > - dir.c: avoid stat() in valid_cached_dir() > - status: add a failing test showing a core.untrackedCache bug > > Some bugs around "untracked cache" feature have been fixed. > > Will merge to 'next'. It must be Murphy's law or something, but even though the bug has been there for years I just had some internal users again run into the bug this fixes, so I'm building an internal v2.16.1 + custom patches (this included). > * ab/sha1dc-build (2017-12-12) 4 commits > . Makefile: use the sha1collisiondetection submodule by default > - sha1dc_git.h: re-arrange an ifdef chain for a subsequent change > - Makefile: under "make dist", include the sha1collisiondetection submodule > - Makefile: don't error out under DC_SHA1_EXTERNAL if DC_SHA1_SUBMODULE=auto > > Push the submodule version of collision-detecting SHA-1 hash > implementation a bit harder on builders. > > The earlier two may make sense, but leaning toward rejecting the last step. > cf.This has been lingering for a long time. I think it makes sense just to merge the first 3 down and then we can discuss 4/4 in another submission. [12]/4 solve real bugs we have now, 3/4 is harmless to merge down (and makes 4/4 smaller when we get around to discussing it again), it's just 4/4 that's been stalling this. Do you want to peel of 4/4 and just keep 1-3 should I submit another version without 4/4?
Re: [PATCH] tag: add --edit option
On Thu, Feb 1, 2018 at 5:43 AM, Nicolas Morey-Chaisemartinwrote: > Le 01/02/2018 à 11:34, Nicolas Morey-Chaisemartin a écrit : >> Le 01/02/2018 à 11:16, Eric Sunshine a écrit : >>> A little below this change is where launch_editor() is actually >>> invoked. If it fails for some reason, it prints: >>> >>> Please supply the message using either -m or -F option. >>> >>> which seems a bit counterintuitive if the user *did* specify one of >>> those options along with --edit. I wonder if that message needs to be >>> adjusted. >>> >> Yes I'll fix this. > I just checked what commit.c does and it seems to behave as my patch: > if (launch_editor(git_path_commit_editmsg(), NULL, env.argv)) { > fprintf(stderr, > _("Please supply the message using either -m or -F option.\n")); > > To be honest the message is not that clear either. > If I'm reading launch_editor right most (or all) its falire are du to a > failure to launch the editor or the editor crashed/exited with an error. > In this case, I wouldn't advise the user to use -m or -F but to fix its > editor. Indeed, I also looked at the implementation of launch_editor(), and my "wondering" about whether the message needed adjustment was just that. The message seems somewhat counterintuitive in this case, but I didn't necessarily have a better suggestion. A valid response, therefore, might be to punt on it and leave that change for the future, or perhaps take it on as a second patch which adjusts the message in both commands. I don't have strong feelings about it at this time.
Re: [PATCH] tag: add --edit option
Le 01/02/2018 à 11:34, Nicolas Morey-Chaisemartin a écrit : > > Le 01/02/2018 à 11:16, Eric Sunshine a écrit : >> On Thu, Feb 1, 2018 at 4:49 AM, Nicolas Morey-Chaisemartin >>wrote: >>> Add a --edit option whichs allows modifying the messages provided by -m or >>> -F, >>> the same way git commit --edit does. >>> >>> Signed-off-by: Nicolas Morey-Chaisemartin >>> --- >>> diff --git a/builtin/tag.c b/builtin/tag.c >>> @@ -224,7 +225,7 @@ static void create_tag(const struct object_id *object, >>> const char *tag, >>> - if (!opt->message_given) { >>> + if (!opt->message_given || opt->use_editor) { >>> >>> - if (!is_null_oid(prev)) { >>> + if (opt->message_given) { >>> + write_or_die(fd, buf->buf, buf->len); >>> + strbuf_reset(buf); >>> + } else if (!is_null_oid(prev)) { >>> write_tag_body(fd, prev); >>> } else { >> A little below this change is where launch_editor() is actually >> invoked. If it fails for some reason, it prints: >> >> Please supply the message using either -m or -F option. >> >> which seems a bit counterintuitive if the user *did* specify one of >> those options along with --edit. I wonder if that message needs to be >> adjusted. >> > Yes I'll fix this. I just checked what commit.c does and it seems to behave as my patch: if (launch_editor(git_path_commit_editmsg(), NULL, env.argv)) { fprintf(stderr, _("Please supply the message using either -m or -F option.\n")); exit(1); } To be honest the message is not that clear either. If I'm reading launch_editor right most (or all) its falire are du to a failure to launch the editor or the editor crashed/exited with an error. In this case, I wouldn't advise the user to use -m or -F but to fix its editor. Nicolas
Re: Some rough edges of core.fsmonitor
On Tue, Jan 30, 2018 at 6:16 AM, Ben Peartwrote: > > > On 1/29/2018 4:40 AM, Duy Nguyen wrote: >> >> On Sat, Jan 27, 2018 at 12:43:41PM +0100, Ævar Arnfjörð Bjarmason wrote: >>> >>> b) with fsmonitor >>> >>> $ time GIT_TRACE_PERFORMANCE=1 ~/g/git/git-status >>> 12:34:23.833625 read-cache.c:1890 performance: 0.049485685 s: >>> read cache .git/index >> >> >> This is sort of off topic but may be interesting for big repo guys. It >> looks like read cache's time is partially dominated by malloc(). >> > > That is correct. We have tried a few different ways to address this. First > was my patch series [1] that would parallelize all of the read cache code. > > We quickly found that malloc() was the biggest culprit and by speeding that > up, we got most of the wins. At Peff's recommendation [2], we looked into > using tcmalloc but found that 1) it has bugs on Windows and 2) it isn't > being actively maintained so it didn't seem those bugs would ever get fixed. > > We are currently working on a patch that will use a refactored version of > the mem_pool in fast-import.c to do block allocations of the cache entries > which is giving us about a 22% improvement in "git status" times. One My apologies if this has been discussed in the second half of 2017 which I have no idea what happened. I just wonder if it's possible to design a "file format" that is basically a memory dump of lots of struct cache_entry? This "file" will stay in a shared memory somewhere and never get written down to disk. Since it's very close to the data structure we have in core, the most we need to do after mmap'ing it (and keeping it mmap'd until the end) is adjust some pointers. These entries are of course read-only. When you modify/create new entries, they are created new, using the old malloc(). We just need to make sure not free the read-only cache_entry entries and munmap() the whole thing when we discard_index(). This opens up another option to deal with the large UNTR and TREE extensions in a similar way. These will be your next headache after you have reduced parse time for main entries. > challenge has been ensuring that cache entries are not passed from one > index/mem_pool to another which could cause access after free bugs. We kind of have something close to that, but not entirely the same. When split index is used, the same cache_entry can appear in two index_state structs. Of course you can free only one of them (and you can only do so when you know both index_state are gone). I see some code cleanup opportunity :) > > [1] > https://public-inbox.org/git/20171109141737.47976-1-benpe...@microsoft.com/ > [2] > https://public-inbox.org/git/20171120153846.v5b7ho42yzrzn...@sigill.intra.peff.net/ -- Duy
Re: [PATCH] tag: add --edit option
Le 01/02/2018 à 11:16, Eric Sunshine a écrit : > On Thu, Feb 1, 2018 at 4:49 AM, Nicolas Morey-Chaisemartin >wrote: >> Add a --edit option whichs allows modifying the messages provided by -m or >> -F, >> the same way git commit --edit does. >> >> Signed-off-by: Nicolas Morey-Chaisemartin >> --- >> diff --git a/builtin/tag.c b/builtin/tag.c >> @@ -224,7 +225,7 @@ static void create_tag(const struct object_id *object, >> const char *tag, >> - if (!opt->message_given) { >> + if (!opt->message_given || opt->use_editor) { >> >> - if (!is_null_oid(prev)) { >> + if (opt->message_given) { >> + write_or_die(fd, buf->buf, buf->len); >> + strbuf_reset(buf); >> + } else if (!is_null_oid(prev)) { >> write_tag_body(fd, prev); >> } else { > A little below this change is where launch_editor() is actually > invoked. If it fails for some reason, it prints: > > Please supply the message using either -m or -F option. > > which seems a bit counterintuitive if the user *did* specify one of > those options along with --edit. I wonder if that message needs to be > adjusted. > Yes I'll fix this. >> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh >> @@ -452,6 +452,23 @@ test_expect_success \ >> +get_tag_header annotated-tag-edit $commit commit $time >expect >> +echo "An edited message" >>expect >> +test_expect_success 'set up editor' ' >> + cat >editor <<-\EOF && >> + #!/bin/sh >> + sed -e "s/A message/An edited message/g" <"$1" >"$1-" >> + mv "$1-" "$1" >> + EOF >> + chmod 755 editor > If you use write_script() to create the fake editor, then it supplies > the #!/bin/sh line for you and does the 'chmod', so you only need to > supply the actual script payload. Also, other "editors" in this test > file are named "fakeeditor", so perhaps follow suit. > > write_script fakeeditor <<-\EOF > sed -e "s/A message/An edited message/g" <"$1" >"$1-" > mv "$1-" "$1" > EOF > I dumbly copied the test from commit --edit as it was my reference. I'll fix the names and switch to write_script. Thanks Nicolas
Re: [PATCH v2 01/41] parse-options: support --git-completion-helper
On Thu, Feb 1, 2018 at 4:54 PM, Eric Sunshinewrote: > On Wed, Jan 31, 2018 at 7:05 PM, Duy Nguyen wrote: >> On Thu, Feb 1, 2018 at 4:04 AM, Eric Sunshine >> wrote: >>> On Wed, Jan 31, 2018 at 6:05 AM, Nguyễn Thái Ngọc Duy >>> wrote: Dangerous/Unpopular options could be hidden with the new "NOCOMPLETE" flag. >>> >>> I wonder if this option should be named DANGEROUS rather than >>> NOCOMPLETE to better reflect its intention. >> >> It's not only for dangerous options (I forgot to mention this in the >> commit message, I will in v3). The --continue|--abort|--skip should >> only show up when you are in a middle of rebase/am/cherry-pick. >> git-completion.bash handles this case separately and only put them in >> the completion list when appropriate. --git-completion-helper must >> not include these or the trick done by git-completion.bash becomes >> useless. >> >> Interesting. So we now have two classes of "no complete". One can't be >> configurable (--continue|--abort|--skip) and one can. I'll use two >> separate flags for these, though I'm not adding the configuration >> option right now. > > I don't see that as convincing argument for two classes of "no > complete". Since git-completion.bash already special-cases > rebase/am/cherry-pick for --continue|--abort|--skip, it is not far > fetched that that special-case treatment can be extended slightly to > also filter out those three options from the list returned by > --git-completion-helper. I agree that is possible, but it's a bit tricky to do the filtering right in bash (all options are sent back as one line instead of one per line, which is easier to process by command line tools). On top of that, I think we want git-completion.bash to be fast, the more commands we execute there, the unhappier Windows users are. It is too possible to do this kind of filtering just once, before caching. It does not fit well to how I designed __gitcomp_builtin so I need to sit back and see how to sort that out. Long term though, I think we would want more and more completion logic built in. One of those things I have in mind is to let --git-completion-helper (or some other new option) to provide completion for possible option values (e.g. true/false, values for "git merge --strategy=", "git status --untrack-files="). That may also include completion of --continue|--abort|.. in C code, something like this roughly # separate command blocks because we still need to cache them in shell if [ -f .git/rebase-apply ]; then __gitcomp $(git $cmd --git-completion-helper=in-progress) else __gitcomp $(git $cmd --git-completion-helper) fi which means eventually we have to flag these options differently. Having said all that, these are the things I imagine we _might_ do. I have not really thought it through and nor do I have a clear plan forward at this point, so they may end up being just rubbish thoughts. > So, if that special case is handled entirely by the completion script, > then that leaves only the "dangerous" options, which requires only a > single flag. -- Duy
Re: [PATCH] tag: add --edit option
On Thu, Feb 1, 2018 at 4:49 AM, Nicolas Morey-Chaisemartinwrote: > Add a --edit option whichs allows modifying the messages provided by -m or -F, > the same way git commit --edit does. > > Signed-off-by: Nicolas Morey-Chaisemartin > --- > diff --git a/builtin/tag.c b/builtin/tag.c > @@ -224,7 +225,7 @@ static void create_tag(const struct object_id *object, > const char *tag, > - if (!opt->message_given) { > + if (!opt->message_given || opt->use_editor) { > > - if (!is_null_oid(prev)) { > + if (opt->message_given) { > + write_or_die(fd, buf->buf, buf->len); > + strbuf_reset(buf); > + } else if (!is_null_oid(prev)) { > write_tag_body(fd, prev); > } else { A little below this change is where launch_editor() is actually invoked. If it fails for some reason, it prints: Please supply the message using either -m or -F option. which seems a bit counterintuitive if the user *did* specify one of those options along with --edit. I wonder if that message needs to be adjusted. > diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh > @@ -452,6 +452,23 @@ test_expect_success \ > +get_tag_header annotated-tag-edit $commit commit $time >expect > +echo "An edited message" >>expect > +test_expect_success 'set up editor' ' > + cat >editor <<-\EOF && > + #!/bin/sh > + sed -e "s/A message/An edited message/g" <"$1" >"$1-" > + mv "$1-" "$1" > + EOF > + chmod 755 editor If you use write_script() to create the fake editor, then it supplies the #!/bin/sh line for you and does the 'chmod', so you only need to supply the actual script payload. Also, other "editors" in this test file are named "fakeeditor", so perhaps follow suit. write_script fakeeditor <<-\EOF sed -e "s/A message/An edited message/g" <"$1" >"$1-" mv "$1-" "$1" EOF
[PATCH v2 3/3] perf/aggregate: sort JSON fields in output
It is much easier to diff the output against a previous one when the fields are sorted. Helped-by: Philip OakleySigned-off-by: Christian Couder --- t/perf/aggregate.perl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) The only change compared to v1 is a typo fix suggested by Philip in the commti message. diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index a609292491..749d6689f9 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -253,7 +253,7 @@ sub print_codespeed_results { } } - print to_json(\@data, {utf8 => 1, pretty => 1}), "\n"; + print to_json(\@data, {utf8 => 1, pretty => 1, canonical => 1}), "\n"; } binmode STDOUT, ":utf8" or die "PANIC on binmode: $!"; -- 2.16.0.rc2.45.g09a1bbd803
[PATCH v2 2/3] perf/aggregate: add --reponame option
This makes it easier to use the aggregate script on the command line when one wants to get the "environment" fields set in the codespeed output. Previously setting GIT_REPO_NAME was needed for this purpose. Helped-by: Eric SunshineSigned-off-by: Christian Couder --- t/perf/aggregate.perl | 15 +-- 1 file changed, 13 insertions(+), 2 deletions(-) The only change compared to v1 is a logical change suggested by Eric in the 'if ... elsif ... else ...' sequence that sets $environment. diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index bbf0f30898..a609292491 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -37,7 +37,7 @@ sub format_times { } my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests, -$codespeed, $subsection); +$codespeed, $subsection, $reponame); while (scalar @ARGV) { my $arg = $ARGV[0]; my $dir; @@ -55,6 +55,15 @@ while (scalar @ARGV) { } next; } + if ($arg eq "--reponame") { + shift @ARGV; + $reponame = $ARGV[0]; + shift @ARGV; + if (! $reponame) { + die "empty reponame"; + } + next; + } last if -f $arg or $arg eq "--"; if (! -d $arg) { my $rev = Git::command_oneline(qw(rev-parse --verify), $arg); @@ -210,7 +219,9 @@ sub print_codespeed_results { } my $environment; - if (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") { + if ($reponame) { + $environment = $reponame; + } elsif (exists $ENV{GIT_PERF_REPO_NAME} and $ENV{GIT_PERF_REPO_NAME} ne "") { $environment = $ENV{GIT_PERF_REPO_NAME}; } elsif (exists $ENV{GIT_TEST_INSTALLED} and $ENV{GIT_TEST_INSTALLED} ne "") { $environment = $ENV{GIT_TEST_INSTALLED}; -- 2.16.0.rc2.45.g09a1bbd803
[PATCH v2 1/3] perf/aggregate: add --subsection option
This makes it easier to use the aggregate script on the command line, to get results from subsections. Previously setting GIT_PERF_SUBSECTION was needed for this purpose. Signed-off-by: Christian Couder--- t/perf/aggregate.perl | 33 - 1 file changed, 24 insertions(+), 9 deletions(-) This small patch series contains a few small Codespeed related usability improvements of the perf/aggregate.perl script on top the cc/codespeed patch series that was recently merged into master. Changes compared to v1 are described in each patch if any. The discussion thread about v1 is there: https://public-inbox.org/git/20180128111843.2690-1-chrisc...@tuxfamily.org/T/#u diff --git a/t/perf/aggregate.perl b/t/perf/aggregate.perl index 5c439f6bc2..bbf0f30898 100755 --- a/t/perf/aggregate.perl +++ b/t/perf/aggregate.perl @@ -36,7 +36,8 @@ sub format_times { return $out; } -my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests, $codespeed); +my (@dirs, %dirnames, %dirabbrevs, %prefixes, @tests, +$codespeed, $subsection); while (scalar @ARGV) { my $arg = $ARGV[0]; my $dir; @@ -45,6 +46,15 @@ while (scalar @ARGV) { shift @ARGV; next; } + if ($arg eq "--subsection") { + shift @ARGV; + $subsection = $ARGV[0]; + shift @ARGV; + if (! $subsection) { + die "empty subsection"; + } + next; + } last if -f $arg or $arg eq "--"; if (! -d $arg) { my $rev = Git::command_oneline(qw(rev-parse --verify), $arg); @@ -76,10 +86,15 @@ if (not @tests) { } my $resultsdir = "test-results"; -my $results_section = ""; -if (exists $ENV{GIT_PERF_SUBSECTION} and $ENV{GIT_PERF_SUBSECTION} ne "") { - $resultsdir .= "/" . $ENV{GIT_PERF_SUBSECTION}; - $results_section = $ENV{GIT_PERF_SUBSECTION}; + +if (! $subsection and +exists $ENV{GIT_PERF_SUBSECTION} and +$ENV{GIT_PERF_SUBSECTION} ne "") { + $subsection = $ENV{GIT_PERF_SUBSECTION}; +} + +if ($subsection) { + $resultsdir .= "/" . $subsection; } my @subtests; @@ -183,15 +198,15 @@ sub print_default_results { } sub print_codespeed_results { - my ($results_section) = @_; + my ($subsection) = @_; my $project = "Git"; my $executable = `uname -s -m`; chomp $executable; - if ($results_section ne "") { - $executable .= ", " . $results_section; + if ($subsection) { + $executable .= ", " . $subsection; } my $environment; @@ -233,7 +248,7 @@ sub print_codespeed_results { binmode STDOUT, ":utf8" or die "PANIC on binmode: $!"; if ($codespeed) { - print_codespeed_results($results_section); + print_codespeed_results($subsection); } else { print_default_results(); } -- 2.16.0.rc2.45.g09a1bbd803
Hello My Dear Friend,
I have a business proposal in the tune of $10.2 Million USD for you to handle with me. I have opportunity to transfer this abandon fund to your bank account in your country which belongs to our client. I am inviting you in this transaction where this money can be shared between us at ratio of 60/40% and help the needy around us don’t be afraid of anything I am with you I will instruct you what you will do to maintain this fund. Please kindly contact me with your information's if you are interested in this transaction for more details. Your Name:.. Your Bank Name:. Your Account Number:... Your Telephone Number: Your Country And Address: Your Age And Sex:... Thanks Mrs.Zainab Ahmed,
Re: [PATCH v2 3/3] rebase: introduce and use pseudo-ref ORIG_COMMIT
On Thu, Feb 1, 2018 at 6:18 AM, Junio C Hamanowrote: > Nguyễn Thái Ngọc Duy writes: > >> The new command `git rebase --show-current-patch` is useful for seeing >> the commit related to the current rebase state. Some however may find >> the "git show" command behind it too limiting. You may want to >> increase context lines, do a diff that ignores whitespaces... >> >> For these advanced use cases, the user can execute any command they >> want with the new pseudo ref ORIG_COMMIT. >> >> This also helps show where the stopped commit is from, which is hard >> to see from the previous patch which implements --show-current-patch. >> >> Helped-by: Tim Landscheidt >> Signed-off-by: Nguyễn Thái Ngọc Duy >> --- > > Hmph, how is this new file conceptually different from existing ones > like CHERRY_PICK_HEAD? Conceptually the same, except that CHERRY_PICK_HEAD can't be reused because it's specifically tied to git-cherry-pick (there's even code that delete this ref if cherry-pick is run as part of rebase, and git-status uses this ref to see if a cherry-pick is in progress). There's also REVERT_HEAD in sequencer.c, same purpose but for git-revert. Perhaps I should rename this new ref to REBASE_HEAD to follow the same naming? -- Duy
Re: [PATCH] gitignore.txt: elaborate shell glob syntax
On Wed, Jan 31, 2018 at 03:22:46PM -0800, Junio C Hamano wrote: > Nguyễn Thái Ngọc Duywrites: Argh.. stray patches strike again. It's not supposed to be in this thread but in https://public-inbox.org/git/%3C20180130101351.GA761@ash%3E/ > > > `.gitignore` file). > > > > - - Otherwise, Git treats the pattern as a shell glob suitable > > - for consumption by fnmatch(3) with the FNM_PATHNAME flag: > > - wildcards in the pattern will not match a / in the pathname. > > - For example, "Documentation/{asterisk}.html" matches > > - "Documentation/git.html" but not "Documentation/ppc/ppc.html" > > - or "tools/perf/Documentation/perf.html". > > + - Otherwise, Git treats the pattern as a shell glob: '{asterisk}' > > + matches anything except '/', '?' matches any one character except > > + '/' and '[]' matches one character in a selected range. See > > + fnmatch(3) and the FNM_PATHNAME flag for a more accurate > > + description. > > Where the original did not quote single letters at all, this uses a > pair of single quotes. Did you make sure it renders well in HTML > and manpage already or should I do that for you before applying? I didn't. I thought I didn't add any weird symbols. I was wrong. These are now wrapped as "`stuff`" to be displayed the same way as in nearby paragraphs. Verified both man and HTML pages are rendered well. > I think what you wrote is accurate enough already, and those who > want to go to fnmatch(3) would do so not for accuracy but for > authority ;-) Perhaps s/accurate/detailed/? Well there are rooms for guessing, for example "matches anything" does not tell you straight that it can match multiple characters. Anyway fixed too. -- 8< -- Subject: [PATCH v2] gitignore.txt: elaborate shell glob syntax `fnmatch(3)` is a great mention if the intended audience is programmers. For normal users it's probably better to spell out what a shell glob is. This paragraph is updated to roughly tell (or remind) what the main wildcards are supposed to do. All the details are still hidden away behind the `fnmatch(3)` wall because bringing the whole specification here may be too much. Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/gitignore.txt | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Documentation/gitignore.txt b/Documentation/gitignore.txt index 63260f0056..ff5d7f9ed6 100644 --- a/Documentation/gitignore.txt +++ b/Documentation/gitignore.txt @@ -102,12 +102,11 @@ PATTERN FORMAT (relative to the toplevel of the work tree if not from a `.gitignore` file). - - Otherwise, Git treats the pattern as a shell glob suitable - for consumption by fnmatch(3) with the FNM_PATHNAME flag: - wildcards in the pattern will not match a / in the pathname. - For example, "Documentation/{asterisk}.html" matches - "Documentation/git.html" but not "Documentation/ppc/ppc.html" - or "tools/perf/Documentation/perf.html". + - Otherwise, Git treats the pattern as a shell glob: "`*`" matches + anything except "`/`", "`?`" matches any one character except "`/`" + and "`[]`" matches one character in a selected range. See + fnmatch(3) and the FNM_PATHNAME flag for a more detailed + description. - A leading slash matches the beginning of the pathname. For example, "/{asterisk}.c" matches "cat-file.c" but not -- 2.16.1.205.g271f633410 -- 8< --
Re: [PATCH v2 01/41] parse-options: support --git-completion-helper
On Wed, Jan 31, 2018 at 7:05 PM, Duy Nguyenwrote: > On Thu, Feb 1, 2018 at 4:04 AM, Eric Sunshine wrote: >> On Wed, Jan 31, 2018 at 6:05 AM, Nguyễn Thái Ngọc Duy >> wrote: >>> Dangerous/Unpopular >>> options could be hidden with the new "NOCOMPLETE" flag. >> >> I wonder if this option should be named DANGEROUS rather than >> NOCOMPLETE to better reflect its intention. > > It's not only for dangerous options (I forgot to mention this in the > commit message, I will in v3). The --continue|--abort|--skip should > only show up when you are in a middle of rebase/am/cherry-pick. > git-completion.bash handles this case separately and only put them in > the completion list when appropriate. --git-completion-helper must > not include these or the trick done by git-completion.bash becomes > useless. > > Interesting. So we now have two classes of "no complete". One can't be > configurable (--continue|--abort|--skip) and one can. I'll use two > separate flags for these, though I'm not adding the configuration > option right now. I don't see that as convincing argument for two classes of "no complete". Since git-completion.bash already special-cases rebase/am/cherry-pick for --continue|--abort|--skip, it is not far fetched that that special-case treatment can be extended slightly to also filter out those three options from the list returned by --git-completion-helper. So, if that special case is handled entirely by the completion script, then that leaves only the "dangerous" options, which requires only a single flag.
[PATCH] tag: add --edit option
Add a --edit option whichs allows modifying the messages provided by -m or -F, the same way git commit --edit does. Signed-off-by: Nicolas Morey-Chaisemartin--- Documentation/git-tag.txt | 6 ++ builtin/tag.c | 11 +-- t/t7004-tag.sh| 34 ++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/Documentation/git-tag.txt b/Documentation/git-tag.txt index 956fc019f984..b9e5a993bea0 100644 --- a/Documentation/git-tag.txt +++ b/Documentation/git-tag.txt @@ -167,6 +167,12 @@ This option is only applicable when listing tags without annotation lines. Implies `-a` if none of `-a`, `-s`, or `-u ` is given. +-e:: +--edit:: + The message taken from file with `-F` and command line with + `-m` are usually used as the tag message unmodified. + This option lets you further edit the message taken from these sources. + --cleanup=:: This option sets how the tag message is cleaned up. The '' can be one of 'verbatim', 'whitespace' and 'strip'. The diff --git a/builtin/tag.c b/builtin/tag.c index a7e6a5b0f234..91c60829d5f9 100644 --- a/builtin/tag.c +++ b/builtin/tag.c @@ -194,6 +194,7 @@ static int build_tag_object(struct strbuf *buf, int sign, struct object_id *resu struct create_tag_options { unsigned int message_given:1; + unsigned int use_editor:1; unsigned int sign; enum { CLEANUP_NONE, @@ -224,7 +225,7 @@ static void create_tag(const struct object_id *object, const char *tag, tag, git_committer_info(IDENT_STRICT)); - if (!opt->message_given) { + if (!opt->message_given || opt->use_editor) { int fd; /* write the template message before editing: */ @@ -233,7 +234,10 @@ static void create_tag(const struct object_id *object, const char *tag, if (fd < 0) die_errno(_("could not create file '%s'"), path); - if (!is_null_oid(prev)) { + if (opt->message_given) { + write_or_die(fd, buf->buf, buf->len); + strbuf_reset(buf); + } else if (!is_null_oid(prev)) { write_tag_body(fd, prev); } else { struct strbuf buf = STRBUF_INIT; @@ -372,6 +376,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) static struct ref_sorting *sorting = NULL, **sorting_tail = struct ref_format format = REF_FORMAT_INIT; int icase = 0; + int edit_flag = 0; struct option options[] = { OPT_CMDMODE('l', "list", , N_("list tag names"), 'l'), { OPTION_INTEGER, 'n', NULL, , N_("n"), @@ -386,6 +391,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) OPT_CALLBACK('m', "message", , N_("message"), N_("tag message"), parse_msg_arg), OPT_FILENAME('F', "file", , N_("read message from file")), + OPT_BOOL('e', "edit", _flag, N_("force edit of commit")), OPT_BOOL('s', "sign", , N_("annotated and GPG-signed tag")), OPT_STRING(0, "cleanup", _arg, N_("mode"), N_("how to strip spaces and #comments from message")), @@ -524,6 +530,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix) die(_("tag '%s' already exists"), tag); opt.message_given = msg.given || msgfile; + opt.use_editor = edit_flag; if (!cleanup_arg || !strcmp(cleanup_arg, "strip")) opt.cleanup_mode = CLEANUP_ALL; diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh index a9af2de9960b..60e3a53f297f 100755 --- a/t/t7004-tag.sh +++ b/t/t7004-tag.sh @@ -452,6 +452,23 @@ test_expect_success \ test_cmp expect actual ' +get_tag_header annotated-tag-edit $commit commit $time >expect +echo "An edited message" >>expect +test_expect_success 'set up editor' ' + cat >editor <<-\EOF && + #!/bin/sh + sed -e "s/A message/An edited message/g" <"$1" >"$1-" + mv "$1-" "$1" + EOF + chmod 755 editor +' +test_expect_success \ + 'creating an annotated tag with -m message --edit should succeed' ' + EDITOR=./editor git tag -m "A message" --edit annotated-tag-edit && + get_tag_msg annotated-tag-edit >actual && + test_cmp expect actual +' + cat >msgfile >expect +test_expect_success 'set up editor' ' + cat >editor <<-\EOF && + #!/bin/sh + sed -e "s/Another message/Another edited message/g" <"$1" >"$1-" + mv "$1-" "$1" + EOF + chmod 755 editor +' +test_expect_success \ + 'creating an annotated tag with -F messagefile --edit should succeed' ' + EDITOR=./editor git tag -F msgfile