[RFC/PATCH] worktree: replace "checkout --to" with "worktree new"
The command "git checkout --to " is something of an anachronism, encompassing functionality somewhere between "checkout" and "clone". The introduction of the git-worktree command, however, provides a proper and intuitive place to house such functionality. Consequently, re-implement "git checkout --to" as "git worktree new". As a side-effect, linked worktree creation becomes much more discoverable with its own dedicated command, whereas `--to` was easily overlooked amid the plethora of options recognized by git-checkout. Signed-off-by: Eric Sunshine --- I've long felt that Duy's linked-worktree functionality was a bit oddly named as "git checkout --to", but, since I could never come up with a better name, I never made mention of it. However, with Duy's introduction of the git-worktree command[1], we now have a much more appropriate and discoverable place to house the "git checkout --to" functionality, and upon seeing his patch, I was ready to reply with the suggestion to relocate "git checkout --to" to "git worktree new", however, Junio beat me to it[2]. So, in response, this patch does exactly that. It applies atop [1]. This is primarily a code and documentation relocation patch, with minor new code added to builtin/worktree.c. Specifically: * git-checkout.txt:"--to" description moved to git-worktree.txt:"new". * git-checkout.txt:"MULTIPLE WORKING TREES" moved to git-worktree.txt:"DESCRIPTION", with "checkout --to" replaced by "worktree new" as necessary. git-worktree.txt could probably use a bit of reorganization, but that can be done as a separate patch. * builtin/checkout.c:remove_junk() and remove_junk_on_signal() moved verbatim to builtin/worktree.c. * builtin/checkout.c:prepare_linked_checkout() moved to builtin/worktree.c:new_worktree() nearly verbatim. The following small changes were needed (which might possibly be better done as separate preparatory patches): - The "no branch specified" check was dropped since git-worktree lacks the machinery for parsing git-checkout command-line arguments, and thus simply doesn't know if a branch/ref was provided, or in what form. I'm not sure yet how to replace this check. - checkout.c:prepare_linked_checkout() (temporarily) fakes up a HEAD ref with a valid object-id in the new worktree to pacify is_git_directory(). It does so using the branch/ref from the command-line which it already resolved. worktree.c, however, doesn't have access to this information, so I instead added code to resolve and use HEAD for the fakement. - The "Enter %s (identifier %s)" message is suppressed in checkout.c if --quiet is specified, however, worktree.c does not have a --quiet option, so the message is printed unconditionally. - argv[] for the sub git-checkout invocation is hand-crafted in worktree.c rather than merely being re-used from the original "git checkout --to" as it was in checkout.c. * builtin/worktree.c:new() is new. It recognizes a --force option ("git worktree new --force ") which allows a branch to be checked out in a new worktree even if already checked out in some other worktree (thus, mirroring the functionality of "git checkout --ignore-other-worktrees"). * t2025-checkout-to.sh became t2025-worktree-new.sh. I'm not sure if the test number still makes sense or if it should be changed, however, it resides alongside its t2026-prune-linked-checkouts.sh counterpart. [1]: http://git.661346.n2.nabble.com/PATCH-worktree-new-place-for-git-prune-worktrees-tp7634619.html [2]: http://git.661346.n2.nabble.com/PATCH-worktree-new-place-for-git-prune-worktrees-tp7634619p7634638.html Documentation/git-checkout.txt| 72 -- Documentation/git-worktree.txt| 79 ++- builtin/checkout.c| 152 + builtin/worktree.c| 157 ++ t/{t2025-checkout-to.sh => t2025-worktree-new.sh} | 44 +++--- t/t2026-prune-linked-checkouts.sh | 2 +- 6 files changed, 260 insertions(+), 246 deletions(-) rename t/{t2025-checkout-to.sh => t2025-worktree-new.sh} (56%) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index d263a56..e19f03a 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -225,13 +225,6 @@ This means that you can use `git checkout -p` to selectively discard edits from your current working tree. See the ``Interactive Mode'' section of linkgit:git-add[1] to learn how to operate the `--patch` mode. ---to=:: - Check out a branch in a separate working directory at - ``. A new working directory is linked to the current - repository, sharing everything except working directory - specific files such as HEAD, index... See "MULTIPLE WORKING - TREES" section for more information. - --ignore-other-worktrees:: `git checkout` r
Re: [PATCH v4 00/44] Make git-am a builtin
On Sun, Jun 28, 2015 at 10:01 PM, Stefan Beller wrote: >> This is a re-roll of [WIP v3]. This patch series is now out of WIP, as ... > >> This WIP patch series rewrites git-am.sh into optimized C builtin/am.c, and >> is >> part of my GSoC project to rewrite git-pull and git-am into C builtins[1]. >> > > I assume the later is just a left over from an old cover letter. :) The series makes sense to me, all the comments were minor nits. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 19/44] builtin-am: implement --3way, am.threeway
> +/** > + * Builds a index that contains just the blobs needed for a 3way merge. an index -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 28/44] builtin-am: pass git-apply's options to git-apply
On Sun, Jun 28, 2015 at 7:05 AM, Paul Tan wrote: > git-am.sh recognizes some of git-apply's options, and would pass them to > git-apply: > > * --whitespace, since 8c31cb8 (git-am: --whitespace=x option., > 2006-02-28) > > * -C, since 67dad68 (add -C[NUM] to git-am, 2007-02-08) > > * -p, since 2092a1f (Teach git-am to pass -p option down to git-apply, > 2007-02-11) > > * --directory, since b47dfe9 (git-am: add --directory= option, > 2009-01-11) > > * --reject, since b80da42 (git-am: implement --reject option passed to > git-apply, 2009-01-23) > > * --ignore-space-change, --ignore-whitespace, since 86c91f9 (git apply: > option to ignore whitespace differences, 2009-08-04) > > * --exclude, since 77e9e49 (am: pass exclude down to apply, 2011-08-03) > > * --include, since 58725ef (am: support --include option, 2012-03-28) > > * --reject, since b80da42 (git-am: implement --reject option passed to > git-apply, 2009-01-23) > > Re-implement support for these options in builtin/am.c. > > Signed-off-by: Paul Tan > --- > builtin/am.c | 47 +++ > 1 file changed, 47 insertions(+) > > diff --git a/builtin/am.c b/builtin/am.c > index 55989e5..5aab627 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -115,6 +115,8 @@ struct am_state { > /* one of the enum scissors_type values */ > int scissors; > > + struct argv_array git_apply_opts; > + > /* override error message when patch failure occurs */ > const char *resolvemsg; > > @@ -147,6 +149,8 @@ static void am_state_init(struct am_state *state, const > char *dir) > git_config_get_bool("am.messageid", &state->message_id); > > state->scissors = SCISSORS_UNSET; > + > + argv_array_init(&state->git_apply_opts); > } > > /** > @@ -168,6 +172,8 @@ static void am_state_release(struct am_state *state) > > if (state->msg) > free(state->msg); > + > + argv_array_clear(&state->git_apply_opts); > } > > /** > @@ -447,6 +453,11 @@ static void am_load(struct am_state *state) > else > state->scissors = SCISSORS_UNSET; > > + read_state_file(&sb, state, "apply-opt", 1); > + argv_array_clear(&state->git_apply_opts); > + if (sq_dequote_to_argv_array(sb.buf, &state->git_apply_opts) < 0) > + die(_("could not parse %s"), am_path(state, "apply-opt")); > + > state->rebasing = !!file_exists(am_path(state, "rebasing")); > > strbuf_release(&sb); > @@ -621,6 +632,7 @@ static void am_setup(struct am_state *state, enum > patch_format patch_format, > { > unsigned char curr_head[GIT_SHA1_RAWSZ]; > const char *str; > + struct strbuf sb = STRBUF_INIT; > > if (!patch_format) > patch_format = detect_patch_format(paths); > @@ -683,6 +695,9 @@ static void am_setup(struct am_state *state, enum > patch_format patch_format, > > write_file(am_path(state, "scissors"), 1, "%s", str); > > + sq_quote_argv(&sb, state->git_apply_opts.argv, 0); > + write_file(am_path(state, "apply-opt"), 1, "%s", sb.buf); > + > if (state->rebasing) > write_file(am_path(state, "rebasing"), 1, "%s", ""); > else > @@ -707,6 +722,8 @@ static void am_setup(struct am_state *state, enum > patch_format patch_format, > write_file(am_path(state, "next"), 1, "%d", state->cur); > > write_file(am_path(state, "last"), 1, "%d", state->last); > + > + strbuf_release(&sb); > } > > /** > @@ -1099,6 +1116,8 @@ static int run_apply(const struct am_state *state, > const char *index_file) > > argv_array_push(&cp.args, "apply"); > > + argv_array_pushv(&cp.args, state->git_apply_opts.argv); > + > if (index_file) > argv_array_push(&cp.args, "--cached"); > else > @@ -1125,6 +1144,7 @@ static int build_fake_ancestor(const struct am_state > *state, const char *index_f > > cp.git_cmd = 1; > argv_array_push(&cp.args, "apply"); > + argv_array_pushv(&cp.args, state->git_apply_opts.argv); > argv_array_pushf(&cp.args, "--build-fake-ancestor=%s", index_file); > argv_array_push(&cp.args, am_path(state, "patch")); > > @@ -1616,9 +1636,36 @@ int cmd_am(int argc, const char **argv, const char > *prefix) > PARSE_OPT_NOARG | PARSE_OPT_NONEG, NULL, 0}, > OPT_BOOL('c', "scissors", &state.scissors, > N_("strip everything before a scissors line")), > + OPT_PASSTHRU_ARGV(0, "whitespace", &state.git_apply_opts, > N_("action"), > + N_("pass it through git-apply"), > + 0), > + OPT_PASSTHRU_ARGV(0, "ignore-space-change", > &state.git_apply_opts, NULL, > + N_("pass it through git-apply"), > + PARSE_OPT_NOARG), > + OPT_PASSTHRU_ARGV(0, "ignore-whitespace", > &state.git_apply
Re: [PATCH v4 31/44] builtin-am: implement -S/--gpg-sign, commit.gpgsign
On Sun, Jun 28, 2015 at 7:05 AM, Paul Tan wrote: > Since 3b4e395 (am: add the --gpg-sign option, 2014-02-01), git-am.sh > supported the --gpg-sign option, and would pass it to git-commit-tree, > thus GPG-signing the commit object. > > Re-implement this option in builtin/am.c. > > git-commit-tree would also sign the commit by default if the > commit.gpgsign setting is true. Since we do not run commit-tree, we > re-implement this behavior by handling the commit.gpgsign setting > ourselves. > > Signed-off-by: Paul Tan > --- > builtin/am.c | 11 ++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/builtin/am.c b/builtin/am.c > index 80850e8..d44f5e2 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -124,6 +124,8 @@ struct am_state { > > int ignore_date; > > + const char *sign_commit; > + > int rebasing; > }; > > @@ -134,6 +136,7 @@ struct am_state { > static void am_state_init(struct am_state *state, const char *dir) > { > const char *quiet; > + int sign_commit; I needed to read this patch a few times as this patch introduces `sign_commit` twice. This is mostly a review problem I'd guess as in the code it just affects this method and you'd see all the code of the method easily compared to hunks sent via email. But renaming this variable doesn't hurt. > > memset(state, 0, sizeof(*state)); > > @@ -155,6 +158,9 @@ static void am_state_init(struct am_state *state, const > char *dir) > state->scissors = SCISSORS_UNSET; > > argv_array_init(&state->git_apply_opts); > + > + if (!git_config_get_bool("commit.gpgsign", &sign_commit)) > + state->sign_commit = sign_commit ? "" : NULL; > } > > /** > @@ -1272,7 +1278,7 @@ static void do_commit(const struct am_state *state) > state->ignore_date ? "" : state->author_date, 1); > > if (commit_tree(state->msg, state->msg_len, tree, parents, commit, > - author, NULL)) > + author, state->sign_commit)) > die(_("failed to write commit object")); > > reflog_msg = getenv("GIT_REFLOG_ACTION"); > @@ -1694,6 +1700,9 @@ int cmd_am(int argc, const char **argv, const char > *prefix) > N_("lie about committer date")), > OPT_BOOL(0, "ignore-date", &state.ignore_date, > N_("use current timestamp for author date")), > + { OPTION_STRING, 'S', "gpg-sign", &state.sign_commit, > N_("key-id"), > + N_("GPG-sign commits"), > + PARSE_OPT_OPTARG, NULL, (intptr_t) "" }, > OPT_HIDDEN_BOOL(0, "rebasing", &state.rebasing, > N_("(internal use for git-rebase)")), > OPT_END() > -- > 2.5.0.rc0.76.gb2c6e93 > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
What's cooking in git.git (Jun 2015, #07; Mon, 29)
Here are the topics that have been cooking. Commits prefixed with '-' are only in 'pu' (proposed updates) while commits prefixed with '+' are in 'next'. Some of the topics in flight have overlaps with each other and have been excluded from 'pu'; most notably, I think the remainder of bc/object-id needs to wait until the for-each-ref topic from Karthik settles and then rebased on it, or something. A couple of "hotfix" topics that just went into 'next' should be part of -rc1 early next month, but other than that there aren't anything ultra-urgent cooking right now. Read those "Will merge to 'master'" as if they were suffixed with "soon after the upcoming release". You can find the changes described here in the integration branches of the repositories listed at http://git-blame.blogspot.com/p/git-public-repositories.html -- [Graduated to "master"] * nd/diff-i-t-a (2015-06-23) 1 commit + Revert "diff-lib.c: adjust position of i-t-a entries in diff" The "let's show paths added with -N as 'new' in status output" change was done without enough consideration on potential fallouts on the codepaths that do not have anything to do with "status" and caused regression to at least one widely used "wsadd" alias. -- [New Topics] None of these is particularly urgent. * dt/refs-backend-preamble (2015-06-29) 7 commits - git-stash: use git-reflog instead of creating files - git-reflog: add create and exists functions - refs: new public ref function: safe_create_reflog - refs: break out check for reflog autocreation - bisect: treat BISECT_HEAD as a ref - cherry-pick: treat CHERRY_PICK_HEAD and REVERT_HEAD as refs - refs.c: add err arguments to reflog functions In preparation for allowing different "backends" to store the refs in a way different from the traditional "one ref per file in $GIT_DIR or in a $GIT_DIR/packed-refs file" filesystem storage, reduce direct filesystem access to ref-like things like CHERRY_PICK_HEAD from scripts and programs. Will merge to 'next'. * et/http-proxyauth (2015-06-29) 1 commit - http: always use any proxy auth method available We used to ask libCURL to use the most secure authentication method available when talking to an HTTP proxy only when we were told to talk to one via configuration variables. We now ask libCURL to always use the most secure authentication method, because the user can tell libCURL to use an HTTP proxy via an environment variable without using configuration variables. Looked sensible. An extra set of eyes appreciated, but I think this is ready. Will merge to 'next'. * jc/fsck-retire-require-eoh (2015-06-28) 1 commit - fsck: it is OK for a tag and a commit to lack the body A fix to a minor regression to "git fsck" in v2.2 era that started complaining about a body-less tag object when it lacks a separator empty line after its header to separate it with a non-existent body. Will merge to 'next'. * jk/date-mode-format (2015-06-29) 3 commits - introduce "format" date-mode - convert "enum date_mode" into a struct - show-branch: use DATE_RELATIVE instead of magic number Teach "git log" and friends a new "--date=format:..." option to format timestamps using system's strftime(3). Will merge to 'next'. * jk/still-interesting (2015-06-29) 1 commit - revision.c: remove unneeded check for NULL Code clean-up. Will merge to 'next'. * nd/export-worktree (2015-06-26) 1 commit - setup: set env $GIT_WORK_TREE when work tree is set, like $GIT_DIR Running an aliased command from a subdirectory when the .git thing in the working tree is a gitfile pointing elsewhere did not work. Will merge to 'next'. * sb/p5310-and-chain (2015-06-26) 1 commit - p5310: Fix broken && chain in performance test Code clean-up. Will merge to 'next'. * jc/rerere (2015-06-28) 5 commits - rerere: report autoupdated paths only after actually updating them - rerere: write out each record of MERGE_RR in one go - rerere: lift PATH_MAX limitation - t4200: rerere a merge with two identical conflicts - rerere: fix an off-by-one non-bug Code clean-up (so far). -- [Stalled] * sg/config-name-only (2015-05-28) 3 commits - completion: use new 'git config' options to reliably list variable names - SQUASH - config: add options to list only variable names "git config --list" output was hard to parse when values consist of multiple lines. Introduce a way to show only the keys. Adding a single --name-only option may be a better way to go than adding two new options. Expecting a reroll. * kb/i18n-doc (2015-06-15) 1 commit - Documentation/i18n.txt: clarify character encoding support Comments (other than $gmane/271657)? * kb/use-nsec-doc (2015-06-15) 1 commit - Makefile / racy-git.txt: clarify USE_NSEC prerequisites Comments (other than $gmane/271656)? * kk/log-merges-conf
Re: [PATCH 3/3] introduce "format" date-mode
On Thu, Jun 25, 2015 at 12:55:45PM -0400, Jeff King wrote: > This feeds the format directly to strftime. Besides being a > little more flexible, the main advantage is that your system > strftime may know more about your locale's preferred format > (e.g., how to spell the days of the week). > > Signed-off-by: Jeff King > --- > diff --git a/strbuf.c b/strbuf.c > index 0d4f4e5..a7ba028 100644 > --- a/strbuf.c > +++ b/strbuf.c > @@ -709,3 +709,32 @@ char *xstrfmt(const char *fmt, ...) > +void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm) > +{ > + size_t len; > + > + /* > + * strftime reports "0" if it could not fit the result in the buffer. > + * Unfortunately, it also reports "0" if the requested time string > + * takes 0 bytes. So if we were to probe and grow, we have to choose > + * some arbitrary cap beyond which we guess that the format probably > + * just results in a 0-length output. Since we have to choose some > + * reasonable cap anyway, and since it is not that big, we may > + * as well just grow to their in the first place. > + */ > + strbuf_grow(sb, 128); > + len = strftime(sb->buf + sb->len, sb->alloc - sb->len, fmt, tm); > + > + if (!len) { > + /* > + * Either we failed, or the format actually produces a 0-length > + * output. There's not much we can do, so we leave it blank. > + * However, the output array is left in an undefined state, so > + * we must re-assert our NUL terminator. > + */ > + sb->buf[sb->len] = '\0'; > + } else { > + sb->len += len; > + } > +} Clients of strbuf rightly expect the buffer to grow as needed in order to complete the requested operation. It is, therefore, both weird and expectation-breaking for strbuf_addftime() to lack this behavior. Worse, it doesn't even signal when the format has failed due to insufficient buffer space. How about taking this approach (or something similar), instead, which grows the strbuf as needed? --- 8< --- void strbuf_addftime(struct strbuf *sb, const char *fmt, const struct tm *tm) { size_t len; struct strbuf f = STRBUF_INIT; /* * This is a bit tricky since strftime returns 0 if the result did not * fit in the supplied buffer, as well as when the formatted time has * zero length. In the former case, we need to grow the buffer and try * again. To distinguish between the two cases, we supply strftime with * a format string one character longer than what the client supplied, * which ensures that a successful format will have non-zero length, * and then drop the extra character from the formatted time before * returning. */ strbuf_addf(&f, "%s ", fmt); do { strbuf_grow(sb, 128); len = strftime(sb->buf + sb->len, sb->alloc - sb->len, f.buf, tm); } while (!len); strbuf_setlen(sb, sb->len + len - 1); strbuf_release(&f); } --- 8< --- If this is performance critical code, then the augmented format string can be constructed with less expensive functions than strbuf_addf(). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 13/17] engine.pl: provide more debug print statements
On 25.06.2015 02:03, Philip Oakley wrote: --- a/contrib/buildsystems/engine.pl +++ b/contrib/buildsystems/engine.pl @@ -41,6 +41,7 @@ EOM # Parse command-line options while (@ARGV) { my $arg = shift @ARGV; + #print "Arg: $arg \n"; if ("$arg" eq "-h" || "$arg" eq "--help" || "$arg" eq "-?") { showUsage(); exit(0); @@ -129,6 +130,7 @@ sub parseMakeOutput print "Parsing GNU Make output to figure out build structure...\n"; my $line = 0; while (my $text = shift @makedry) { + #print "Make: $text\n"; # show the makedry line Please never commit code that's been commented out. Also see http://dev.solita.fi/2013/07/04/whats-in-a-good-commit.html ;-) -- Sebastian Schuberth -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 38/44] builtin-am: support and auto-detect StGit patches
Eric Sunshine writes: > On Mon, Jun 29, 2015 at 4:42 PM, Stefan Beller wrote: >> On Sun, Jun 28, 2015 at 7:06 AM, Paul Tan wrote: >>> +/** >>> + * Returns true if `str` consists of only whitespace, false otherwise. >>> + */ >>> +static int str_isspace(const char *str) >>> +{ >>> + while (*str) >>> + if (!isspace(*(str)++)) >>> + return 0; >> ... >> while (*str && !isspace(*(str)++)) >> return 0; > ... > Ugh. Please don't break the logic with this strange and bogus transformation. > > If you really want it to read more idiomatically, try: > > for (; *s; s++) > if (!isspace(*s)) > return 0; ;-). Regardless of the loop structure, I find *(str)++ especially ugly and confusing. I'd understand if it were *(str++) but the parentheses pair is unnecessary. Not using any increment inside isspace(), like you showed, is the most readable. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] --count feature for git shortlog
My apologies, I misunderstood and thought rev-list didn't take filenames. Lawrence Siebert On Mon, Jun 29, 2015 at 10:04 AM, Junio C Hamano wrote: > Lawrence Siebert writes: > >> I was using it to sort files >> by commit count when provided a list of files, which git rev-list >> doesn't really work for. > > What makes you say rev-list does not work (perhaps 'really' is the > key word there?) > > git rev-list --no-merges maint..master -- Makefile > git shortlog --no-merges maint..master -- Makefile > git log --oneline --no-merges maint..master -- Makefile > > all gives me results that are consistent among them. > -- About Me: http://about.me/lawrencesiebert Constantly Coding: http://constantcoding.blogspot.com -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 7/7] git-stash: use git-reflog instead of creating files
OK, I'll squash in the last bit and restart today's integration run. Everything looks much nicer than any previous versions ;-) Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 38/44] builtin-am: support and auto-detect StGit patches
On Mon, Jun 29, 2015 at 4:42 PM, Stefan Beller wrote: > On Sun, Jun 28, 2015 at 7:06 AM, Paul Tan wrote: >> +/** >> + * Returns true if `str` consists of only whitespace, false otherwise. >> + */ >> +static int str_isspace(const char *str) >> +{ >> + while (*str) >> + if (!isspace(*(str)++)) >> + return 0; > > (nit:) > This looks a bit weird when first reading it, maybe combine the 2 conditions? > > while (*str && !isspace(*(str)++)) > return 0; > > The isspace checks for both tabs and whitespaces IIRC, so SP TAB SP > would be valid here > (returning 1). Ugh. Please don't break the logic with this strange and bogus transformation. If you really want it to read more idiomatically, try: for (; *s; s++) if (!isspace(*s)) return 0; -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 0/7] refs backend preamble
On Mon, 2015-06-29 at 13:31 -0700, Junio C Hamano wrote: > David Turner writes: > > > Minor formatting fixes from Junio Hamano. > > There is another. Would it be simpler for me to push this sort of fixup to github and and just mention that a new patchset is available? If so, here it is: https://github.com/dturner-tw/git.git dturner/pluggable-backends-preamble If not, I'll re-send. Of course for real reviews, I'll continue sending patches to the mailing list using git send-email. > By the way, "unused variable" is not a formatting fix. Indeed, it is not. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 38/44] builtin-am: support and auto-detect StGit patches
On Sun, Jun 28, 2015 at 7:06 AM, Paul Tan wrote: > Since c574e68 (git-am foreign patch support: StGIT support, 2009-05-27), > git-am.sh supported converting StGit patches into RFC2822 mail patches > that can be parsed with git-mailinfo. > > Implement this by introducing two functions in builtin/am.c: > stgit_patch_to_mail() and split_mail_conv(). > > stgit_patch_to_mail() is a callback function for split_mail_conv(), and > contains the logic for converting an StGit patch into an RFC2822 mail > patch. > > split_mail_conv() implements the logic to go through each file in the > `paths` list, reading from stdin where specified, and calls the callback > function to write the converted patch to the corresponding output file > in the state directory. This interface should be generic enough to > support other foreign patch formats in the future. > > Since 15ced75 (git-am foreign patch support: autodetect some patch > formats, 2009-05-27), git-am.sh is able to auto-detect StGit patches. > Re-implement this in builtin/am.c. > > Signed-off-by: Paul Tan > --- > builtin/am.c | 132 > ++- > 1 file changed, 131 insertions(+), 1 deletion(-) > > diff --git a/builtin/am.c b/builtin/am.c > index b54fdbd..b73498e 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -65,9 +65,22 @@ static int linelen(const char *msg) > return strchrnul(msg, '\n') - msg; > } > > +/** > + * Returns true if `str` consists of only whitespace, false otherwise. > + */ > +static int str_isspace(const char *str) > +{ > + while (*str) > + if (!isspace(*(str)++)) > + return 0; (nit:) This looks a bit weird when first reading it, maybe combine the 2 conditions? while (*str && !isspace(*(str)++)) return 0; The isspace checks for both tabs and whitespaces IIRC, so SP TAB SP would be valid here (returning 1). > + > + return 1; > +} > + > enum patch_format { > PATCH_FORMAT_UNKNOWN = 0, > - PATCH_FORMAT_MBOX > + PATCH_FORMAT_MBOX, > + PATCH_FORMAT_STGIT > }; > > enum keep_type { > @@ -651,6 +664,8 @@ static int detect_patch_format(const char **paths) > { > enum patch_format ret = PATCH_FORMAT_UNKNOWN; > struct strbuf l1 = STRBUF_INIT; > + struct strbuf l2 = STRBUF_INIT; > + struct strbuf l3 = STRBUF_INIT; > FILE *fp; > > /* > @@ -676,6 +691,23 @@ static int detect_patch_format(const char **paths) > goto done; > } > > + strbuf_reset(&l2); > + strbuf_getline_crlf(&l2, fp); > + strbuf_reset(&l3); > + strbuf_getline_crlf(&l3, fp); > + > + /* > +* If the second line is empty and the third is a From, Author or Date > +* entry, this is likely an StGit patch. > +*/ > + if (l1.len && !l2.len && > + (starts_with(l3.buf, "From:") || > +starts_with(l3.buf, "Author:") || > +starts_with(l3.buf, "Date:"))) { > + ret = PATCH_FORMAT_STGIT; > + goto done; > + } > + > if (l1.len && is_mail(fp)) { > ret = PATCH_FORMAT_MBOX; > goto done; > @@ -716,6 +748,100 @@ static int split_mail_mbox(struct am_state *state, > const char **paths, int keep_ > } > > /** > + * Callback signature for split_mail_conv(). The foreign patch should be > + * read from `in`, and the converted patch (in RFC2822 mail format) should be > + * written to `out`. Return 0 on success, or -1 on failure. > + */ > +typedef int (*mail_conv_fn)(FILE *out, FILE *in, int keep_cr); > + > +/** > + * Calls `fn` for each file in `paths` to convert the foreign patch to the > + * RFC2822 mail format suitable for parsing with git-mailinfo. > + * > + * Returns 0 on success, -1 on failure. > + */ > +static int split_mail_conv(mail_conv_fn fn, struct am_state *state, > + const char **paths, int keep_cr) > +{ > + static const char *stdin_only[] = {"-", NULL}; > + int i; > + > + if (!*paths) > + paths = stdin_only; > + > + for (i = 0; *paths; paths++, i++) { > + FILE *in, *out; > + const char *mail; > + int ret; > + > + if (!strcmp(*paths, "-")) > + in = stdin; > + else > + in = fopen(*paths, "r"); > + > + if (!in) > + return error(_("could not open '%s' for reading: %s"), > + *paths, strerror(errno)); > + > + mail = mkpath("%s/%0*d", state->dir, state->prec, i + 1); > + > + out = fopen(mail, "w"); > + if (!out) > + return error(_("could not open '%s' for writing: %s"), > + mail, strerror(errno)); > + > + ret = fn(out, in, keep_cr); > + > + fclose(out)
Re: [PATCH v4 40/44] builtin-am: support and auto-detect mercurial patches
On Mon, Jun 29, 2015 at 1:32 PM, Stefan Beller wrote: > On Sun, Jun 28, 2015 at 7:06 AM, Paul Tan wrote: >> + tz = tz / (60 * 60) * 100 + tz % (60 * 60); > > What happens if we have a negative input not matching a full hour, say -5400 ? > (would equate to 0130 in git) > > for calculating the minutes we would only need to take % 3600 (which > you do), but > then we still need to divide by 60 to convert seconds to minutes? > That said, I wonder if we have some helper functions around somewhere as we need to convert the timezone data at many places. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 40/44] builtin-am: support and auto-detect mercurial patches
On Sun, Jun 28, 2015 at 7:06 AM, Paul Tan wrote: > Since 0cfd112 (am: preliminary support for hg patches, 2011-08-29), > git-am.sh could convert mercurial patches to an RFC2822 mail patch > suitable for parsing with git-mailinfo, and queue them in the state > directory for application. > > Since 15ced75 (git-am foreign patch support: autodetect some patch > formats, 2009-05-27), git-am.sh was able to auto-detect mercurial > patches by checking if the file begins with the line: > > # HG changeset patch > > Re-implement the above in builtin/am.c. > > Signed-off-by: Paul Tan > --- > builtin/am.c | 73 > +++- > 1 file changed, 72 insertions(+), 1 deletion(-) > > diff --git a/builtin/am.c b/builtin/am.c > index 1576bd4..5c86e6f 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -81,7 +81,8 @@ enum patch_format { > PATCH_FORMAT_UNKNOWN = 0, > PATCH_FORMAT_MBOX, > PATCH_FORMAT_STGIT, > - PATCH_FORMAT_STGIT_SERIES > + PATCH_FORMAT_STGIT_SERIES, > + PATCH_FORMAT_HG > }; > > enum keep_type { > @@ -697,6 +698,11 @@ static int detect_patch_format(const char **paths) > goto done; > } > > + if (!strcmp(l1.buf, "# HG changeset patch")) { > + ret = PATCH_FORMAT_HG; > + goto done; > + } > + > strbuf_reset(&l2); > strbuf_getline_crlf(&l2, fp); > strbuf_reset(&l3); > @@ -895,6 +901,67 @@ static int split_mail_stgit_series(struct am_state > *state, const char **paths, > } > > /** > + * A split_patches_conv() callback that converts a mercurial patch to a > RFC2822 > + * message suitable for parsing with git-mailinfo. > + */ > +static int hg_patch_to_mail(FILE *out, FILE *in, int keep_cr) > +{ > + struct strbuf sb = STRBUF_INIT; > + > + while (!strbuf_getline(&sb, in, '\n')) { > + const char *str; > + > + if (skip_prefix(sb.buf, "# User ", &str)) > + fprintf(out, "From: %s\n", str); > + else if (skip_prefix(sb.buf, "# Date ", &str)) { > + unsigned long timestamp; > + long tz; > + char *end; > + > + errno = 0; > + timestamp = strtoul(str, &end, 10); > + if (errno) > + return error(_("invalid timestamp")); > + > + if (!skip_prefix(end, " ", &str)) > + return error(_("invalid Date line")); > + > + errno = 0; > + tz = strtol(str, &end, 10); > + if (errno) > + return error(_("invalid timezone offset")); > + > + if (*end) > + return error(_("invalid Date line")); > + > + /* > +* mercurial's timezone is in seconds west of UTC, > +* however git's timezone is in hours + minutes east > of > +* UTC. Convert it. > +*/ > + tz = tz / (60 * 60) * 100 + tz % (60 * 60); What happens if we have a negative input not matching a full hour, say -5400 ? (would equate to 0130 in git) for calculating the minutes we would only need to take % 3600 (which you do), but then we still need to divide by 60 to convert seconds to minutes? > + tz = -tz; > + > + fprintf(out, "Date: %s\n", show_date(timestamp, tz, > DATE_RFC2822)); > + } else if (starts_with(sb.buf, "# ")) { > + continue; > + } else { > + fprintf(out, "\n%s\n", sb.buf); > + break; > + } > + } > + > + strbuf_reset(&sb); > + while (strbuf_fread(&sb, 8192, in) > 0) { > + fwrite(sb.buf, 1, sb.len, out); > + strbuf_reset(&sb); > + } > + > + strbuf_release(&sb); > + return 0; > +} > + > +/** > * Splits a list of files/directories into individual email patches. Each > path > * in `paths` must be a file/directory that is formatted according to > * `patch_format`. > @@ -926,6 +993,8 @@ static int split_mail(struct am_state *state, enum > patch_format patch_format, > return split_mail_conv(stgit_patch_to_mail, state, paths, > keep_cr); > case PATCH_FORMAT_STGIT_SERIES: > return split_mail_stgit_series(state, paths, keep_cr); > + case PATCH_FORMAT_HG: > + return split_mail_conv(hg_patch_to_mail, state, paths, > keep_cr); > default: > die("BUG: invalid patch_format"); > } > @@ -1960,6 +2029,8 @@ static int parse_opt_patchformat(const struct option > *opt, const char *arg, int > *opt_value = PATCH_FORMAT_ST
Re: [PATCH v6 0/7] refs backend preamble
David Turner writes: > Minor formatting fixes from Junio Hamano. There is another. By the way, "unused variable" is not a formatting fix. git-bisect.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-bisect.sh b/git-bisect.sh index dddcc89..2fd8ea6 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -210,7 +210,7 @@ check_expected_revs() { } bisect_head_exists() { -git rev-parse --quiet --verify "BISECT_HEAD" >/dev/null + git rev-parse --quiet --verify "BISECT_HEAD" >/dev/null } bisect_skip() { -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 06/10] bisect: don't mix option parsing and non-trivial code
Matthieu, are you allowing your editor to corrupt the number of lines in the hunk on the @@ ... @@ hunk header? "diff" mode in Emacs does that, and there may be other editors that has the same bug, but please be careful---they make the patch unapplicable. Count the preimage lines in the hunk below. I count 24 but somebody is lying there. > @@ -102,25 +103,27 @@ bisect_start() { > die "$(eval_gettext "'\$arg' does not appear to > be a valid revision")" > break > } > - > - # The user ran "git bisect start > - # ", hence did not explicitly specify > - # the terms, but we are already starting to > - # set references named with the default terms, > - # and won't be able to change afterwards. > - must_write_terms=1 > - > - case $bad_seen in > - 0) state=$TERM_BAD ; bad_seen=1 ;; > - *) state=$TERM_GOOD ;; > - esac > - eval="$eval bisect_write '$state' '$rev' 'nolog' &&" > + revs="$revs $rev" > shift > ;; > esac > done > > + for rev in $revs > + do > + # The user ran "git bisect start > + # ", hence did not explicitly specify > + # the terms, but we are already starting to > + # set references named with the default terms, > + # and won't be able to change afterwards. > + must_write_terms=1 > + > + case $bad_seen in > + 0) state=$TERM_BAD ; bad_seen=1 ;; > + *) state=$TERM_GOOD ;; > + esac > + eval="$eval bisect_write '$state' '$rev' 'nolog' &&" > + done > # > # Verify HEAD. > # > -- > 2.5.0.rc0.10.gd2bff5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv7 1/3] git-rebase -i: add command "drop" to remove a commit
Instead of removing a line to remove the commit, you can use the command "drop" (just like "pick" or "edit"). It has the same effect as deleting the line (removing the commit) except that you keep a visual trace of your actions, allowing a better control and reducing the possibility of removing a commit by mistake. Signed-off-by: Galan Rémi --- Documentation/git-rebase.txt | 3 +++ git-rebase--interactive.sh| 3 ++- t/lib-rebase.sh | 4 ++-- t/t3404-rebase-interactive.sh | 18 ++ 4 files changed, 25 insertions(+), 3 deletions(-) diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 1d01baa..34bd070 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -514,6 +514,9 @@ rebasing. If you just want to edit the commit message for a commit, replace the command "pick" with the command "reword". +To drop a commit, replace the command "pick" with "drop", or just +delete the matching line. + If you want to fold two or more commits into one, replace the command "pick" for the second and subsequent commits with "squash" or "fixup". If the commits had different authors, the folded commit will be diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index dc3133f..72abf90 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -152,6 +152,7 @@ Commands: s, squash = use commit, but meld into previous commit f, fixup = like "squash", but discard this commit's log message x, exec = run command (the rest of the line) using shell + d, drop = remove commit These lines can be re-ordered; they are executed from top to bottom. @@ -505,7 +506,7 @@ do_next () { rm -f "$msg" "$author_script" "$amend" "$state_dir"/stopped-sha || exit read -r command sha1 rest < "$todo" case "$command" in - "$comment_char"*|''|noop) + "$comment_char"*|''|noop|drop|d) mark_action_done ;; pick|p) diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index 6bd2522..fdbc900 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -14,7 +14,7 @@ # specified line. # # " " -- add a line with the specified command -# ("squash", "fixup", "edit", or "reword") and the SHA1 taken +# ("squash", "fixup", "edit", "reword" or "drop") and the SHA1 taken # from the specified line. # # "exec_cmd_with_args" -- add an "exec cmd with args" line. @@ -46,7 +46,7 @@ set_fake_editor () { action=pick for line in $FAKE_LINES; do case $line in - squash|fixup|edit|reword) + squash|fixup|edit|reword|drop) action="$line";; exec*) echo "$line" | sed 's/_/ /g' >> "$1";; diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index ac429a0..3d059e5 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1102,4 +1102,22 @@ test_expect_success 'rebase -i commits that overwrite untracked files (no ff)' ' test $(git cat-file commit HEAD | sed -ne \$p) = I ' +rebase_setup_and_clean () { + test_when_finished " + git checkout master && + test_might_fail git branch -D $1 && + test_might_fail git rebase --abort + " && + git checkout -b $1 master +} + +test_expect_success 'drop' ' + rebase_setup_and_clean drop-test && + set_fake_editor && + FAKE_LINES="1 drop 2 3 drop 4 5" git rebase -i --root && + test E = $(git cat-file commit HEAD | sed -ne \$p) && + test C = $(git cat-file commit HEAD^ | sed -ne \$p) && + test A = $(git cat-file commit HEAD^^ | sed -ne \$p) +' + test_done -- 2.4.3.532.gf6210e5.dirty -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCHv7 2/3] git rebase -i: warn about removed commits
Check if commits were removed (i.e. a line was deleted) and print warnings or stop git rebase depending on the value of the configuration variable rebase.missingCommitsCheck. This patch gives the user the possibility to avoid silent loss of information (losing a commit through deleting the line in this case) if he wants. Add the configuration variable rebase.missingCommitsCheck. - When unset or set to "ignore", no checking is done. - When set to "warn", the commits are checked, warnings are displayed but git rebase still proceeds. - When set to "error", the commits are checked, warnings are displayed and the rebase is stopped. (The user can then use 'git rebase --edit-todo' and 'git rebase --continue', or 'git rebase --abort') rebase.missingCommitsCheck defaults to "ignore". Signed-off-by: Galan Rémi --- Documentation/config.txt | 11 + Documentation/git-rebase.txt | 6 +++ git-rebase--interactive.sh| 104 -- t/t3404-rebase-interactive.sh | 66 +++ 4 files changed, 184 insertions(+), 3 deletions(-) diff --git a/Documentation/config.txt b/Documentation/config.txt index 3e37b93..0360e7b 100644 --- a/Documentation/config.txt +++ b/Documentation/config.txt @@ -2169,6 +2169,17 @@ rebase.autoStash:: successful rebase might result in non-trivial conflicts. Defaults to false. +rebase.missingCommitsCheck:: + If set to "warn", git rebase -i will print a warning if some + commits are removed (e.g. a line was deleted), however the + rebase will still proceed. If set to "error", it will print + the previous warning and stop the rebase, 'git rebase + --edit-todo' can then be used to correct the error. If set to + "ignore", no checking is done. + To drop a commit without warning or error, use the `drop` + command in the todo-list. + Defaults to "ignore". + receive.advertiseAtomic:: By default, git-receive-pack will advertise the atomic push capability to its clients. If you don't want to this capability diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 34bd070..2ca3b8d 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -213,6 +213,12 @@ rebase.autoSquash:: rebase.autoStash:: If set to true enable '--autostash' option by default. +rebase.missingCommitsCheck:: + If set to "warn", print warnings about removed commits in + interactive mode. If set to "error", print the warnings and + stop the rebase. If set to "ignore", no checking is + done. "ignore" by default. + OPTIONS --- --onto :: diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 72abf90..66daacf 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -834,6 +834,104 @@ add_exec_commands () { mv "$1.new" "$1" } +# Print the list of the SHA-1 of the commits +# from stdin to stdout +todo_list_to_sha_list () { + git stripspace --strip-comments | + while read -r command sha1 rest + do + case $command in + "$comment_char"*|''|noop|x|"exec") + ;; + *) + long_sha=$(git rev-list --no-walk "$sha1" 2>/dev/null) + printf "%s\n" "$long_sha" + ;; + esac + done +} + +# Use warn for each line in stdin +warn_lines () { + while read -r line + do + warn " - $line" + done +} + +# Switch to the branch in $into and notify it in the reflog +checkout_onto () { + GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name" + output git checkout $onto || die_abort "could not detach HEAD" + git update-ref ORIG_HEAD $orig_head +} + +# Check if the user dropped some commits by mistake +# Behaviour determined by rebase.missingCommitsCheck. +check_todo_list () { + raise_error=f + + check_level=$(git config --get rebase.missingCommitsCheck) + check_level=${check_level:-ignore} + # Don't be case sensitive + check_level=$(printf '%s' "$check_level" | tr 'A-Z' 'a-z') + + case "$check_level" in + warn|error) + # Get the SHA-1 of the commits + todo_list_to_sha_list <"$todo".backup >"$todo".oldsha1 + todo_list_to_sha_list <"$todo" >"$todo".newsha1 + + # Sort the SHA-1 and compare them + sort -u "$todo".oldsha1 >"$todo".oldsha1+ + mv "$todo".oldsha1+ "$todo".oldsha1 + sort -u "$todo".newsha1 >"$todo".newsha1+ + mv "$todo".newsha1+ "$todo".newsha1 + comm -2 -3 "$todo".oldsha1 "$todo".newsha1 >"$todo".miss + + # Warn about missing commits + if test -s "$todo".miss + then + test "$check_level" = error && r
[PATCHv7 3/3] git rebase -i: add static check for commands and SHA-1
Check before the start of the rebasing if the commands exists, and for the commands expecting a SHA-1, check if the SHA-1 is present and corresponds to a commit. In case of error, print the error, stop git rebase and prompt the user to fix with 'git rebase --edit-todo' or to abort. This allows to avoid doing half of a rebase before finding an error and giving back what's left of the todo list to the user and prompt him to fix when it might be too late for him to do so (he might have to abort and restart the rebase). Signed-off-by: Galan Rémi --- git-rebase--interactive.sh| 75 +++ t/lib-rebase.sh | 5 +++ t/t3404-rebase-interactive.sh | 39 ++ 3 files changed, 119 insertions(+) diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh index 66daacf..ec4a068 100644 --- a/git-rebase--interactive.sh +++ b/git-rebase--interactive.sh @@ -834,6 +834,73 @@ add_exec_commands () { mv "$1.new" "$1" } +# Check if the SHA-1 passed as an argument is a +# correct one, if not then print $2 in "$todo".badsha +# $1: the SHA-1 to test +# $2: the line to display if incorrect SHA-1 +check_commit_sha () { + badsha=0 + if test -z $1 + then + badsha=1 + else + sha1_verif="$(git rev-parse --verify --quiet $1^{commit})" + if test -z $sha1_verif + then + badsha=1 + fi + fi + + if test $badsha -ne 0 + then + warn "Warning: the SHA-1 is missing or isn't" \ + "a commit in the following line:" + warn " - $2" + warn + fi + + return $badsha +} + +# prints the bad commits and bad commands +# from the todolist in stdin +check_bad_cmd_and_sha () { + retval=0 + git stripspace --strip-comments | + ( + while read -r line + do + IFS=' ' + set x $line + shift + command=$1 + sha1=$2 + + case $command in + ''|noop|x|"exec") + # Doesn't expect a SHA-1 + ;; + pick|p|drop|d|reword|r|edit|e|squash|s|fixup|f) + check_commit_sha $sha1 "$line" + if test $? -ne 0 + then + retval=1 + fi + ;; + *) + warn "Warning: the command isn't recognized" \ + "in the following line:" + warn " - $line" + warn + retval=1 + ;; + esac + done + + return $retval + ) +} + # Print the list of the SHA-1 of the commits # from stdin to stdout todo_list_to_sha_list () { @@ -868,6 +935,8 @@ checkout_onto () { # Check if the user dropped some commits by mistake # Behaviour determined by rebase.missingCommitsCheck. +# Check if there is an unrecognized command or a +# bad SHA-1 in a command. check_todo_list () { raise_error=f @@ -919,6 +988,12 @@ check_todo_list () { ;; esac + check_bad_cmd_and_sha <"$todo" + if test $? -ne 0 + then + raise_error=t + fi + if test $raise_error = t then # Checkout before the first commit of the diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh index fdbc900..9a96e15 100644 --- a/t/lib-rebase.sh +++ b/t/lib-rebase.sh @@ -54,6 +54,11 @@ set_fake_editor () { echo '# comment' >> "$1";; ">") echo >> "$1";; + bad) + action="badcmd";; + fakesha) + echo "$action XXX False commit" >> "$1" + action=pick;; *) sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1" action=pick;; diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 904a2d0..9b2c51c 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1186,4 +1186,43 @@ test_expect_success 'rebase -i respects rebase.missingCommitsCheck = error' ' test B = $(git cat-file commit HEAD^ | sed -ne \$p) ' +cat >expectexpect
rebase -i: drop, missing commits and static checks
Changes between versions: In t3404: Changed 'test_rebase_end' to 'rebase_setup_and_clean'. Changed the indentation in 'rebase_setup_and_clean'. Changed the names of the branches created in my tests (avoid names like 'tmp'). Added 'test_might_fail' in front of 'git branch -D'. Remove 'test_config rebase.missingCommitsCheck error' in the last test ('static check of bad SHA-1') because it was useless. In git-rebase--interactive.sh: Errors found in commands and SHA-1 (static check) are displayed on the spot. I used return values to signal to the calling functions if there is an error. The whole while block in 'check_bad_cmd_and_sha' with the return is in a '( [...] )' block because 'retval' would not have been correctly affected when getting out of the loop since the while is in a pipe. A thought occured to me: Shouldn't all the checking also be called in a 'rebase --continue', considering that it can be called after a 'rebase --edit-todo' ? (Right now it is only called after closing the editor in 'rebase -i') [PATCHv7 1/3] git-rebase -i: add command "drop" to remove a commit [PATCHv7 2/3] git rebase -i: warn about removed commits [PATCHv7 3/3] git rebase -i: add static check for commands and SHA-1 Rémi (It seems that with 'send-email', 1 time out of 2 I get: '5.7.8 Error: authentication failed: authentication failure') -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 5/7] refs: new public ref function: safe_create_reflog
The safe_create_reflog function creates a reflog, if it does not already exist. The log_ref_setup function becomes private and gains a force_create parameter to force the creation of a reflog even if log_all_ref_updates is false or the refname is not one of the special refnames. The new parameter also reduces the need to store, modify, and restore the log_all_ref_updates global before reflog creation. In a moment, we will use this to add reflog creation commands to git-reflog. Signed-off-by: David Turner --- builtin/checkout.c | 10 +- refs.c | 25 + refs.h | 2 +- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 93f63d3..9f68399 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -620,19 +620,11 @@ static void update_refs_for_switch(const struct checkout_opts *opts, if (opts->new_branch) { if (opts->new_orphan_branch) { if (opts->new_branch_log && !log_all_ref_updates) { - int temp; - struct strbuf log_file = STRBUF_INIT; - int ret; const char *ref_name; struct strbuf err = STRBUF_INIT; ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch); - temp = log_all_ref_updates; - log_all_ref_updates = 1; - ret = log_ref_setup(ref_name, &log_file, &err); - log_all_ref_updates = temp; - strbuf_release(&log_file); - if (ret) { + if (safe_create_reflog(ref_name, &err, 1)) { fprintf(stderr, _("Can not do reflog for '%s'. %s\n"), opts->new_orphan_branch, err.buf); strbuf_release(&err); diff --git a/refs.c b/refs.c index 30e81ba..1e53ef0 100644 --- a/refs.c +++ b/refs.c @@ -3128,8 +3128,14 @@ static int should_autocreate_reflog(const char *refname) !strcmp(refname, "HEAD"); } -/* This function will fill in *err and return -1 on failure */ -int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) +/* + * This function creates a reflog for a ref. If force_create = 0, the + * reflog will only be created for certain refs (those for which + * should_autocreate_reflog returns non-zero. Otherwise, it will be + * created regardless of the ref name. This function will fill in *err + * and return -1 on failure + */ +static int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err, int force_create) { int logfd, oflags = O_APPEND | O_WRONLY; char *logfile; @@ -3138,7 +3144,7 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf logfile = sb_logfile->buf; /* make sure the rest of the function can't change "logfile" */ sb_logfile = NULL; - if (should_autocreate_reflog(refname)) { + if (force_create || should_autocreate_reflog(refname)) { if (safe_create_leading_directories(logfile) < 0) { strbuf_addf(err, "unable to create directory for %s. " "%s", logfile, strerror(errno)); @@ -3173,6 +3179,17 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf return 0; } + +int safe_create_reflog(const char *refname, struct strbuf *err, int force_create) +{ + int ret; + struct strbuf sb = STRBUF_INIT; + + ret = log_ref_setup(refname, &sb, err, force_create); + strbuf_release(&sb); + return ret; +} + static int log_ref_write_fd(int fd, const unsigned char *old_sha1, const unsigned char *new_sha1, const char *committer, const char *msg) @@ -3209,7 +3226,7 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1, if (log_all_ref_updates < 0) log_all_ref_updates = !is_bare_repository(); - result = log_ref_setup(refname, sb_log_file, err); + result = log_ref_setup(refname, sb_log_file, err, 0); if (result) return result; diff --git a/refs.h b/refs.h index debdefc..3b90e16 100644 --- a/refs.h +++ b/refs.h @@ -228,7 +228,7 @@ int pack_refs(unsigned int flags); /* * Setup reflog before using. Fill in err and return -1 on failure. */ -int log_ref_setup(const char *refname, struct strbuf *logfile, struct strbuf *err); +int safe_create_reflog(const char *refname, struct strbuf *err, int force_create); /** Reads log for the value of ref during at_time. **/ extern int read_ref_at(const char *ref
[PATCH v6 2/7] cherry-pick: treat CHERRY_PICK_HEAD and REVERT_HEAD as refs
Instead of directly writing to and reading from files in $GIT_DIR, use ref API to interact with CHERRY_PICK_HEAD and REVERT_HEAD. Signed-off-by: David Turner --- branch.c | 4 ++-- builtin/commit.c | 6 +++--- builtin/merge.c | 2 +- contrib/completion/git-prompt.sh | 4 ++-- git-gui/lib/commit.tcl | 2 +- sequencer.c | 39 --- t/t7509-commit.sh| 4 ++-- wt-status.c | 6 ++ 8 files changed, 33 insertions(+), 34 deletions(-) diff --git a/branch.c b/branch.c index b002435..ec598aa 100644 --- a/branch.c +++ b/branch.c @@ -302,8 +302,8 @@ void create_branch(const char *head, void remove_branch_state(void) { - unlink(git_path("CHERRY_PICK_HEAD")); - unlink(git_path("REVERT_HEAD")); + delete_ref("CHERRY_PICK_HEAD", NULL, REF_NODEREF); + delete_ref("REVERT_HEAD", NULL, REF_NODEREF); unlink(git_path("MERGE_HEAD")); unlink(git_path("MERGE_RR")); unlink(git_path("MERGE_MSG")); diff --git a/builtin/commit.c b/builtin/commit.c index b5b1158..53c7e90 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -168,7 +168,7 @@ static void determine_whence(struct wt_status *s) { if (file_exists(git_path("MERGE_HEAD"))) whence = FROM_MERGE; - else if (file_exists(git_path("CHERRY_PICK_HEAD"))) { + else if (ref_exists("CHERRY_PICK_HEAD")) { whence = FROM_CHERRY_PICK; if (file_exists(git_path(SEQ_DIR))) sequencer_in_use = 1; @@ -1777,8 +1777,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix) } ref_transaction_free(transaction); - unlink(git_path("CHERRY_PICK_HEAD")); - unlink(git_path("REVERT_HEAD")); + delete_ref("CHERRY_PICK_HEAD", NULL, REF_NODEREF); + delete_ref("REVERT_HEAD", NULL, REF_NODEREF); unlink(git_path("MERGE_HEAD")); unlink(git_path("MERGE_MSG")); unlink(git_path("MERGE_MODE")); diff --git a/builtin/merge.c b/builtin/merge.c index 46aacd6..3e2ae2f 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1206,7 +1206,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) else die(_("You have not concluded your merge (MERGE_HEAD exists).")); } - if (file_exists(git_path("CHERRY_PICK_HEAD"))) { + if (ref_exists("CHERRY_PICK_HEAD")) { if (advice_resolve_conflict) die(_("You have not concluded your cherry-pick (CHERRY_PICK_HEAD exists).\n" "Please, commit your changes before you merge.")); diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 366f0bc..e2c5583 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -415,9 +415,9 @@ __git_ps1 () fi elif [ -f "$g/MERGE_HEAD" ]; then r="|MERGING" - elif [ -f "$g/CHERRY_PICK_HEAD" ]; then + elif git rev-parse --quiet --verify "CHERRY_PICK_HEAD" >/dev/null; then r="|CHERRY-PICKING" - elif [ -f "$g/REVERT_HEAD" ]; then + elif git rev-parse --quiet --verify "REVERT_HEAD" >/dev/null; then r="|REVERTING" elif [ -f "$g/BISECT_LOG" ]; then r="|BISECTING" diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl index 864b687..2b08b13 100644 --- a/git-gui/lib/commit.tcl +++ b/git-gui/lib/commit.tcl @@ -409,7 +409,7 @@ A rescan will be automatically started now. catch {file delete [gitdir MERGE_MSG]} catch {file delete [gitdir SQUASH_MSG]} catch {file delete [gitdir GITGUI_MSG]} - catch {file delete [gitdir CHERRY_PICK_HEAD]} + catch {git update-ref -d --no-deref CHERRY_PICK_HEAD} # -- Let rerere do its thing. # diff --git a/sequencer.c b/sequencer.c index f8421a8..44c43e5 100644 --- a/sequencer.c +++ b/sequencer.c @@ -158,21 +158,22 @@ static void free_message(struct commit *commit, struct commit_message *msg) unuse_commit_buffer(commit, msg->message); } -static void write_cherry_pick_head(struct commit *commit, const char *pseudoref) +static void write_cherry_pick_head(struct commit *commit, const char *ref) { - const char *filename; - int fd; - struct strbuf buf = STRBUF_INIT; + struct strbuf err = STRBUF_INIT; + void *transaction; - strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1)); + transaction = ref_transaction_begin(&err); + if (!transaction) + die(_("Could not create transaction: %s"), err.buf); - filename = git_path("%s", pseudoref); - fd = open(filename, O_WRONLY | O_CREAT, 0666); - if (fd < 0) -
[PATCH v6 7/7] git-stash: use git-reflog instead of creating files
This is in support of alternate ref backends which don't necessarily store reflogs as files. Signed-off-by: David Turner --- git-stash.sh | 4 ++-- refs.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/git-stash.sh b/git-stash.sh index 8e9e2cd..27155bc 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -184,7 +184,7 @@ store_stash () { fi # Make sure the reflog for stash is kept. - : >>"$(git rev-parse --git-path logs/$ref_stash)" + git reflog create "$ref_stash" git update-ref -m "$stash_msg" $ref_stash $w_commit ret=$? test $ret != 0 && test -z $quiet && @@ -262,7 +262,7 @@ save_stash () { say "$(gettext "No local changes to save")" exit 0 fi - test -f "$(git rev-parse --git-path logs/$ref_stash)" || + git reflog exists $ref_stash || clear_stash || die "$(gettext "Cannot initialize stash")" create_stash "$stash_msg" $untracked diff --git a/refs.c b/refs.c index 1e53ef0..0f240c9 100644 --- a/refs.c +++ b/refs.c @@ -3125,6 +3125,7 @@ static int should_autocreate_reflog(const char *refname) return starts_with(refname, "refs/heads/") || starts_with(refname, "refs/remotes/") || starts_with(refname, "refs/notes/") || + !strcmp(refname, "refs/stash") || !strcmp(refname, "HEAD"); } -- 2.0.4.315.gad8727a-twtrsrc -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 3/7] bisect: treat BISECT_HEAD as a ref
Instead of directly writing to and reading from files in $GIT_DIR, use ref API to interact with BISECT_HEAD. Signed-off-by: David Turner --- git-bisect.sh | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/git-bisect.sh b/git-bisect.sh index ae3fec2..dddcc89 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -35,7 +35,7 @@ _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40" bisect_head() { - if test -f "$GIT_DIR/BISECT_HEAD" + if bisect_head_exists then echo BISECT_HEAD else @@ -209,6 +209,10 @@ check_expected_revs() { done } +bisect_head_exists() { +git rev-parse --quiet --verify "BISECT_HEAD" >/dev/null +} + bisect_skip() { all='' for arg in "$@" @@ -310,7 +314,7 @@ bisect_next() { bisect_next_check good # Perform all bisection computation, display and checkout - git bisect--helper --next-all $(test -f "$GIT_DIR/BISECT_HEAD" && echo --no-checkout) + git bisect--helper --next-all $(bisect_head_exists && echo --no-checkout) res=$? # Check if we should exit because bisection is finished @@ -377,7 +381,7 @@ bisect_reset() { usage ;; esac - if ! test -f "$GIT_DIR/BISECT_HEAD" && ! git checkout "$branch" -- + if ! bisect_head_exists && ! git checkout "$branch" -- then die "$(eval_gettext "Could not check out original HEAD '\$branch'. Try 'git bisect reset '.")" -- 2.0.4.315.gad8727a-twtrsrc -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 4/7] refs: Break out check for reflog autocreation
This is just for clarity. Signed-off-by: David Turner --- refs.c | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/refs.c b/refs.c index c97ca02..30e81ba 100644 --- a/refs.c +++ b/refs.c @@ -3118,6 +3118,16 @@ static int copy_msg(char *buf, const char *msg) return cp - buf; } +static int should_autocreate_reflog(const char *refname) +{ + if (!log_all_ref_updates) + return 0; + return starts_with(refname, "refs/heads/") || + starts_with(refname, "refs/remotes/") || + starts_with(refname, "refs/notes/") || + !strcmp(refname, "HEAD"); +} + /* This function will fill in *err and return -1 on failure */ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) { @@ -3128,11 +3138,7 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf logfile = sb_logfile->buf; /* make sure the rest of the function can't change "logfile" */ sb_logfile = NULL; - if (log_all_ref_updates && - (starts_with(refname, "refs/heads/") || -starts_with(refname, "refs/remotes/") || -starts_with(refname, "refs/notes/") || -!strcmp(refname, "HEAD"))) { + if (should_autocreate_reflog(refname)) { if (safe_create_leading_directories(logfile) < 0) { strbuf_addf(err, "unable to create directory for %s. " "%s", logfile, strerror(errno)); -- 2.0.4.315.gad8727a-twtrsrc -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v6 6/7] git-reflog: add create and exists functions
These are necessary because alternate ref backends might store reflogs somewhere other than .git/logs. Code that now directly manipulates .git/logs should instead go through git-reflog. In a moment, we will use these functions to make git stash work with alternate ref backends. Signed-off-by: David Turner --- Documentation/git-reflog.txt | 10 ++ builtin/reflog.c | 75 +++- t/t1411-reflog-show.sh | 12 +++ 3 files changed, 96 insertions(+), 1 deletion(-) diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt index 5e7908e..2bf8aa6 100644 --- a/Documentation/git-reflog.txt +++ b/Documentation/git-reflog.txt @@ -23,6 +23,8 @@ depending on the subcommand: [--dry-run] [--verbose] [--all | ...] 'git reflog delete' [--rewrite] [--updateref] [--dry-run] [--verbose] ref@\{specifier\}... +'git reflog create' ... +'git reflog exists' Reference logs, or "reflogs", record when the tips of branches and other references were updated in the local repository. Reflogs are @@ -52,6 +54,14 @@ argument must be an _exact_ entry (e.g. "`git reflog delete master@{2}`"). This subcommand is also typically not used directly by end users. +The "create" subcommand creates a reflog for one or more refs. Most +refs (those under refs/heads, refs/remotes, and refs/tags) will +automatically have reflogs created. Other refs will not. This command +allows manual ref creation. + +The "exists" subcommand checks whether a ref has a reflog. It exists +with zero status if the reflog exists, and non-zero status if it does +not. OPTIONS --- diff --git a/builtin/reflog.c b/builtin/reflog.c index c2eb8ff..3080865 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -13,6 +13,10 @@ static const char reflog_expire_usage[] = "git reflog expire [--expire=] [--expire-unreachable=] [--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] [--verbose] [--all] ..."; static const char reflog_delete_usage[] = "git reflog delete [--rewrite] [--updateref] [--dry-run | -n] [--verbose] ..."; +static const char reflog_create_usage[] = +"git reflog create ..."; +static const char reflog_exists_usage[] = +"git reflog exists "; static unsigned long default_reflog_expire; static unsigned long default_reflog_expire_unreachable; @@ -699,12 +703,75 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix) return status; } +static int cmd_reflog_create(int argc, const char **argv, const char *prefix) +{ + int i, status = 0, start = 0; + struct strbuf err = STRBUF_INIT; + + for (i = 1; i < argc; i++) { + const char *arg = argv[i]; + if (!strcmp(arg, "--")) { + i++; + break; + } + else if (arg[0] == '-') + usage(reflog_create_usage); + else + break; + } + + start = i; + + if (argc - start < 1) + return error("Nothing to create?"); + + for (i = start; i < argc; i++) { + if (check_refname_format(argv[i], REFNAME_ALLOW_ONELEVEL)) + die("invalid ref format: %s", argv[i]); + } + for (i = start; i < argc; i++) { + if (safe_create_reflog(argv[i], &err, 1)) { + error("could not create reflog %s: %s", argv[i], + err.buf); + status = 1; + strbuf_release(&err); + } + } + return status; +} + +static int cmd_reflog_exists(int argc, const char **argv, const char *prefix) +{ + int i, start = 0; + + for (i = 1; i < argc; i++) { + const char *arg = argv[i]; + if (!strcmp(arg, "--")) { + i++; + break; + } + else if (arg[0] == '-') + usage(reflog_exists_usage); + else + break; + } + + start = i; + + if (argc - start != 1) + usage(reflog_exists_usage); + + if (check_refname_format(argv[start], REFNAME_ALLOW_ONELEVEL)) + die("invalid ref format: %s", argv[start]); + return !reflog_exists(argv[start]); +} + /* * main "reflog" */ static const char reflog_usage[] = -"git reflog [ show | expire | delete ]"; +"git reflog [ show | expire | delete | create | exists ]"; int cmd_reflog(int argc, const char **argv, const char *prefix) { @@ -724,5 +791,11 @@ int cmd_reflog(int argc, const char **argv, const char *prefix) if (!strcmp(argv[1], "delete")) return cmd_reflog_delete(argc - 1, argv + 1, prefix); + if (!strcmp(argv[1], "create")) + return cmd_reflog_create(argc - 1, argv + 1, prefix); + + if (!strcmp(argv[1], "exists")) + return cmd_reflog
[PATCH v6 1/7] refs.c: add err arguments to reflog functions
Add an err argument to log_ref_setup that can explain the reason for a failure. This then eliminates the need to manage errno through this function since we can just add strerror(errno) to the err string when meaningful. No callers relied on errno from this function for anything else than the error message. Also add err arguments to private functions write_ref_to_lockfile, log_ref_write_1, commit_ref_update. This again eliminates the need to manage errno in these functions. Update of a patch by Ronnie Sahlberg. Signed-off-by: Ronnie Sahlberg Signed-off-by: David Turner --- builtin/checkout.c | 8 ++-- refs.c | 111 - refs.h | 4 +- 3 files changed, 66 insertions(+), 57 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index c018ab3..93f63d3 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -624,16 +624,18 @@ static void update_refs_for_switch(const struct checkout_opts *opts, struct strbuf log_file = STRBUF_INIT; int ret; const char *ref_name; + struct strbuf err = STRBUF_INIT; ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch); temp = log_all_ref_updates; log_all_ref_updates = 1; - ret = log_ref_setup(ref_name, &log_file); + ret = log_ref_setup(ref_name, &log_file, &err); log_all_ref_updates = temp; strbuf_release(&log_file); if (ret) { - fprintf(stderr, _("Can not do reflog for '%s'\n"), - opts->new_orphan_branch); + fprintf(stderr, _("Can not do reflog for '%s'. %s\n"), + opts->new_orphan_branch, err.buf); + strbuf_release(&err); return; } } diff --git a/refs.c b/refs.c index fb568d7..c97ca02 100644 --- a/refs.c +++ b/refs.c @@ -2975,9 +2975,11 @@ static int rename_ref_available(const char *oldname, const char *newname) return ret; } -static int write_ref_to_lockfile(struct ref_lock *lock, const unsigned char *sha1); +static int write_ref_to_lockfile(struct ref_lock *lock, +const unsigned char *sha1, struct strbuf *err); static int commit_ref_update(struct ref_lock *lock, -const unsigned char *sha1, const char *logmsg); +const unsigned char *sha1, const char *logmsg, +struct strbuf *err); int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg) { @@ -3038,9 +3040,10 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms } hashcpy(lock->old_oid.hash, orig_sha1); - if (write_ref_to_lockfile(lock, orig_sha1) || - commit_ref_update(lock, orig_sha1, logmsg)) { - error("unable to write current sha1 into %s", newrefname); + if (write_ref_to_lockfile(lock, orig_sha1, &err) || + commit_ref_update(lock, orig_sha1, logmsg, &err)) { + error("unable to write current sha1 into %s: %s", newrefname, err.buf); + strbuf_release(&err); goto rollback; } @@ -3056,9 +3059,11 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms flag = log_all_ref_updates; log_all_ref_updates = 0; - if (write_ref_to_lockfile(lock, orig_sha1) || - commit_ref_update(lock, orig_sha1, NULL)) - error("unable to write current sha1 into %s", oldrefname); + if (write_ref_to_lockfile(lock, orig_sha1, &err) || + commit_ref_update(lock, orig_sha1, NULL, &err)) { + error("unable to write current sha1 into %s: %s", oldrefname, err.buf); + strbuf_release(&err); + } log_all_ref_updates = flag; rollbacklog: @@ -3113,8 +3118,8 @@ static int copy_msg(char *buf, const char *msg) return cp - buf; } -/* This function must set a meaningful errno on failure */ -int log_ref_setup(const char *refname, struct strbuf *sb_logfile) +/* This function will fill in *err and return -1 on failure */ +int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) { int logfd, oflags = O_APPEND | O_WRONLY; char *logfile; @@ -3129,9 +3134,8 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile) starts_with(refname, "refs/notes/") || !strcmp(refname, "HEAD
[PATCH v6 0/7] refs backend preamble
Minor formatting fixes from Junio Hamano. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 42/44] builtin-am: implement legacy -b/--binary option
On Sun, Jun 28, 2015 at 7:06 AM, Paul Tan wrote: > The -b/--binary option was initially implemented in 087b674 (git-am: > --binary; document --resume and --binary., 2005-11-16). The option will > pass the --binary flag to git-apply to allow it to apply binary patches. > > However, in 2b6eef9 (Make apply --binary a no-op., 2006-09-06), --binary > was been made a no-op in git-apply. Following that, since cb3a160 > (git-am: ignore --binary option, 2008-08-09), the --binary option in > git-am is ignored as well. > > In 6c15a1c (am: officially deprecate -b/--binary option, 2012-03-13), > the --binary option was tweaked to its present behavior: when set, the > message: > > The -b/--binary option has been a no-op for long time, and it > will be removed. Please do not use it anymore. > > will be printed. I wonder if now would be the right time? The rewrite aim's at full feature compatibility, but we may want to revert this commit on top of patch 44 later. > > Re-implement this in builtin/am.c. > > Signed-off-by: Paul Tan > --- > builtin/am.c | 7 +++ > 1 file changed, 7 insertions(+) > > diff --git a/builtin/am.c b/builtin/am.c > index f148f05..a46aa74 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -2148,6 +2148,7 @@ enum resume_mode { > int cmd_am(int argc, const char **argv, const char *prefix) > { > struct am_state state; > + int binary = -1; > int keep_cr = -1; > int patch_format = PATCH_FORMAT_UNKNOWN; > enum resume_mode resume = RESUME_FALSE; > @@ -2161,6 +2162,8 @@ int cmd_am(int argc, const char **argv, const char > *prefix) > struct option options[] = { > OPT_BOOL('i', "interactive", &state.interactive, > N_("run interactively")), > + OPT_HIDDEN_BOOL('b', "binary", &binary, > + N_("(historical option -- no-op")), > OPT_BOOL('3', "3way", &state.threeway, > N_("allow fall back on 3way merging if needed")), > OPT__QUIET(&state.quiet, N_("be quiet")), > @@ -2261,6 +2264,10 @@ int cmd_am(int argc, const char **argv, const char > *prefix) > > argc = parse_options(argc, argv, prefix, options, usage, 0); > > + if (binary >= 0) > + fprintf_ln(stderr, _("The -b/--binary option has been a no-op > for long time, and\n" > + "it will be removed. Please do not use it > anymore.")); > + > if (read_index_preload(&the_index, NULL) < 0) > die(_("failed to read the index")); > > -- > 2.5.0.rc0.76.gb2c6e93 > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 43/44] builtin-am: check for valid committer ident
On Sun, Jun 28, 2015 at 7:06 AM, Paul Tan wrote: > When commit_tree() is called, if the user does not have an explicit > committer ident configured, it will attempt to construct a default > committer ident based on the user's and system's info (e.g. gecos field, > hostname etc.) However, if a default committer ident is unable to be > constructed, commit_tree() will die(). However, at this point of git-am, s/. However,/, but/ ? > there will already be changes made to the index and work tree. > > This can be confusing to new users, and as such since d64e6b0 (Keep > Porcelainish from failing by broken ident after making changes., > 2006-02-18) git-am.sh will check to see if the committer ident has been > configured, or a default one can be constructed, before even starting to > apply patches. > > Re-implement this in builtin/am.c. > > Signed-off-by: Paul Tan > --- > builtin/am.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/builtin/am.c b/builtin/am.c > index a46aa74..1cb02c8 100644 > --- a/builtin/am.c > +++ b/builtin/am.c > @@ -2268,6 +2268,9 @@ int cmd_am(int argc, const char **argv, const char > *prefix) > fprintf_ln(stderr, _("The -b/--binary option has been a no-op > for long time, and\n" > "it will be removed. Please do not use it > anymore.")); > > + /* Ensure a valid committer ident can be constructed */ > + git_committer_info(IDENT_STRICT); > + > if (read_index_preload(&the_index, NULL) < 0) > die(_("failed to read the index")); > > -- > 2.5.0.rc0.76.gb2c6e93 > -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2015, #06; Wed, 24)
> * js/rebase-i-clean-up-upon-continue-to-skip (2015-06-23) 2 commits > - rebase -i: do not leave a CHERRY_PICK_HEAD file behind > - t3404: demonstrate CHERRY_PICK_HEAD bug > > Abandoning an already applied change in "git rebase -i" with > "--continue" left CHERRY_PICK_HEAD and confused later steps. > > Will merge to 'next'. I need a little tweak here: diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh index 7fd1330..f236128 100755 --- a/t/t3404-rebase-interactive.sh +++ b/t/t3404-rebase-interactive.sh @@ -1228,7 +1228,7 @@ test_expect_success 'rebase --continue removes CHERRY_PICK_HEAD' ' git checkout -b commit-to-skip && for double in X 3 1 do - seq 5 | sed "s/$double/&&/" >seq && + test_seq 5 | sed "s/$double/&&/" >seq && -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 04/11] for-each-ref: add '--points-at' option
On Mon, Jun 29, 2015 at 11:16 PM, Junio C Hamano wrote: > Karthik Nayak writes: > >> Add the '--points-at' option provided by 'ref-filter'. The option >> lets the user to pick only refs which point to a particular >> commit. > > It somehow feels strange that the option name is points-at and all > the explanation (like the above and also in the doc) talks about > pointing to an object. Not limited to this patch but the previous > one had the same, I think. > Will have a look and change, thanks :) >> diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh >> index b1fa8d4..7269a66 100644 >> --- a/t/t6302-for-each-ref-filter.sh >> +++ b/t/t6302-for-each-ref-filter.sh >> @@ -16,4 +16,24 @@ test_expect_success 'setup some history and refs' ' >> git update-ref refs/odd/spot master >> ' >> >> +test_expect_success 'filtering with --points-at' ' >> + cat >expect <<-\EOF && >> + refs/heads/master >> + refs/odd/spot >> + refs/tags/three >> + EOF >> + git for-each-ref --format="%(refname)" --points-at=master >actual && >> + test_cmp expect actual >> +' >> + >> +test_expect_success 'check signed tags with --points-at' ' >> + cat >expect <<-\EOF && >> + refs/heads/side >> + refs/tags/four >> + refs/tags/signed-tag four >> + EOF >> + git for-each-ref --format="%(refname) %(*subject)" --points-at=side >> >actual && >> + test_cmp expect actual >> +' > > This shows that we would want to add a "annotated doubly" tag in the > preparation step 01/11; the expected outcome is that it will not > show in the output, I think. > > Thanks. Will add! -- Regards, Karthik Nayak -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 03/11] ref-filter: implement '--points-at' option
On Mon, Jun 29, 2015 at 11:10 PM, Junio C Hamano wrote: > Karthik Nayak writes: > >> +/* >> + * Given a ref (sha1, refname) see if it points to one of the sha1s >> + * in a sha1_array. >> + */ >> +static int match_points_at(struct sha1_array *points_at, const unsigned >> char *sha1, >> +const char *refname) >> +{ >> + struct object *obj; >> + >> + if (!points_at || !points_at->nr) >> + return 1; >> + >> + if (sha1_array_lookup(points_at, sha1) >= 0) >> + return 1; >> + >> + obj = parse_object_or_die(sha1, refname); >> + if (obj->type == OBJ_TAG && >> + sha1_array_lookup(points_at, ((struct tag *)obj)->tagged->sha1) >= >> 0) >> + return 1; >> + >> + return 0; >> +} >> + > > Interesting. I think the change done while copying the code does > not change anything from the original (other than that the helper > lost its ability to return the peeled object name), and I think you > shouldn't make any change while copying the code that would change > the benaviour, but I notice a few things that we might want to keep > in mind and revisit them later (i.e. might be a good idea to add > NEEDSWORK comment to record them near the function): Reverted the change. OK will do and add this, I will work on this after GSoC is done. But like you said I'll add a comment so if someone wants to they can work on it for now. > > - The original only peeled one level of indirection, so does this >implementation. But is that really what we want, I wonder? > >After doing: > >$ git tag -a -m 'annotated' atag $commit >$ git tag -a -m 'annotated doubly' dtag atag > >atag^0, dtag^0 and $commit all refer to the same commit object. >Do we want to miss dtag with --point-at=$commit? > > - As we are in for-each-ref (or eventually tag -l) that is walking >the cached refs, we may know what refname peels to without >parsing the object at all. Could it be more efficient to ask >peel_ref() for the pointee without doing parse_object() >ourselves? > Shouldn't both these scenarios be solved together by using peel_ref()? After what you said I just tried a hacked up version of using peel_ref() rather than parsing the object, tried it out on the Linux tree. $time git tag -l --points-at=HEAD~501 git tag -l --points-at=HEAD~501 0.03s user 0.01s system 97% cpu 0.044 total *Using the modified version which uses peel_ref() * $time ../git/git tag -l --points-at=HEAD~501 ../git/git tag -l --points-at=HEAD~501 0.01s user 0.02s system 90% cpu 0.033 total This was the average of around 5 tests, Might not be the best way to check, but I'm sure there's an improvement. Thanks :) -- Regards, Karthik Nayak -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 04/11] for-each-ref: add '--points-at' option
On Tue, Jun 30, 2015 at 12:08 AM, Junio C Hamano wrote: > Karthik Nayak writes: > >> +test_expect_success 'check signed tags with --points-at' ' >> + cat >expect <<-\EOF && >> + refs/heads/side >> + refs/tags/four >> + refs/tags/signed-tag four >> + EOF >> + git for-each-ref --format="%(refname) %(*subject)" --points-at=side >> >actual && >> + test_cmp expect actual >> +' > > This piece seems to fail without a trailing whitespace in the > expected file. I initially suspected that they were dropped > on my end while applying with "git am --whitespace=fix", but going > back to my mailbox, it seems that what gmane received does not have > them in the first place: > > sed -e "s/Z$//" <<-\EOF && > refs/heads/side Z > refs/tags/four Z > refs/tags/signed-tag four > EOF > > or something like that to make the EOL more visible to those who are > reading the tests, perhaps? Thanks, will use sed like you suggested. -- Regards, Karthik Nayak -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v5 0/7] refs backend preamble
There might be other issues but from cursory read, at least the following needs to be split and squashed into patches that introduce them. Otherwise, it was a very pleasant read. Thanks. Documentation/git-reflog.txt | 4 ++-- builtin/reflog.c | 2 +- refs.c | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt index 7ab2c42..2bf8aa6 100644 --- a/Documentation/git-reflog.txt +++ b/Documentation/git-reflog.txt @@ -23,8 +23,8 @@ depending on the subcommand: [--dry-run] [--verbose] [--all | ...] 'git reflog delete' [--rewrite] [--updateref] [--dry-run] [--verbose] ref@\{specifier\}... -'git reflog create ... -'git reflog exists +'git reflog create' ... +'git reflog exists' Reference logs, or "reflogs", record when the tips of branches and other references were updated in the local repository. Reflogs are diff --git a/builtin/reflog.c b/builtin/reflog.c index 1ecbfb6..3080865 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -742,7 +742,7 @@ static int cmd_reflog_create(int argc, const char **argv, const char *prefix) static int cmd_reflog_exists(int argc, const char **argv, const char *prefix) { - int i, status = 0, start = 0; + int i, start = 0; for (i = 1; i < argc; i++) { const char *arg = argv[i]; diff --git a/refs.c b/refs.c index 55235cf..831d31c 100644 --- a/refs.c +++ b/refs.c @@ -2911,7 +2911,7 @@ static int rename_ref_available(const char *oldname, const char *newname) } static int write_ref_to_lockfile(struct ref_lock *lock, -const unsigned char *sha1, struct strbuf* err); +const unsigned char *sha1, struct strbuf *err); static int commit_ref_update(struct ref_lock *lock, const unsigned char *sha1, const char *logmsg, struct strbuf *err); -- 2.5.0-rc0-169-ge0d9173 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 01/11] t6302: for-each-ref tests for ref-filter APIs
On Mon, Jun 29, 2015 at 11:44 PM, Junio C Hamano wrote: > Karthik Nayak writes: > >> t/t6302-for-each-ref-filter.sh | 19 +++ >> 1 file changed, 19 insertions(+) >> create mode 100644 t/t6302-for-each-ref-filter.sh > > non-executable tests: t6302-for-each-ref-filter.sh Renaming it defaulted to 644, will change, thanks :) -- Regards, Karthik Nayak -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 04/11] for-each-ref: add '--points-at' option
Karthik Nayak writes: > +test_expect_success 'check signed tags with --points-at' ' > + cat >expect <<-\EOF && > + refs/heads/side > + refs/tags/four > + refs/tags/signed-tag four > + EOF > + git for-each-ref --format="%(refname) %(*subject)" --points-at=side > >actual && > + test_cmp expect actual > +' This piece seems to fail without a trailing whitespace in the expected file. I initially suspected that they were dropped on my end while applying with "git am --whitespace=fix", but going back to my mailbox, it seems that what gmane received does not have them in the first place: sed -e "s/Z$//" <<-\EOF && refs/heads/side Z refs/tags/four Z refs/tags/signed-tag four EOF or something like that to make the EOL more visible to those who are reading the tests, perhaps? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 06/11] ref-filter: implement '--merged' and '--no-merged' options
Junio C Hamano writes: > Karthik Nayak writes: > >> +static void do_merge_filter(struct ref_filter_cbdata *ref_cbdata) >> +{ >> ... >> +for (i = 0; i < array->nr; i++) { >> +struct ref_array_item *item = array->items[i]; >> +add_pending_object(&revs, &item->commit->object, item->refname); >> +to_clear[i] = item->commit; >> +} >> + >> +filter->merge_commit->object.flags |= UNINTERESTING; >> +add_pending_object(&revs, &filter->merge_commit->object, ""); >> + >> +revs.limited = 1; >> +if (prepare_revision_walk(&revs)) >> +die(_("revision walk setup failed")); >> + ... > > Did this come from somewhere else (e.g. tag -l or branch -l)? If > so, you'd need a note similar to what you added in [02/11] to the > original. Ah, now I see this came from a part of print_ref_list in builtin/branch.c; you would want to leave a note there. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC/PATCH 5/9] ref-filter: add option to match literal pattern
Karthik Nayak writes: > Since 'ref-filter' only has an option to match path names. That is not a whole sentence ;-) > Add an option for regular pattern matching. > Mentored-by: Christian Couder > Mentored-by: Matthieu Moy > Signed-off-by: Karthik Nayak > - if (flag & REF_BAD_NAME) { > - warning("ignoring ref with broken name %s", refname); > - return 0; > - } > - Hmm, where did this check go in the new code? Or is it now OK not to warn or ignore, and if so why? > if (flag & REF_ISBROKEN) { > warning("ignoring broken ref %s", refname); > return 0; > } > > - if (*filter->name_patterns && > !match_name_as_path(filter->name_patterns, refname)) > + if (!filter_pattern_match(filter, refname)) > return 0; > > if (!match_points_at(&filter->points_at, oid->hash, refname)) > diff --git a/ref-filter.h b/ref-filter.h > index 6b6fb96..a4809c8 100644 > --- a/ref-filter.h > +++ b/ref-filter.h > @@ -54,7 +54,8 @@ struct ref_filter { > } merge; > struct commit *merge_commit; > > - unsigned int with_commit_tag_algo: 1; > + unsigned int with_commit_tag_algo: 1, > + match_as_path: 1; Lose SP on both sides of the colon, or have SP on both sides (same for the last patch in the previous series). -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 01/11] t6302: for-each-ref tests for ref-filter APIs
Karthik Nayak writes: > t/t6302-for-each-ref-filter.sh | 19 +++ > 1 file changed, 19 insertions(+) > create mode 100644 t/t6302-for-each-ref-filter.sh non-executable tests: t6302-for-each-ref-filter.sh -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 06/11] ref-filter: implement '--merged' and '--no-merged' options
Karthik Nayak writes: > +static void do_merge_filter(struct ref_filter_cbdata *ref_cbdata) > +{ > + struct rev_info revs; > + int i, old_nr; > + struct ref_filter *filter = ref_cbdata->filter; > + struct ref_array *array = ref_cbdata->array; > + struct commit **to_clear = xcalloc(sizeof(struct commit *), array->nr); > + > + init_revisions(&revs, NULL); > + > + for (i = 0; i < array->nr; i++) { > + struct ref_array_item *item = array->items[i]; > + add_pending_object(&revs, &item->commit->object, item->refname); > + to_clear[i] = item->commit; > + } > + > + filter->merge_commit->object.flags |= UNINTERESTING; > + add_pending_object(&revs, &filter->merge_commit->object, ""); > + > + revs.limited = 1; > + if (prepare_revision_walk(&revs)) > + die(_("revision walk setup failed")); > + > + old_nr = array->nr; > + array->nr = 0; > + > + for (i = 0; i < old_nr; i++) { > + struct ref_array_item *item = array->items[i]; > + struct commit *commit = item->commit; > + > + int is_merged = !!(commit->object.flags & UNINTERESTING); > + > + if (is_merged == (filter->merge == REF_FILTER_MERGED_INCLUDE)) > + array->items[array->nr++] = array->items[i]; > + else > + free_array_item(item); > + } > + > + for (i = 0; i < old_nr; i++) > + clear_commit_marks(to_clear[i], ALL_REV_FLAGS); > + clear_commit_marks(filter->merge_commit, ALL_REV_FLAGS); > + free(to_clear); > +} Did this come from somewhere else (e.g. tag -l or branch -l)? If so, you'd need a note similar to what you added in [02/11] to the original. I also have a feeling that compared to an implementation based on paint_down_to_common(), including is_descendant_of(), this may be less precise (e.g. it would be confused with clock skew that lasts more than SLOP commits). If we are inventing a new helper (as opposed to moving an existing one), we'd probably be better off doing a thin wrapper around paint_down_to_common() than calling revision-walk machinery. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 0/7] refs backend preamble
This splits v4's patch 4/6 into two patches. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 7/7] git-stash: use git-reflog instead of creating files
This is in support of alternate ref backends which don't necessarily store reflogs as files. Signed-off-by: David Turner --- git-stash.sh | 4 ++-- refs.c | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/git-stash.sh b/git-stash.sh index 8e9e2cd..27155bc 100755 --- a/git-stash.sh +++ b/git-stash.sh @@ -184,7 +184,7 @@ store_stash () { fi # Make sure the reflog for stash is kept. - : >>"$(git rev-parse --git-path logs/$ref_stash)" + git reflog create "$ref_stash" git update-ref -m "$stash_msg" $ref_stash $w_commit ret=$? test $ret != 0 && test -z $quiet && @@ -262,7 +262,7 @@ save_stash () { say "$(gettext "No local changes to save")" exit 0 fi - test -f "$(git rev-parse --git-path logs/$ref_stash)" || + git reflog exists $ref_stash || clear_stash || die "$(gettext "Cannot initialize stash")" create_stash "$stash_msg" $untracked diff --git a/refs.c b/refs.c index 0ece8f2..813b1fc 100644 --- a/refs.c +++ b/refs.c @@ -3125,6 +3125,7 @@ static int should_autocreate_reflog(const char *refname) return starts_with(refname, "refs/heads/") || starts_with(refname, "refs/remotes/") || starts_with(refname, "refs/notes/") || + !strcmp(refname, "refs/stash") || !strcmp(refname, "HEAD"); } -- 2.0.4.315.gad8727a-twtrsrc -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 2/7] cherry-pick: treat CHERRY_PICK_HEAD and REVERT_HEAD as refs
Instead of directly writing to and reading from files in $GIT_DIR, use ref API to interact with CHERRY_PICK_HEAD and REVERT_HEAD. Signed-off-by: David Turner --- branch.c | 4 ++-- builtin/commit.c | 6 +++--- builtin/merge.c | 2 +- contrib/completion/git-prompt.sh | 4 ++-- git-gui/lib/commit.tcl | 2 +- sequencer.c | 39 --- t/t7509-commit.sh| 4 ++-- wt-status.c | 6 ++ 8 files changed, 33 insertions(+), 34 deletions(-) diff --git a/branch.c b/branch.c index b002435..ec598aa 100644 --- a/branch.c +++ b/branch.c @@ -302,8 +302,8 @@ void create_branch(const char *head, void remove_branch_state(void) { - unlink(git_path("CHERRY_PICK_HEAD")); - unlink(git_path("REVERT_HEAD")); + delete_ref("CHERRY_PICK_HEAD", NULL, REF_NODEREF); + delete_ref("REVERT_HEAD", NULL, REF_NODEREF); unlink(git_path("MERGE_HEAD")); unlink(git_path("MERGE_RR")); unlink(git_path("MERGE_MSG")); diff --git a/builtin/commit.c b/builtin/commit.c index b5b1158..53c7e90 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -168,7 +168,7 @@ static void determine_whence(struct wt_status *s) { if (file_exists(git_path("MERGE_HEAD"))) whence = FROM_MERGE; - else if (file_exists(git_path("CHERRY_PICK_HEAD"))) { + else if (ref_exists("CHERRY_PICK_HEAD")) { whence = FROM_CHERRY_PICK; if (file_exists(git_path(SEQ_DIR))) sequencer_in_use = 1; @@ -1777,8 +1777,8 @@ int cmd_commit(int argc, const char **argv, const char *prefix) } ref_transaction_free(transaction); - unlink(git_path("CHERRY_PICK_HEAD")); - unlink(git_path("REVERT_HEAD")); + delete_ref("CHERRY_PICK_HEAD", NULL, REF_NODEREF); + delete_ref("REVERT_HEAD", NULL, REF_NODEREF); unlink(git_path("MERGE_HEAD")); unlink(git_path("MERGE_MSG")); unlink(git_path("MERGE_MODE")); diff --git a/builtin/merge.c b/builtin/merge.c index 46aacd6..3e2ae2f 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -1206,7 +1206,7 @@ int cmd_merge(int argc, const char **argv, const char *prefix) else die(_("You have not concluded your merge (MERGE_HEAD exists).")); } - if (file_exists(git_path("CHERRY_PICK_HEAD"))) { + if (ref_exists("CHERRY_PICK_HEAD")) { if (advice_resolve_conflict) die(_("You have not concluded your cherry-pick (CHERRY_PICK_HEAD exists).\n" "Please, commit your changes before you merge.")); diff --git a/contrib/completion/git-prompt.sh b/contrib/completion/git-prompt.sh index 366f0bc..e2c5583 100644 --- a/contrib/completion/git-prompt.sh +++ b/contrib/completion/git-prompt.sh @@ -415,9 +415,9 @@ __git_ps1 () fi elif [ -f "$g/MERGE_HEAD" ]; then r="|MERGING" - elif [ -f "$g/CHERRY_PICK_HEAD" ]; then + elif git rev-parse --quiet --verify "CHERRY_PICK_HEAD" >/dev/null; then r="|CHERRY-PICKING" - elif [ -f "$g/REVERT_HEAD" ]; then + elif git rev-parse --quiet --verify "REVERT_HEAD" >/dev/null; then r="|REVERTING" elif [ -f "$g/BISECT_LOG" ]; then r="|BISECTING" diff --git a/git-gui/lib/commit.tcl b/git-gui/lib/commit.tcl index 864b687..2b08b13 100644 --- a/git-gui/lib/commit.tcl +++ b/git-gui/lib/commit.tcl @@ -409,7 +409,7 @@ A rescan will be automatically started now. catch {file delete [gitdir MERGE_MSG]} catch {file delete [gitdir SQUASH_MSG]} catch {file delete [gitdir GITGUI_MSG]} - catch {file delete [gitdir CHERRY_PICK_HEAD]} + catch {git update-ref -d --no-deref CHERRY_PICK_HEAD} # -- Let rerere do its thing. # diff --git a/sequencer.c b/sequencer.c index f8421a8..44c43e5 100644 --- a/sequencer.c +++ b/sequencer.c @@ -158,21 +158,22 @@ static void free_message(struct commit *commit, struct commit_message *msg) unuse_commit_buffer(commit, msg->message); } -static void write_cherry_pick_head(struct commit *commit, const char *pseudoref) +static void write_cherry_pick_head(struct commit *commit, const char *ref) { - const char *filename; - int fd; - struct strbuf buf = STRBUF_INIT; + struct strbuf err = STRBUF_INIT; + void *transaction; - strbuf_addf(&buf, "%s\n", sha1_to_hex(commit->object.sha1)); + transaction = ref_transaction_begin(&err); + if (!transaction) + die(_("Could not create transaction: %s"), err.buf); - filename = git_path("%s", pseudoref); - fd = open(filename, O_WRONLY | O_CREAT, 0666); - if (fd < 0) -
[PATCH v5 4/7] refs: Break out check for reflog autocreation
This is just for clarity. Signed-off-by: David Turner --- refs.c | 16 +++- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/refs.c b/refs.c index b34a54a..dd76721 100644 --- a/refs.c +++ b/refs.c @@ -3118,6 +3118,16 @@ static int copy_msg(char *buf, const char *msg) return cp - buf; } +static int should_autocreate_reflog(const char *refname) +{ + if (!log_all_ref_updates) + return 0; + return starts_with(refname, "refs/heads/") || + starts_with(refname, "refs/remotes/") || + starts_with(refname, "refs/notes/") || + !strcmp(refname, "HEAD"); +} + /* This function will fill in *err and return -1 on failure */ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) { @@ -3128,11 +3138,7 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf logfile = sb_logfile->buf; /* make sure the rest of the function can't change "logfile" */ sb_logfile = NULL; - if (log_all_ref_updates && - (starts_with(refname, "refs/heads/") || -starts_with(refname, "refs/remotes/") || -starts_with(refname, "refs/notes/") || -!strcmp(refname, "HEAD"))) { + if (should_autocreate_reflog(refname)) { if (safe_create_leading_directories(logfile) < 0) { strbuf_addf(err, "unable to create directory for %s. " "%s", logfile, strerror(errno)); -- 2.0.4.315.gad8727a-twtrsrc -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v5 5/7] refs: new public ref function: safe_create_reflog
The safe_create_reflog function creates a reflog, if it does not already exist. The log_ref_setup function becomes private and gains a force_create parameter to force the creation of a reflog even if log_all_ref_updates is false or the refname is not one of the special refnames. The new parameter also reduces the need to store, modify, and restore the log_all_ref_updates global before reflog creation. In a moment, we will use this to add reflog creation commands to git-reflog. Signed-off-by: David Turner --- builtin/checkout.c | 10 +- refs.c | 25 + refs.h | 2 +- 3 files changed, 23 insertions(+), 14 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 93f63d3..9f68399 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -620,19 +620,11 @@ static void update_refs_for_switch(const struct checkout_opts *opts, if (opts->new_branch) { if (opts->new_orphan_branch) { if (opts->new_branch_log && !log_all_ref_updates) { - int temp; - struct strbuf log_file = STRBUF_INIT; - int ret; const char *ref_name; struct strbuf err = STRBUF_INIT; ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch); - temp = log_all_ref_updates; - log_all_ref_updates = 1; - ret = log_ref_setup(ref_name, &log_file, &err); - log_all_ref_updates = temp; - strbuf_release(&log_file); - if (ret) { + if (safe_create_reflog(ref_name, &err, 1)) { fprintf(stderr, _("Can not do reflog for '%s'. %s\n"), opts->new_orphan_branch, err.buf); strbuf_release(&err); diff --git a/refs.c b/refs.c index dd76721..0ece8f2 100644 --- a/refs.c +++ b/refs.c @@ -3128,8 +3128,14 @@ static int should_autocreate_reflog(const char *refname) !strcmp(refname, "HEAD"); } -/* This function will fill in *err and return -1 on failure */ -int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) +/* + * This function creates a reflog for a ref. If force_create = 0, the + * reflog will only be created for certain refs (those for which + * should_autocreate_reflog returns non-zero. Otherwise, it will be + * created regardless of the ref name. This function will fill in *err + * and return -1 on failure + */ +static int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err, int force_create) { int logfd, oflags = O_APPEND | O_WRONLY; char *logfile; @@ -3138,7 +3144,7 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf logfile = sb_logfile->buf; /* make sure the rest of the function can't change "logfile" */ sb_logfile = NULL; - if (should_autocreate_reflog(refname)) { + if (force_create || should_autocreate_reflog(refname)) { if (safe_create_leading_directories(logfile) < 0) { strbuf_addf(err, "unable to create directory for %s. " "%s", logfile, strerror(errno)); @@ -3173,6 +3179,17 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf return 0; } + +int safe_create_reflog(const char *refname, struct strbuf *err, int force_create) +{ + int ret; + struct strbuf sb = STRBUF_INIT; + + ret = log_ref_setup(refname, &sb, err, force_create); + strbuf_release(&sb); + return ret; +} + static int log_ref_write_fd(int fd, const unsigned char *old_sha1, const unsigned char *new_sha1, const char *committer, const char *msg) @@ -3209,7 +3226,7 @@ static int log_ref_write_1(const char *refname, const unsigned char *old_sha1, if (log_all_ref_updates < 0) log_all_ref_updates = !is_bare_repository(); - result = log_ref_setup(refname, sb_log_file, err); + result = log_ref_setup(refname, sb_log_file, err, 0); if (result) return result; diff --git a/refs.h b/refs.h index debdefc..3b90e16 100644 --- a/refs.h +++ b/refs.h @@ -228,7 +228,7 @@ int pack_refs(unsigned int flags); /* * Setup reflog before using. Fill in err and return -1 on failure. */ -int log_ref_setup(const char *refname, struct strbuf *logfile, struct strbuf *err); +int safe_create_reflog(const char *refname, struct strbuf *err, int force_create); /** Reads log for the value of ref during at_time. **/ extern int read_ref_at(const char *ref
[PATCH v5 6/7] git-reflog: add create and exists functions
These are necessary because alternate ref backends might store reflogs somewhere other than .git/logs. Code that now directly manipulates .git/logs should instead go through git-reflog. In a moment, we will use these functions to make git stash work with alternate ref backends. Signed-off-by: David Turner --- Documentation/git-reflog.txt | 10 ++ builtin/reflog.c | 75 +++- t/t1411-reflog-show.sh | 12 +++ 3 files changed, 96 insertions(+), 1 deletion(-) diff --git a/Documentation/git-reflog.txt b/Documentation/git-reflog.txt index 5e7908e..7ab2c42 100644 --- a/Documentation/git-reflog.txt +++ b/Documentation/git-reflog.txt @@ -23,6 +23,8 @@ depending on the subcommand: [--dry-run] [--verbose] [--all | ...] 'git reflog delete' [--rewrite] [--updateref] [--dry-run] [--verbose] ref@\{specifier\}... +'git reflog create ... +'git reflog exists Reference logs, or "reflogs", record when the tips of branches and other references were updated in the local repository. Reflogs are @@ -52,6 +54,14 @@ argument must be an _exact_ entry (e.g. "`git reflog delete master@{2}`"). This subcommand is also typically not used directly by end users. +The "create" subcommand creates a reflog for one or more refs. Most +refs (those under refs/heads, refs/remotes, and refs/tags) will +automatically have reflogs created. Other refs will not. This command +allows manual ref creation. + +The "exists" subcommand checks whether a ref has a reflog. It exists +with zero status if the reflog exists, and non-zero status if it does +not. OPTIONS --- diff --git a/builtin/reflog.c b/builtin/reflog.c index c2eb8ff..1ecbfb6 100644 --- a/builtin/reflog.c +++ b/builtin/reflog.c @@ -13,6 +13,10 @@ static const char reflog_expire_usage[] = "git reflog expire [--expire=] [--expire-unreachable=] [--rewrite] [--updateref] [--stale-fix] [--dry-run | -n] [--verbose] [--all] ..."; static const char reflog_delete_usage[] = "git reflog delete [--rewrite] [--updateref] [--dry-run | -n] [--verbose] ..."; +static const char reflog_create_usage[] = +"git reflog create ..."; +static const char reflog_exists_usage[] = +"git reflog exists "; static unsigned long default_reflog_expire; static unsigned long default_reflog_expire_unreachable; @@ -699,12 +703,75 @@ static int cmd_reflog_delete(int argc, const char **argv, const char *prefix) return status; } +static int cmd_reflog_create(int argc, const char **argv, const char *prefix) +{ + int i, status = 0, start = 0; + struct strbuf err = STRBUF_INIT; + + for (i = 1; i < argc; i++) { + const char *arg = argv[i]; + if (!strcmp(arg, "--")) { + i++; + break; + } + else if (arg[0] == '-') + usage(reflog_create_usage); + else + break; + } + + start = i; + + if (argc - start < 1) + return error("Nothing to create?"); + + for (i = start; i < argc; i++) { + if (check_refname_format(argv[i], REFNAME_ALLOW_ONELEVEL)) + die("invalid ref format: %s", argv[i]); + } + for (i = start; i < argc; i++) { + if (safe_create_reflog(argv[i], &err, 1)) { + error("could not create reflog %s: %s", argv[i], + err.buf); + status = 1; + strbuf_release(&err); + } + } + return status; +} + +static int cmd_reflog_exists(int argc, const char **argv, const char *prefix) +{ + int i, status = 0, start = 0; + + for (i = 1; i < argc; i++) { + const char *arg = argv[i]; + if (!strcmp(arg, "--")) { + i++; + break; + } + else if (arg[0] == '-') + usage(reflog_exists_usage); + else + break; + } + + start = i; + + if (argc - start != 1) + usage(reflog_exists_usage); + + if (check_refname_format(argv[start], REFNAME_ALLOW_ONELEVEL)) + die("invalid ref format: %s", argv[start]); + return !reflog_exists(argv[start]); +} + /* * main "reflog" */ static const char reflog_usage[] = -"git reflog [ show | expire | delete ]"; +"git reflog [ show | expire | delete | create | exists ]"; int cmd_reflog(int argc, const char **argv, const char *prefix) { @@ -724,5 +791,11 @@ int cmd_reflog(int argc, const char **argv, const char *prefix) if (!strcmp(argv[1], "delete")) return cmd_reflog_delete(argc - 1, argv + 1, prefix); + if (!strcmp(argv[1], "create")) + return cmd_reflog_create(argc - 1, argv + 1, prefix); + + if (!strcmp(argv[1], "exists")) + return
[PATCH v5 1/7] refs.c: add err arguments to reflog functions
Add an err argument to log_ref_setup that can explain the reason for a failure. This then eliminates the need to manage errno through this function since we can just add strerror(errno) to the err string when meaningful. No callers relied on errno from this function for anything else than the error message. Also add err arguments to private functions write_ref_to_lockfile, log_ref_write_1, commit_ref_update. This again eliminates the need to manage errno in these functions. Update of a patch by Ronnie Sahlberg. Signed-off-by: Ronnie Sahlberg Signed-off-by: David Turner --- builtin/checkout.c | 8 ++-- refs.c | 111 - refs.h | 4 +- 3 files changed, 66 insertions(+), 57 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index c018ab3..93f63d3 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -624,16 +624,18 @@ static void update_refs_for_switch(const struct checkout_opts *opts, struct strbuf log_file = STRBUF_INIT; int ret; const char *ref_name; + struct strbuf err = STRBUF_INIT; ref_name = mkpath("refs/heads/%s", opts->new_orphan_branch); temp = log_all_ref_updates; log_all_ref_updates = 1; - ret = log_ref_setup(ref_name, &log_file); + ret = log_ref_setup(ref_name, &log_file, &err); log_all_ref_updates = temp; strbuf_release(&log_file); if (ret) { - fprintf(stderr, _("Can not do reflog for '%s'\n"), - opts->new_orphan_branch); + fprintf(stderr, _("Can not do reflog for '%s'. %s\n"), + opts->new_orphan_branch, err.buf); + strbuf_release(&err); return; } } diff --git a/refs.c b/refs.c index fb568d7..b34a54a 100644 --- a/refs.c +++ b/refs.c @@ -2975,9 +2975,11 @@ static int rename_ref_available(const char *oldname, const char *newname) return ret; } -static int write_ref_to_lockfile(struct ref_lock *lock, const unsigned char *sha1); +static int write_ref_to_lockfile(struct ref_lock *lock, +const unsigned char *sha1, struct strbuf* err); static int commit_ref_update(struct ref_lock *lock, -const unsigned char *sha1, const char *logmsg); +const unsigned char *sha1, const char *logmsg, +struct strbuf *err); int rename_ref(const char *oldrefname, const char *newrefname, const char *logmsg) { @@ -3038,9 +3040,10 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms } hashcpy(lock->old_oid.hash, orig_sha1); - if (write_ref_to_lockfile(lock, orig_sha1) || - commit_ref_update(lock, orig_sha1, logmsg)) { - error("unable to write current sha1 into %s", newrefname); + if (write_ref_to_lockfile(lock, orig_sha1, &err) || + commit_ref_update(lock, orig_sha1, logmsg, &err)) { + error("unable to write current sha1 into %s: %s", newrefname, err.buf); + strbuf_release(&err); goto rollback; } @@ -3056,9 +3059,11 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms flag = log_all_ref_updates; log_all_ref_updates = 0; - if (write_ref_to_lockfile(lock, orig_sha1) || - commit_ref_update(lock, orig_sha1, NULL)) - error("unable to write current sha1 into %s", oldrefname); + if (write_ref_to_lockfile(lock, orig_sha1, &err) || + commit_ref_update(lock, orig_sha1, NULL, &err)) { + error("unable to write current sha1 into %s: %s", oldrefname, err.buf); + strbuf_release(&err); + } log_all_ref_updates = flag; rollbacklog: @@ -3113,8 +3118,8 @@ static int copy_msg(char *buf, const char *msg) return cp - buf; } -/* This function must set a meaningful errno on failure */ -int log_ref_setup(const char *refname, struct strbuf *sb_logfile) +/* This function will fill in *err and return -1 on failure */ +int log_ref_setup(const char *refname, struct strbuf *sb_logfile, struct strbuf *err) { int logfd, oflags = O_APPEND | O_WRONLY; char *logfile; @@ -3129,9 +3134,8 @@ int log_ref_setup(const char *refname, struct strbuf *sb_logfile) starts_with(refname, "refs/notes/") || !strcmp(refname, "HEAD
[PATCH v5 3/7] bisect: treat BISECT_HEAD as a ref
Instead of directly writing to and reading from files in $GIT_DIR, use ref API to interact with BISECT_HEAD. Signed-off-by: David Turner --- git-bisect.sh | 10 +++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/git-bisect.sh b/git-bisect.sh index ae3fec2..dddcc89 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -35,7 +35,7 @@ _x40="$_x40$_x40$_x40$_x40$_x40$_x40$_x40$_x40" bisect_head() { - if test -f "$GIT_DIR/BISECT_HEAD" + if bisect_head_exists then echo BISECT_HEAD else @@ -209,6 +209,10 @@ check_expected_revs() { done } +bisect_head_exists() { +git rev-parse --quiet --verify "BISECT_HEAD" >/dev/null +} + bisect_skip() { all='' for arg in "$@" @@ -310,7 +314,7 @@ bisect_next() { bisect_next_check good # Perform all bisection computation, display and checkout - git bisect--helper --next-all $(test -f "$GIT_DIR/BISECT_HEAD" && echo --no-checkout) + git bisect--helper --next-all $(bisect_head_exists && echo --no-checkout) res=$? # Check if we should exit because bisection is finished @@ -377,7 +381,7 @@ bisect_reset() { usage ;; esac - if ! test -f "$GIT_DIR/BISECT_HEAD" && ! git checkout "$branch" -- + if ! bisect_head_exists && ! git checkout "$branch" -- then die "$(eval_gettext "Could not check out original HEAD '\$branch'. Try 'git bisect reset '.")" -- 2.0.4.315.gad8727a-twtrsrc -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 04/11] for-each-ref: add '--points-at' option
Karthik Nayak writes: > Add the '--points-at' option provided by 'ref-filter'. The option > lets the user to pick only refs which point to a particular > commit. It somehow feels strange that the option name is points-at and all the explanation (like the above and also in the doc) talks about pointing to an object. Not limited to this patch but the previous one had the same, I think. > diff --git a/t/t6302-for-each-ref-filter.sh b/t/t6302-for-each-ref-filter.sh > index b1fa8d4..7269a66 100644 > --- a/t/t6302-for-each-ref-filter.sh > +++ b/t/t6302-for-each-ref-filter.sh > @@ -16,4 +16,24 @@ test_expect_success 'setup some history and refs' ' > git update-ref refs/odd/spot master > ' > > +test_expect_success 'filtering with --points-at' ' > + cat >expect <<-\EOF && > + refs/heads/master > + refs/odd/spot > + refs/tags/three > + EOF > + git for-each-ref --format="%(refname)" --points-at=master >actual && > + test_cmp expect actual > +' > + > +test_expect_success 'check signed tags with --points-at' ' > + cat >expect <<-\EOF && > + refs/heads/side > + refs/tags/four > + refs/tags/signed-tag four > + EOF > + git for-each-ref --format="%(refname) %(*subject)" --points-at=side > >actual && > + test_cmp expect actual > +' This shows that we would want to add a "annotated doubly" tag in the preparation step 01/11; the expected outcome is that it will not show in the output, I think. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v6 03/11] ref-filter: implement '--points-at' option
Karthik Nayak writes: > +/* > + * Given a ref (sha1, refname) see if it points to one of the sha1s > + * in a sha1_array. > + */ > +static int match_points_at(struct sha1_array *points_at, const unsigned char > *sha1, > +const char *refname) > +{ > + struct object *obj; > + > + if (!points_at || !points_at->nr) > + return 1; > + > + if (sha1_array_lookup(points_at, sha1) >= 0) > + return 1; > + > + obj = parse_object_or_die(sha1, refname); > + if (obj->type == OBJ_TAG && > + sha1_array_lookup(points_at, ((struct tag *)obj)->tagged->sha1) >= > 0) > + return 1; > + > + return 0; > +} > + Interesting. I think the change done while copying the code does not change anything from the original (other than that the helper lost its ability to return the peeled object name), and I think you shouldn't make any change while copying the code that would change the benaviour, but I notice a few things that we might want to keep in mind and revisit them later (i.e. might be a good idea to add NEEDSWORK comment to record them near the function): - The original only peeled one level of indirection, so does this implementation. But is that really what we want, I wonder? After doing: $ git tag -a -m 'annotated' atag $commit $ git tag -a -m 'annotated doubly' dtag atag atag^0, dtag^0 and $commit all refer to the same commit object. Do we want to miss dtag with --point-at=$commit? - As we are in for-each-ref (or eventually tag -l) that is walking the cached refs, we may know what refname peels to without parsing the object at all. Could it be more efficient to ask peel_ref() for the pointee without doing parse_object() ourselves? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 01/44] wrapper: implement xopen()
Torsten Bögershausen writes: >> +int xopen(const char *path, int oflag, ...) >> +{ >> +mode_t mode = 0; >> +va_list ap; >> + >> +va_start(ap, oflag); >> +if (oflag & O_CREAT) >> +mode = va_arg(ap, mode_t); >> +va_end(ap); >> + >> +assert(path); >> + > 2 remarks: > - I don't know if and why we need the assert() here (but don't know if > we have a strategie in Git for assert()) There is no bright-line rules, but I think it is sensible to remove this. Nobody sane would throw a NULL at open(2) and xopen() is supposed to imitate that interface. We do protect ourselves from careless use of our own API, but no need to clutter the code with overly zealous check against insane code, I would say. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] --count feature for git shortlog
Lawrence Siebert writes: > I was using it to sort files > by commit count when provided a list of files, which git rev-list > doesn't really work for. What makes you say rev-list does not work (perhaps 'really' is the key word there?) git rev-list --no-merges maint..master -- Makefile git shortlog --no-merges maint..master -- Makefile git log --oneline --no-merges maint..master -- Makefile all gives me results that are consistent among them. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] http: always use any proxy auth method available
Enrique Tobis writes: > You did guess correctly. As you said you are not an expert in this > area, should I wait until someone else chimes in or is this enough to > resubmit for inclusion? Assuming my revised explanation is acceptable, > of course. I'll queue the patch as-is with your updated log message to explain it. Maybe somebody else has more comment on it, but I think the change makes sense. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] --count feature for git shortlog
Junio, I appreciate your help. Okay, That all makes sense. I would note that something like: git shortlog -s "$FILENAME: | cut -f 1 | paste -sd+ - | bc seems like it run much faster then: git log --oneline "$FILENAME" | wc -l Which was why I was looking at shortlog. I was using it to sort files by commit count when provided a list of files, which git rev-list doesn't really work for. Anyway I can try and put it in log proper, if you think that's the best place. Thank you, Lawrence Siebert -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v11 00/10] bisect terms
Matthieu Moy writes: > So, here's a reroll that tries to address the ongoing discussion. > > The first patches are preparatory steps, which are IMHO good > regardless of the features. I kept the user-interface to chose terms > at the end, and tried to keep the UI patches as small as possible. > > I have the feeling that "bisect: add the terms old/new" should be > dropped, but I have no strong opinion on that. If you like the > feature, say so. If you think the feature doesn't bring enough, and > should eventually be obsoleted by "guess which commit is old and which > is new", say so too. I personally do not mind having old/new, as long as it does not make it harder to build on top of the codebase (and existing user experience) to eventually transition to "you can always say good or bad and we'll figure out which of old/new they map to". Obviously we will never guess which is old/new when the user uses old/new as the chosen terms, and having to special case that might complicat things, which is where that "as long as it does not make it harder" above comes from. > The beginning of the series didn't change much since v10. The major > change is that I gave up using "git bisect terms ", and > implemented the same feature in "git bisect start". We're losing the > ability to run "git bisect terms" several times to change the terms > before we use them, but I'm not sure this was a useful feature. OTOH, > we're back to the principle "git bisect start" starts from a fresh > state, this avoids confusing the situation where the user has leftover > from yesterday's "git bisect terms". And the code is much, much > simpler. I like that direction. But let's first wait to see what others say. Thanks. > > Antoine Delaite (4): > bisect: correction of typo > bisect: replace hardcoded "bad|good" by variables > bisect: simplify the addition of new bisect terms > bisect: add the terms old/new > > Matthieu Moy (5): > Documentation/bisect: move getting help section to the end > bisect: don't mix option parsing and non-trivial code > bisect: sanity check on terms > bisect: add 'git bisect terms' to view the current terms > bisect: allow setting any user-specified in 'git bisect start' > > Michael Haggerty (1): > Documentation/bisect: revise overall content > > Documentation/git-bisect.txt | 236 --- > bisect.c | 94 > git-bisect.sh| 255 > +++ > revision.c | 19 +++- > t/t6030-bisect-porcelain.sh | 137 ++- > 5 files changed, 612 insertions(+), 129 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10.1 7/7] bisect: allow any terms set by user
Matthieu Moy writes: > So, my proposal would be to remove the "old/new" patch from the series, > and keep the other patches. > > What do you think? Let me answer after reading v11 through. >> but now it would be more clear that $name_good and $name_bad is a bad >> way to name internal variables and files in $GIT_DIR. The inferred 'ah >> you are hunting for regression' mode would call old ones 'bad' and new >> ones 'good', they have to be given value neutral names, e.g. $name_old >> and $name_new. > > Ideally, the whole code would be ported to use old/new, but the more I > read the code the more I agree with Christian actually: we still have > many more instances of good/bad in the code (e.g. functions > check_good_are_ancestors_of_bad, for_each_good_bisect_ref, ...), so > having just name_new/name_old would make the code very inconsistant. Oh, no question about that. I was hoping that we would at least get the concensus that we should move all to old/new and these good/bad in code no longer make sense. It just was that introducing new variables and functions whose names follow the convention that reflects the world view that is no longer valid (i.e. good is always old and bad is always new) in a series that introduces the new world view somehow felt wrong. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10.1 7/7] bisect: allow any terms set by user
Matthieu Moy writes: > Junio C Hamano writes: > >> I moderately hate to see both from aesthetics point of view, but can >> we at least lose "--name-" prefix? > > I changed it to --term- prefix, but I'd rather not drop it. When reading > "--old=foo", it is not clear to me whether the meaning should be "the > term used for old is foo" or "mark foo as old". The longer version does > not have this problem. Yeah, my suggestion was based on one assumption I did not mention, which is that we do not need to crowd "bisect start" with this option when we have "bisect terms", as long as "bisect start" does not have to take both "what are the terms" and "which commits are painted using which one of the two terms" on its command line, the "--name-" prefix was unnecessary. But if you are dropping "bisect terms" and allowing the terms specified only from "bisect start" as you mentioned in the cover letter of v11, that changes the equation. And I think "start is the place that sets up a clean slate, and that is the only place where you can optionally declare your custom terms" is a very sensible design. I do not have a problem with "--terms" prefix in that case. Thanks. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] 4th release candidate of Git for Windows 2.x, was Re: 3rd release candidate of Git for Windows 2.x
Hi Konstantin, On 2015-06-29 17:54, Konstantin Khomoutov wrote: > I've finally took time to switch from my old "msys1" release to this > RC4, and immediately got hit by the fact Git is now speaking to me in > Russian, which is not what I want (previously this behaviour was only > exhibited by `git gui` and `gitk`). > > Should I make Git see LC_MESSAGES=en (or other thing like LANG) in the > environment or is there some Git-local method to affect this behaviour? > I tried to grep the release notes using relevant keywords but was left > empty-handed. Personally, I would use LC_ALL=C. Maybe that's good for you, too? I guess this would also make for a fine opportunity to add an option to the installer to switch the localization off? Ciao, Johannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] worktree: new place for "git prune --worktrees"
Nguyễn Thái Ngọc Duy writes: > Commit 23af91d (prune: strategies for linked checkouts - 2014-11-30) > adds "--worktrees" to "git prune" without realizing that "git prune" is > for object database only. This patch moves the same functionality to a > new command "git worktree". > > Signed-off-by: Nguyễn Thái Ngọc Duy > --- > In future I probably move the big block of text in git-checkout.txt to > git-worktree.txt and add "git worktree list". But let's start with > something small and simple before "git prune --worktrees" is shipped > out. Thanks. I notice that after applying this, builtin/prune.c does not revert to the original before 23af91d. It stops including "dir.h" and it adds an extra blank line. We've been including a header that is not necessary even in v2.4.0, it seems. We used to walk $GIT_DIR/objects ourselves to find loose object files and "dir.h" was needed for is_dot_or_dotdot(); for_each_loose_file_in_objdir() is what we use these days to hide the implementation details these days; 27e1e22 forgot to remove the inclusion when it did this. The C code part is mostly just \C-x \C-v and I found nothing questionable. > diff --git a/command-list.txt b/command-list.txt > index b17c011..2a94137 100644 > --- a/command-list.txt > +++ b/command-list.txt > @@ -148,4 +148,5 @@ git-verify-pack > plumbinginterrogators > git-verify-tag ancillaryinterrogators > gitweb ancillaryinterrogators > git-whatchanged ancillaryinterrogators > +git-worktreemainporcelain I doubt that a helper that is primarily spawned from "gc" as its implementation detail is more mainporcelain than "git config" is: git-config ancillarymanipulators I also wonder if "git worktree" command should have a mode that works in a way similar to how new-workdir (in contrib/workdir) does, instead of an option "checkout --to" that looks just out of place as "worktree prune" was out of place in "prune". The feature is doing a lot more than what "checkout" normally does (somewhere in between "checkout" and "clone", I would say), and it may be cleaner to use an independent command "git worktree" to manage a separate worktree. And when that happens, the command should definitely be classified as a mainporcelain. > diff --git a/git.c b/git.c > index 44374b1..fa77bc9 100644 > --- a/git.c > +++ b/git.c > @@ -483,6 +483,7 @@ static struct cmd_struct commands[] = { > { "verify-tag", cmd_verify_tag, RUN_SETUP }, > { "version", cmd_version }, > { "whatchanged", cmd_whatchanged, RUN_SETUP }, > + { "worktree", cmd_worktree, RUN_SETUP }, We do not NEED_WORK_TREE because we can create a new worktree off of a bare repository? -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] 4th release candidate of Git for Windows 2.x, was Re: 3rd release candidate of Git for Windows 2.x
On Mon, 29 Jun 2015 16:37:54 +0200 Johannes Schindelin wrote: > >> I just uploaded the 4th release candidate for the upcoming Git for > >> Windows 2.x release. Please find the download link here: > >> > >> https://git-for-windows.github.io/#download > >> > >> The most important changes are the update to Git 2.4.5 and a fix > >> for the crash when running Git Bash with a legacy `TERM` setting > >> (this should help 3rd party software to upgrade to Git for Windows > >> 2.x). > > > > Thanks. > > It seems that this link: > > > >https://github.com/git-for-windows/git/releases/latest > > > > doesn't point to the latest release. > > > > Might be because the tags have the same date ? > > Wooops. Sorry for being so slow (been interviewing today). It should > be correct now, can you verify, please? Thanks for making all this real, Johannes! I've finally took time to switch from my old "msys1" release to this RC4, and immediately got hit by the fact Git is now speaking to me in Russian, which is not what I want (previously this behaviour was only exhibited by `git gui` and `gitk`). Should I make Git see LC_MESSAGES=en (or other thing like LANG) in the environment or is there some Git-local method to affect this behaviour? I tried to grep the release notes using relevant keywords but was left empty-handed. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v11 03/10] Documentation/bisect: revise overall content
From: Michael Haggerty Thoroughly revise the "git bisect" manpage, including: * Beef up the "Description" section. * Make the first long example less specific to kernel development. * De-emphasize implementation details in a couple of places. * Add "(roughly N steps)" in the places where example output is shown. * Properly markup code within the prose. * Lots of wordsmithing. Signed-off-by: Michael Haggerty Signed-off-by: Matthieu Moy --- Documentation/git-bisect.txt | 122 --- 1 file changed, 68 insertions(+), 54 deletions(-) diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt index 2bdc3b8..e97f2de 100644 --- a/Documentation/git-bisect.txt +++ b/Documentation/git-bisect.txt @@ -3,7 +3,7 @@ git-bisect(1) NAME -git-bisect - Find by binary search the change that introduced a bug +git-bisect - Use binary search to find the commit that introduced a bug SYNOPSIS @@ -16,7 +16,6 @@ DESCRIPTION The command takes various subcommands, and different options depending on the subcommand: - git bisect help git bisect start [--no-checkout] [ [...]] [--] [...] git bisect bad [] git bisect good [...] @@ -26,58 +25,71 @@ on the subcommand: git bisect replay git bisect log git bisect run ... + git bisect help -This command uses 'git rev-list --bisect' to help drive the -binary search process to find which change introduced a bug, given an -old "good" commit object name and a later "bad" commit object name. +This command uses a binary search algorithm to find which commit in +your project's history introduced a bug. You use it by first telling +it a "bad" commit that is known to contain the bug, and a "good" +commit that is known to be before the bug was introduced. Then `git +bisect` picks a commit between those two endpoints and asks you +whether the selected commit is "good" or "bad". It continues narrowing +down the range until it finds the exact commit that introduced the +change. Basic bisect commands: start, bad, good ~~~ -Using the Linux kernel tree as an example, basic use of the bisect -command is as follows: +As an example, suppose you are trying to find the commit that broke a +feature that was known to work in version `v2.6.13-rc2` of your +project. You start a bisect session as follows: $ git bisect start $ git bisect bad # Current version is bad -$ git bisect good v2.6.13-rc2# v2.6.13-rc2 was the last version - # tested that was good +$ git bisect good v2.6.13-rc2# v2.6.13-rc2 is known to be good + + +Once you have specified at least one bad and one good commit, `git +bisect` selects a commit in the middle of that range of history, +checks it out, and outputs something similar to the following: + + +Bisecting: 675 revisions left to test after this (roughly 10 steps) -When you have specified at least one bad and one good version, the -command bisects the revision tree and outputs something similar to -the following: +You should now compile the checked-out version and test it. If that +version works correctly, type -Bisecting: 675 revisions left to test after this +$ git bisect good -The state in the middle of the set of revisions is then checked out. -You would now compile that kernel and boot it. If the booted kernel -works correctly, you would then issue the following command: +If that version is broken, type -$ git bisect good # this one is good +$ git bisect bad -The output of this command would be something similar to the following: +Then `git bisect` will respond with something like -Bisecting: 337 revisions left to test after this +Bisecting: 337 revisions left to test after this (roughly 9 steps) -You keep repeating this process, compiling the tree, testing it, and -depending on whether it is good or bad issuing the command "git bisect good" -or "git bisect bad" to ask for the next bisection. +Keep repeating the process: compile the tree, test it, and depending +on whether it is good or bad run `git bisect good` or `git bisect bad` +to ask for the next commit that needs testing. + +Eventually there will be no more revisions left to inspect, and the +command will print out a description of the first bad commit. The +reference `refs/bisect/bad` will be left pointing at that commit. -Eventually there will be no more revisions left to bisect, and you -will have been left with the
[PATCH v11 00/10] bisect terms
Hi, So, here's a reroll that tries to address the ongoing discussion. The first patches are preparatory steps, which are IMHO good regardless of the features. I kept the user-interface to chose terms at the end, and tried to keep the UI patches as small as possible. I have the feeling that "bisect: add the terms old/new" should be dropped, but I have no strong opinion on that. If you like the feature, say so. If you think the feature doesn't bring enough, and should eventually be obsoleted by "guess which commit is old and which is new", say so too. The beginning of the series didn't change much since v10. The major change is that I gave up using "git bisect terms ", and implemented the same feature in "git bisect start". We're losing the ability to run "git bisect terms" several times to change the terms before we use them, but I'm not sure this was a useful feature. OTOH, we're back to the principle "git bisect start" starts from a fresh state, this avoids confusing the situation where the user has leftover from yesterday's "git bisect terms". And the code is much, much simpler. Antoine Delaite (4): bisect: correction of typo bisect: replace hardcoded "bad|good" by variables bisect: simplify the addition of new bisect terms bisect: add the terms old/new Matthieu Moy (5): Documentation/bisect: move getting help section to the end bisect: don't mix option parsing and non-trivial code bisect: sanity check on terms bisect: add 'git bisect terms' to view the current terms bisect: allow setting any user-specified in 'git bisect start' Michael Haggerty (1): Documentation/bisect: revise overall content Documentation/git-bisect.txt | 236 --- bisect.c | 94 git-bisect.sh| 255 +++ revision.c | 19 +++- t/t6030-bisect-porcelain.sh | 137 ++- 5 files changed, 612 insertions(+), 129 deletions(-) -- 2.5.0.rc0.10.gd2bff5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v11 07/10] bisect: sanity check on terms
This is currently only a defensive check since the only terms are bad/good and new/old, which pass it, but this is a preparation step for accepting user-supplied terms. Signed-off-by: Matthieu Moy --- git-bisect.sh | 33 + 1 file changed, 33 insertions(+) diff --git a/git-bisect.sh b/git-bisect.sh index f32fd2d..0b3c820 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -535,9 +535,42 @@ get_terms () { write_terms () { TERM_BAD=$1 TERM_GOOD=$2 + if test "$TERM_BAD" = "$TERM_GOOD" + then + die "$(gettext "please use two different terms")" + fi + check_term_format "$TERM_BAD" bad + check_term_format "$TERM_GOOD" good printf '%s\n%s\n' "$TERM_BAD" "$TERM_GOOD" >"$GIT_DIR/BISECT_TERMS" } +check_term_format () { + term=$1 + git check-ref-format refs/bisect/"$term" || + die "$(eval_gettext "'\$term' is not a valid term")" + case "$term" in + help|start|terms|skip|next|reset|visualize|replay|log|run) + die "$(eval_gettext "can't use the builtin command '\$term' as a term")" + ;; + bad|new) + if test "$2" != bad + then + # In theory, nothing prevents swapping + # completely good and bad, but this situation + # could be confusing and hasn't been tested + # enough. Forbid it for now. + die "$(eval_gettext "can't change the meaning of term '\$term'")" + fi + ;; + good|old) + if test "$2" != good + then + die "$(eval_gettext "can't change the meaning of term '\$term'")" + fi + ;; + esac +} + check_and_set_terms () { cmd="$1" case "$cmd" in -- 2.5.0.rc0.10.gd2bff5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v11 06/10] bisect: don't mix option parsing and non-trivial code
As-is, the revisions that appear on the command-line are processed in order. This would mix badly with code that changes the configuration (e.g. change $TERM_GOOD and $TERM_BAD) while processing the options. Signed-off-by: Matthieu Moy --- git-bisect.sh | 30 +- 1 file changed, 17 insertions(+), 13 deletions(-) diff --git a/git-bisect.sh b/git-bisect.sh index dcd7e59..f32fd2d 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -78,6 +78,7 @@ bisect_start() { bad_seen=0 eval='' must_write_terms=0 + revs='' if test "z$(git rev-parse --is-bare-repository)" != zfalse then mode=--no-checkout @@ -102,25 +103,27 @@ bisect_start() { die "$(eval_gettext "'\$arg' does not appear to be a valid revision")" break } - - # The user ran "git bisect start - # ", hence did not explicitly specify - # the terms, but we are already starting to - # set references named with the default terms, - # and won't be able to change afterwards. - must_write_terms=1 - - case $bad_seen in - 0) state=$TERM_BAD ; bad_seen=1 ;; - *) state=$TERM_GOOD ;; - esac - eval="$eval bisect_write '$state' '$rev' 'nolog' &&" + revs="$revs $rev" shift ;; esac done + for rev in $revs + do + # The user ran "git bisect start + # ", hence did not explicitly specify + # the terms, but we are already starting to + # set references named with the default terms, + # and won't be able to change afterwards. + must_write_terms=1 + + case $bad_seen in + 0) state=$TERM_BAD ; bad_seen=1 ;; + *) state=$TERM_GOOD ;; + esac + eval="$eval bisect_write '$state' '$rev' 'nolog' &&" + done # # Verify HEAD. # -- 2.5.0.rc0.10.gd2bff5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v11 09/10] bisect: add 'git bisect terms' to view the current terms
Signed-off-by: Matthieu Moy --- Documentation/git-bisect.txt | 10 ++ git-bisect.sh| 39 ++- t/t6030-bisect-porcelain.sh | 20 3 files changed, 68 insertions(+), 1 deletion(-) diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt index abaf462..4dd6295 100644 --- a/Documentation/git-bisect.txt +++ b/Documentation/git-bisect.txt @@ -19,6 +19,7 @@ on the subcommand: git bisect start [--no-checkout] [ [...]] [--] [...] git bisect (bad|new) [] git bisect (good|old) [...] + git bisect terms [--term-good | --term-bad] git bisect skip [(|)...] git bisect reset [] git bisect visualize @@ -157,6 +158,15 @@ git bisect new [...] to indicate that it was after. +To get a reminder of the currently used terms, use + + +git bisect terms + + +You can get just the old (respectively new) term with `git bisect term +--term-old` or `git bisect term --term-good`. + Bisect visualize diff --git a/git-bisect.sh b/git-bisect.sh index 42e1cee..da86d9e 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -1,6 +1,6 @@ #!/bin/sh -USAGE='[help|start|bad|good|new|old|skip|next|reset|visualize|replay|log|run]' +USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]' LONG_USAGE='git bisect help print this long help message. git bisect start [--no-checkout] [ [...]] [--] [...] @@ -11,6 +11,8 @@ git bisect (bad|new) [] git bisect (good|old) [...] mark ... known-good revisions/ revisions before change in a given property. +git bisect terms [--term-good | --term-bad] + show the terms used for old and new commits (default: bad, good) git bisect skip [(|)...] mark ... untestable revisions. git bisect next @@ -453,6 +455,8 @@ bisect_replay () { eval "$cmd" ;; "$TERM_GOOD"|"$TERM_BAD"|skip) bisect_write "$command" "$rev" ;; + terms) + bisect_terms $rev ;; *) die "$(gettext "?? what are you talking about?")" ;; esac @@ -606,6 +610,37 @@ bisect_voc () { esac } +bisect_terms () { + get_terms + if ! test -s "$GIT_DIR/BISECT_TERMS" + then + die "$(gettext "no terms defined")" + fi + case "$#" in + 0) + gettextln "Your current terms are $TERM_GOOD for the old state +and $TERM_BAD for the new state." + ;; + 1) + arg=$1 + case "$arg" in + --term-good|--term-old) + printf '%s\n' "$TERM_GOOD" + ;; + --term-bad|--term-new) + printf '%s\n' "$TERM_BAD" + ;; + *) + die "$(eval_gettext "invalid argument \$arg for 'git bisect terms'. +Supported options are: --term-good|--term-old and --term-bad|--term-new.")" + ;; + esac + ;; + *) + usage ;; + esac +} + case "$#" in 0) usage ;; @@ -635,6 +670,8 @@ case "$#" in bisect_log ;; run) bisect_run "$@" ;; + terms) + bisect_terms "$@" ;; *) usage ;; esac diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 983c503..9393488 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -797,4 +797,24 @@ test_expect_success 'bisect cannot mix old/new and good/bad' ' test_must_fail git bisect old $HASH1 ' +test_expect_success 'bisect terms needs 0 or 1 argument' ' + git bisect reset && + test_must_fail git bisect terms only-one && + test_must_fail git bisect terms 1 2 && + test_must_fail git bisect terms 2>actual && + echo "no terms defined" >expected && + test_cmp expected actual +' + +test_expect_success 'bisect terms shows good/bad after start' ' + git bisect reset && + git bisect start HEAD $HASH1 && + git bisect terms --term-good >actual && + echo good >expected && + test_cmp expected actual && + git bisect terms --term-bad >actual && + echo bad >expected && + test_cmp expected actual +' + test_done -- 2.5.0.rc0.10.gd2bff5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v11 08/10] bisect: add the terms old/new
From: Antoine Delaite When not looking for a regression during a bisect but for a fix or a change in another given property, it can be confusing to use 'good' and 'bad'. This patch introduce `git bisect new` and `git bisect old` as an alternative to 'bad' and good': the commits which have a certain property must be marked as `new` and the ones which do not as `old`. The output will be the first commit after the change in the property. During a new/old bisect session you cannot use bad/good commands and vice-versa. Some commands are still not available for old/new: * git rev-list --bisect does not treat the revs/bisect/new and revs/bisect/old-SHA1 files. Old discussions: - http://thread.gmane.org/gmane.comp.version-control.git/86063 introduced bisect fix unfixed to find fix. - http://thread.gmane.org/gmane.comp.version-control.git/182398 discussion around bisect yes/no or old/new. - http://thread.gmane.org/gmane.comp.version-control.git/199758 last discussion and reviews New discussions: - http://thread.gmane.org/gmane.comp.version-control.git/271320 ( v2 1/7-4/7 ) - http://comments.gmane.org/gmane.comp.version-control.git/271343 ( v2 5/7-7/7 ) Signed-off-by: Antoine Delaite Signed-off-by: Louis Stuber Signed-off-by: Valentin Duperray Signed-off-by: Franck Jonas Signed-off-by: Lucien Kong Signed-off-by: Thomas Nguy Signed-off-by: Huynh Khoi Nguyen Nguyen Signed-off-by: Matthieu Moy --- Documentation/git-bisect.txt | 58 ++-- bisect.c | 11 ++--- git-bisect.sh| 26 +--- t/t6030-bisect-porcelain.sh | 38 + 4 files changed, 119 insertions(+), 14 deletions(-) diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt index e97f2de..abaf462 100644 --- a/Documentation/git-bisect.txt +++ b/Documentation/git-bisect.txt @@ -17,8 +17,8 @@ The command takes various subcommands, and different options depending on the subcommand: git bisect start [--no-checkout] [ [...]] [--] [...] - git bisect bad [] - git bisect good [...] + git bisect (bad|new) [] + git bisect (good|old) [...] git bisect skip [(|)...] git bisect reset [] git bisect visualize @@ -36,6 +36,13 @@ whether the selected commit is "good" or "bad". It continues narrowing down the range until it finds the exact commit that introduced the change. +In fact, `git bisect` can be used to find the commit that changed +*any* property of your project; e.g., the commit that fixed a bug, or +the commit that caused a benchmark's performance to improve. To +support this more general usage, the terms "old" and "new" can be used +in place of "good" and "bad". See +section "Alternate terms" below for more information. + Basic bisect commands: start, bad, good ~~~ @@ -111,6 +118,45 @@ bad revision, while `git bisect reset HEAD` will leave you on the current bisection commit and avoid switching commits at all. +Alternate terms +~~~ + +Sometimes you are not looking for the commit that introduced a +breakage, but rather for a commit that caused a change between some +other "old" state and "new" state. For example, you might be looking +for the commit that introduced a particular fix. Or you might be +looking for the first commit in which the source-code filenames were +finally all converted to your company's naming standard. Or whatever. + +In such cases it can be very confusing to use the terms "good" and +"bad" to refer to "the state before the change" and "the state after +the change". So instead, you can use the terms "old" and "new", +respectively, in place of "good" and "bad". (But note that you cannot +mix "good" and "bad" with "old" and "new" in a single session.) + +In this more general usage, you provide `git bisect` with a "new" +commit has some property and an "old" commit that doesn't have that +property. Each time `git bisect` checks out a commit, you test if that +commit has the property. If it does, mark the commit as "new"; +otherwise, mark it as "old". When the bisection is done, `git bisect` +will report which commit introduced the property. + +To use "old" and "new" instead of "good" and bad, you must run `git +bisect start` without commits as argument and then run the following +commands to add the commits: + + +git bisect old [] + + +to indicate that a commit was before the sought change, or + + +git bisect new [...] + + +to indicate that it was after. + Bisect visualize @@ -387,6 +433,14 @@ In this case, when 'git bisect run' finishes, bisect/bad will refer to a commit has at least one p
[PATCH v11 10/10] bisect: allow setting any user-specified in 'git bisect start'
This allows a natural user-interface when looking for any change in the code, not just regression. For example: git bisect start --term-old fast --term-new slow git bisect fast git bisect slow ... There were several proposed user-interfaces for this feature. This patch implements it as options to 'git bisect start' for the following reasons: * By construction, the terms will be valid for one and only one bisection. * Unlike positional arguments, using named options avoid having to remember an order. * We can combine user-defined terms and passing old/new commits as argument to "git bisect start". * The implementation is relatively simple. See previous discussions: http://mid.gmane.org/1435337896-20709-3-git-send-email-matthieu@imag.fr Signed-off-by: Matthieu Moy --- Documentation/git-bisect.txt | 37 +++-- git-bisect.sh| 21 +++- t/t6030-bisect-porcelain.sh | 77 3 files changed, 132 insertions(+), 3 deletions(-) diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt index 4dd6295..340f3c1 100644 --- a/Documentation/git-bisect.txt +++ b/Documentation/git-bisect.txt @@ -16,7 +16,8 @@ DESCRIPTION The command takes various subcommands, and different options depending on the subcommand: - git bisect start [--no-checkout] [ [...]] [--] [...] + git bisect start [--term-{old,good}= --term-{new,bad}=] + [--no-checkout] [ [...]] [--] [...] git bisect (bad|new) [] git bisect (good|old) [...] git bisect terms [--term-good | --term-bad] @@ -41,7 +42,7 @@ In fact, `git bisect` can be used to find the commit that changed *any* property of your project; e.g., the commit that fixed a bug, or the commit that caused a benchmark's performance to improve. To support this more general usage, the terms "old" and "new" can be used -in place of "good" and "bad". See +in place of "good" and "bad", or you can choose your own terms. See section "Alternate terms" below for more information. Basic bisect commands: start, bad, good @@ -167,6 +168,31 @@ git bisect terms You can get just the old (respectively new) term with `git bisect term --term-old` or `git bisect term --term-good`. +If you would like to use your own terms instead of "bad"/"good" or +"new"/"old", you can choose any names you like (except existing bisect +subcommands like `reset`, `start`, ...) by starting the +bisection using + + +git bisect start --term-old --term-new + + +For example, if you are looking for a commit that introduced a +performance regression, you might use + + +git bisect start --term-old fast --term-new slow + + +Or if you are looking for the commit that fixed a bug, you might use + + +git bisect start --term-new fixed --term-old broken + + +Then, use `git bisect ` and `git bisect ` instead +of `git bisect good` and `git bisect bad` to mark commits. + Bisect visualize @@ -450,6 +476,13 @@ $ git bisect start $ git bisect new HEAD# current commit is marked as new $ git bisect old HEAD~10 # the tenth commit from now is marked as old ++ +or: + +$ git bisect start --term-old broken --term-new fixed +$ git bisect fixed +$ git bisect broken HEAD~10 + Getting help diff --git a/git-bisect.sh b/git-bisect.sh index da86d9e..718902b 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -3,7 +3,8 @@ USAGE='[help|start|bad|good|new|old|terms|skip|next|reset|visualize|replay|log|run]' LONG_USAGE='git bisect help print this long help message. -git bisect start [--no-checkout] [ [...]] [--] [...] +git bisect start [--term-{old,good}= --term-{new,bad}=] + [--no-checkout] [ [...]] [--] [...] reset bisect state and start bisection. git bisect (bad|new) [] mark a known-bad revision/ @@ -99,6 +100,24 @@ bisect_start() { --no-checkout) mode=--no-checkout shift ;; + --term-good|--term-old) + shift + must_write_terms=1 + TERM_GOOD=$1 + shift ;; + --term-good=*|--term-old=*) + must_write_terms=1 + TERM_GOOD=${1#*=} + shift ;; + --term-bad|--term-new) + shift + must_write_terms=1 + TERM_BAD=$1 + shift ;; + --term-bad=*|--term-new=*) + must_write_terms=1 + TERM_BAD=${1#*=} + shift ;;
[PATCH v11 05/10] bisect: simplify the addition of new bisect terms
From: Antoine Delaite We create a file BISECT_TERMS in the repository .git to be read during a bisection. There's no user-interface yet, but "git bisect" works if terms other than old/new or bad/good are set in .git/BISECT_TERMS. The fonctions to be changed if we add new terms are quite few. In git-bisect.sh: check_and_set_terms bisect_voc Co-authored-by: Louis Stuber Tweaked-by: Matthieu Moy Signed-off-by: Antoine Delaite Signed-off-by: Louis Stuber Signed-off-by: Valentin Duperray Signed-off-by: Franck Jonas Signed-off-by: Lucien Kong Signed-off-by: Thomas Nguy Signed-off-by: Huynh Khoi Nguyen Nguyen Signed-off-by: Matthieu Moy --- bisect.c | 33 +++-- git-bisect.sh | 79 +-- revision.c| 19 -- 3 files changed, 119 insertions(+), 12 deletions(-) diff --git a/bisect.c b/bisect.c index a96e485..857cf59 100644 --- a/bisect.c +++ b/bisect.c @@ -905,6 +905,36 @@ static void show_diff_tree(const char *prefix, struct commit *commit) } /* + * The terms used for this bisect session are stored in BISECT_TERMS. + * We read them and store them to adapt the messages accordingly. + * Default is bad/good. + */ +void read_bisect_terms(const char **read_bad, const char **read_good) +{ + struct strbuf str = STRBUF_INIT; + const char *filename = git_path("BISECT_TERMS"); + FILE *fp = fopen(filename, "r"); + + if (!fp) { + if (errno == ENOENT) { + *read_bad = "bad"; + *read_good = "good"; + return; + } else { + die("could not read file '%s': %s", filename, + strerror(errno)); + } + } else { + strbuf_getline(&str, fp, '\n'); + *read_bad = strbuf_detach(&str, NULL); + strbuf_getline(&str, fp, '\n'); + *read_good = strbuf_detach(&str, NULL); + } + strbuf_release(&str); + fclose(fp); +} + +/* * We use the convention that exiting with an exit code 10 means that * the bisection process finished successfully. * In this case the calling shell script should exit 0. @@ -920,8 +950,7 @@ int bisect_next_all(const char *prefix, int no_checkout) const unsigned char *bisect_rev; char bisect_rev_hex[GIT_SHA1_HEXSZ + 1]; - term_bad = "bad"; - term_good = "good"; + read_bisect_terms(&term_bad, &term_good); if (read_bisect_refs()) die("reading bisect refs failed"); diff --git a/git-bisect.sh b/git-bisect.sh index fcbed22..dcd7e59 100755 --- a/git-bisect.sh +++ b/git-bisect.sh @@ -77,6 +77,7 @@ bisect_start() { orig_args=$(git rev-parse --sq-quote "$@") bad_seen=0 eval='' + must_write_terms=0 if test "z$(git rev-parse --is-bare-repository)" != zfalse then mode=--no-checkout @@ -101,6 +102,14 @@ bisect_start() { die "$(eval_gettext "'\$arg' does not appear to be a valid revision")" break } + + # The user ran "git bisect start + # ", hence did not explicitly specify + # the terms, but we are already starting to + # set references named with the default terms, + # and won't be able to change afterwards. + must_write_terms=1 + case $bad_seen in 0) state=$TERM_BAD ; bad_seen=1 ;; *) state=$TERM_GOOD ;; @@ -172,6 +181,10 @@ bisect_start() { } && git rev-parse --sq-quote "$@" >"$GIT_DIR/BISECT_NAMES" && eval "$eval true" && + if test $must_write_terms -eq 1 + then + write_terms "$TERM_BAD" "$TERM_GOOD" + fi && echo "git bisect start$orig_args" >>"$GIT_DIR/BISECT_LOG" || exit # # Check if we can proceed to the next bisect state. @@ -232,6 +245,7 @@ bisect_skip() { bisect_state() { bisect_autostart state=$1 + check_and_set_terms $state case "$#,$state" in 0,*) die "$(gettext "Please call 'bisect_state' with at least one argument.")" ;; @@ -291,15 +305,17 @@ bisect_next_check() { : bisect without $TERM_GOOD... ;; *) - + bad_syn=$(bisect_voc bad) + good_syn=$(bisect_voc good) if test -s "$GIT_DIR/BISECT_START" then - gettextln "You need to give me at least one good and one bad revision. -(You can use \"git bisect bad\" and \"git bisect good\" for that.)" >&2 + + eval_gettextln "You need to give me at least one \$bad_syn and one \$good_syn revision. +(You can use \"git bi
[PATCH v11 04/10] bisect: replace hardcoded "bad|good" by variables
From: Antoine Delaite To add new tags like old/new and have keywords less confusing, the first step is to avoid hardcoding the keywords. The default mode is still bad/good. Signed-off-by: Antoine Delaite Signed-off-by: Louis Stuber Signed-off-by: Valentin Duperray Signed-off-by: Franck Jonas Signed-off-by: Lucien Kong Signed-off-by: Thomas Nguy Signed-off-by: Huynh Khoi Nguyen Nguyen Signed-off-by: Matthieu Moy --- bisect.c | 54 +- git-bisect.sh | 57 +++-- 2 files changed, 68 insertions(+), 43 deletions(-) diff --git a/bisect.c b/bisect.c index 5b8357d..a96e485 100644 --- a/bisect.c +++ b/bisect.c @@ -21,6 +21,9 @@ static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL}; static const char *argv_show_branch[] = {"show-branch", NULL, NULL}; static const char *argv_update_ref[] = {"update-ref", "--no-deref", "BISECT_HEAD", NULL, NULL}; +static const char *term_bad; +static const char *term_good; + /* Remember to update object flag allocation in object.h */ #define COUNTED(1u<<16) @@ -403,15 +406,21 @@ struct commit_list *find_bisection(struct commit_list *list, static int register_ref(const char *refname, const struct object_id *oid, int flags, void *cb_data) { - if (!strcmp(refname, "bad")) { + struct strbuf good_prefix = STRBUF_INIT; + strbuf_addstr(&good_prefix, term_good); + strbuf_addstr(&good_prefix, "-"); + + if (!strcmp(refname, term_bad)) { current_bad_oid = xmalloc(sizeof(*current_bad_oid)); oidcpy(current_bad_oid, oid); - } else if (starts_with(refname, "good-")) { + } else if (starts_with(refname, good_prefix.buf)) { sha1_array_append(&good_revs, oid->hash); } else if (starts_with(refname, "skip-")) { sha1_array_append(&skipped_revs, oid->hash); } + strbuf_release(&good_prefix); + return 0; } @@ -634,7 +643,7 @@ static void exit_if_skipped_commits(struct commit_list *tried, return; printf("There are only 'skip'ped commits left to test.\n" - "The first bad commit could be any of:\n"); + "The first %s commit could be any of:\n", term_bad); print_commit_list(tried, "%s\n", "%s\n"); if (bad) printf("%s\n", oid_to_hex(bad)); @@ -732,18 +741,24 @@ static void handle_bad_merge_base(void) if (is_expected_rev(current_bad_oid)) { char *bad_hex = oid_to_hex(current_bad_oid); char *good_hex = join_sha1_array_hex(&good_revs, ' '); - - fprintf(stderr, "The merge base %s is bad.\n" - "This means the bug has been fixed " - "between %s and [%s].\n", - bad_hex, bad_hex, good_hex); - + if (!strcmp(term_bad, "bad") && !strcmp(term_good, "good")) { + fprintf(stderr, "The merge base %s is bad.\n" + "This means the bug has been fixed " + "between %s and [%s].\n", + bad_hex, bad_hex, good_hex); + } else { + fprintf(stderr, "The merge base %s is %s.\n" + "This means the first '%s' commit is " + "between %s and [%s].\n", + bad_hex, term_bad, term_good, bad_hex, good_hex); + } exit(3); } - fprintf(stderr, "Some good revs are not ancestor of the bad rev.\n" + fprintf(stderr, "Some %s revs are not ancestor of the %s rev.\n" "git bisect cannot work properly in this case.\n" - "Maybe you mistook good and bad revs?\n"); + "Maybe you mistook %s and %s revs?\n", + term_good, term_bad, term_good, term_bad); exit(1); } @@ -755,10 +770,10 @@ static void handle_skipped_merge_base(const unsigned char *mb) warning("the merge base between %s and [%s] " "must be skipped.\n" - "So we cannot be sure the first bad commit is " + "So we cannot be sure the first %s commit is " "between %s and %s.\n" "We continue anyway.", - bad_hex, good_hex, mb_hex, bad_hex); + bad_hex, good_hex, term_bad, mb_hex, bad_hex); free(good_hex); } @@ -839,7 +854,7 @@ static void check_good_are_ancestors_of_bad(const char *prefix, int no_checkout) int fd; if (!current_bad_oid) - die("a bad revision is needed"); + die("a %s revision is needed", term_bad); /* Check if file BISECT_ANCESTORS_OK exists. */ if (!stat(filename, &st) && S_ISREG(st.st_mode)) @@ -905,6 +920,8 @@ int bisect_
[PATCH v11 01/10] bisect: correction of typo
From: Antoine Delaite Signed-off-by: Antoine Delaite Signed-off-by: Matthieu Moy --- bisect.c| 2 +- t/t6030-bisect-porcelain.sh | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bisect.c b/bisect.c index 03d5cd9..5b8357d 100644 --- a/bisect.c +++ b/bisect.c @@ -743,7 +743,7 @@ static void handle_bad_merge_base(void) fprintf(stderr, "Some good revs are not ancestor of the bad rev.\n" "git bisect cannot work properly in this case.\n" - "Maybe you mistake good and bad revs?\n"); + "Maybe you mistook good and bad revs?\n"); exit(1); } diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 06b4868..9e2c203 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -362,7 +362,7 @@ test_expect_success 'bisect starting with a detached HEAD' ' test_expect_success 'bisect errors out if bad and good are mistaken' ' git bisect reset && test_must_fail git bisect start $HASH2 $HASH4 2> rev_list_error && - grep "mistake good and bad" rev_list_error && + grep "mistook good and bad" rev_list_error && git bisect reset ' -- 2.5.0.rc0.10.gd2bff5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v11 02/10] Documentation/bisect: move getting help section to the end
Signed-off-by: Matthieu Moy --- Documentation/git-bisect.txt | 11 +-- 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/Documentation/git-bisect.txt b/Documentation/git-bisect.txt index 4cb52a7..2bdc3b8 100644 --- a/Documentation/git-bisect.txt +++ b/Documentation/git-bisect.txt @@ -31,12 +31,6 @@ This command uses 'git rev-list --bisect' to help drive the binary search process to find which change introduced a bug, given an old "good" commit object name and a later "bad" commit object name. -Getting help - - -Use "git bisect" to get a short usage description, and "git bisect -help" or "git bisect -h" to get a long usage description. - Basic bisect commands: start, bad, good ~~~ @@ -379,6 +373,11 @@ In this case, when 'git bisect run' finishes, bisect/bad will refer to a commit has at least one parent whose reachable graph is fully traversable in the sense required by 'git pack objects'. +Getting help + + +Use `git bisect` to get a short usage description, and `git bisect +help` or `git bisect -h` to get a long usage description. SEE ALSO -- 2.5.0.rc0.10.gd2bff5d -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10.1 7/7] bisect: allow any terms set by user
Christian Couder writes: > On Mon, Jun 29, 2015 at 11:32 AM, Matthieu Moy > wrote: >> bisect is all about finding the commit where a property has changed, > > That is your interpretation of this command. On the man page there is: > > git-bisect - Find by binary search the change that introduced a bug > > So its stated purpose is to find the first "bad" commit. Not to find a fix. This is a limitation of the current bisect, but the discussion is precisely about removing this limitation. I still don't understand what "risk" we are taking by doing the bisection anyway. I can't imagine a case where we would harm the user by doing so. I just tested with Mercurial, and looking for a fix instead of a regression just works: $ hg bisect --good 4 $ hg bisect --bad 1 Testing changeset 2:d75a2d042c99 (3 changesets remaining, ~1 tests) 1 files updated, 0 files merged, 0 files removed, 0 files unresolved $ hg bisect --bad Testing changeset 3:9d27d9c02e28 (2 changesets remaining, ~1 tests) 1 files updated, 0 files merged, 0 files removed, 0 files unresolved $ hg bisect --bad The first good revision is: changeset: 4:1dd9bb959eb6 tag: tip user:Matthieu Moy date:Mon Jun 29 17:07:51 2015 +0200 summary: foo I don't see anything wrong with this. (OTOH, "hg bisect" does not accept revisions which aren't parent of each other) -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] fsck: it is OK for a tag and a commit to lack the body
Hi Junio, On 2015-06-29 07:42, Junio C Hamano wrote: > Johannes Schindelin writes: > >> Hmm. Maybe we should still warn when there is no empty line finishing >> the header explicitly, or at least make it FSCK_IGNORE by default so >> that maintainers who like a stricter check can upgrade the condition >> to an error? > > [...] > > And such a check can certainly be added in the future True. I take my suggestion back. Thanks for the reality check, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] 4th release candidate of Git for Windows 2.x, was Re: 3rd release candidate of Git for Windows 2.x
Am 29.06.2015 um 16:37 schrieb Johannes Schindelin: > Hi Stefan, > > On 2015-06-29 11:07, Stefan Näwe wrote: >> Am 29.06.2015 um 10:30 schrieb Johannes Schindelin: > >>> I just uploaded the 4th release candidate for the upcoming Git for >>> Windows 2.x release. Please find the download link here: >>> >>> https://git-for-windows.github.io/#download >>> >>> The most important changes are the update to Git 2.4.5 and a fix for the >>> crash when running Git Bash >>> with a legacy `TERM` setting (this should help 3rd party software to >>> upgrade to Git for Windows 2.x). >> >> Thanks. >> It seems that this link: >> >>https://github.com/git-for-windows/git/releases/latest >> >> doesn't point to the latest release. >> >> Might be because the tags have the same date ? > > Wooops. Sorry for being so slow (been interviewing today). It should be > correct now, can you verify, please? Yes. https://github.com/git-for-windows/git/releases/latest redirects to "Fourth release candidate of Git for Windows 2.x" now. Thanks, Stefan -- /dev/random says: Documentation is the castor oil of programming. python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')" GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9 9666 829B 49C5 9221 27AF -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: What's cooking in git.git (Jun 2015, #06; Wed, 24)
What can I do to help un-stall my gitweb patches? > [Stalled] > > * tf/gitweb-project-listing (2015-03-19) 5 commits > - gitweb: make category headings into links when they are directories > - gitweb: optionally set project category from its pathname > - gitweb: add a link under the search box to clear a project filter > - gitweb: if the PATH_INFO is incomplete, use it as a project_filter > - gitweb: fix typo in man page > > Update gitweb to make it more pleasant to deal with a hierarchical > forest of repositories. > > Any comments from those who use or have their own code in Gitweb? Tony. -- f.anthony.n.finchhttp://dotat.at/ South Utsire: Westerly 4, occasionally 5 in east. Smooth or slight, occasionally moderate in east. Fog patches later. Moderate, occasionally very poor later. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] 4th release candidate of Git for Windows 2.x, was Re: 3rd release candidate of Git for Windows 2.x
Hi Stefan, On 2015-06-29 11:07, Stefan Näwe wrote: > Am 29.06.2015 um 10:30 schrieb Johannes Schindelin: >> I just uploaded the 4th release candidate for the upcoming Git for >> Windows 2.x release. Please find the download link here: >> >> https://git-for-windows.github.io/#download >> >> The most important changes are the update to Git 2.4.5 and a fix for the >> crash when running Git Bash >> with a legacy `TERM` setting (this should help 3rd party software to upgrade >> to Git for Windows 2.x). > > Thanks. > It seems that this link: > >https://github.com/git-for-windows/git/releases/latest > > doesn't point to the latest release. > > Might be because the tags have the same date ? Wooops. Sorry for being so slow (been interviewing today). It should be correct now, can you verify, please? Thanks, Dscho -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] worktree: new place for "git prune --worktrees"
Commit 23af91d (prune: strategies for linked checkouts - 2014-11-30) adds "--worktrees" to "git prune" without realizing that "git prune" is for object database only. This patch moves the same functionality to a new command "git worktree". Signed-off-by: Nguyễn Thái Ngọc Duy --- In future I probably move the big block of text in git-checkout.txt to git-worktree.txt and add "git worktree list". But let's start with something small and simple before "git prune --worktrees" is shipped out. .gitignore | 1 + Documentation/git-prune.txt | 3 - Documentation/git-worktree.txt (new) | 48 + Makefile | 1 + builtin.h| 1 + builtin/gc.c | 2 +- builtin/prune.c | 99 -- builtin/worktree.c (new) | 133 +++ command-list.txt | 1 + git.c| 1 + t/t2026-prune-linked-checkouts.sh| 22 +++--- 11 files changed, 198 insertions(+), 114 deletions(-) create mode 100644 Documentation/git-worktree.txt create mode 100644 builtin/worktree.c diff --git a/.gitignore b/.gitignore index 422c538..a685ec1 100644 --- a/.gitignore +++ b/.gitignore @@ -171,6 +171,7 @@ /git-verify-tag /git-web--browse /git-whatchanged +/git-worktree /git-write-tree /git-core-*/?* /gitweb/GITWEB-BUILD-OPTIONS diff --git a/Documentation/git-prune.txt b/Documentation/git-prune.txt index 1cf3bed..7a493c8 100644 --- a/Documentation/git-prune.txt +++ b/Documentation/git-prune.txt @@ -48,9 +48,6 @@ OPTIONS --expire :: Only expire loose objects older than . ---worktrees:: - Prune dead working tree information in $GIT_DIR/worktrees. - ...:: In addition to objects reachable from any of our references, keep objects diff --git a/Documentation/git-worktree.txt b/Documentation/git-worktree.txt new file mode 100644 index 000..b9072e0 --- /dev/null +++ b/Documentation/git-worktree.txt @@ -0,0 +1,48 @@ +git-worktree(1) +=== + +NAME + +git-worktree - Manage multiple worktrees + + +SYNOPSIS + +[verse] +'git worktree prune' [-n] [-v] [--expire ] + +DESCRIPTION +--- + +Manage multiple worktrees attached to the same repository. These are +created by the command `git checkout --to`. + +COMMANDS + +prune:: + +Prune working tree information in $GIT_DIR/worktrees. + +OPTIONS +--- + +-n:: +--dry-run:: + Do not remove anything; just report what it would + remove. + +-v:: +--verbose:: + Report all removals. + +--expire :: + Only expire unused worktrees older than . + +SEE ALSO + + +linkgit:git-checkout[1] + +GIT +--- +Part of the linkgit:git[1] suite diff --git a/Makefile b/Makefile index 149f1c7..89785f6 100644 --- a/Makefile +++ b/Makefile @@ -909,6 +909,7 @@ BUILTIN_OBJS += builtin/var.o BUILTIN_OBJS += builtin/verify-commit.o BUILTIN_OBJS += builtin/verify-pack.o BUILTIN_OBJS += builtin/verify-tag.o +BUILTIN_OBJS += builtin/worktree.o BUILTIN_OBJS += builtin/write-tree.o GITLIBS = $(LIB_FILE) $(XDIFF_LIB) diff --git a/builtin.h b/builtin.h index b87df70..9e04f97 100644 --- a/builtin.h +++ b/builtin.h @@ -133,6 +133,7 @@ extern int cmd_verify_commit(int argc, const char **argv, const char *prefix); extern int cmd_verify_tag(int argc, const char **argv, const char *prefix); extern int cmd_version(int argc, const char **argv, const char *prefix); extern int cmd_whatchanged(int argc, const char **argv, const char *prefix); +extern int cmd_worktree(int argc, const char **argv, const char *prefix); extern int cmd_write_tree(int argc, const char **argv, const char *prefix); extern int cmd_verify_pack(int argc, const char **argv, const char *prefix); extern int cmd_show_ref(int argc, const char **argv, const char *prefix); diff --git a/builtin/gc.c b/builtin/gc.c index 36fe333..4957c39 100644 --- a/builtin/gc.c +++ b/builtin/gc.c @@ -293,7 +293,7 @@ int cmd_gc(int argc, const char **argv, const char *prefix) argv_array_pushl(&reflog, "reflog", "expire", "--all", NULL); argv_array_pushl(&repack, "repack", "-d", "-l", NULL); argv_array_pushl(&prune, "prune", "--expire", NULL); - argv_array_pushl(&prune_worktrees, "prune", "--worktrees", "--expire", NULL); + argv_array_pushl(&prune_worktrees, "worktree", "prune", "--expire", NULL); argv_array_pushl(&rerere, "rerere", "gc", NULL); gc_config(); diff --git a/builtin/prune.c b/builtin/prune.c index 0c73246..10b03d3 100644 --- a/builtin/prune.c +++ b/builtin/prune.c @@ -6,7 +6,6 @@ #include "reachable.h" #include "parse-options.h" #include "progress.h" -#include "dir.h" static const char * const prune_usage[] = { N_("git prune [-n] [-v] [--expire ] [--] [...]"), @@ -76,95 +75,6 @@ static int prune_subdir(int nr, const char *path, void *data) retur
Re: [PATCH v10.1 7/7] bisect: allow any terms set by user
On Mon, Jun 29, 2015 at 11:32 AM, Matthieu Moy wrote: > Christian Couder writes: [...] > I find it particularly frustrating that we issue the message: > > "The merge base %s is bad.\n" > "This means the bug has been fixed " > "between %s and [%s].\n" I find it a good safety feature. > bisect is all about finding the commit where a property has changed, That is your interpretation of this command. On the man page there is: git-bisect - Find by binary search the change that introduced a bug So its stated purpose is to find the first "bad" commit. Not to find a fix. This doesn't mean that we cannot or should not make it possible to find a fix, but we shoudn't try to find a fix when there is doubt about whether the user wants to find a fix or the first bad commit. > and > here it stops, saying "I know it's between A and B, but I won't go > further". You can interpret it that way, but my interpretation of this is that the real reason why we stop is because we don't know what the users really wants to do, find the fix, or really find the first bad commit. >>> In particular when bisecting from two branches, the user knows that >>> branch A is good, and branch B is bad, but does not necessarily know >>> whether it's a regression in B or a >>> fix in A. The fact that bisect can find out should be just "normal" from >>> the user point of view. There's no mistake involved, nothing to fix, and >>> nothing that went wrong. >> >> Well in this case, it's possible that the merge base is bad and what >> the user is interested in is the first bad commit that was commited >> before the merge base. We just don't know, in the case the merge base >> is bad, what is more interesting for the user. > > The question asked by the user is "X is good, Y is bad, tell me where > exactly it changed". That's your interpretation of the question asked by the user. It can be interpreted otherwise. > We can't know for sure what is "interesting" for > the user, but we can answer his question anyway. You can answer your interpretation of the question, yes, but this means taking risks that we don't need to take in my opinion. If people really want it, we can still have an option called for example --find-fix that could automatically try to find the fix when the merge base is "bad" without displaying the message that annoys you. It would make it explicit that it is ok to find a fix. > Similarly, there can be several commits that introduce the same > regression (this may happen if you cherry picked the guilty commit from > branch A to branch B, and then merged A and B, or if you broke, fixed, > and then broke again). bisect finds one transition from good to bad, but > not all. It may or may not be the one the user wanted, but we can't > know. Yeah, but this is different as we would still find a first "bad" commit, not a fix. >>> I think I prefer "term" to "name". >> >> Ok with that. I agree that it would be more consistent to have a "git >> bisect terms" and "--term-{old,new,bad,good}". > > OK, I've applied it. Great! -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10.1 7/7] bisect: allow any terms set by user
Junio C Hamano writes: > I moderately hate to see both from aesthetics point of view, but can > we at least lose "--name-" prefix? I changed it to --term- prefix, but I'd rather not drop it. When reading "--old=foo", it is not clear to me whether the meaning should be "the term used for old is foo" or "mark foo as old". The longer version does not have this problem. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10.1 7/7] bisect: allow any terms set by user
Christian Couder writes: > On Mon, Jun 29, 2015 at 9:34 AM, Matthieu Moy > >> As a user, when I >> discovered "git bisect", I was actually surprised that it expected one >> particular order between good and bad. I would have expected to be able >> to say "this is good, this is bad, tell me where it changed" without >> having an idea of who's good and who's bad. > > Maybe, but it's not how it has been developed. ... but it's how it would behave if we had this guessing feature. The user does not have to care whether we internally need "good is an ancestor of bad" if we can provide a user-interface which does not need that. I find it particularly frustrating that we issue the message: "The merge base %s is bad.\n" "This means the bug has been fixed " "between %s and [%s].\n" bisect is all about finding the commit where a property has changed, and here it stops, saying "I know it's between A and B, but I won't go further". >> In particular when bisecting from two branches, the user knows that >> branch A is good, and branch B is bad, but does not necessarily know >> whether it's a regression in B or a >> fix in A. The fact that bisect can find out should be just "normal" from >> the user point of view. There's no mistake involved, nothing to fix, and >> nothing that went wrong. > > Well in this case, it's possible that the merge base is bad and what > the user is interested in is the first bad commit that was commited > before the merge base. We just don't know, in the case the merge base > is bad, what is more interesting for the user. The question asked by the user is "X is good, Y is bad, tell me where exactly it changed". We can't know for sure what is "interesting" for the user, but we can answer his question anyway. Similarly, there can be several commits that introduce the same regression (this may happen if you cherry picked the guilty commit from branch A to branch B, and then merged A and B, or if you broke, fixed, and then broke again). bisect finds one transition from good to bad, but not all. It may or may not be the one the user wanted, but we can't know. >> I think I prefer "term" to "name". > > Ok with that. I agree that it would be more consistent to have a "git > bisect terms" and "--term-{old,new,bad,good}". OK, I've applied it. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [msysGit] 4th release candidate of Git for Windows 2.x, was Re: 3rd release candidate of Git for Windows 2.x
Am 29.06.2015 um 10:30 schrieb Johannes Schindelin: > Hi all, > > I just uploaded the 4th release candidate for the upcoming Git for > Windows 2.x release. Please find the download link here: > > https://git-for-windows.github.io/#download > > The most important changes are the update to Git 2.4.5 and a fix for the > crash when running Git Bash > with a legacy `TERM` setting (this should help 3rd party software to upgrade > to Git for Windows 2.x). Thanks. It seems that this link: https://github.com/git-for-windows/git/releases/latest doesn't point to the latest release. Might be because the tags have the same date ? Thanks anyway! Stefan -- /dev/random says: Who needs comedians? Journalists are much more laughable! python -c "print '73746566616e2e6e616577654061746c61732d656c656b74726f6e696b2e636f6d'.decode('hex')" GPG Key fingerprint = 2DF5 E01B 09C3 7501 BCA9 9666 829B 49C5 9221 27AF -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
4th release candidate of Git for Windows 2.x, was Re: 3rd release candidate of Git for Windows 2.x
Hi all, I just uploaded the 4th release candidate for the upcoming Git for Windows 2.x release. Please find the download link here: https://git-for-windows.github.io/#download The most important changes are the update to Git 2.4.5 and a fix for the crash when running Git Bash with a legacy `TERM` setting (this should help 3rd party software to upgrade to Git for Windows 2.x). Please find the release notes here: https://github.com/git-for-windows/build-extra/blob/master/installer/ReleaseNotes.md Another step, Johannes -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10.1 7/7] bisect: allow any terms set by user
On Mon, Jun 29, 2015 at 9:34 AM, Matthieu Moy wrote: > Christian Couder writes: > >> On Sun, Jun 28, 2015 at 8:46 AM, Michael Haggerty >> wrote: >>> I understand that the user might make a mistake when marking the initial >>> commits, but as soon as bisect says >>> >>> Commit is an ancestor of , so I >>> will look for the commit that caused the transition from >>> "xyzzy" to "plugh". >>> >>> then I hope the user will notice and correct her/his mistake. >> >> This looks fragile to me. Unfortunately many users will probably not >> read it and continue, and then spend a lot of time later trying to >> understand what went wrong, > > I don't understand what you mean by "went wrong". It happens that users mistake the "good" and the "bad" commits when giving them to git bisect. Right now in the most common case, we can error out because we know that a "bad" commit cannot be an ancestor of a "good" commit. > As a user, when I > discovered "git bisect", I was actually surprised that it expected one > particular order between good and bad. I would have expected to be able > to say "this is good, this is bad, tell me where it changed" without > having an idea of who's good and who's bad. Maybe, but it's not how it has been developed. > In particular when bisecting > from two branches, the user knows that branch A is good, and branch B is > bad, but does not necessarily know whether it's a regression in B or a > fix in A. The fact that bisect can find out should be just "normal" from > the user point of view. There's no mistake involved, nothing to fix, and > nothing that went wrong. Well in this case, it's possible that the merge base is bad and what the user is interested in is the first bad commit that was commited before the merge base. We just don't know, in the case the merge base is bad, what is more interesting for the user. So I disagree with you and Michael that we should decide that the user is interested by the fix in this case. It's better to error out like we do now and let the user decide what he/she wants rather than decide for him/her that he/she is interested by the fix. >> By the way we could use "mark" or "term" instead of "name" in the >> option name (like --mark-old or --term-old) and in the code too if it >> looks clearer. > > I prefer "term" to "mark" because "mark" is both a verb and a noun, so > --mark-old=foo could mean both "mark foo as old" or "the name of the > marks for old commits is foo". > > I think I prefer "term" to "name". Ok with that. I agree that it would be more consistent to have a "git bisect terms" and "--term-{old,new,bad,good}". Thanks, Christian. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10.1 7/7] bisect: allow any terms set by user
Christian Couder writes: > On Sun, Jun 28, 2015 at 8:46 AM, Michael Haggerty > wrote: >> I understand that the user might make a mistake when marking the initial >> commits, but as soon as bisect says >> >> Commit is an ancestor of , so I >> will look for the commit that caused the transition from >> "xyzzy" to "plugh". >> >> then I hope the user will notice and correct her/his mistake. > > This looks fragile to me. Unfortunately many users will probably not > read it and continue, and then spend a lot of time later trying to > understand what went wrong, I don't understand what you mean by "went wrong". As a user, when I discovered "git bisect", I was actually surprised that it expected one particular order between good and bad. I would have expected to be able to say "this is good, this is bad, tell me where it changed" without having an idea of who's good and who's bad. In particular when bisecting from two branches, the user knows that branch A is good, and branch B is bad, but does not necessarily know whether it's a regression in B or a fix in A. The fact that bisect can find out should be just "normal" from the user point of view. There's no mistake involved, nothing to fix, and nothing that went wrong. > By the way we could use "mark" or "term" instead of "name" in the > option name (like --mark-old or --term-old) and in the code too if it > looks clearer. I prefer "term" to "mark" because "mark" is both a verb and a noun, so --mark-old=foo could mean both "mark foo as old" or "the name of the marks for old commits is foo". I think I prefer "term" to "name". -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v10.1 7/7] bisect: allow any terms set by user
Junio C Hamano writes: > I _think_ bulk of Antoine and Matthieu's work can be salvaged/reused > to implement the proposal, I'm obviously biaised since I already spent time on the "bisect terms" idea, and I would hate to see my work and Antoine & Louis' thrown away. But I have to admit that I do like the idea of "git bisect" figuring out which commit is the old and which commit is the new one. It's much easier to implement after the series. I'm currently forbidding redefining "good" as "bad" and vice versa, but that's just to avoid confusion and because I didn't test this case, but it should just work. So, essentially an implementation of this "guess who's old and who's new" could be: if we find a good commit that is an ancestor of an old commit, swap the terms in BISECT_TERMS. When this happens before we started to set any refs, this should do the trick. In general, we should rename the good-$sha1 reference to good and the bad to bad-$sha1 (there are corner-cases where the user already specified several good-$sha1 refs, in which case we would need to discard some of them). I'm getting out of Git time budget, so I can't be the one doing this, at least not soon. So, one option is to take the series as-is, and wait for someone to implement the "guess who's old and who's new" later on top of it. One drawback would be that we'd end up having the not-so-useful feature "bisect terms" in the user-interface. At least, I am now convinced that hardcoding the pair old/new is not needed. In the short term, we can have "git bisect start --name-old foo --name-new bar" which avoids the "One needs to remember in which order to give terms" issue we used to have, so we don't need to clutter the user-interface with many ways to do the same thing. OTOH, the "bisect terms" feature wouldn't be completely useless: not everything is good or bad, so leaving the option to the user to tag "slow/fast", "present/absent", ... still makes sense. So, my proposal would be to remove the "old/new" patch from the series, and keep the other patches. What do you think? > but now it would be more clear that $name_good and $name_bad is a bad > way to name internal variables and files in $GIT_DIR. The inferred 'ah > you are hunting for regression' mode would call old ones 'bad' and new > ones 'good', they have to be given value neutral names, e.g. $name_old > and $name_new. Ideally, the whole code would be ported to use old/new, but the more I read the code the more I agree with Christian actually: we still have many more instances of good/bad in the code (e.g. functions check_good_are_ancestors_of_bad, for_each_good_bisect_ref, ...), so having just name_new/name_old would make the code very inconsistant. It's easier to read the code thinking "good revs are old, bad revs are recent; maybe some magic swapped the terms but I don't need to worry about this" for now. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html