Improve support for 'git config gc.reflogExpire never'
If I configure bare repo with git config gc.pruneExpire never git config gc.reflogExpire never then git will never garbage collect any commit ever stored in the repo. This is what I want. However, all commits referenced only via reflog are kept as loose objects and will not be included in packs. In long run this will cause warning: There are too many unreachable loose objects; run 'git prune' to remove them. and the performance of the repository will slowly decrease. If I have understood correctly, git should notice that reflog will keep referenced commits forever and include all those commits in packs instead of keeping those as loose forever. A more generic behavior might be to always compress all loose commits in (one?) special pack so the performance would stay good even if gc.reflogExpire is very long instead of "never". Discussion about this case: https://stackoverflow.com/questions/28092485/how-to-prevent-garbage-collection-in-git https://stackoverflow.com/questions/55043693/is-it-possible-to-get-git-gc-to-pack-reflog-objects -- Mikko
Re: Error fetching submodule from submodule
Thanks Jeff You are completely right! It works as I expect if I remember the extra parameter `--recursive` when doing `git submodule update --init --recursive` Thanks a lot for your feedback! This is really useful! I learned a useful thing today :) /Jesper On Thu, Mar 7, 2019 at 7:08 PM Jeff King wrote: > > On Thu, Mar 07, 2019 at 12:07:21PM +0100, Jesper Rønn-Jensen wrote: > > > Hi I think I may have found an error in the way git handles a > > submodule's submodule. Read further for the example (extracted from a > > real project). > > First off, thank you for the example script. It made understanding > what's going on so much easier. :) > > > * I have a main repository which has some submodules defined. > > * One of the submodules is a common submodule which is also included > > in one of the other submodules > > * When running `git fetch --recurse-submodules` I get an error. > > I think the presence of common_submodule in the main repo is actually a > red herring. if you remove the last two lines of this stanza: > > > git setup main_repos > > pushd main_repos > > git submodule add ../common_submodule > > git commit -m 'added submodule to main_repos' > > the outcome is the same. > > > # This line fails with error code 1 "Could not access submodule > > 'common_submodule'" > > git fetch --recurse-submodules > > It looks like "fetch" is smart enough to initialize a submodule when > necessary, but not smart enough to do so recursively. If I replace that > line with: > > git submodule update --init --recursive > > then it works as I'd expect. Likewise, cloning the repository with: > > git clone --recurse-submodules main_repos foo > > does what you'd want. > > After that, I think "git fetch --recurse-submodules" does what you want, > because the submodule repository is already initialized. > > I'm not sure to what degree git-fetch intended to support initializing > submodules. But it certainly seems like a bug that it handles the top > layer but does not recurse (i.e., it should either handle all or none). > > Hopefully the commands above at least give you a workaround. > > -Peff -- Jesper Rønn-Jensen Nine A/S Mobile: +45 2989 1822 Blog http://justaddwater.dk/ jespe...@gmail.com (Private e-mail and Google Talk IM)
Re: [PATCH v3 1/2] worktree: fix worktree add race.
Junio, it seems 2/2 is stuck in an endless discussion. But 1/2 is good regardless, maybe pick it up now and let 2/2 come later whenever it's ready? On Wed, Feb 20, 2019 at 11:16 PM Michal Suchanek wrote: > > Git runs a stat loop to find a worktree name that's available and then does > mkdir on the found name. Turn it to mkdir loop to avoid another invocation of > worktree add finding the same free name and creating the directory first. > > Signed-off-by: Michal Suchanek > --- > v2: > - simplify loop exit condition > - exit early if the mkdir fails for reason other than already present > worktree > - make counter unsigned > --- > builtin/worktree.c | 12 +++- > 1 file changed, 7 insertions(+), 5 deletions(-) > > diff --git a/builtin/worktree.c b/builtin/worktree.c > index 3f9907fcc994..85a604cfe98c 100644 > --- a/builtin/worktree.c > +++ b/builtin/worktree.c > @@ -268,10 +268,10 @@ static int add_worktree(const char *path, const char > *refname, > struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; > struct strbuf sb = STRBUF_INIT; > const char *name; > - struct stat st; > struct child_process cp = CHILD_PROCESS_INIT; > struct argv_array child_env = ARGV_ARRAY_INIT; > - int counter = 0, len, ret; > + unsigned int counter = 0; > + int len, ret; > struct strbuf symref = STRBUF_INIT; > struct commit *commit = NULL; > int is_branch = 0; > @@ -295,8 +295,12 @@ static int add_worktree(const char *path, const char > *refname, > if (safe_create_leading_directories_const(sb_repo.buf)) > die_errno(_("could not create leading directories of '%s'"), > sb_repo.buf); > - while (!stat(sb_repo.buf, &st)) { > + > + while (mkdir(sb_repo.buf, 0777)) { > counter++; > + if ((errno != EEXIST) || !counter /* overflow */) > + die_errno(_("could not create directory of '%s'"), > + sb_repo.buf); > strbuf_setlen(&sb_repo, len); > strbuf_addf(&sb_repo, "%d", counter); > } > @@ -306,8 +310,6 @@ static int add_worktree(const char *path, const char > *refname, > atexit(remove_junk); > sigchain_push_common(remove_junk_on_signal); > > - if (mkdir(sb_repo.buf, 0777)) > - die_errno(_("could not create directory of '%s'"), > sb_repo.buf); > junk_git_dir = xstrdup(sb_repo.buf); > is_junk = 1; > > -- > 2.20.1 > -- Duy
[PATCH v5 1/1] worktree add: sanitize worktree names
Worktree names are based on $(basename $GIT_WORK_TREE). They aren't significant until 3a3b9d8cde (refs: new ref types to make per-worktree refs visible to all worktrees - 2018-10-21), where worktree name could be part of a refname and must follow refname rules. Update 'worktree add' code to remove special characters to follow these rules. In the future the user will be able to specify the worktree name by themselves if they're not happy with this dumb character substitution. Reported-by: Konstantin Kharlamov Helped-by: Jeff King Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/worktree.c | 10 +++- refs.c | 103 refs.h | 6 +++ t/t2400-worktree-add.sh | 5 ++ 4 files changed, 104 insertions(+), 20 deletions(-) diff --git a/builtin/worktree.c b/builtin/worktree.c index 6cc094a453..756cf3a417 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -275,6 +275,7 @@ static int add_worktree(const char *path, const char *refname, struct strbuf symref = STRBUF_INIT; struct commit *commit = NULL; int is_branch = 0; + struct strbuf sb_name = STRBUF_INIT; validate_worktree_add(path, opts); @@ -290,7 +291,13 @@ static int add_worktree(const char *path, const char *refname, die(_("invalid reference: %s"), refname); name = worktree_basename(path, &len); - git_path_buf(&sb_repo, "worktrees/%.*s", (int)(path + len - name), name); + strbuf_add(&sb, name, path + len - name); + sanitize_refname_component(sb.buf, &sb_name); + if (!sb_name.len) + BUG("How come '%s' becomes empty after sanitization?", sb.buf); + strbuf_reset(&sb); + name = sb_name.buf; + git_path_buf(&sb_repo, "worktrees/%s", name); len = sb_repo.len; if (safe_create_leading_directories_const(sb_repo.buf)) die_errno(_("could not create leading directories of '%s'"), @@ -416,6 +423,7 @@ static int add_worktree(const char *path, const char *refname, strbuf_release(&symref); strbuf_release(&sb_repo); strbuf_release(&sb_git); + strbuf_release(&sb_name); return ret; } diff --git a/refs.c b/refs.c index 142888a40a..e9f83018f0 100644 --- a/refs.c +++ b/refs.c @@ -72,30 +72,57 @@ static unsigned char refname_disposition[256] = { * - it ends with ".lock", or * - it contains a "@{" portion */ -static int check_refname_component(const char *refname, int *flags) +static int check_refname_component(const char *refname, int *flags, + struct strbuf *sanitized) { const char *cp; char last = '\0'; + size_t component_start; + + if (sanitized) + component_start = sanitized->len; for (cp = refname; ; cp++) { int ch = *cp & 255; unsigned char disp = refname_disposition[ch]; + + if (sanitized && disp != 1) + strbuf_addch(sanitized, ch); + switch (disp) { case 1: goto out; case 2: - if (last == '.') - return -1; /* Refname contains "..". */ + if (last == '.') { /* Refname contains "..". */ + if (sanitized) + sanitized->len--; /* collapse ".." to single "." */ + else + return -1; + } break; case 3: - if (last == '@') - return -1; /* Refname contains "@{". */ + if (last == '@') { /* Refname contains "@{". */ + if (sanitized) + sanitized->buf[sanitized->len-1] = '-'; + else + return -1; + } break; case 4: - return -1; + /* forbidden char */ + if (sanitized) + sanitized->buf[sanitized->len-1] = '-'; + else + return -1; + break; case 5: - if (!(*flags & REFNAME_REFSPEC_PATTERN)) - return -1; /* refspec can't be a pattern */ + if (!(*flags & REFNAME_REFSPEC_PATTERN)) { + /* refspec can't be a pattern */ + if (sanitized) + sanitized->buf[sanitized->len-1] = '-'; + else + return -1; + }
[PATCH v5 0/1] worktree add: sanitize worktree names
v5 is basically Jeff's version from one of the replies in v4, where check_refname_component is enhanced to optionally sanitize. I was reluctant to go this way because it makes check_refname_component more complex (turns out still manageable) and burns worktree rules in it. But there may never be the second sanitization user, we deal with it when it comes. As said, refs.c is pretty much Jeff's except two major changes: - handle foo.lock.lock correctly by stripping .lock repeatedly - sanitize refname _components_ instead of full refs. I could construct worktrees/ and pass to Jeff's sanitize_refname. But then I need to strip worktrees/ after that. I took credits so that bugs come to me first (then I'll blame him anyway while doing some evil laughs) Nguyễn Thái Ngọc Duy (1): worktree add: sanitize worktree names builtin/worktree.c | 10 +++- refs.c | 103 refs.h | 6 +++ t/t2400-worktree-add.sh | 5 ++ 4 files changed, 104 insertions(+), 20 deletions(-) -- 2.21.0.rc1.337.gdf7f8d0522
ls-remote set timeout
Hi there! Im new to this list - so hello, hope I'm welcome. My problem is: I have a configuration for my bash saved on a private git-repo. Every time, i start bash, my .bashrc checks this repo out to get all changes (alias, some functions, $PS1 and so on). So i can have my working environment on all my servers with personal login. Now I'm working on a new customer, where github.com is not reachable (firewall/proxy). Parts of my configuration (some plugins/scripts for vim) cannot be updated there, because they are hosted on github.com. :-/ Now i tried to fiddle in a check, if a repo is reachable in my .bashrc. And i found out, that git ls-remote is a good choice, because it handles redirections from http to https correctly behind this proxy. (direct https links to git-repos do even not work in this surrounding... don't ask why, please). I can check, if my private repo (git bare repo with gitweb) is reachable (http pull, ssh is also closed!!!) with git ls-remote. But this check hangs on github.com in case of a redirection from the proxy to a "this is forbidden"-site... . And it hangs forever (1 Minute, 2 Minutes or even really forever!) Is it possible, to include a "--connection-timeout" and/or the "--max-time" option for curl, that i can give to my "git ls-remote" command? So i can call git --connection-timeout 3 -m 3 ls-remote and the command stops after 3 seconds with an errorcode, which I can evaluate? I tried netcat and curl directly. In this environment only git ls-remote will work correctly on reachable repos, but it hangs on blocked... :-/ Thank you for your interests Jakob
Re: [PATCH v3 1/2] worktree: fix worktree add race.
On Fri, Mar 8, 2019 at 4:20 AM Duy Nguyen wrote: > Junio, it seems 2/2 is stuck in an endless discussion. But 1/2 is good > regardless, maybe pick it up now and let 2/2 come later whenever it's > ready? Yep, 1/2 seems a good idea and has not been controversial. It may not solve all the race conditions, but it is a good step forward.
Re: [GSoC][PATCH v2 3/3] t3600: use helpers to replace test -d/f/e/s
On Fri, Mar 8, 2019 at 12:38 AM Junio C Hamano wrote: > An unrelated tangent, but what do you think of this patch? In the > context of testing "git rm", if foo is a dangling symbolic link, > "git rm foo && test_path_is_missing foo" would need something like > this to work correctly, I would think. > > test_path_is_missing () { > - if test -e "$1" > + if test -e "$1" || test -L "$1" > then > echo "Path exists:" > ls -ld "$1" Makes sense. Won't we also want: test_path_exists () { -if ! test -e "$1" + if ! test -e "$1" && ! test -L "$1" then echo "Path $1 doesn't exist. $2" or something like that?
PRIVEE
Good day , my name is David William , i sent you a mail and there was no response , please confirm that you did get this mail for more details. Regards. David William
[PATCH v3 02/21] git-checkout.txt: fix one syntax line
can be omitted in this syntax, and it's actually documented a few paragraphs down: You could omit , in which case the command degenerates to "check out the current branch", which is a glorified no-op with rather expensive side-effects to show only the tracking information, if exists, for the current branch. --- Documentation/git-checkout.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 99c8c0dc0f..28817cfa41 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -23,7 +23,7 @@ or the specified tree. If no paths are given, 'git checkout' will also update `HEAD` to set the specified branch as the current branch. -'git checkout' :: +'git checkout' []:: To prepare for working on , switch to it by updating the index and the files in the working tree, and by pointing HEAD at the branch. Local modifications to the files in the -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v3 00/21] Add new command "switch"
v3 contains document and completion updates based on v2's feedback. It also contains some extra git-checkout.txt updates (blame Eric for this, he points out problems in git-switch.txt and makes me want to go fix git-checkout.txt too). The series is now based on 'master' (yay!) Nguyễn Thái Ngọc Duy (21): git-checkout.txt: spell out --no-option git-checkout.txt: fix one syntax line doc: document --overwrite-ignore git-checkout.txt: fix monospace typeset t: rename t2014-switch.sh to t2014-checkout-switch.sh checkout: factor out some code in parse_branchname_arg() checkout: make "opts" in cmd_checkout() a pointer checkout: move 'confict_style' and 'dwim_..' to checkout_opts checkout: split options[] array in three pieces checkout: split part of it to new command 'switch' switch: better names for -b and -B switch: remove -l switch: stop accepting pathspec switch: reject "do nothing" case switch: only allow explicit detached HEAD switch: add short option for --detach switch: no implicit dwim, use --guess to dwim switch: no worktree status unless real branch switch happens t: add tests for switch completion: support switch doc: promote "git switch" .gitignore| 1 + Documentation/config/advice.txt | 13 +- Documentation/config/branch.txt | 4 +- Documentation/config/checkout.txt | 17 +- Documentation/config/diff.txt | 3 +- Documentation/git-branch.txt | 12 +- Documentation/git-check-ref-format.txt| 3 +- Documentation/git-checkout.txt| 119 +++-- Documentation/git-format-patch.txt| 2 +- Documentation/git-merge-base.txt | 2 +- Documentation/git-merge.txt | 5 + Documentation/git-rebase.txt | 2 +- Documentation/git-remote.txt | 2 +- Documentation/git-rerere.txt | 10 +- Documentation/git-reset.txt | 20 +- Documentation/git-stash.txt | 9 +- Documentation/git-switch.txt | 259 ++ Documentation/gitattributes.txt | 3 +- Documentation/gitcore-tutorial.txt| 19 +- Documentation/giteveryday.txt | 24 +- Documentation/githooks.txt| 8 +- Documentation/gittutorial.txt | 4 +- Documentation/gitworkflows.txt| 3 +- Documentation/revisions.txt | 2 +- Documentation/user-manual.txt | 56 +-- Makefile | 1 + advice.c | 11 +- builtin.h | 1 + builtin/checkout.c| 471 +- command-list.txt | 1 + contrib/completion/git-completion.bash| 27 + git.c | 1 + parse-options-cb.c| 17 + parse-options.h | 1 + sha1-name.c | 2 +- t/t1090-sparse-checkout-scope.sh | 14 - ...014-switch.sh => t2014-checkout-switch.sh} | 0 t/t2020-checkout-detach.sh| 16 +- t/t2060-switch.sh | 87 39 files changed, 837 insertions(+), 415 deletions(-) create mode 100644 Documentation/git-switch.txt rename t/{t2014-switch.sh => t2014-checkout-switch.sh} (100%) create mode 100755 t/t2060-switch.sh Range-diff dựa trên v2: -: -- > 1: 949f3dd4fd git-checkout.txt: spell out --no-option 1: 8358b9ca36 = 2: 1ddbbae3e2 git-checkout.txt: fix one syntax line 2: 1686ccbf8d ! 3: b0cb2372db doc: document --overwrite-ignore @@ -14,14 +14,15 @@ out anyway. In other words, the ref can be held by more than one worktree. -+--[no-]overwrite-ignore:: ++--overwrite-ignore:: ++--no-overwrite-ignore:: + Silently overwrite ignored files when switching branches. This -+ is the default behavior. Use --no-overwrite-ignore to abort ++ is the default behavior. Use `--no-overwrite-ignore` to abort + the operation when the new branch contains ignored files. + - --[no-]recurse-submodules:: + --recurse-submodules:: + --no-recurse-submodules:: Using --recurse-submodules will update the content of all initialized - submodules according to the commit recorded in the superproject. If diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt --- a/Documentation/git-merge.txt @@ -30,9 +31,10 @@ Allow the rerere mechanism to update the index with the result of auto-conflict resolution if possible. -+--[no-]overwrite-ignore:: ++--overwrite-ignore:: ++--no-overwrite-ignore:: + Silently overwrite ignored files from
[PATCH v3 03/21] doc: document --overwrite-ignore
I added this option in git-checkout and git-merge in c1d7036b6b (checkout,merge: disallow overwriting ignored files with --no-overwrite-ignore - 2011-11-27) but did not remember to update documentation. This completes that commit. --- Documentation/git-checkout.txt | 6 ++ Documentation/git-merge.txt| 5 + 2 files changed, 11 insertions(+) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 28817cfa41..5280d1f9ed 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -271,6 +271,12 @@ Note that this option uses the no overlay mode by default (see also out anyway. In other words, the ref can be held by more than one worktree. +--overwrite-ignore:: +--no-overwrite-ignore:: + Silently overwrite ignored files when switching branches. This + is the default behavior. Use `--no-overwrite-ignore` to abort + the operation when the new branch contains ignored files. + --recurse-submodules:: --no-recurse-submodules:: Using --recurse-submodules will update the content of all initialized diff --git a/Documentation/git-merge.txt b/Documentation/git-merge.txt index 4cc86469f3..6a9163d8fe 100644 --- a/Documentation/git-merge.txt +++ b/Documentation/git-merge.txt @@ -87,6 +87,11 @@ will be appended to the specified message. Allow the rerere mechanism to update the index with the result of auto-conflict resolution if possible. +--overwrite-ignore:: +--no-overwrite-ignore:: + Silently overwrite ignored files from the merge result. This + is the default behavior. Use `--no-overwrite-ignore` to abort. + --abort:: Abort the current conflict resolution process, and try to reconstruct the pre-merge state. -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v3 01/21] git-checkout.txt: spell out --no-option
It's easier to search for and also less cryptic. --- Documentation/git-checkout.txt | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index f179b43732..99c8c0dc0f 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -118,7 +118,8 @@ OPTIONS --quiet:: Quiet, suppress feedback messages. ---[no-]progress:: +--progress:: +--no-progress:: Progress status is reported on the standard error stream by default when it is attached to a terminal, unless `--quiet` is specified. This flag enables progress reporting even if not @@ -262,7 +263,7 @@ edits from your current working tree. See the ``Interactive Mode'' section of linkgit:git-add[1] to learn how to operate the `--patch` mode. + Note that this option uses the no overlay mode by default (see also -`--[no-]overlay`), and currently doesn't support overlay mode. +`--overlay`), and currently doesn't support overlay mode. --ignore-other-worktrees:: `git checkout` refuses when the wanted ref is already checked @@ -270,7 +271,8 @@ Note that this option uses the no overlay mode by default (see also out anyway. In other words, the ref can be held by more than one worktree. ---[no-]recurse-submodules:: +--recurse-submodules:: +--no-recurse-submodules:: Using --recurse-submodules will update the content of all initialized submodules according to the commit recorded in the superproject. If local modifications in a submodule would be overwritten the checkout @@ -283,7 +285,8 @@ Note that this option uses the no overlay mode by default (see also Do not attempt to create a branch if a remote tracking branch of the same name exists. ---[no-]overlay:: +--overlay:: +--no-overlay:: In the default overlay mode, `git checkout` never removes files from the index or the working tree. When specifying `--no-overlay`, files that appear in the index and -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v3 06/21] checkout: factor out some code in parse_branchname_arg()
This is in preparation for the new command restore, which also needs to parse opts->source_tree but does not need all the disambiguation logic. --- builtin/checkout.c | 51 -- 1 file changed, 31 insertions(+), 20 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 0e6037b296..cbdcb1417f 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1081,6 +1081,34 @@ static int git_checkout_config(const char *var, const char *value, void *cb) return git_xmerge_config(var, value, NULL); } +static void setup_new_branch_info_and_source_tree( + struct branch_info *new_branch_info, + struct checkout_opts *opts, + struct object_id *rev, + const char *arg) +{ + struct tree **source_tree = &opts->source_tree; + struct object_id branch_rev; + + new_branch_info->name = arg; + setup_branch_path(new_branch_info); + + if (!check_refname_format(new_branch_info->path, 0) && + !read_ref(new_branch_info->path, &branch_rev)) + oidcpy(rev, &branch_rev); + else + new_branch_info->path = NULL; /* not an existing branch */ + + new_branch_info->commit = lookup_commit_reference_gently(the_repository, rev, 1); + if (!new_branch_info->commit) { + /* not a commit */ + *source_tree = parse_tree_indirect(rev); + } else { + parse_commit_or_die(new_branch_info->commit); + *source_tree = get_commit_tree(new_branch_info->commit); + } +} + static int parse_branchname_arg(int argc, const char **argv, int dwim_new_local_branch_ok, struct branch_info *new_branch_info, @@ -1088,10 +1116,8 @@ static int parse_branchname_arg(int argc, const char **argv, struct object_id *rev, int *dwim_remotes_matched) { - struct tree **source_tree = &opts->source_tree; const char **new_branch = &opts->new_branch; int argcount = 0; - struct object_id branch_rev; const char *arg; int dash_dash_pos; int has_dash_dash = 0; @@ -1213,26 +1239,11 @@ static int parse_branchname_arg(int argc, const char **argv, argv++; argc--; - new_branch_info->name = arg; - setup_branch_path(new_branch_info); - - if (!check_refname_format(new_branch_info->path, 0) && - !read_ref(new_branch_info->path, &branch_rev)) - oidcpy(rev, &branch_rev); - else - new_branch_info->path = NULL; /* not an existing branch */ + setup_new_branch_info_and_source_tree(new_branch_info, opts, rev, arg); - new_branch_info->commit = lookup_commit_reference_gently(the_repository, rev, 1); - if (!new_branch_info->commit) { - /* not a commit */ - *source_tree = parse_tree_indirect(rev); - } else { - parse_commit_or_die(new_branch_info->commit); - *source_tree = get_commit_tree(new_branch_info->commit); - } - - if (!*source_tree) /* case (1): want a tree */ + if (!opts->source_tree) /* case (1): want a tree */ die(_("reference is not a tree: %s"), arg); + if (!has_dash_dash) { /* case (3).(d) -> (1) */ /* * Do not complain the most common case -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v3 05/21] t: rename t2014-switch.sh to t2014-checkout-switch.sh
The old name does not really say that this is about 'checkout -b'. See 49d833dc07 (Revert "checkout branch: prime cache-tree fully" - 2009-05-12) for more information --- t/{t2014-switch.sh => t2014-checkout-switch.sh} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename t/{t2014-switch.sh => t2014-checkout-switch.sh} (100%) diff --git a/t/t2014-switch.sh b/t/t2014-checkout-switch.sh similarity index 100% rename from t/t2014-switch.sh rename to t/t2014-checkout-switch.sh -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v3 04/21] git-checkout.txt: fix monospace typeset
--- Documentation/git-checkout.txt | 60 +- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 5280d1f9ed..1b9d689933 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -24,14 +24,14 @@ also update `HEAD` to set the specified branch as the current branch. 'git checkout' []:: - To prepare for working on , switch to it by updating + To prepare for working on ``, switch to it by updating the index and the files in the working tree, and by pointing HEAD at the branch. Local modifications to the files in the working tree are kept, so that they can be committed to the - . + ``. + -If is not found but there does exist a tracking branch in -exactly one remote (call it ) with a matching name, treat as +If `` is not found but there does exist a tracking branch in +exactly one remote (call it ``) with a matching name, treat as equivalent to + @@ -47,7 +47,7 @@ branches from there if `` is ambiguous but exists on the 'origin' remote. See also `checkout.defaultRemote` in linkgit:git-config[1]. + -You could omit , in which case the command degenerates to +You could omit ``, in which case the command degenerates to "check out the current branch", which is a glorified no-op with rather expensive side-effects to show only the tracking information, if exists, for the current branch. @@ -61,7 +61,7 @@ if exists, for the current branch. `--track` without `-b` implies branch creation; see the description of `--track` below. + -If `-B` is given, is created if it doesn't exist; otherwise, it +If `-B` is given, `` is created if it doesn't exist; otherwise, it is reset. This is the transactional equivalent of + @@ -75,25 +75,25 @@ successful. 'git checkout' --detach []:: 'git checkout' [--detach] :: - Prepare to work on top of , by detaching HEAD at it + Prepare to work on top of ``, by detaching HEAD at it (see "DETACHED HEAD" section), and updating the index and the files in the working tree. Local modifications to the files in the working tree are kept, so that the resulting working tree will be the state recorded in the commit plus the local modifications. + -When the argument is a branch name, the `--detach` option can +When the `` argument is a branch name, the `--detach` option can be used to detach HEAD at the tip of the branch (`git checkout -` would check out that branch without detaching HEAD). +``` would check out that branch without detaching HEAD). + -Omitting detaches HEAD at the tip of the current branch. +Omitting `` detaches HEAD at the tip of the current branch. 'git checkout' [] [--] ...:: Overwrite paths in the working tree by replacing with the - contents in the index or in the (most often a - commit). When a is given, the paths that - match the are updated both in the index and in + contents in the index or in the `` (most often a + commit). When a `` is given, the paths that + match the `` are updated both in the index and in the working tree. + The index may contain unmerged entries because of a previous failed merge. @@ -155,12 +155,12 @@ on your side branch as `theirs` (i.e. "one contributor's work on top of it"). -b :: - Create a new branch named and start it at - ; see linkgit:git-branch[1] for details. + Create a new branch named `` and start it at + ``; see linkgit:git-branch[1] for details. -B :: - Creates the branch and start it at ; - if it already exists, then reset it to . This is + Creates the branch `` and start it at ``; + if it already exists, then reset it to ``. This is equivalent to running "git branch" with "-f"; see linkgit:git-branch[1] for details. @@ -191,19 +191,19 @@ explicitly give a name with `-b` in such a case. Rather than checking out a branch to work on it, check out a commit for inspection and discardable experiments. This is the default behavior of "git checkout " when -is not a branch name. See the "DETACHED HEAD" section + `` is not a branch name. See the "DETACHED HEAD" section below for details. --orphan :: - Create a new 'orphan' branch, named , started from -and switch to it. The first commit made on this + Create a new 'orphan' branch, named ``, started from + `` and switch to it. The first commit made on this new branch will have no parents and it will be the root of a new history totally disconnected from all the other branches and commits. + The index and the working tree are adjusted as if you had previously run "git checkout ". This allows you to start a new history -that records a set of paths similar
[PATCH v3 12/21] switch: remove -l
This option is ancient. Nowadays reflog is enabled by default and automatically created for new branches. Keep it in git-checkout only. --- builtin/checkout.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 4c3f0f6ac7..a731f983c4 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1372,7 +1372,6 @@ static struct option *add_common_switch_branch_options( struct checkout_opts *opts, struct option *prevopts) { struct option options[] = { - OPT_BOOL('l', NULL, &opts->new_branch_log, N_("create reflog for new branch")), OPT_BOOL(0, "detach", &opts->force_detach, N_("detach HEAD at named commit")), OPT_SET_INT('t', "track", &opts->track, N_("set upstream info for new branch"), BRANCH_TRACK_EXPLICIT), @@ -1573,6 +1572,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) N_("create and checkout a new branch")), OPT_STRING('B', NULL, &opts.new_branch_force, N_("branch"), N_("create/reset and checkout a branch")), + OPT_BOOL('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")), OPT_END() }; int ret; -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v3 08/21] checkout: move 'confict_style' and 'dwim_..' to checkout_opts
These local variables are referenced by struct option[]. This struct will soon be broken down, moved away and we can't rely on local variables anymore. Move these two to struct checkout_opts in preparation for that. --- builtin/checkout.c | 17 ++--- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index baefe3c942..4b43433941 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -47,6 +47,8 @@ struct checkout_opts { int show_progress; int count_checkout_paths; int overlay_mode; + int no_dwim_new_local_branch; + /* * If new checkout options are added, skip_merge_working_tree * should be updated accordingly. @@ -58,6 +60,7 @@ struct checkout_opts { int new_branch_log; enum branch_track track; struct diff_options diff_options; + char *conflict_style; int branch_exists; const char *prefix; @@ -1344,8 +1347,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) struct checkout_opts real_opts; struct checkout_opts *opts = &real_opts; struct branch_info new_branch_info; - char *conflict_style = NULL; - int dwim_new_local_branch, no_dwim_new_local_branch = 0; + int dwim_new_local_branch; int dwim_remotes_matched = 0; struct option options[] = { OPT__QUIET(&opts->quiet, N_("suppress progress reporting")), @@ -1370,12 +1372,12 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) OPT_BOOL_F(0, "overwrite-ignore", &opts->overwrite_ignore, N_("update ignored files (default)"), PARSE_OPT_NOCOMPLETE), - OPT_STRING(0, "conflict", &conflict_style, N_("style"), + OPT_STRING(0, "conflict", &opts->conflict_style, N_("style"), N_("conflict style (merge or diff3)")), OPT_BOOL('p', "patch", &opts->patch_mode, N_("select hunks interactively")), OPT_BOOL(0, "ignore-skip-worktree-bits", &opts->ignore_skipworktree, N_("do not limit pathspecs to sparse entries only")), - OPT_BOOL(0, "no-guess", &no_dwim_new_local_branch, + OPT_BOOL(0, "no-guess", &opts->no_dwim_new_local_branch, N_("do not second guess 'git checkout '")), OPT_BOOL(0, "ignore-other-worktrees", &opts->ignore_other_worktrees, N_("do not check if another worktree is holding the given ref")), @@ -1393,6 +1395,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) opts->prefix = prefix; opts->show_progress = -1; opts->overlay_mode = -1; + opts->no_dwim_new_local_branch = 0; git_config(git_checkout_config, opts); @@ -1401,7 +1404,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, checkout_usage, PARSE_OPT_KEEP_DASHDASH); - dwim_new_local_branch = !no_dwim_new_local_branch; + dwim_new_local_branch = !opts->no_dwim_new_local_branch; if (opts->show_progress < 0) { if (opts->quiet) opts->show_progress = 0; @@ -1409,9 +1412,9 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) opts->show_progress = isatty(2); } - if (conflict_style) { + if (opts->conflict_style) { opts->merge = 1; /* implied */ - git_xmerge_config("merge.conflictstyle", conflict_style, NULL); + git_xmerge_config("merge.conflictstyle", opts->conflict_style, NULL); } if ((!!opts->new_branch + !!opts->new_branch_force + !!opts->new_orphan_branch) > 1) -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v3 13/21] switch: stop accepting pathspec
This command is about switching branch (or creating a new one) and should not accept pathspec. This helps simplify ambiguation handling. The other two ("git checkout" and "git restore") of course do accept pathspec as before. --- builtin/checkout.c | 20 +++- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index a731f983c4..1b1181b220 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -53,6 +53,7 @@ struct checkout_opts { int count_checkout_paths; int overlay_mode; int no_dwim_new_local_branch; + int accept_pathspec; /* * If new checkout options are added, skip_merge_working_tree @@ -1175,10 +1176,16 @@ static int parse_branchname_arg(int argc, const char **argv, if (!argc) return 0; + if (!opts->accept_pathspec) { + if (argc > 1) + die(_("only one reference expected")); + has_dash_dash = 1; /* helps disambiguate */ + } + arg = argv[0]; dash_dash_pos = -1; for (i = 0; i < argc; i++) { - if (!strcmp(argv[i], "--")) { + if (opts->accept_pathspec && !strcmp(argv[i], "--")) { dash_dash_pos = i; break; } @@ -1212,11 +1219,12 @@ static int parse_branchname_arg(int argc, const char **argv, recover_with_dwim = 0; /* -* Accept "git checkout foo" and "git checkout foo --" -* as candidates for dwim. +* Accept "git checkout foo", "git checkout foo --" +* and "git switch foo" as candidates for dwim. */ if (!(argc == 1 && !has_dash_dash) && - !(argc == 2 && has_dash_dash)) + !(argc == 2 && has_dash_dash) && + opts->accept_pathspec) recover_with_dwim = 0; if (recover_with_dwim) { @@ -1261,7 +1269,7 @@ static int parse_branchname_arg(int argc, const char **argv, */ if (argc) verify_non_filename(opts->prefix, arg); - } else { + } else if (opts->accept_pathspec) { argcount++; argv++; argc--; @@ -1579,6 +1587,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) memset(&opts, 0, sizeof(opts)); opts.no_dwim_new_local_branch = 0; + opts.accept_pathspec = 1; options = parse_options_dup(checkout_options); options = add_common_options(&opts, options); @@ -1606,6 +1615,7 @@ int cmd_switch(int argc, const char **argv, const char *prefix) memset(&opts, 0, sizeof(opts)); opts.no_dwim_new_local_branch = 0; + opts.accept_pathspec = 0; options = parse_options_dup(switch_options); options = add_common_options(&opts, options); -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v3 14/21] switch: reject "do nothing" case
"git checkout" can be executed without any arguments. What it does is not exactly great: it switches from HEAD to HEAD and shows worktree modification as a side effect. Make switch reject this case. Just use "git status" if you want that side effect. For switch, you have to either - really switch a branch - (explicitly) detach from the current branch - create a new branch --- builtin/checkout.c | 9 + 1 file changed, 9 insertions(+) diff --git a/builtin/checkout.c b/builtin/checkout.c index 1b1181b220..f9f7ee2936 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -54,6 +54,7 @@ struct checkout_opts { int overlay_mode; int no_dwim_new_local_branch; int accept_pathspec; + int switch_branch_doing_nothing_is_ok; /* * If new checkout options are added, skip_merge_working_tree @@ -1334,6 +1335,12 @@ static int checkout_branch(struct checkout_opts *opts, die(_("Cannot switch branch to a non-commit '%s'"), new_branch_info->name); + if (!opts->switch_branch_doing_nothing_is_ok && + !new_branch_info->name && + !opts->new_branch && + !opts->force_detach) + die(_("missing branch or commit argument")); + if (new_branch_info->path && !opts->force_detach && !opts->new_branch && !opts->ignore_other_worktrees) { int flag; @@ -1587,6 +1594,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) memset(&opts, 0, sizeof(opts)); opts.no_dwim_new_local_branch = 0; + opts.switch_branch_doing_nothing_is_ok = 1; opts.accept_pathspec = 1; options = parse_options_dup(checkout_options); @@ -1616,6 +1624,7 @@ int cmd_switch(int argc, const char **argv, const char *prefix) memset(&opts, 0, sizeof(opts)); opts.no_dwim_new_local_branch = 0; opts.accept_pathspec = 0; + opts.switch_branch_doing_nothing_is_ok = 0; options = parse_options_dup(switch_options); options = add_common_options(&opts, options); -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v3 15/21] switch: only allow explicit detached HEAD
"git checkout " will checkout the commit in question and detach HEAD from the current branch. It is naturally a right thing to do once you get git references. But detached HEAD is a scary concept to new users because we show a lot of warnings and stuff, and it could be hard to get out of (until you know better). To keep switch a bit more friendly to new users, we only allow entering detached HEAD mode when --detach is given. "git switch" must take a branch (unless you create a new branch, then of course switch can take any commit-ish) --- builtin/checkout.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/builtin/checkout.c b/builtin/checkout.c index f9f7ee2936..2e150f0175 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -45,6 +45,7 @@ struct checkout_opts { int merge; int force; int force_detach; + int implicit_detach; int writeout_stage; int overwrite_ignore; int ignore_skipworktree; @@ -1341,6 +1342,14 @@ static int checkout_branch(struct checkout_opts *opts, !opts->force_detach) die(_("missing branch or commit argument")); + if (!opts->implicit_detach && + !opts->force_detach && + !opts->new_branch && + !opts->new_branch_force && + new_branch_info->name && + !new_branch_info->path) + die(_("a branch is expected, got %s"), new_branch_info->name); + if (new_branch_info->path && !opts->force_detach && !opts->new_branch && !opts->ignore_other_worktrees) { int flag; @@ -1596,6 +1605,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) opts.no_dwim_new_local_branch = 0; opts.switch_branch_doing_nothing_is_ok = 1; opts.accept_pathspec = 1; + opts.implicit_detach = 1; options = parse_options_dup(checkout_options); options = add_common_options(&opts, options); @@ -1625,6 +1635,7 @@ int cmd_switch(int argc, const char **argv, const char *prefix) opts.no_dwim_new_local_branch = 0; opts.accept_pathspec = 0; opts.switch_branch_doing_nothing_is_ok = 0; + opts.implicit_detach = 0; options = parse_options_dup(switch_options); options = add_common_options(&opts, options); -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v3 17/21] switch: no implicit dwim, use --guess to dwim
Similar to automatic detach, this behavior could be confusing because it can sometimes create a new branch without a user asking it to, especially when the user is still not aware about this feature. In the future, perhaps we could have a config key to disable these safety nets and let 'switch' do automatic detach or dwim again. But that will be opt-in after the user knows what is what. For now give a short option if you want to use it often. --- Documentation/git-checkout.txt | 38 -- builtin/checkout.c | 16 +++--- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index ac355dc3f3..2b776c1269 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -31,22 +31,13 @@ branch. ``. + If `` is not found but there does exist a tracking branch in -exactly one remote (call it ``) with a matching name, treat as -equivalent to +exactly one remote (call it ``) with a matching name and +`--no-guess` is not specified, treat as equivalent to + $ git checkout -b --track / + -If the branch exists in multiple remotes and one of them is named by -the `checkout.defaultRemote` configuration variable, we'll use that -one for the purposes of disambiguation, even if the `` isn't -unique across all remotes. Set it to -e.g. `checkout.defaultRemote=origin` to always checkout remote -branches from there if `` is ambiguous but exists on the -'origin' remote. See also `checkout.defaultRemote` in -linkgit:git-config[1]. -+ You could omit ``, in which case the command degenerates to "check out the current branch", which is a glorified no-op with rather expensive side-effects to show only the tracking information, @@ -183,6 +174,27 @@ explicitly give a name with `-b` in such a case. Do not set up "upstream" configuration, even if the branch.autoSetupMerge configuration variable is true. +--guess:: +--no-guess:: + If `` is not found but there does exist a tracking + branch in exactly one remote (call it ``) with a + matching name, treat as equivalent to ++ + +$ git checkout -b --track / + ++ +If the branch exists in multiple remotes and one of them is named by +the `checkout.defaultRemote` configuration variable, we'll use that +one for the purposes of disambiguation, even if the `` isn't +unique across all remotes. Set it to +e.g. `checkout.defaultRemote=origin` to always checkout remote +branches from there if `` is ambiguous but exists on the +'origin' remote. See also `checkout.defaultRemote` in +linkgit:git-config[1]. ++ +Use `--no-guess` to disable this. + -l:: Create the new branch's reflog; see linkgit:git-branch[1] for details. @@ -287,10 +299,6 @@ Note that this option uses the no overlay mode by default (see also Just like linkgit:git-submodule[1], this will detach the submodules HEAD. ---no-guess:: - Do not attempt to create a branch if a remote tracking branch - of the same name exists. - --overlay:: --no-overlay:: In the default overlay mode, `git checkout` never diff --git a/builtin/checkout.c b/builtin/checkout.c index 0866aeba83..8a89df4f36 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -53,7 +53,7 @@ struct checkout_opts { int show_progress; int count_checkout_paths; int overlay_mode; - int no_dwim_new_local_branch; + int dwim_new_local_branch; int accept_pathspec; int switch_branch_doing_nothing_is_ok; @@ -1403,8 +1403,6 @@ static struct option *add_common_switch_branch_options( OPT_BOOL_F(0, "overwrite-ignore", &opts->overwrite_ignore, N_("update ignored files (default)"), PARSE_OPT_NOCOMPLETE), - OPT_BOOL(0, "no-guess", &opts->no_dwim_new_local_branch, -N_("second guess 'git checkout '")), OPT_BOOL(0, "ignore-other-worktrees", &opts->ignore_other_worktrees, N_("do not check if another worktree is holding the given ref")), OPT_END() @@ -1441,7 +1439,6 @@ static int checkout_main(int argc, const char **argv, const char *prefix, { struct branch_info new_branch_info; int dwim_remotes_matched = 0; - int dwim_new_local_branch; memset(&new_branch_info, 0, sizeof(new_branch_info)); opts->overwrite_ignore = 1; @@ -1456,7 +1453,6 @@ static int checkout_main(int argc, const char **argv, const char *prefix, argc = parse_options(argc, argv, prefix, options, usagestr, PARSE_OPT_KEEP_DASHDASH); - dwim_new_local_branch = !opts->no_dwim_new_local_branch; if (opts->show_progress < 0) { if (opts->quiet) opts->show_progress = 0; @@ -1516,7 +1512,7 @@ stati
[PATCH v3 16/21] switch: add short option for --detach
"git checkout" automatically detaches branches and --detach is not that useful (--no-detach is more likely). But for "switch", you may want to use it more often once you're used to detached HEAD. This of course adds -d to git-checkout but it does not harm (yet?) to do it. --- builtin/checkout.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 2e150f0175..0866aeba83 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1396,7 +1396,7 @@ static struct option *add_common_switch_branch_options( struct checkout_opts *opts, struct option *prevopts) { struct option options[] = { - OPT_BOOL(0, "detach", &opts->force_detach, N_("detach HEAD at named commit")), + OPT_BOOL('d', "detach", &opts->force_detach, N_("detach HEAD at named commit")), OPT_SET_INT('t', "track", &opts->track, N_("set upstream info for new branch"), BRANCH_TRACK_EXPLICIT), OPT_STRING(0, "orphan", &opts->new_orphan_branch, N_("new-branch"), N_("new unparented branch")), -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v3 10/21] checkout: split part of it to new command 'switch'
"git checkout" doing too many things is a source of confusion for many users (and it even bites old timers sometimes). To remedy that, the command will be split into two new ones: switch and something-to-checkout-paths. The good old "git checkout" command is still here and will be until all (or most of users) are sick of it. See the new man page for the final design of switch. The actual implementation though is still pretty much the same as "git checkout" and not completely aligned with the man page. Following patches will adjust their behavior to match the man page. --- .gitignore| 1 + Documentation/config/advice.txt | 13 +- Documentation/config/branch.txt | 4 +- Documentation/config/checkout.txt | 9 +- Documentation/config/diff.txt | 3 +- Documentation/git-checkout.txt| 4 + Documentation/git-switch.txt | 259 ++ Documentation/gitattributes.txt | 3 +- Documentation/githooks.txt| 8 +- Makefile | 1 + builtin.h | 1 + builtin/checkout.c| 60 +-- command-list.txt | 1 + git.c | 1 + 14 files changed, 341 insertions(+), 27 deletions(-) create mode 100644 Documentation/git-switch.txt diff --git a/.gitignore b/.gitignore index 7374587f9d..c687b92b1c 100644 --- a/.gitignore +++ b/.gitignore @@ -167,6 +167,7 @@ /git-submodule /git-submodule--helper /git-svn +/git-switch /git-symbolic-ref /git-tag /git-unpack-file diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt index 88620429ea..239d479506 100644 --- a/Documentation/config/advice.txt +++ b/Documentation/config/advice.txt @@ -42,7 +42,8 @@ advice.*:: state in the output of linkgit:git-status[1], in the template shown when writing commit messages in linkgit:git-commit[1], and in the help message shown - by linkgit:git-checkout[1] when switching branch. + by linkgit:git-switch[1] or + linkgit:git-checkout[1] when switching branch. statusUoption:: Advise to consider using the `-u` option to linkgit:git-status[1] when the command takes more than 2 seconds to enumerate untracked @@ -62,12 +63,14 @@ advice.*:: your information is guessed from the system username and domain name. detachedHead:: - Advice shown when you used linkgit:git-checkout[1] to - move to the detach HEAD state, to instruct how to create - a local branch after the fact. + Advice shown when you used + linkgit:git-switch[1] or linkgit:git-checkout[1] + to move to the detach HEAD state, to instruct how to + create a local branch after the fact. checkoutAmbiguousRemoteBranchName:: Advice shown when the argument to - linkgit:git-checkout[1] ambiguously resolves to a + linkgit:git-checkout[1] and linkgit:git-switch[1] + ambiguously resolves to a remote tracking branch on more than one remote in situations where an unambiguous argument would have otherwise caused a remote-tracking branch to be diff --git a/Documentation/config/branch.txt b/Documentation/config/branch.txt index 019d60ede2..8050466159 100644 --- a/Documentation/config/branch.txt +++ b/Documentation/config/branch.txt @@ -1,5 +1,5 @@ branch.autoSetupMerge:: - Tells 'git branch' and 'git checkout' to set up new branches + Tells 'git branch', 'git switch' and 'git checkout' to set up new branches so that linkgit:git-pull[1] will appropriately merge from the starting point branch. Note that even if this option is not set, this behavior can be chosen per-branch using the `--track` @@ -11,7 +11,7 @@ branch.autoSetupMerge:: branch. This option defaults to true. branch.autoSetupRebase:: - When a new branch is created with 'git branch' or 'git checkout' + When a new branch is created with 'git branch', 'git switch' or 'git checkout' that tracks another branch, this variable tells Git to set up pull to rebase instead of merge (see "branch..rebase"). When `never`, rebase is never automatically set to true. diff --git a/Documentation/config/checkout.txt b/Documentation/config/checkout.txt index c4118fa196..d6872ffa83 100644 --- a/Documentation/config/checkout.txt +++ b/Documentation/config/checkout.txt @@ -1,5 +1,6 @@ checkout.defaultRemote:: - When you run 'git checkout ' and only have one + When you run 'git checkout ' + or 'git switch ' and only have one remote, it may implicitly fall back on checking out and tracking e.g. 'origin/'. This stops working as soon as you have more than one re
[PATCH v3 09/21] checkout: split options[] array in three pieces
This is a preparation step for introducing new commands that do parts of what checkout does. There will be two new commands, one is about switching branches, detaching HEAD... one about checking out paths. These share the a subset of command line options. The rest of command line options are separate. --- builtin/checkout.c | 82 +- parse-options-cb.c | 17 ++ parse-options.h| 1 + 3 files changed, 77 insertions(+), 23 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 4b43433941..7d23083282 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1342,15 +1342,31 @@ static int checkout_branch(struct checkout_opts *opts, return switch_branches(opts, new_branch_info); } -int cmd_checkout(int argc, const char **argv, const char *prefix) +static struct option *add_common_options(struct checkout_opts *opts, +struct option *prevopts) { - struct checkout_opts real_opts; - struct checkout_opts *opts = &real_opts; - struct branch_info new_branch_info; - int dwim_new_local_branch; - int dwim_remotes_matched = 0; struct option options[] = { OPT__QUIET(&opts->quiet, N_("suppress progress reporting")), + { OPTION_CALLBACK, 0, "recurse-submodules", NULL, + "checkout", "control recursive updating of submodules", + PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater }, + OPT_BOOL(0, "progress", &opts->show_progress, N_("force progress reporting")), + OPT__FORCE(&opts->force, N_("force checkout (throw away local modifications)"), + PARSE_OPT_NOCOMPLETE), + OPT_BOOL('m', "merge", &opts->merge, N_("perform a 3-way merge with the new branch")), + OPT_STRING(0, "conflict", &opts->conflict_style, N_("style"), + N_("conflict style (merge or diff3)")), + OPT_END() + }; + struct option *newopts = parse_options_concat(prevopts, options); + free(prevopts); + return newopts; +} + +static struct option *add_switch_branch_options(struct checkout_opts *opts, + struct option *prevopts) +{ + struct option options[] = { OPT_STRING('b', NULL, &opts->new_branch, N_("branch"), N_("create and checkout a new branch")), OPT_STRING('B', NULL, &opts->new_branch_force, N_("branch"), @@ -1360,34 +1376,49 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) OPT_SET_INT('t', "track", &opts->track, N_("set upstream info for new branch"), BRANCH_TRACK_EXPLICIT), OPT_STRING(0, "orphan", &opts->new_orphan_branch, N_("new-branch"), N_("new unparented branch")), + OPT_BOOL_F(0, "overwrite-ignore", &opts->overwrite_ignore, + N_("update ignored files (default)"), + PARSE_OPT_NOCOMPLETE), + OPT_BOOL(0, "no-guess", &opts->no_dwim_new_local_branch, +N_("second guess 'git checkout '")), + OPT_BOOL(0, "ignore-other-worktrees", &opts->ignore_other_worktrees, +N_("do not check if another worktree is holding the given ref")), + OPT_END() + }; + struct option *newopts = parse_options_concat(prevopts, options); + free(prevopts); + return newopts; +} + +static struct option *add_checkout_path_options(struct checkout_opts *opts, + struct option *prevopts) +{ + struct option options[] = { OPT_SET_INT_F('2', "ours", &opts->writeout_stage, N_("checkout our version for unmerged files"), 2, PARSE_OPT_NONEG), OPT_SET_INT_F('3', "theirs", &opts->writeout_stage, N_("checkout their version for unmerged files"), 3, PARSE_OPT_NONEG), - OPT__FORCE(&opts->force, N_("force checkout (throw away local modifications)"), - PARSE_OPT_NOCOMPLETE), - OPT_BOOL('m', "merge", &opts->merge, N_("perform a 3-way merge with the new branch")), - OPT_BOOL_F(0, "overwrite-ignore", &opts->overwrite_ignore, - N_("update ignored files (default)"), - PARSE_OPT_NOCOMPLETE), - OPT_STRING(0, "conflict", &opts->conflict_style, N_("style"), - N_("conflict style (merge or diff3)")), OPT_BOOL('p', "patch", &opts->patch_mode, N_("select hunks interactively")), OPT_BOOL(0, "ignore-skip-worktree-bits", &opts->ignore_skipworktree,
[PATCH v3 11/21] switch: better names for -b and -B
The shortcut of these options do not make much sense when used with switch. And their descriptions are also tied to checkout. Move -b/-B to cmd_checkout() and new -c/-C with the same functionality in cmd_switch_branch() --- builtin/checkout.c | 32 +--- 1 file changed, 21 insertions(+), 11 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 1eff10dbef..4c3f0f6ac7 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1368,14 +1368,10 @@ static struct option *add_common_options(struct checkout_opts *opts, return newopts; } -static struct option *add_switch_branch_options(struct checkout_opts *opts, - struct option *prevopts) +static struct option *add_common_switch_branch_options( + struct checkout_opts *opts, struct option *prevopts) { struct option options[] = { - OPT_STRING('b', NULL, &opts->new_branch, N_("branch"), - N_("create and checkout a new branch")), - OPT_STRING('B', NULL, &opts->new_branch_force, N_("branch"), - N_("create/reset and checkout a branch")), OPT_BOOL('l', NULL, &opts->new_branch_log, N_("create reflog for new branch")), OPT_BOOL(0, "detach", &opts->force_detach, N_("detach HEAD at named commit")), OPT_SET_INT('t', "track", &opts->track, N_("set upstream info for new branch"), @@ -1571,15 +1567,22 @@ static int checkout_main(int argc, const char **argv, const char *prefix, int cmd_checkout(int argc, const char **argv, const char *prefix) { struct checkout_opts opts; - struct option *options = NULL; + struct option *options; + struct option checkout_options[] = { + OPT_STRING('b', NULL, &opts.new_branch, N_("branch"), + N_("create and checkout a new branch")), + OPT_STRING('B', NULL, &opts.new_branch_force, N_("branch"), + N_("create/reset and checkout a branch")), + OPT_END() + }; int ret; memset(&opts, 0, sizeof(opts)); opts.no_dwim_new_local_branch = 0; - options = parse_options_dup(options); + options = parse_options_dup(checkout_options); options = add_common_options(&opts, options); - options = add_switch_branch_options(&opts, options); + options = add_common_switch_branch_options(&opts, options); options = add_checkout_path_options(&opts, options); ret = checkout_main(argc, argv, prefix, &opts, @@ -1592,14 +1595,21 @@ int cmd_switch(int argc, const char **argv, const char *prefix) { struct checkout_opts opts; struct option *options = NULL; + struct option switch_options[] = { + OPT_STRING('c', "create", &opts.new_branch, N_("branch"), + N_("create and switch to a new branch")), + OPT_STRING('C', "force-create", &opts.new_branch_force, N_("branch"), + N_("create/reset and switch to a branch")), + OPT_END() + }; int ret; memset(&opts, 0, sizeof(opts)); opts.no_dwim_new_local_branch = 0; - options = parse_options_dup(options); + options = parse_options_dup(switch_options); options = add_common_options(&opts, options); - options = add_switch_branch_options(&opts, options); + options = add_common_switch_branch_options(&opts, options); ret = checkout_main(argc, argv, prefix, &opts, options, switch_branch_usage); -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v3 07/21] checkout: make "opts" in cmd_checkout() a pointer
"opts" will soon be moved out of cmd_checkout(). To keep changes in that patch smaller, convert "opts" to a pointer and keep the real thing behind "real_opts". --- builtin/checkout.c | 115 +++-- 1 file changed, 58 insertions(+), 57 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index cbdcb1417f..baefe3c942 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1341,82 +1341,83 @@ static int checkout_branch(struct checkout_opts *opts, int cmd_checkout(int argc, const char **argv, const char *prefix) { - struct checkout_opts opts; + struct checkout_opts real_opts; + struct checkout_opts *opts = &real_opts; struct branch_info new_branch_info; char *conflict_style = NULL; int dwim_new_local_branch, no_dwim_new_local_branch = 0; int dwim_remotes_matched = 0; struct option options[] = { - OPT__QUIET(&opts.quiet, N_("suppress progress reporting")), - OPT_STRING('b', NULL, &opts.new_branch, N_("branch"), + OPT__QUIET(&opts->quiet, N_("suppress progress reporting")), + OPT_STRING('b', NULL, &opts->new_branch, N_("branch"), N_("create and checkout a new branch")), - OPT_STRING('B', NULL, &opts.new_branch_force, N_("branch"), + OPT_STRING('B', NULL, &opts->new_branch_force, N_("branch"), N_("create/reset and checkout a branch")), - OPT_BOOL('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")), - OPT_BOOL(0, "detach", &opts.force_detach, N_("detach HEAD at named commit")), - OPT_SET_INT('t', "track", &opts.track, N_("set upstream info for new branch"), + OPT_BOOL('l', NULL, &opts->new_branch_log, N_("create reflog for new branch")), + OPT_BOOL(0, "detach", &opts->force_detach, N_("detach HEAD at named commit")), + OPT_SET_INT('t', "track", &opts->track, N_("set upstream info for new branch"), BRANCH_TRACK_EXPLICIT), - OPT_STRING(0, "orphan", &opts.new_orphan_branch, N_("new-branch"), N_("new unparented branch")), - OPT_SET_INT_F('2', "ours", &opts.writeout_stage, + OPT_STRING(0, "orphan", &opts->new_orphan_branch, N_("new-branch"), N_("new unparented branch")), + OPT_SET_INT_F('2', "ours", &opts->writeout_stage, N_("checkout our version for unmerged files"), 2, PARSE_OPT_NONEG), - OPT_SET_INT_F('3', "theirs", &opts.writeout_stage, + OPT_SET_INT_F('3', "theirs", &opts->writeout_stage, N_("checkout their version for unmerged files"), 3, PARSE_OPT_NONEG), - OPT__FORCE(&opts.force, N_("force checkout (throw away local modifications)"), + OPT__FORCE(&opts->force, N_("force checkout (throw away local modifications)"), PARSE_OPT_NOCOMPLETE), - OPT_BOOL('m', "merge", &opts.merge, N_("perform a 3-way merge with the new branch")), - OPT_BOOL_F(0, "overwrite-ignore", &opts.overwrite_ignore, + OPT_BOOL('m', "merge", &opts->merge, N_("perform a 3-way merge with the new branch")), + OPT_BOOL_F(0, "overwrite-ignore", &opts->overwrite_ignore, N_("update ignored files (default)"), PARSE_OPT_NOCOMPLETE), OPT_STRING(0, "conflict", &conflict_style, N_("style"), N_("conflict style (merge or diff3)")), - OPT_BOOL('p', "patch", &opts.patch_mode, N_("select hunks interactively")), - OPT_BOOL(0, "ignore-skip-worktree-bits", &opts.ignore_skipworktree, + OPT_BOOL('p', "patch", &opts->patch_mode, N_("select hunks interactively")), + OPT_BOOL(0, "ignore-skip-worktree-bits", &opts->ignore_skipworktree, N_("do not limit pathspecs to sparse entries only")), OPT_BOOL(0, "no-guess", &no_dwim_new_local_branch, N_("do not second guess 'git checkout '")), - OPT_BOOL(0, "ignore-other-worktrees", &opts.ignore_other_worktrees, + OPT_BOOL(0, "ignore-other-worktrees", &opts->ignore_other_worktrees, N_("do not check if another worktree is holding the given ref")), { OPTION_CALLBACK, 0, "recurse-submodules", NULL, "checkout", "control recursive updating of submodules", PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater }, - OPT_BOOL(0, "progress", &opts.show_progress, N_("force progress reporting")), - OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mo
[PATCH v3 20/21] completion: support switch
Completion support for --guess could be made better. If no --detach is given, we should only provide a list of refs/heads/* and dwim ones, not the entire ref space. But I still can't penetrate that __git_refs() function yet. --- contrib/completion/git-completion.bash | 27 ++ 1 file changed, 27 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 976e4a6548..7fcf28d437 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2158,6 +2158,33 @@ _git_status () __git_complete_index_file "$complete_opt" } +_git_switch () +{ + case "$cur" in + --conflict=*) + __gitcomp "diff3 merge" "" "${cur##--conflict=}" + ;; + --*) + __gitcomp_builtin switch + ;; + *) + # check if ---guess was specified to enable DWIM mode + local track_opt= only_local_ref=n + if [ -n "$(__git_find_on_cmdline "-g --guess")" ]; then + track_opt='--track' + fi + if [ -z "$(__git_find_on_cmdline "-d --detach")" ]; then + only_local_ref=y + fi + if [ $only_local_ref = y -a -n "$track_opt"]; then + __gitcomp_direct "$(__git_heads "" "$cur" " ")" + else + __git_complete_refs $track_opt + fi + ;; + esac +} + __git_config_get_set_variables () { local prevword word config_file= c=$cword -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v3 21/21] doc: promote "git switch"
The new command "git switch" is added to avoid the confusion of one-command-do-all "git checkout" for new users. They are also helpful to avoid ambiguation context. For these reasons, promote it everywhere possible. This includes documentation, suggestions/advice from other commands... --- Documentation/git-branch.txt | 12 +++--- Documentation/git-check-ref-format.txt | 3 +- Documentation/git-format-patch.txt | 2 +- Documentation/git-merge-base.txt | 2 +- Documentation/git-rebase.txt | 2 +- Documentation/git-remote.txt | 2 +- Documentation/git-rerere.txt | 10 ++--- Documentation/git-reset.txt| 20 - Documentation/git-stash.txt| 9 +++-- Documentation/gitcore-tutorial.txt | 19 + Documentation/giteveryday.txt | 24 +-- Documentation/gittutorial.txt | 4 +- Documentation/gitworkflows.txt | 3 +- Documentation/revisions.txt| 2 +- Documentation/user-manual.txt | 56 +- advice.c | 11 +++-- sha1-name.c| 2 +- t/t2020-checkout-detach.sh | 16 18 files changed, 101 insertions(+), 98 deletions(-) diff --git a/Documentation/git-branch.txt b/Documentation/git-branch.txt index 0cd87ddeff..1e2d89b174 100644 --- a/Documentation/git-branch.txt +++ b/Documentation/git-branch.txt @@ -48,7 +48,7 @@ The command's second form creates a new branch head named which points to the current `HEAD`, or if given. Note that this will create the new branch, but it will not switch the -working tree to it; use "git checkout " to switch to the +working tree to it; use "git switch " to switch to the new branch. When a local branch is started off a remote-tracking branch, Git sets up the @@ -198,7 +198,7 @@ This option is only applicable in non-verbose mode. + This behavior is the default when the start point is a remote-tracking branch. Set the branch.autoSetupMerge configuration variable to `false` if you -want `git checkout` and `git branch` to always behave as if `--no-track` +want `git switch`, `git checkout` and `git branch` to always behave as if `--no-track` were given. Set it to `always` if you want this behavior when the start-point is either a local or remote-tracking branch. @@ -297,7 +297,7 @@ Start development from a known tag:: $ git clone git://git.kernel.org/pub/scm/.../linux-2.6 my2.6 $ cd my2.6 $ git branch my2.6.14 v2.6.14 <1> -$ git checkout my2.6.14 +$ git switch my2.6.14 + <1> This step and the next one could be combined into a single step with @@ -322,9 +322,9 @@ $ git branch -D test<2> NOTES - -If you are creating a branch that you want to checkout immediately, it is -easier to use the git checkout command with its `-b` option to create -a branch and check it out with a single command. +If you are creating a branch that you want to switch to immediately, +it is easier to use the "git switch" command with its `-c` option to +do the same thing with a single command. The options `--contains`, `--no-contains`, `--merged` and `--no-merged` serve four related but different purposes: diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt index d9de992585..ee6a4144fb 100644 --- a/Documentation/git-check-ref-format.txt +++ b/Documentation/git-check-ref-format.txt @@ -88,7 +88,8 @@ but it is explicitly forbidden at the beginning of a branch name). When run with `--branch` option in a repository, the input is first expanded for the ``previous checkout syntax'' `@{-n}`. For example, `@{-1}` is a way to refer the last thing that -was checked out using "git checkout" operation. This option should be +was checked out using "git switch" or "git checkout" operation. +This option should be used by porcelains to accept this syntax anywhere a branch name is expected, so they can act as if you typed the branch name. As an exception note that, the ``previous checkout operation'' might result diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 1af85d404f..0a24a5679e 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -421,7 +421,7 @@ One way to test if your MUA is set up correctly is: * Apply it: $ git fetch master:test-apply -$ git checkout test-apply +$ git switch test-apply $ git reset --hard $ git am a.patch diff --git a/Documentation/git-merge-base.txt b/Documentation/git-merge-base.txt index 9f07f4f6ed..261d5c1164 100644 --- a/Documentation/git-merge-base.txt +++ b/Documentation/git-merge-base.txt @@ -149,7 +149,7 @@ instead. Discussion on fork-point mode - -After working on the `topic` branch created with `git checkout -b +After working on the `topic` branch created with `git switch -c topic
[PATCH v3 19/21] t: add tests for switch
--- t/t2060-switch.sh | 87 +++ 1 file changed, 87 insertions(+) create mode 100755 t/t2060-switch.sh diff --git a/t/t2060-switch.sh b/t/t2060-switch.sh new file mode 100755 index 00..1e1e834c1b --- /dev/null +++ b/t/t2060-switch.sh @@ -0,0 +1,87 @@ +#!/bin/sh + +test_description='switch basic functionality' + +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit first && + git branch first-branch && + test_commit second && + test_commit third && + git remote add origin nohost:/nopath && + git update-ref refs/remotes/origin/foo first-branch +' + +test_expect_success 'switch branch no arguments' ' + test_must_fail git switch +' + +test_expect_success 'switch branch' ' + git switch first-branch && + test_path_is_missing second.t +' + +test_expect_success 'switch to a commit' ' + test_must_fail git switch master^{commit} +' + +test_expect_success 'switch and detach' ' + test_when_finished git switch master && + git switch --detach master^{commit} && + test_must_fail git symbolic-ref HEAD +' + +test_expect_success 'switch and detach current branch' ' + test_when_finished git switch master && + git switch master && + git switch --detach && + test_must_fail git symbolic-ref HEAD +' + +test_expect_success 'switch and create branch' ' + test_when_finished git switch master && + git switch -c temp master^ && + test_cmp_rev master^ refs/heads/temp && + echo refs/heads/temp >expected-branch && + git symbolic-ref HEAD >actual-branch && + test_cmp expected-branch actual-branch +' + +test_expect_success 'force create branch from HEAD' ' + test_when_finished git switch master && + git switch --detach master && + git switch -C temp && + test_cmp_rev master refs/heads/temp && + echo refs/heads/temp >expected-branch && + git symbolic-ref HEAD >actual-branch && + test_cmp expected-branch actual-branch +' + +test_expect_success 'new orphan branch' ' + test_when_finished git switch master && + git switch --orphan new-orphan master^ && + test_commit orphan && + git cat-file commit refs/heads/new-orphan >commit && + ! grep ^parent commit +' + +test_expect_success 'switching ignores file of same branch name' ' + test_when_finished git switch master && + : >first-branch && + git switch first-branch && + echo refs/heads/first-branch >expected && + git symbolic-ref HEAD >actual && + test_commit expected actual +' + +test_expect_success 'guess and create branch ' ' + test_when_finished git switch master && + test_must_fail git switch foo && + git switch --guess foo && + echo refs/heads/foo >expected && + git symbolic-ref HEAD >actual && + test_cmp expected actual +' + +test_done -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v3 18/21] switch: no worktree status unless real branch switch happens
When we switch from one branch to another, it makes sense to show a summary of local changes since there could be conflicts, or some files left modified When switch is used solely for creating a new branch (and "switch" to the same commit) or detaching, we don't really need to show anything. "git checkout" does it anyway for historical reasons. But we can start with a clean slate with switch and don't have to. This essentially reverts fa655d8411 (checkout: optimize "git checkout -b " - 2018-08-16) and make it default for switch, but also for -B and --detach. Users of big repos are encouraged to move to switch. --- Documentation/config/checkout.txt | 8 -- builtin/checkout.c| 134 ++ t/t1090-sparse-checkout-scope.sh | 14 3 files changed, 8 insertions(+), 148 deletions(-) diff --git a/Documentation/config/checkout.txt b/Documentation/config/checkout.txt index d6872ffa83..6b646813ab 100644 --- a/Documentation/config/checkout.txt +++ b/Documentation/config/checkout.txt @@ -16,11 +16,3 @@ will checkout the '' branch on another remote, and by linkgit:git-worktree[1] when 'git worktree add' refers to a remote branch. This setting might be used for other checkout-like commands or functionality in the future. - -checkout.optimizeNewBranch:: - Optimizes the performance of "git checkout -b " when - using sparse-checkout. When set to true, git will not update the - repo based on the current sparse-checkout settings. This means it - will not update the skip-worktree bit in the index nor add/remove - files in the working directory to reflect the current sparse checkout - settings nor will it show the local changes. diff --git a/builtin/checkout.c b/builtin/checkout.c index 8a89df4f36..4903359b49 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -26,8 +26,6 @@ #include "submodule.h" #include "advice.h" -static int checkout_optimize_new_branch; - static const char * const checkout_usage[] = { N_("git checkout [] "), N_("git checkout [] [] -- ..."), @@ -56,11 +54,7 @@ struct checkout_opts { int dwim_new_local_branch; int accept_pathspec; int switch_branch_doing_nothing_is_ok; - - /* -* If new checkout options are added, skip_merge_working_tree -* should be updated accordingly. -*/ + int only_merge_on_switching_branches; const char *new_branch; const char *new_branch_force; @@ -564,112 +558,6 @@ static void setup_branch_path(struct branch_info *branch) branch->path = strbuf_detach(&buf, NULL); } -/* - * Skip merging the trees, updating the index and working directory if and - * only if we are creating a new branch via "git checkout -b ." - */ -static int skip_merge_working_tree(const struct checkout_opts *opts, - const struct branch_info *old_branch_info, - const struct branch_info *new_branch_info) -{ - /* -* Do the merge if sparse checkout is on and the user has not opted in -* to the optimized behavior -*/ - if (core_apply_sparse_checkout && !checkout_optimize_new_branch) - return 0; - - /* -* We must do the merge if we are actually moving to a new commit. -*/ - if (!old_branch_info->commit || !new_branch_info->commit || - !oideq(&old_branch_info->commit->object.oid, - &new_branch_info->commit->object.oid)) - return 0; - - /* -* opts->patch_mode cannot be used with switching branches so is -* not tested here -*/ - - /* -* opts->quiet only impacts output so doesn't require a merge -*/ - - /* -* Honor the explicit request for a three-way merge or to throw away -* local changes -*/ - if (opts->merge || opts->force) - return 0; - - /* -* --detach is documented as "updating the index and the files in the -* working tree" but this optimization skips those steps so fall through -* to the regular code path. -*/ - if (opts->force_detach) - return 0; - - /* -* opts->writeout_stage cannot be used with switching branches so is -* not tested here -*/ - - /* -* Honor the explicit ignore requests -*/ - if (!opts->overwrite_ignore || opts->ignore_skipworktree || - opts->ignore_other_worktrees) - return 0; - - /* -* opts->show_progress only impacts output so doesn't require a merge -*/ - - /* -* opts->overlay_mode cannot be used with switching branches so is -* not tested here -*/ - - /* -* If we aren't creating a new branch any changes or updates will -* happen in the existing branch. Since that could only be updating -* the index and working directory, we d
[PATCH v1 07/11] restore: default to --source=HEAD when only --index is specified
"git restore --index" does not make much sense since we're told to restore the index from the (by default) index. Set default source to HEAD in this case. This is basically the same as "git reset -- ". Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/checkout.c | 6 ++ 1 file changed, 6 insertions(+) diff --git a/builtin/checkout.c b/builtin/checkout.c index 07b431be51..f06439dbeb 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1426,6 +1426,12 @@ static int checkout_main(int argc, const char **argv, const char *prefix, } if (opts->checkout_index < 0 || opts->checkout_worktree < 0) BUG("these flags should be non-negative by now"); + /* +* convenient shortcut: "git restore --index" equals +* "git restore --index --source HEAD" +*/ + if (!opts->from_treeish && opts->checkout_index && !opts->checkout_worktree) + opts->from_treeish = "HEAD"; /* * From here on, new_branch will contain the branch to be checked out, -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v1 09/11] t: add tests for restore
Signed-off-by: Nguyễn Thái Ngọc Duy --- t/lib-patch-mode.sh | 12 t/t2070-restore.sh (new +x) | 77 ++ t/t2071-restore-patch.sh (new +x) | 105 ++ 3 files changed, 194 insertions(+) diff --git a/t/lib-patch-mode.sh b/t/lib-patch-mode.sh index 06c3c91762..cfd76bf987 100644 --- a/t/lib-patch-mode.sh +++ b/t/lib-patch-mode.sh @@ -2,28 +2,40 @@ . ./test-lib.sh +# set_state +# +# Prepare the content for path in worktree and the index as specified. set_state () { echo "$3" > "$1" && git add "$1" && echo "$2" > "$1" } +# save_state +# +# Save index/worktree content of in the files _worktree_ +# and _index_ save_state () { noslash="$(echo "$1" | tr / _)" && cat "$1" > _worktree_"$noslash" && git show :"$1" > _index_"$noslash" } +# set_and_save_state set_and_save_state () { set_state "$@" && save_state "$1" } +# verify_state verify_state () { test "$(cat "$1")" = "$2" && test "$(git show :"$1")" = "$3" } +# verify_saved_state +# +# Call verify_state with expected contents from the last save_state verify_saved_state () { noslash="$(echo "$1" | tr / _)" && verify_state "$1" "$(cat _worktree_"$noslash")" "$(cat _index_"$noslash")" diff --git a/t/t2070-restore.sh b/t/t2070-restore.sh new file mode 100755 index 00..df91bf54bc --- /dev/null +++ b/t/t2070-restore.sh @@ -0,0 +1,77 @@ +#!/bin/sh + +test_description='restore basic functionality' + +. ./test-lib.sh + +test_expect_success 'setup' ' + test_commit first && + echo first-and-a-half >>first.t && + git add first.t && + test_commit second && + echo one >one && + echo two >two && + echo untracked >untracked && + echo ignored >ignored && + echo /ignored >.gitignore && + git add one two .gitignore && + git update-ref refs/heads/one master +' + +test_expect_success 'restore without pathspec is not ok' ' + test_must_fail git restore && + test_must_fail git restore --source=first +' + +test_expect_success 'restore -p without pathspec is fine' ' + echo q >cmd && + git restore -p expected && + echo dirty >>one && + git restore one && + test_cmp expected one +' + +test_expect_success 'restore a file on worktree from another branch' ' + test_when_finished git reset --hard && + git cat-file blob first:./first.t >expected && + git restore --source=first first.t && + test_cmp expected first.t && + git cat-file blob HEAD:./first.t >expected && + git show :first.t >actual && + test_cmp expected actual +' + +test_expect_success 'restore a file in the index from another branch' ' + test_when_finished git reset --hard && + git cat-file blob first:./first.t >expected && + git restore --source=first --index first.t && + git show :first.t >actual && + test_cmp expected actual && + git cat-file blob HEAD:./first.t >expected && + test_cmp expected first.t +' + +test_expect_success 'restore a file in both the index and worktree from another branch' ' + test_when_finished git reset --hard && + git cat-file blob first:./first.t >expected && + git restore --source=first --index --worktree first.t && + git show :first.t >actual && + test_cmp expected actual && + test_cmp expected first.t +' + +test_expect_success 'restore --index uses HEAD as source' ' + test_when_finished git reset --hard && + git cat-file blob :./first.t >expected && + echo index-dirty >>first.t && + git add first.t && + git restore --index first.t && + git cat-file blob :./first.t >actual && + test_cmp expected actual +' + +test_done diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh new file mode 100755 index 00..46ebcb2413 --- /dev/null +++ b/t/t2071-restore-patch.sh @@ -0,0 +1,105 @@ +#!/bin/sh + +test_description='git restore --patch' + +. ./lib-patch-mode.sh + +test_expect_success PERL 'setup' ' + mkdir dir && + echo parent >dir/foo && + echo dummy >bar && + git add bar dir/foo && + git commit -m initial && + test_tick && + test_commit second dir/foo head && + set_and_save_state bar bar_work bar_index && + save_head +' + +# note: bar sorts before dir/foo, so the first 'n' is always to skip 'bar' + +test_expect_success PERL 'saying "n" does nothing' ' + set_and_save_state dir/foo work head && + test_write_lines n n | git restore -p && + verify_saved_state bar && + verify_saved_state dir/foo +' + +test_expect_success PERL 'git restore -p' ' + set_and_save_state dir/foo work head && + test_write_lines n y | git restore -p && + verify_saved_state bar && + verify_state dir/foo head head +' + +test_expect_success PERL 'git re
[PATCH v1 05/11] checkout: factor out worktree checkout code
Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/checkout.c | 108 + 1 file changed, 59 insertions(+), 49 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 9e59bf792f..5fb85e7b73 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -326,17 +326,73 @@ static void mark_ce_for_checkout_no_overlay(struct cache_entry *ce, } } +static int checkout_worktree(const struct checkout_opts *opts) +{ + struct checkout state = CHECKOUT_INIT; + int nr_checkouts = 0, nr_unmerged = 0; + int errs = 0; + int pos; + + state.force = 1; + state.refresh_cache = 1; + state.istate = &the_index; + + enable_delayed_checkout(&state); + for (pos = 0; pos < active_nr; pos++) { + struct cache_entry *ce = active_cache[pos]; + if (ce->ce_flags & CE_MATCHED) { + if (!ce_stage(ce)) { + errs |= checkout_entry(ce, &state, + NULL, &nr_checkouts); + continue; + } + if (opts->writeout_stage) + errs |= checkout_stage(opts->writeout_stage, + ce, pos, + &state, + &nr_checkouts, opts->overlay_mode); + else if (opts->merge) + errs |= checkout_merged(pos, &state, + &nr_unmerged); + pos = skip_same_name(ce, pos) - 1; + } + } + remove_marked_cache_entries(&the_index, 1); + remove_scheduled_dirs(); + errs |= finish_delayed_checkout(&state, &nr_checkouts); + + if (opts->count_checkout_paths) { + if (nr_unmerged) + fprintf_ln(stderr, Q_("Recreated %d merge conflict", + "Recreated %d merge conflicts", + nr_unmerged), + nr_unmerged); + if (opts->source_tree) + fprintf_ln(stderr, Q_("Updated %d path from %s", + "Updated %d paths from %s", + nr_checkouts), + nr_checkouts, + find_unique_abbrev(&opts->source_tree->object.oid, + DEFAULT_ABBREV)); + else if (!nr_unmerged || nr_checkouts) + fprintf_ln(stderr, Q_("Updated %d path from the index", + "Updated %d paths from the index", + nr_checkouts), + nr_checkouts); + } + + return errs; +} + static int checkout_paths(const struct checkout_opts *opts, const char *revision) { int pos; - struct checkout state = CHECKOUT_INIT; static char *ps_matched; struct object_id rev; struct commit *head; int errs = 0; struct lock_file lock_file = LOCK_INIT; - int nr_checkouts = 0, nr_unmerged = 0; trace2_cmd_mode(opts->patch_mode ? "patch" : "path"); @@ -422,53 +478,7 @@ static int checkout_paths(const struct checkout_opts *opts, return 1; /* Now we are committed to check them out */ - state.force = 1; - state.refresh_cache = 1; - state.istate = &the_index; - - enable_delayed_checkout(&state); - for (pos = 0; pos < active_nr; pos++) { - struct cache_entry *ce = active_cache[pos]; - if (ce->ce_flags & CE_MATCHED) { - if (!ce_stage(ce)) { - errs |= checkout_entry(ce, &state, - NULL, &nr_checkouts); - continue; - } - if (opts->writeout_stage) - errs |= checkout_stage(opts->writeout_stage, - ce, pos, - &state, - &nr_checkouts, opts->overlay_mode); - else if (opts->merge) - errs |= checkout_merged(pos, &state, - &nr_unmerged); - pos = skip_same_name(ce, pos) - 1; - } - } - remove_marked_cache_entries(&the_index, 1); - remove_scheduled_dirs(); - errs |= finish_delayed_checkout(&
[PATCH v1 08/11] restore: support --patch
git-restore is different from git-checkout that it only restores the worktree by default, not both worktree and index. add--interactive needs some update to support this mode. Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/checkout.c| 5 ++-- git-add--interactive.perl | 52 +++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index f06439dbeb..73de110d42 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -436,9 +436,10 @@ static int checkout_paths(const struct checkout_opts *opts, patch_mode = "--patch=checkout"; else if (opts->checkout_index && !opts->checkout_worktree) patch_mode = "--patch=reset"; + else if (!opts->checkout_index && opts->checkout_worktree) + patch_mode = "--patch=worktree"; else - die(_("'%s' with only '%s' is not currently supported"), - "--patch", "--worktree"); + BUG("either flag must have been set"); return run_add_interactive(revision, patch_mode, &opts->pathspec); } diff --git a/git-add--interactive.perl b/git-add--interactive.perl index 20eb81cc92..3dfb3629c9 100755 --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@ -149,6 +149,20 @@ sub colored { FILTER => undef, IS_REVERSE => 0, }, + 'worktree_head' => { + DIFF => 'diff-index -p', + APPLY => sub { apply_patch 'apply -R', @_ }, + APPLY_CHECK => 'apply -R', + FILTER => undef, + IS_REVERSE => 1, + }, + 'worktree_nothead' => { + DIFF => 'diff-index -R -p', + APPLY => sub { apply_patch 'apply', @_ }, + APPLY_CHECK => 'apply', + FILTER => undef, + IS_REVERSE => 0, + }, ); $patch_mode = 'stage'; @@ -1049,6 +1063,12 @@ sub color_diff { marked for discarding."), checkout_nothead => N__( "If the patch applies cleanly, the edited hunk will immediately be +marked for applying."), + worktree_head => N__( +"If the patch applies cleanly, the edited hunk will immediately be +marked for discarding."), + worktree_nothead => N__( +"If the patch applies cleanly, the edited hunk will immediately be marked for applying."), ); @@ -1259,6 +1279,18 @@ sub edit_hunk_loop { n - do not apply this hunk to index and worktree q - quit; do not apply this hunk or any of the remaining ones a - apply this hunk and all later hunks in the file +d - do not apply this hunk or any of the later hunks in the file"), + worktree_head => N__( +"y - discard this hunk from worktree +n - do not discard this hunk from worktree +q - quit; do not discard this hunk or any of the remaining ones +a - discard this hunk and all later hunks in the file +d - do not discard this hunk or any of the later hunks in the file"), + worktree_nothead => N__( +"y - apply this hunk to worktree +n - do not apply this hunk to worktree +q - quit; do not apply this hunk or any of the remaining ones +a - apply this hunk and all later hunks in the file d - do not apply this hunk or any of the later hunks in the file"), ); @@ -1421,6 +1453,16 @@ sub display_hunks { deletion => N__("Apply deletion to index and worktree [y,n,q,a,d%s,?]? "), hunk => N__("Apply this hunk to index and worktree [y,n,q,a,d%s,?]? "), }, + worktree_head => { + mode => N__("Discard mode change from worktree [y,n,q,a,d%s,?]? "), + deletion => N__("Discard deletion from worktree [y,n,q,a,d%s,?]? "), + hunk => N__("Discard this hunk from worktree [y,n,q,a,d%s,?]? "), + }, + worktree_nothead => { + mode => N__("Apply mode change to worktree [y,n,q,a,d%s,?]? "), + deletion => N__("Apply deletion to worktree [y,n,q,a,d%s,?]? "), + hunk => N__("Apply this hunk to worktree [y,n,q,a,d%s,?]? "), + }, ); sub patch_update_file { @@ -1756,6 +1798,16 @@ sub process_args { 'checkout_head' : 'checkout_nothead'); $arg = shift @ARGV or die __("missing --"); } + } elsif ($1 eq 'worktree') { + $arg = shift @ARGV or die __("missing --"); + if ($arg eq '--') { + $patch_mode = 'checkout_index'; + } else { + $patch_mode_revision = $arg; + $patch_mode = ($arg eq 'HEAD' ? + 'worktree_head' : 'worktree_nothead'); +
[PATCH v1 06/11] restore: add --worktree and --index
'git checkout ' updates both index and worktree. But updating the index when you want to restore worktree files is non-intuitive. The index contains the data ready for the next commit, and there's no indication that the user will want to commit the restored versions. 'git restore' therefore by default only touches worktree. The user has the option to update either the index with git restore --source= --index (1) or update both with git restore --source= --index --worktree (2) PS. Orignally I wanted to make worktree update default and form (1) would add index update while also updating the worktree, and the user would need to do "--index --no-worktree" to update index only. But it looks really confusing that "--index" option alone updates both. So now form (2) is used for both, which reads much more obvious. PPS. Yes form (1) overlaps with "git reset ". I don't know if we can ever turn "git reset" back to "_always_ reset HEAD and optionally do something else". Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/checkout.c | 74 ++ 1 file changed, 68 insertions(+), 6 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 5fb85e7b73..07b431be51 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -62,6 +62,8 @@ struct checkout_opts { int switch_branch_doing_nothing_is_ok; int only_merge_on_switching_branches; int empty_pathspec_ok; + int checkout_index; + int checkout_worktree; const char *new_branch; const char *new_branch_force; @@ -393,6 +395,7 @@ static int checkout_paths(const struct checkout_opts *opts, struct commit *head; int errs = 0; struct lock_file lock_file = LOCK_INIT; + int checkout_index; trace2_cmd_mode(opts->patch_mode ? "patch" : "path"); @@ -418,9 +421,26 @@ static int checkout_paths(const struct checkout_opts *opts, die(_("Cannot update paths and switch to branch '%s' at the same time."), opts->new_branch); - if (opts->patch_mode) - return run_add_interactive(revision, "--patch=checkout", - &opts->pathspec); + if (!opts->checkout_worktree && !opts->checkout_index) + die(_("neither '%s' or '%s' is specified"), + "--index", "--worktree"); + + if (!opts->source_tree && !opts->checkout_worktree) + die(_("'%s' must be used when '%s' is not specified"), + "--worktree", "--source"); + + if (opts->patch_mode) { + const char *patch_mode; + + if (opts->checkout_index && opts->checkout_worktree) + patch_mode = "--patch=checkout"; + else if (opts->checkout_index && !opts->checkout_worktree) + patch_mode = "--patch=reset"; + else + die(_("'%s' with only '%s' is not currently supported"), + "--patch", "--worktree"); + return run_add_interactive(revision, patch_mode, &opts->pathspec); + } repo_hold_locked_index(the_repository, &lock_file, LOCK_DIE_ON_ERROR); if (read_cache_preload(&opts->pathspec) < 0) @@ -478,10 +498,30 @@ static int checkout_paths(const struct checkout_opts *opts, return 1; /* Now we are committed to check them out */ - errs |= checkout_worktree(opts); + if (opts->checkout_worktree) + errs |= checkout_worktree(opts); - if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK)) - die(_("unable to write new index file")); + /* +* Allow updating the index when checking out from the index. +* This is to save new stat info. +*/ + if (opts->checkout_worktree && !opts->checkout_index && !opts->source_tree) + checkout_index = 1; + else + checkout_index = opts->checkout_index; + + if (checkout_index) { + if (write_locked_index(&the_index, &lock_file, COMMIT_LOCK)) + die(_("unable to write new index file")); + } else { + /* +* NEEDSWORK: if --worktree is not specified, we +* should save stat info of checked out files in the +* index to avoid the next (potentially costly) +* refresh. But it's a bit tricker to do... +*/ + rollback_lock_file(&lock_file); + } read_ref_full("HEAD", 0, &rev, NULL); head = lookup_commit_reference_gently(the_repository, &rev, 1); @@ -1373,6 +1413,20 @@ static int checkout_main(int argc, const char **argv, const char *prefix, if (opts->overlay_mode == 1 && opts->patch_mode) die(_("-p and --overlay are mutually exclusive")); + if (opts->checkout_index >= 0 || opts->checkout_w
[PATCH v1 00/11] And new command "restore"
This is the companion of "git switch" [1] and is based on that topic. This command peforms the "checkout paths" from git-checkout, git-reset and also has a third mode to reset only worktree, leaving the index alone. For new people not aware of previous discussions, this command is supposed to be a friendlier replacement for "git checkout" and hopefully fixes some warts that have grown over the time in git-checkout. For this reason, the last patch starts to recommend "git restore" everywhere. For old people, I'm surprised nobody reacts to the command rename from restore-files to restore. "restore" vs "reset" is certainly confusing. My hope was to remove "common porcelain" status from reset, but I probably will not succeed in doing that. Some open issues from the last discussion [2] - intent-to-add support. This will come but maybe later. - --index has a different meaning in git-apply. I don't have a good suggestion except "yeah we have two different worlds now, the old and the new one" The series is also available at [3] [1] https://public-inbox.org/git/20190308095752.8574-1-pclo...@gmail.com [2] https://public-inbox.org/git/cacsjy8cqhwec3b6egpepurejfox7c17x61-wqq5koirzykr...@mail.gmail.com/ [3] https://gitlab.com/pclouds/git/tree/switch-and-restore Nguyễn Thái Ngọc Duy (11): checkout: split part of it to new command 'restore' restore: take tree-ish from --source option instead restore: make pathspec mandatory restore: disable overlay mode by default checkout: factor out worktree checkout code restore: add --worktree and --index restore: default to --source=HEAD when only --index is specified restore: support --patch t: add tests for restore completion: support restore doc: promote "git restore" .gitignore | 1 + Documentation/config/interactive.txt | 3 +- Documentation/git-checkout.txt | 1 + Documentation/git-clean.txt| 2 +- Documentation/git-commit.txt | 4 +- Documentation/git-format-patch.txt | 2 +- Documentation/git-reset.txt| 6 +- Documentation/git-restore.txt (new)| 172 Documentation/git-revert.txt | 4 +- Documentation/gitcli.txt | 4 +- Documentation/giteveryday.txt | 4 +- Documentation/gittutorial-2.txt| 4 +- Documentation/gittutorial.txt | 2 +- Documentation/user-manual.txt | 14 +- Makefile | 1 + builtin.h | 1 + builtin/checkout.c | 259 +++-- builtin/clone.c| 2 +- builtin/commit.c | 2 +- command-list.txt | 3 +- contrib/completion/git-completion.bash | 15 ++ git-add--interactive.perl | 52 + git.c | 1 + t/lib-patch-mode.sh| 12 ++ t/t2070-restore.sh (new +x)| 77 t/t2071-restore-patch.sh (new +x) | 105 ++ t/t7508-status.sh | 82 t/t7512-status-help.sh | 20 +- wt-status.c| 26 ++- 29 files changed, 736 insertions(+), 145 deletions(-) create mode 100644 Documentation/git-restore.txt create mode 100755 t/t2070-restore.sh create mode 100755 t/t2071-restore-patch.sh -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v1 01/11] checkout: split part of it to new command 'restore'
Previously the switching branch business of 'git checkout' becomes a new command. This adds restore command for the checking out paths path. Similar to switch, a new man page is added to describe what the command will become. The implementation will be updated shortly to match the man page. A couple of differences from 'git checkout' worth highlighting: - 'restore' by default will only update worktree. This matters more when --source is specified ('checkout ' updates both worktree and index). - 'restore --index' can be used to restore the index. This command overlaps with 'git reset '. - both worktree and index could also be restored at the same time (from a tree). This overlaps with 'git checkout ' - default source for restoring worktree and index is the index and HEAD respectively. A different (tree) source could be specified as with --source (*). - when both index and worktree are restored, --source must be specified since the default source for these two individual targets are different (**) - --no-overlay is enabled by default, if an entry is missing in the source, restoring means deleting the entry (*) I originally went with --from instead of --source. I still think --from is a better name. The short option -f however is already taken by force. And I do think short option is good to have, e.g. to write -s@ or -s@^ instead of --source=HEAD. (**) If you sit down and think about it, moving worktree's source from the index to HEAD makes sense, but nobody is really thinking it through when they type the commands. Signed-off-by: Nguyễn Thái Ngọc Duy --- .gitignore | 1 + Documentation/config/interactive.txt | 3 +- Documentation/git-checkout.txt | 1 + Documentation/git-restore.txt (new) | 172 +++ Makefile | 1 + builtin.h| 1 + builtin/checkout.c | 26 command-list.txt | 1 + git.c| 1 + 9 files changed, 206 insertions(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index c687b92b1c..fb377106be 100644 --- a/.gitignore +++ b/.gitignore @@ -143,6 +143,7 @@ /git-request-pull /git-rerere /git-reset +/git-restore /git-rev-list /git-rev-parse /git-revert diff --git a/Documentation/config/interactive.txt b/Documentation/config/interactive.txt index ad846dd7c9..a2d3c7ec44 100644 --- a/Documentation/config/interactive.txt +++ b/Documentation/config/interactive.txt @@ -2,7 +2,8 @@ interactive.singleKey:: In interactive commands, allow the user to provide one-letter input with a single key (i.e., without hitting enter). Currently this is used by the `--patch` mode of - linkgit:git-add[1], linkgit:git-checkout[1], linkgit:git-commit[1], + linkgit:git-add[1], linkgit:git-checkout[1], + linkgit:git-restore[1], linkgit:git-commit[1], linkgit:git-reset[1], and linkgit:git-stash[1]. Note that this setting is silently ignored if portable keystroke input is not available; requires the Perl module Term::ReadKey. diff --git a/Documentation/git-checkout.txt b/Documentation/git-checkout.txt index 2b776c1269..e107099c8c 100644 --- a/Documentation/git-checkout.txt +++ b/Documentation/git-checkout.txt @@ -571,6 +571,7 @@ $ git add frotz SEE ALSO linkgit:git-switch[1] +linkgit:git-restore[1] GIT --- diff --git a/Documentation/git-restore.txt b/Documentation/git-restore.txt new file mode 100644 index 00..a667a5ced4 --- /dev/null +++ b/Documentation/git-restore.txt @@ -0,0 +1,172 @@ +git-restore(1) +== + +NAME + +git-restore - Restore working tree files + +SYNOPSIS + +[verse] +'git restore' [] [--source=] ... +'git restore' (-p|--patch) [--source=] [...] + +DESCRIPTION +--- +Restore paths in the working tree by replacing with the contents in +the restore source or remove if the paths do not exist in the restore +source. The source is by default the index but could be from a commit. +The command can also optionally restore content in the index from a +commit. + +When a `` is given, the paths that match the `` are +updated both in the index and in the working tree. + +OPTIONS +--- +-s:: +--source=:: + Restore the working tree files with the content from the given + tree or any revision that leads to a tree (e.g. a commit or a + branch). + +-p:: +--patch:: + Interactively select hunks in the difference between the + `` (or the index, if unspecified) and the working + tree. See the ``Interactive Mode'' section of linkgit:git-add[1] + to learn how to operate the `--patch` mode. + +-W:: +--worktree:: +-I:: +--index:: + Specify the restore location. If neither option is specified, + by default the working tree is restored. If `--index` is + specified without `--worktree` or `--source`, `--source=HEAD`
[PATCH v1 03/11] restore: make pathspec mandatory
"git restore" without arguments does not make much sense when it's about restoring files (what files now?). We could default to either git restore . or git restore :/ Neither is intuitive. Make the user always give pathspec, force the user to think the scope of restore they want because this is a destructive operation. "git restore -p" without pathspec is an exception to this because it really is a separate mode. It will be treated as running patch mode on the whole worktree. Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/checkout.c | 7 +++ 1 file changed, 7 insertions(+) diff --git a/builtin/checkout.c b/builtin/checkout.c index 838343d6aa..c52ce13d2a 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -61,6 +61,7 @@ struct checkout_opts { int accept_pathspec; int switch_branch_doing_nothing_is_ok; int only_merge_on_switching_branches; + int empty_pathspec_ok; const char *new_branch; const char *new_branch_force; @@ -1427,6 +1428,10 @@ static int checkout_main(int argc, const char **argv, const char *prefix, die(_("reference is not a tree: %s"), opts->from_treeish); } + if (opts->accept_pathspec && !opts->empty_pathspec_ok && !argc && + !opts->patch_mode) /* patch mode is special */ + die(_("pathspec is required")); + if (argc) { parse_pathspec(&opts->pathspec, 0, opts->patch_mode ? PATHSPEC_PREFIX_ORIGIN : 0, @@ -1511,6 +1516,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) opts.accept_ref = 1; opts.accept_pathspec = 1; opts.implicit_detach = 1; + opts.empty_pathspec_ok = 1; options = parse_options_dup(checkout_options); options = add_common_options(&opts, options); @@ -1570,6 +1576,7 @@ int cmd_restore(int argc, const char **argv, const char *prefix) memset(&opts, 0, sizeof(opts)); opts.accept_ref = 0; opts.accept_pathspec = 1; + opts.empty_pathspec_ok = 0; options = parse_options_dup(restore_options); options = add_common_options(&opts, options); -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v1 10/11] completion: support restore
Completion for restore is straightforward. We could still do better though by give the list of just tracked files instead of all present ones. But let's leave it for later. Signed-off-by: Nguyễn Thái Ngọc Duy --- contrib/completion/git-completion.bash | 15 +++ 1 file changed, 15 insertions(+) diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash index 7fcf28d437..0b22e68187 100644 --- a/contrib/completion/git-completion.bash +++ b/contrib/completion/git-completion.bash @@ -2483,6 +2483,21 @@ _git_reset () __git_complete_refs } +_git_restore () +{ + case "$cur" in + --conflict=*) + __gitcomp "diff3 merge" "" "${cur##--conflict=}" + ;; + --source=*) + __git_complete_refs --cur="${cur##--source=}" + ;; + --*) + __gitcomp_builtin restore + ;; + esac +} + __git_revert_inprogress_options="--continue --quit --abort" _git_revert () -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v1 04/11] restore: disable overlay mode by default
Overlay mode is considered confusing when the command is about restoring files on worktree. Disable it by default. The user can still turn it on, or use 'git checkout' which still has overlay mode on by default. While at there make the check in checkout_branch() stricter. Neither --overlay or --no-overlay should be accepted in branch switching mode. Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/checkout.c | 11 +++ 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index c52ce13d2a..9e59bf792f 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -1196,9 +1196,9 @@ static int checkout_branch(struct checkout_opts *opts, die(_("'%s' cannot be used with switching branches"), "--patch"); - if (!opts->overlay_mode) + if (opts->overlay_mode != -1) die(_("'%s' cannot be used with switching branches"), - "--no-overlay"); + "--[no]-overlay"); if (opts->writeout_stage) die(_("'%s' cannot be used with switching branches"), @@ -1313,7 +1313,6 @@ static struct option *add_checkout_path_options(struct checkout_opts *opts, OPT_BOOL('p', "patch", &opts->patch_mode, N_("select hunks interactively")), OPT_BOOL(0, "ignore-skip-worktree-bits", &opts->ignore_skipworktree, N_("do not limit pathspecs to sparse entries only")), - OPT_BOOL(0, "overlay", &opts->overlay_mode, N_("use overlay mode (default)")), OPT_END() }; struct option *newopts = parse_options_concat(prevopts, options); @@ -1333,7 +1332,6 @@ static int checkout_main(int argc, const char **argv, const char *prefix, opts->overwrite_ignore = 1; opts->prefix = prefix; opts->show_progress = -1; - opts->overlay_mode = -1; git_config(git_checkout_config, opts); @@ -1505,6 +1503,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) OPT_BOOL('l', NULL, &opts.new_branch_log, N_("create reflog for new branch")), OPT_BOOL(0, "guess", &opts.dwim_new_local_branch, N_("second guess 'git checkout ' (default)")), + OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode (default)")), OPT_END() }; int ret; @@ -1517,6 +1516,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) opts.accept_pathspec = 1; opts.implicit_detach = 1; opts.empty_pathspec_ok = 1; + opts.overlay_mode = -1; options = parse_options_dup(checkout_options); options = add_common_options(&opts, options); @@ -1551,6 +1551,7 @@ int cmd_switch(int argc, const char **argv, const char *prefix) opts.switch_branch_doing_nothing_is_ok = 0; opts.only_merge_on_switching_branches = 1; opts.implicit_detach = 0; + opts.overlay_mode = -1; options = parse_options_dup(switch_options); options = add_common_options(&opts, options); @@ -1569,6 +1570,7 @@ int cmd_restore(int argc, const char **argv, const char *prefix) struct option restore_options[] = { OPT_STRING('s', "source", &opts.from_treeish, "", N_("where the checkout from")), + OPT_BOOL(0, "overlay", &opts.overlay_mode, N_("use overlay mode")), OPT_END() }; int ret; @@ -1577,6 +1579,7 @@ int cmd_restore(int argc, const char **argv, const char *prefix) opts.accept_ref = 0; opts.accept_pathspec = 1; opts.empty_pathspec_ok = 0; + opts.overlay_mode = 0; options = parse_options_dup(restore_options); options = add_common_options(&opts, options); -- 2.21.0.rc1.337.gdf7f8d0522
[PATCH v1 02/11] restore: take tree-ish from --source option instead
This is another departure from 'git checkout' syntax, which uses -- to separate ref and pathspec. The observation is restore (or "git checkout ,, ") is most often used to restore some files from the index. If this is correct, we can simplify it by taking a way the ref, so that we can write git restore some-file without worrying about some-file being a ref and whether we need to do git restore -- some-file for safety. If the source of the restore comes from a tree, it will be in the form of an option with value, e.g. git restore --source=this-tree some-file This is of course longer to type than using "--". But hopefully it will not be used as often, and it is clearly easier to understand. dwim_new_local_branch is no longer set (or unset) in cmd_restore_files() because it's irrelevant because we don't really care about dwim-ing. With accept_ref being unset, dwim can't happen. Signed-off-by: Nguyễn Thái Ngọc Duy --- builtin/checkout.c | 42 ++ 1 file changed, 34 insertions(+), 8 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index 11dd2ae44c..838343d6aa 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -38,7 +38,7 @@ static const char * const switch_branch_usage[] = { }; static const char * const restore_files_usage[] = { - N_("git restore [] [] -- ..."), + N_("git restore [] [--source=] ..."), NULL, }; @@ -57,6 +57,7 @@ struct checkout_opts { int count_checkout_paths; int overlay_mode; int dwim_new_local_branch; + int accept_ref; int accept_pathspec; int switch_branch_doing_nothing_is_ok; int only_merge_on_switching_branches; @@ -72,6 +73,7 @@ struct checkout_opts { int branch_exists; const char *prefix; struct pathspec pathspec; + const char *from_treeish; struct tree *source_tree; }; @@ -1324,6 +1326,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix, { struct branch_info new_branch_info; int dwim_remotes_matched = 0; + int parseopt_flags = 0; memset(&new_branch_info, 0, sizeof(new_branch_info)); opts->overwrite_ignore = 1; @@ -1335,8 +1338,13 @@ static int checkout_main(int argc, const char **argv, const char *prefix, opts->track = BRANCH_TRACK_UNSPECIFIED; - argc = parse_options(argc, argv, prefix, options, usagestr, -PARSE_OPT_KEEP_DASHDASH); + if (!opts->accept_pathspec && !opts->accept_ref) + BUG("make up your mind, you need to take _something_"); + if (opts->accept_pathspec && opts->accept_ref) + parseopt_flags = PARSE_OPT_KEEP_DASHDASH; + + argc = parse_options(argc, argv, prefix, options, +usagestr, parseopt_flags); if (opts->show_progress < 0) { if (opts->quiet) @@ -1393,7 +1401,7 @@ static int checkout_main(int argc, const char **argv, const char *prefix, * including "last branch" syntax and DWIM-ery for names of * remote branches, erroring out for invalid or ambiguous cases. */ - if (argc) { + if (argc && opts->accept_ref) { struct object_id rev; int dwim_ok = !opts->patch_mode && @@ -1405,6 +1413,18 @@ static int checkout_main(int argc, const char **argv, const char *prefix, &dwim_remotes_matched); argv += n; argc -= n; + } else if (!opts->accept_ref && opts->from_treeish) { + struct object_id rev; + + if (get_oid_mb(opts->from_treeish, &rev)) + die(_("could not resolve %s"), opts->from_treeish); + + setup_new_branch_info_and_source_tree(&new_branch_info, + opts, &rev, + opts->from_treeish); + + if (!opts->source_tree) + die(_("reference is not a tree: %s"), opts->from_treeish); } if (argc) { @@ -1488,6 +1508,7 @@ int cmd_checkout(int argc, const char **argv, const char *prefix) opts.dwim_new_local_branch = 1; opts.switch_branch_doing_nothing_is_ok = 1; opts.only_merge_on_switching_branches = 0; + opts.accept_ref = 1; opts.accept_pathspec = 1; opts.implicit_detach = 1; @@ -1519,6 +1540,7 @@ int cmd_switch(int argc, const char **argv, const char *prefix) memset(&opts, 0, sizeof(opts)); opts.dwim_new_local_branch = 0; + opts.accept_ref = 1; opts.accept_pathspec = 0; opts.switch_branch_doing_nothing_is_ok = 0; opts.only_merge_on_switching_branches = 1; @@ -1537,15 +1559,19 @@ int cmd_switch(int argc, const char **argv, const char *prefix) int cmd_restore(int argc, const char **argv,
[PATCH v1 11/11] doc: promote "git restore"
The new command "git restore" (together with "git switch") are added to avoid the confusion of one-command-do-all "git checkout" for new users. They are also helpful to avoid ambiguation context. For these reasons, promote it everywhere possible. This includes documentation, suggestions/advice from other commands. One nice thing about git-restore is the ability to restore "everything", so it can be used in "git status" advice instead of both "git checkout" and "git reset". The three commands suggested by "git status" are add, rm and restore. "git checkout" is also removed from "git help" (i.e. it's no longer considered a commonly used command) Signed-off-by: Nguyễn Thái Ngọc Duy --- Documentation/git-clean.txt| 2 +- Documentation/git-commit.txt | 4 +- Documentation/git-format-patch.txt | 2 +- Documentation/git-reset.txt| 6 +-- Documentation/git-revert.txt | 4 +- Documentation/gitcli.txt | 4 +- Documentation/giteveryday.txt | 4 +- Documentation/gittutorial-2.txt| 4 +- Documentation/gittutorial.txt | 2 +- Documentation/user-manual.txt | 14 +++-- builtin/clone.c| 2 +- builtin/commit.c | 2 +- command-list.txt | 2 +- t/t7508-status.sh | 82 +++--- t/t7512-status-help.sh | 20 wt-status.c| 26 +++--- 16 files changed, 95 insertions(+), 85 deletions(-) diff --git a/Documentation/git-clean.txt b/Documentation/git-clean.txt index 03056dad0d..8a31ccffea 100644 --- a/Documentation/git-clean.txt +++ b/Documentation/git-clean.txt @@ -64,7 +64,7 @@ OPTIONS directory) and $GIT_DIR/info/exclude, but do still use the ignore rules given with `-e` options. This allows removing all untracked files, including build products. This can be used (possibly in - conjunction with 'git reset') to create a pristine + conjunction with 'git restore' or 'git reset') to create a pristine working directory to test a clean build. -X:: diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt index a85c2c2a4c..e7ac8eb9e8 100644 --- a/Documentation/git-commit.txt +++ b/Documentation/git-commit.txt @@ -56,7 +56,7 @@ summary of what is included by any of the above for the next commit by giving the same set of parameters (options and paths). If you make a commit and then find a mistake immediately after -that, you can recover from it with 'git reset'. +that, you can recover from it with 'git restore' or 'git reset'. OPTIONS @@ -359,7 +359,7 @@ When recording your own work, the contents of modified files in your working tree are temporarily stored to a staging area called the "index" with 'git add'. A file can be reverted back, only in the index but not in the working tree, -to that of the last commit with `git reset HEAD -- `, +to that of the last commit with `git restore --index `, which effectively reverts 'git add' and prevents the changes to this file from participating in the next commit. After building the state to be committed incrementally with these commands, diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt index 0a24a5679e..71c9fe3af3 100644 --- a/Documentation/git-format-patch.txt +++ b/Documentation/git-format-patch.txt @@ -422,7 +422,7 @@ One way to test if your MUA is set up correctly is: $ git fetch master:test-apply $ git switch test-apply -$ git reset --hard +$ git restore --source=HEAD --index --worktree :/ $ git am a.patch If it does not apply correctly, there can be various reasons. diff --git a/Documentation/git-reset.txt b/Documentation/git-reset.txt index cbf901efb4..b70281677f 100644 --- a/Documentation/git-reset.txt +++ b/Documentation/git-reset.txt @@ -28,9 +28,9 @@ This means that `git reset ` is the opposite of `git add `. + After running `git reset ` to update the index entry, you can -use linkgit:git-checkout[1] to check the contents out of the index to -the working tree. -Alternatively, using linkgit:git-checkout[1] and specifying a commit, you +use linkgit:git-restore[1] to check the contents out of the index to +the working tree. Alternatively, using linkgit:git-restore[1] +and specifying a commit with `--source`, you can copy the contents of a path out of a commit to the index and to the working tree in one go. diff --git a/Documentation/git-revert.txt b/Documentation/git-revert.txt index 837707a8fd..c38bc54439 100644 --- a/Documentation/git-revert.txt +++ b/Documentation/git-revert.txt @@ -26,8 +26,8 @@ effect of some earlier commits (often only a faulty one). If you want to throw away all uncommitted changes in your working directory, you should see linkgit:git-reset[1], particularly the `--hard` option. If you want to extract specific files as they were in another commit, you -should see linkgit:git-checkout[1], specifi
One failed self test on Fedora 29
Fedora 29, x86_64. One failed self test: *** t0021-conversion.sh *** ok 1 - setup ok 2 - check ok 3 - expanded_in_repo ok 4 - filter shell-escaped filenames ok 5 - required filter should filter data ok 6 - required filter smudge failure ok 7 - required filter clean failure ok 8 - filtering large input to small output should use little memory ok 9 - filter that does not read is fine ok 10 # skip filter large file (missing EXPENSIVE) ok 11 - filter: clean empty file ok 12 - filter: smudge empty file not ok 13 - disable filter with empty override # # test_config_global filter.disable.smudge false && # test_config_global filter.disable.clean false && # test_config filter.disable.smudge false && # test_config filter.disable.clean false && # # echo "*.disable filter=disable" >.gitattributes && # # echo test >test.disable && # git -c filter.disable.clean= add test.disable 2>err && # test_must_be_empty err && # rm -f test.disable && # git -c filter.disable.smudge= checkout -- test.disable 2>err && # test_must_be_empty err # ok 14 - diff does not reuse worktree files that need cleaning ok 15 - required process filter should filter data ok 16 - required process filter takes precedence ok 17 - required process filter should be used only for "clean" operation only ok 18 - required process filter should process multiple packets ok 19 - required process filter with clean error should fail ok 20 - process filter should restart after unexpected write failure ok 21 - process filter should not be restarted if it signals an error ok 22 - process filter abort stops processing of all further files ok 23 - invalid process filter must fail (and not hang!) ok 24 - delayed checkout in process filter ok 25 - missing file in delayed checkout ok 26 - invalid file in delayed checkout # failed 1 among 26 test(s) 1..26 gmake[2]: *** [Makefile:56: t0021-conversion.sh] Error 1 Does anyone need a config.log or other test data?
fast-import on existing branches
Hi there, I've struggled for quite some time to sort out documented, intended and actual behavior of git fast-import. Unless I'm completely mistaken, it seems to be a straightforward bug, but if that is the case, I am really surprised why nobody else has stumbled over it before: I managed to use fast-import for a chain of commits onto a new branch into an empty repository. I managed to use fast-import to create a new branch starting from an existing parent using the 'from' command as documented. What I failed to do is to add commits on top of an existing branch in a new fast-import stream. As it seems, the variant of using 'commit' without 'from' only works on branches that were created within the same fast-import stream! The different approaches I tried (each with new fast-import stream on existing repo with existing branch) * 'commit' without 'from' -> Error: "Not updating (new tip does not contain ) And indeed looking into the repo afterwards, a new commit exists without any parent. * 'commit' with 'from' both naming the same branch -> Error: "Can't create a branch from itself" The only workarounds that I could find are to either explicitly looking up the top commit on the target branch and hand that to fast-import or create a temporary branch with a different name. Looking through the code of fast-import.c, I can indeed lookup_branch and new_branch only deal with internal data structures and the only point were read_ref is called to actually read existing branches from the repo is in update_branch to check whether the parent was set correctly. What is missing is a call to read_ref in either lookup_branch or new_branch (probably both have to be reworked in some way to handle this cleanly). From all I can see a fix should be fairly straightforward to implement, but I am really not sure whether I have the full picture on this. (I found all of this struggling with git-p4.py which appears to contains a complex and not fully correct mechanism to determine the 'initalParent' that appears to implement just such a workaround.) I would be grateful for any input on this issue! Greetings, Norbert
[PATCH 1/2] modified dir-iterator.c
--- Some places in git use raw API opendir/readdir/closedir to traverse a directory recursively, which usually involves function recursion. Now that we have struct dir_iterator,we have to convert these to use the dir-iterator to simplify the code. Signed-off-by: Sushma Unnibhavi dir-iterator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dir-iterator.c b/dir-iterator.c index f2dcd82fde..a3b5b8929c 100644 --- a/dir-iterator.c +++ b/dir-iterator.c @@ -169,7 +169,7 @@ int dir_iterator_abort(struct dir_iterator *dir_iterator) struct dir_iterator_level *level = &iter->levels[iter->levels_nr - 1]; - if (level->dir && closedir(level->dir)) { + if (level->dir && (struct dir_iterator_int *)(level->dir)) {//changed closedir to (struct dir_iterator_int *) strbuf_setlen(&iter->base.path, level->prefix_len); warning("error closing directory %s: %s", iter->base.path.buf, strerror(errno)); -- 2.17.1
[GSOC][PATCH 1/2] modified dir-iterator.c
--- Some places in git use raw API opendir/readdir/closedir to traverse a directory recursively, which usually involves function recursion. Now that we have struct dir_iterator,we have to convert these to use the dir-iterator to simplify the code. Signed-off-by: Sushma Unnibhavi dir-iterator.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dir-iterator.c b/dir-iterator.c index f2dcd82fde..a3b5b8929c 100644 --- a/dir-iterator.c +++ b/dir-iterator.c @@ -169,7 +169,7 @@ int dir_iterator_abort(struct dir_iterator *dir_iterator) struct dir_iterator_level *level = &iter->levels[iter->levels_nr - 1]; - if (level->dir && closedir(level->dir)) { + if (level->dir && (struct dir_iterator_int *)(level->dir)) {//changed closedir to (struct dir_iterator_int *) strbuf_setlen(&iter->base.path, level->prefix_len); warning("error closing directory %s: %s", iter->base.path.buf, strerror(errno)); -- 2.17.1
Re: [PATCH v13 18/27] stash: convert create to builtin
Hi Peff, On Thu, 7 Mar 2019, Jeff King wrote: > On Mon, Feb 25, 2019 at 11:16:22PM +, Thomas Gummerer wrote: > > > +static void add_pathspecs(struct argv_array *args, > > + struct pathspec ps) { > > Here and elsewhere in the series, I notice that we pass the pathspec > struct by value, which is quite unusual for our codebase (and > potentially confusing, if any of the callers were to mutate the pointers > in the struct). > > Is there any reason this shouldn't be "const struct pathspec *ps" pretty > much throughout the file? I am quite certain that this is merely an oversight. It totes slipped by my review, for example. Thanks for catching! Dscho
how can i "gc" or "prune" objects related to a deleted remote?
writing a short tutorial on how to add a remote and work with it and, as a final step, show how, if one is uninterested in the remote after all, one can simply delete it, but i also want to show how one can then prune or garbage collect the objects related to that remote, but i can't figure out how. as an example, i cloned the linux kernel source tree, then added the linux-next repo as a remote -- the end result was two pack files (the smaller one i'm assuming being for linux-next): $ ls -l .git/objects/pack total 2723632 -r--r--r--. 1 rpjday rpjday1215376 Mar 8 09:44 pack-08cc266c0914e924961a1c8412fdf8746d327d7e.idx -r--r--r--. 1 rpjday rpjday 38402840 Mar 8 09:44 pack-08cc266c0914e924961a1c8412fdf8746d327d7e.pack -r--r--r--. 1 rpjday rpjday 204032716 Mar 8 09:42 pack-1036510bb74967c91093fc0cd8982683a66dbf5f.idx -r--r--r--. 1 rpjday rpjday 254527 Mar 8 09:42 pack-1036510bb74967c91093fc0cd8982683a66dbf5f.pac $ after playing with a couple branches from the new remote, i deleted the remote, then also did things like clear the reflog, delete any local tracking branches related to the deleted remote, and so on, but i can't seem to prune the objects that should no longer be reachable now that i deleted that remote. what am i overlooking? is it because those objects are in a separate pack file? do i need to repack first? what is hanging onto those objects in that second pack file such that they can't be garbage collected? rday -- Robert P. J. Day Ottawa, Ontario, CANADA http://crashcourse.ca/dokuwiki Twitter: http://twitter.com/rpjday LinkedIn: http://ca.linkedin.com/in/rpjday
Re: [PATCH v2] line-log: suppress diff output with "-s"
Hi Peff, On Thu, 7 Mar 2019, Jeff King wrote: > When "-L" is in use, we ignore any diff output format that the user > provides to us, and just always print a patch (with extra context lines > covering the whole area of interest). It's not entirely clear what we > should do with all formats (e.g., should "--stat" show just the diffstat > of the touched lines, or the stat for the whole file?). > > But "-s" is pretty clear: the user probably wants to see just the > commits that touched those lines, without any diff at all. Let's at > least make that work. Agree. The patch looks obviously good. Thank you, Dscho > > Signed-off-by: Jeff King > --- > This is a repost from the thread at: > > > https://public-inbox.org/git/caekqehdfu5zm4ay3ihn0pn1acneomy0wv07pryfab45jn-t...@mail.gmail.com/ > > line-log.c | 6 -- > t/t4211-line-log.sh | 7 +++ > 2 files changed, 11 insertions(+), 2 deletions(-) > > diff --git a/line-log.c b/line-log.c > index 24e21731c4..59248e37cc 100644 > --- a/line-log.c > +++ b/line-log.c > @@ -1103,10 +1103,12 @@ static int process_all_files(struct line_log_data > **range_out, > > int line_log_print(struct rev_info *rev, struct commit *commit) > { > - struct line_log_data *range = lookup_line_range(rev, commit); > > show_log(rev); > - dump_diff_hacky(rev, range); > + if (!(rev->diffopt.output_format & DIFF_FORMAT_NO_OUTPUT)) { > + struct line_log_data *range = lookup_line_range(rev, commit); > + dump_diff_hacky(rev, range); > + } > return 1; > } > > diff --git a/t/t4211-line-log.sh b/t/t4211-line-log.sh > index bd5fe4d148..c9f2036f68 100755 > --- a/t/t4211-line-log.sh > +++ b/t/t4211-line-log.sh > @@ -115,4 +115,11 @@ test_expect_success 'range_set_union' ' > git log $(for x in $(test_seq 200); do echo -L $((2*x)),+1:c.c; done) > ' > > +test_expect_success '-s shows only line-log commits' ' > + git log --format="commit %s" -L1,24:b.c >expect.raw && > + grep ^commit expect.raw >expect && > + git log --format="commit %s" -L1,24:b.c -s >actual && > + test_cmp expect actual > +' > + > test_done > -- > 2.21.0.787.g929e938557 >
Re: fast-import on existing branches
Hi Norbert, On Fri, Mar 8, 2019 at 2:51 AM Norbert Nemec wrote: > > Hi there, > > I've struggled for quite some time to sort out documented, intended and > actual behavior of git fast-import. Unless I'm completely mistaken, it seems > to be a straightforward bug, but if that is the case, I am really surprised > why nobody else has stumbled over it before: > > I managed to use fast-import for a chain of commits onto a new branch into an > empty repository. > I managed to use fast-import to create a new branch starting from an existing > parent using the 'from' command as documented. > > What I failed to do is to add commits on top of an existing branch in a new > fast-import stream. As it seems, the variant of using 'commit' without 'from' > only works on branches that were created within the same fast-import stream! > > The different approaches I tried (each with new fast-import stream on > existing repo with existing branch) > * 'commit' without 'from' > -> Error: "Not updating (new tip does not contain ) > And indeed looking into the repo afterwards, a new commit exists without any > parent. > * 'commit' with 'from' both naming the same branch > -> Error: "Can't create a branch from itself" > The only workarounds that I could find are to either explicitly looking up > the top commit on the target branch and hand that to fast-import or create a > temporary branch with a different name. I would have just used "from " where is something I look up from the current branch I want to update. But, re-looking at the docs, it appears git-fast-import.txt covers this already with a possibly easier syntax: """ The special case of restarting an incremental import from the current branch value should be written as: from refs/heads/branch^0 The `^0` suffix is necessary as fast-import does not permit a branch to start from itself, and the branch is created in memory before the `from` command is even read from the input. Adding `^0` will force fast-import to resolve the commit through Git's revision parsing library, rather than its internal branch table, thereby loading in the existing value of the branch. """ Perhaps try that? > Looking through the code of fast-import.c, I can indeed lookup_branch and > new_branch only deal with internal data structures and the only point were > read_ref is called to actually read existing branches from the repo is in > update_branch to check whether the parent was set correctly. What is missing > is a call to read_ref in either lookup_branch or new_branch (probably both > have to be reworked in some way to handle this cleanly). From all I can see a > fix should be fairly straightforward to implement, but I am really not sure > whether I have the full picture on this. > > (I found all of this struggling with git-p4.py which appears to contains a > complex and not fully correct mechanism to determine the 'initalParent' that > appears to implement just such a workaround.) > > I would be grateful for any input on this issue! Greetings, > Norbert Hope that helps, Elijah
Re: git reset error on Windows
Hi Adrian, On Thu, 7 Mar 2019, Adrian Godong wrote: > Windows 10, git version 2.21.0.windows.1 > > git reset tries to delete folder when last file is removed but failed > to do so if shell is in the deleted folder. > > Repro steps (powershell): > mkdir test > cd test > git init > mkdir dir > cd dir > add-content .\file "" > git add . > git commit -m "1" > git mv .\file .\file2 > git commit -m "2" > git reset --hard HEAD^ > > Deletion of directory 'dir' failed. Should I try again? (y/n) > > Choosing y will repeat the same prompt. Choosing n will complete the > operation correctly. In contrast to Linux, it is not possible to delete a current working directory (unless you delve into horrible hacks like Cygwin does). So this is a restriction of the platform on which you are working, and there is nothing we can do about it, at least as far as I can think of. Ciao, Johannes
New Ft. for Git : Allow resumable cloning of repositories.
Objective: Allow pause and resume functionality while cloning repositories. Below is a rough idea on how this may be achieved. 1) Create a repository_name.json file. 2) repository_name.json will be an index file containing list of all the files in the repository with default status being "False". "False" status of a file signifies that this file is not yet fully downloaded. Something like this: { 'file1.ext' : "False", 'file2.ext' : "False", 'file3.ext' : "False" } 3) As a file finishes downloading, say 'file1.ext' and 'file2.ext' have finished downloading, their status will change to: Something like this: { 'file1.ext' : "True", 'file2.ext' : "True", 'file3.ext' : "False" } 4) Suppose due to some reason, before 'file3.ext' could finish download; cloning is interrupted. 5) After the interruption the repository_name.json and downloaded files are preserved. 6) Now, when cloning of the same repository begins next time, files would be downloaded based on information taken from repository_name.json file. Note 1: Doing this for cloning would be the main objective, further this may be extended for fetching, pulling, and pushing too. Note 2: Since this is gsoc time, please don't take this to be a project idea for gsoc, as it was pointed out on irc that this would be a time intensive functionality. I want to work on building this functionality. Please discuss thoughts on this, so as to make a technically sound to-do list.
[PATCH 1/1] mingw: allow building with an MSYS2 runtime v3.x
From: Johannes Schindelin Recently the Git for Windows project started the upgrade process to a MSYS2 runtime version based on Cygwin v3.x. This has the very notable consequence that `$(uname -r)` no longer reports a version starting with "2", but a version with "3". That breaks our build, as df5218b4c30b (config.mak.uname: support MSys2, 2016-01-13) simply did not expect the version reported by `uname -r` to depend on the underlying Cygwin version: it expected the reported version to match the "2" in "MSYS2". So let's invert that test case to test for *anything else* than a version starting with "1" (for MSys). That should safeguard us for the future, even if Cygwin ends up releasing versionsl like 314.272.65536. Signed-off-by: Johannes Schindelin --- config.mak.uname | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.mak.uname b/config.mak.uname index c8b0e34c31..0207e895a4 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -569,7 +569,7 @@ ifneq (,$(wildcard ../THIS_IS_MSYSGIT)) NO_GETTEXT = YesPlease COMPAT_CLFAGS += -D__USE_MINGW_ACCESS else - ifeq ($(shell expr "$(uname_R)" : '2\.'),2) + ifneq ($(shell expr "$(uname_R)" : '1\.'),2) # MSys2 prefix = /usr/ ifeq (MINGW32,$(MSYSTEM)) -- gitgitgadget
[PATCH 0/1] mingw: fix uname -r test
In df5218b4c30b (config.mak.uname: support MSys2, 2016-01-13), I obviously made the assumption that calling uname -r in MSYS2 would always yield a version number starting with "2". That is incorrect, though, as uname -r reports the version of the Cygwin runtime on which the current MSYS2 runtime is based. This sadly breaks our build as soon as we upgrade to an MSYS2 runtime that is based on Cygwin v3.0.2. Happily, this patch fixes that. Johannes Schindelin (1): mingw: allow building with an MSYS2 runtime v3.x config.mak.uname | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) base-commit: 6e0cc6776106079ed4efa0cc9abace4107657abf Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-160%2Fdscho%2Fmsys2-3.x-v1 Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-160/dscho/msys2-3.x-v1 Pull-Request: https://github.com/gitgitgadget/git/pull/160 -- gitgitgadget
Re: [PATCH 0/2] stash: handle pathspec magic again
Hi Junio, On Fri, 8 Mar 2019, Junio C Hamano wrote: > "Johannes Schindelin via GitGitGadget" > writes: > > > It was reported in https://github.com/git-for-windows/git/issues/2037 that > > git stash -- ':(glob)**/*.testextension is broken. The problem is not even > > the stash operation itself, it happens when the git add part of dropping the > > local changes is spawned: we simply passed the parsed pathspec instead of > > the unparsed one. > > > > While verifying the fix, I also realized that the escape hatch was seriously > > broken by my "backport of the -q option": It introduced four bogus eval > > statements, which really need to be dropped. > > Thanks. > > We seem to be piling many "oops" on top, even after the topic hits > 'next'. But fixes are better late than never ;-). Yes, we do. At least now I do not have the impression that I have to impose on Paul to integrate whatever diffs I came up with, I can now just submit small patch series that are easily reviewed. If you care deeply about the commit history, I hereby offer to you to clean up the built-in stash patches when you say you're ready to advance them to `master`; I will then squash the fixes into the proper places to make it a nicer read, with the promise that the tree will be the same in the end as with the oops-upon-oops patch history that we're piling up in `next`. I would do that for you. > Will queue. Thanks, Dscho
Re: [PATCH v2] line-log: suppress diff output with "-s"
On Fri, Mar 08, 2019 at 04:38:44PM +0100, Johannes Schindelin wrote: > On Thu, 7 Mar 2019, Jeff King wrote: > > > When "-L" is in use, we ignore any diff output format that the user > > provides to us, and just always print a patch (with extra context lines > > covering the whole area of interest). It's not entirely clear what we > > should do with all formats (e.g., should "--stat" show just the diffstat > > of the touched lines, or the stat for the whole file?). > > > > But "-s" is pretty clear: the user probably wants to see just the > > commits that touched those lines, without any diff at all. Let's at > > least make that work. > > Agree. The patch looks obviously good. Thanks. This leaves the other formats as silently ignored. Do we want to do something like this: diff --git a/revision.c b/revision.c index eb8e51bc63..a1b4fe2aa6 100644 --- a/revision.c +++ b/revision.c @@ -2689,6 +2689,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, struct s if (revs->first_parent_only && revs->bisect) die(_("--first-parent is incompatible with --bisect")); + if (revs->line_level_traverse && + (revs->diffopt.output_format & ~(DIFF_FORMAT_PATCH|DIFF_FORMAT_NO_OUTPUT))) + die(_("-L does not yet support diff formats besides -p and -s")); + if (revs->expand_tabs_in_log < 0) revs->expand_tabs_in_log = revs->expand_tabs_in_log_default; ? -Peff
Re: [PATCH 0/1] Drop last MakeMaker reference
Hi Junio, On Fri, 8 Mar 2019, Junio C Hamano wrote: > Johannes Schindelin writes: > > > On Sun, 3 Mar 2019, Junio C Hamano wrote: > > > >> "Johannes Schindelin via GitGitGadget" > >> writes: > >> > >> > Back when we stopped using MakeMaker, we forgot one reference... > >> > > >> > Johannes Schindelin (1): > >> > mingw: drop MakeMaker reference > >> > > >> > config.mak.uname | 1 - > >> > 1 file changed, 1 deletion(-) > >> > > >> > > >> > base-commit: 8104ec994ea3849a968b4667d072fedd1e688642 > >> > Published-As: > >> > https://github.com/gitgitgadget/git/releases/tag/pr-146%2Fdscho%2Fno-perl-makemaker-v1 > >> > Fetch-It-Via: git fetch https://github.com/gitgitgadget/git > >> > pr-146/dscho/no-perl-makemaker-v1 > >> > Pull-Request: https://github.com/gitgitgadget/git/pull/146 > >> > >> Good ;-) > >> Thanks. > > > > Gentle reminder that this has not made it into `pu` yet... > > Thanks. > > I'll try to make it a habit not to respond to 0/1 (but instead to > 1/1) as it is quite inefficient to get to 1/1 from 0/1 at least for > me X-<. Hehe. I do have something for you there: https://github.com/git-for-windows/build-extra/blob/master/apply-from-public-inbox.sh -- snip -- > Or perhaps GGG can learn to avoid 0/1 for a single-patch topic ;-) It is easier, and more consistent, to have a cover letter even then, for things like the broader explanation of the context, the changes since the previous iteration, and the range-diff (which would make v2, v3, v4, etc utterly unreadable from my point of view if they were integrated into the single patches, as I used to do with interdiffs). > Thanks anyway. Will try to apply directly on top of master. Thank you! While we're talking about "directly on top of master"... *after* it got some review, I would love to ask for the same favor for https://public-inbox.org/git/pull.160.git.gitgitgad...@gmail.com On the other hand, it is an old bug, I know, and it will break all CI builds for branches that are based off of older commits, anyway. I guess we'll need some sort of horrible hack in the git-sdk-64-minimal thing, to allow for patching this on the CI side, without adding commits to all of those branches. :-( So: I made up my mind, and that MSYS2 runtime v3.x patch does not need to be fast-tracked; it would not fix all the CI builds... Ciao, Dscho
Re: [RFC PATCH 0/5] Fix some fast-import parsing issues
Hi, On Wed, Feb 20, 2019 at 2:58 PM Elijah Newren wrote: > > I found a few issues with parsing in fast-import (dating back to > I've cc'ed the relevant folks, and have a few patches that fix the > issue and I think make the parser more robust against future issues in > a way that I think is safe enough for backward compatibility, but > "backward compatible enough" might concern some folks; if so, please > take a look at patches 4 and 5. Just thought I'd ping to see if folks have any concerns with this slight tweak to backward compatibility; if not, I'll just repost the patches removing the RFC label.
RE: fast-import on existing branches
Thanks, Elijah, I had indeed missed that block about the ^0 handling. I still don't get why this awkward workaround is required. Why isn't that lookup done by default? Performance can't be the reason, since the same lookup is done lateron anyway, just as correctness check. The way I read the documentation, providing no "from" should continue committing to a branch in any case. I would never have seen the continuation of an incremental import a "special case". There is a number of tools around that sync a git repo from some other source and would regularly need to continue an existing branch. Greetings, Norbert -Original Message- From: Elijah Newren Sent: Friday, March 8, 2019 4:41 PM To: Norbert Nemec Cc: git@vger.kernel.org Subject: Re: fast-import on existing branches Hi Norbert, On Fri, Mar 8, 2019 at 2:51 AM Norbert Nemec wrote: > > Hi there, > > I've struggled for quite some time to sort out documented, intended and > actual behavior of git fast-import. Unless I'm completely mistaken, it seems > to be a straightforward bug, but if that is the case, I am really surprised > why nobody else has stumbled over it before: > > I managed to use fast-import for a chain of commits onto a new branch into an > empty repository. > I managed to use fast-import to create a new branch starting from an existing > parent using the 'from' command as documented. > > What I failed to do is to add commits on top of an existing branch in a new > fast-import stream. As it seems, the variant of using 'commit' without 'from' > only works on branches that were created within the same fast-import stream! > > The different approaches I tried (each with new fast-import stream on > existing repo with existing branch) > * 'commit' without 'from' > -> Error: "Not updating (new tip does not contain > -> ) > And indeed looking into the repo afterwards, a new commit exists without any > parent. > * 'commit' with 'from' both naming the same branch > -> Error: "Can't create a branch from itself" > The only workarounds that I could find are to either explicitly looking up > the top commit on the target branch and hand that to fast-import or create a > temporary branch with a different name. I would have just used "from " where is something I look up from the current branch I want to update. But, re-looking at the docs, it appears git-fast-import.txt covers this already with a possibly easier syntax: """ The special case of restarting an incremental import from the current branch value should be written as: from refs/heads/branch^0 The `^0` suffix is necessary as fast-import does not permit a branch to start from itself, and the branch is created in memory before the `from` command is even read from the input. Adding `^0` will force fast-import to resolve the commit through Git's revision parsing library, rather than its internal branch table, thereby loading in the existing value of the branch. """ Perhaps try that? > Looking through the code of fast-import.c, I can indeed lookup_branch and > new_branch only deal with internal data structures and the only point were > read_ref is called to actually read existing branches from the repo is in > update_branch to check whether the parent was set correctly. What is missing > is a call to read_ref in either lookup_branch or new_branch (probably both > have to be reworked in some way to handle this cleanly). From all I can see a > fix should be fairly straightforward to implement, but I am really not sure > whether I have the full picture on this. > > (I found all of this struggling with git-p4.py which appears to > contains a complex and not fully correct mechanism to determine the > 'initalParent' that appears to implement just such a workaround.) > > I would be grateful for any input on this issue! Greetings, Norbert Hope that helps, Elijah
Re: New Ft. for Git : Allow resumable cloning of repositories.
> Objective: Allow pause and resume functionality while cloning repositories. > > Below is a rough idea on how this may be achieved. This is indeed a nice feature to have, and thanks for details of how this would be accomplished. > 1) Create a repository_name.json file. > 2) repository_name.json will be an index file containing list of all > the files in the repository with default status being "False". >"False" status of a file signifies that this file is not yet fully > downloaded. > > Something like this: > > { > 'file1.ext' : "False", > 'file2.ext' : "False", > 'file3.ext' : "False" > } One issue is that when cloning a repository, we do not download many files - we only download one dynamically generated packfile containing all the objects we want. You might be interested in some work I'm doing to offload part of the packfile response to CDNs: https://public-inbox.org/git/cover.1550963965.git.jonathanta...@google.com/ This means that when cloning/fetching, multiple files could be downloaded, meaning that a scheme like you suggest would be more worthwhile. (In fact, I allude to such a scheme in the design document in patch 5.)
Re: One failed self test on Fedora 29
Hi, Jeffrey Walton wrote: > Fedora 29, x86_64. One failed self test: > > *** t0021-conversion.sh *** [...] > not ok 13 - disable filter with empty override > # > # test_config_global filter.disable.smudge false && > # test_config_global filter.disable.clean false && > # test_config filter.disable.smudge false && > # test_config filter.disable.clean false && > # > # echo "*.disable filter=disable" >.gitattributes && > # > # echo test >test.disable && > # git -c filter.disable.clean= add test.disable 2>err && > # test_must_be_empty err && > # rm -f test.disable && > # git -c filter.disable.smudge= checkout -- test.disable 2>err > && > # test_must_be_empty err > # [...] > # failed 1 among 26 test(s) > 1..26 > gmake[2]: *** [Makefile:56: t0021-conversion.sh] Error 1 > > Does anyone need a config.log or other test data? It would probably help to know what commit you're building. The verbose test output would also be useful, e.g.: cd t && ./t0021-conversion.sh -v -i If it's not reliably reproducible, the --stress* options might help catch a failing run. FWIW, I just built and ran the tests on a Fedora 29 container for master, next, and pu a few times (some with various --stress options) without any test failures. I did this with and without a config.mak from the fedora git packages. I've never used the configure script, it seems like unnecessary overhead. $ git branch -v master 6e0cc67761 Start 2.22 cycle next 541d9dca55 Merge branch 'yb/utf-16le-bom-spellfix' into next * pu 7eadd8ba98 Merge branch 'js/remote-curl-i18n' into pu -- Todd
Re: [PATCH v3 00/21] Add new command "switch"
On 08/03/2019 09:57, Nguyễn Thái Ngọc Duy wrote: [snip] > Range-diff dựa trên v2: > -: -- > 1: 949f3dd4fd git-checkout.txt: spell out --no-option > 1: 8358b9ca36 = 2: 1ddbbae3e2 git-checkout.txt: fix one syntax line > 2: 1686ccbf8d ! 3: b0cb2372db doc: document --overwrite-ignore > @@ -14,14 +14,15 @@ > out anyway. In other words, the ref can be held by more than one > worktree. > > -+--[no-]overwrite-ignore:: > ++--overwrite-ignore:: > ++--no-overwrite-ignore:: Just curious, but why? Is '--[no-]overwrite-ignore' thought to be harder to read? What about the rest of the man-pages? ATB, Ramsay Jones
Re: [PATCH v1 01/11] checkout: split part of it to new command 'restore'
Thanks for working on this; overall looks really good. I've got a few comments here and there on the wording and combinations of options... On Fri, Mar 8, 2019 at 2:17 AM Nguyễn Thái Ngọc Duy wrote: > +SYNOPSIS It might be worth adding some words somewhere to differentiate between `reset` and `restore`. E.g. `git restore` modifies the working tree (and maybe index) to change file content to match some other (usually older) version, but does not update your branch. `git reset` is about modifying which commit your branch points to, meaning possibly removing and/or adding many commits to your branch. It may also make sense to add whatever description you use to the reset manpage. > + > +[verse] > +'git restore' [] [--source=] ... > +'git restore' (-p|--patch) [--source=] [...] So one cannot specify any special options with -p? Does that mean one cannot use it with --index (i.e. this command cannot replace 'git reset -p')? Or is this an oversight in the synopsis? > +DESCRIPTION > +--- > +Restore paths in the working tree by replacing with the contents in > +the restore source or remove if the paths do not exist in the restore > +source. The source is by default the index but could be from a commit. > +The command can also optionally restore content in the index from a > +commit. The first sentence makes it sound like two different operations, when I think it is one. Perhaps: Restore paths in the working tree by replacing with the contents in the restore source (and if the restore source is missing paths found in the working tree, that means deleting those paths from the working tree). > + > +When a `` is given, the paths that match the `` are > +updated both in the index and in the working tree. I thought the default was --worktree. Is this sentence from an older version of your patch series that you forgot to update? > + > +OPTIONS > +--- > +-s:: > +--source=:: > + Restore the working tree files with the content from the given > + tree or any revision that leads to a tree (e.g. a commit or a > + branch). I think that's a little hard to parse. We may not be able to avoid "working tree" vs. "tree" confusion, but the spelling of feels like it should be a separate sentence. Maybe: Restore the working tree files with the content from the given tree. It is common to specify the source tree by naming a commit, branch, or tag. ? > + > +-p:: > +--patch:: > + Interactively select hunks in the difference between the > + `` (or the index, if unspecified) and the working > + tree. See the ``Interactive Mode'' section of linkgit:git-add[1] > + to learn how to operate the `--patch` mode. > + > +-W:: > +--worktree:: > +-I:: > +--index:: > + Specify the restore location. If neither option is specified, > + by default the working tree is restored. If `--index` is > + specified without `--worktree` or `--source`, `--source=HEAD` > + is implied. These options only make sense to use with > + `--source`. Seems like this contains a minor contradiction. Perhaps start the final sentence with: "Otherwise, ..." ? > +-q:: > +--quiet:: > + Quiet, suppress feedback messages. > + > +--progress:: > +--no-progress:: > + Progress status is reported on the standard error stream > + by default when it is attached to a terminal, unless `--quiet` > + is specified. This flag enables progress reporting even if not > + attached to a terminal, regardless of `--quiet`. I'm assuming this means there are feedback messages other than progress feedback? > +-f:: > +--force:: > + If `--source` is not specified, unmerged entries are left alone > + and will not fail the operation. Unmerged entries are always > + replaced if `--source` is specified, regardless of `--force`. This may be slightly confusing, in particular it suggests that --index (or --worktree and --index) are the default. Is --force only useful when --index is specified? If it has utility with --worktree only, what does it do? Also, what happens when there are unmerged entries in the index and someone tries to restore just working tree files -- are the ones corresponding to unmerged entries skipped (if so, silently or with warnings printed for the user?), or does something else happen? > +--ours:: > +--theirs:: > + Check out stage #2 ('ours') or #3 ('theirs') for unmerged > + paths. > ++ > +Note that during `git rebase` and `git pull --rebase`, 'ours' and > +'theirs' may appear swapped. See the explanation of the same options > +in linkgit:git-checkout[1] for details. So sad, but yes you need to mention it. I'm curious what we say for cherry-pick; it's not clear to me whether people think of the current branch as "ours" or the commit they wrote themselves and are trying to bring to this branch as "ours". There are probably examples of both. Not that I think you can do anything about that here or need to ch
Re: [PATCH v2] line-log: suppress diff output with "-s"
Hi Peff, On Fri, 8 Mar 2019, Jeff King wrote: > On Fri, Mar 08, 2019 at 04:38:44PM +0100, Johannes Schindelin wrote: > > > On Thu, 7 Mar 2019, Jeff King wrote: > > > > > When "-L" is in use, we ignore any diff output format that the user > > > provides to us, and just always print a patch (with extra context lines > > > covering the whole area of interest). It's not entirely clear what we > > > should do with all formats (e.g., should "--stat" show just the diffstat > > > of the touched lines, or the stat for the whole file?). > > > > > > But "-s" is pretty clear: the user probably wants to see just the > > > commits that touched those lines, without any diff at all. Let's at > > > least make that work. > > > > Agree. The patch looks obviously good. > > Thanks. This leaves the other formats as silently ignored. I'd be fine with that... but... > Do we want to do something like this: > > diff --git a/revision.c b/revision.c > index eb8e51bc63..a1b4fe2aa6 100644 > --- a/revision.c > +++ b/revision.c > @@ -2689,6 +2689,10 @@ int setup_revisions(int argc, const char **argv, > struct rev_info *revs, struct s > if (revs->first_parent_only && revs->bisect) > die(_("--first-parent is incompatible with --bisect")); > > + if (revs->line_level_traverse && > + (revs->diffopt.output_format & > ~(DIFF_FORMAT_PATCH|DIFF_FORMAT_NO_OUTPUT))) > + die(_("-L does not yet support diff formats besides -p and > -s")); > + > if (revs->expand_tabs_in_log < 0) > revs->expand_tabs_in_log = revs->expand_tabs_in_log_default; Since you already have that patch, why not go wild and apply it, too? ;-) I guess you copy-edited the code from somewhere because you usually do leave spaces around the `|`... I don't care, though ;-) Ciao, Dscho
[PATCH] upload-pack.c: fix a sparse 'NULL pointer' warning
Signed-off-by: Ramsay Jones --- Hi Jonathan, If you need to re-roll your 'jt/fetch-cdn-offload' branch, could you please squash this into the relevant patch (commit 0e821b4427 ("upload-pack: send part of packfile response as uri", 2019-02-23)). Thanks! ATB, Ramsay Jones upload-pack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/upload-pack.c b/upload-pack.c index 534e8951a2..a421df2bbb 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -1164,7 +1164,7 @@ void upload_pack(struct upload_pack_options *options) if (want_obj.nr) { struct object_array have_obj = OBJECT_ARRAY_INIT; get_common_commits(&reader, &have_obj, &want_obj); - create_pack_file(&have_obj, &want_obj, 0); + create_pack_file(&have_obj, &want_obj, NULL); } } -- 2.21.0
Re: [PATCH v5 02/10] add--helper: create builtin helper for interactive add
Hi Junio, On Thu, 21 Feb 2019, Junio C Hamano wrote: > "Daniel Ferreira via GitGitGadget" writes: > > > diff --git a/git.c b/git.c > > index 2dd588674f..cb42591f37 100644 > > --- a/git.c > > +++ b/git.c > > @@ -444,6 +444,7 @@ static int run_builtin(struct cmd_struct *p, int argc, > > const char **argv) > > > > static struct cmd_struct commands[] = { > > { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE }, > > + { "add--helper", cmd_add__helper, RUN_SETUP | NEED_WORK_TREE }, > > { "am", cmd_am, RUN_SETUP | NEED_WORK_TREE }, > > { "annotate", cmd_annotate, RUN_SETUP | NO_PARSEOPT }, > > { "apply", cmd_apply, RUN_SETUP_GENTLY }, > > When adding a more complex replacement command we often cannot write > RUN_SETUP nor NEED_WORK_TREE here and instead do the job ourselves > in cmd_ourcommand() the hard way. But because this one is only for > helping "add -i", we can let the git.c::run_builtin() aka "git > potty" do the hard part, which is very nice. Sadly, I ran into a quite curious (and seemingly equally insurmountable) problem with this approach: when I use `system()` to execute a Git built-in from the Perl script, and when that built-in then consumes even a single byte of stdin, that file descriptor gets closed in the Perl process, and no amount of file descriptor duplication and redirecting that I could come up helped. After calling the built-in helper to run, say, the `update` command, the main interactive add process always exited on me due to EOF on stdin, and I could not make it work. To be sure, this happens only with MSYS2's Perl. If I try the same on Linux (WSL, really), everything works Just Fine. So my current approach is actually the other way round: I started implementing the main loop in C, and implemented all commands except for `patch` so far, and I think I will just hand off to the Perl script for that command for now. Unfortunately that means that my current state looks already quite a bit different than what you reviewed. My hope is, however, that it will make for a nice "story" once I have a version to show that I think could be applied as-is. Ciao, Dscho
Re: Bug: git for Windows spawning a lot of processes recursively
Hi Pierre, On Fri, 22 Feb 2019, Garcia, Pierre wrote: > I'd like to report an issue with git for Windows > > Git version 2.20.1 > Windows 7 x64 (Build 7601: Service Pack 1) > > > Issue: > When running from Git-bash (not even inside a repo), git commands from > built-in are OK, but if I use the exe that is located at C:\Program > Files\Git\bin\git.exe, the process takes long to execute and then exits > with error "error launching git: The filename or extension is too > long.": Is this still happening with v2.21.0? Or for that matter, with the latest snapshot at https://wingit.blob.core.windows.net/files/index.html? Ciao, Johannes > > $ git --version > git version 2.20.1.windows.1 > > $ which git > /mingw64/bin/git > > $ /c/Program\ Files/Git/bin/git.exe --version > error launching git: The filename or extension is too long. > > > It started out of the blue, nothing special was done on the computer, the > previous day I cloned some repos, the next it was not working anymore. > I tried uninstall-reinstall with no success. > I'm using Gitextensions as well, but I verified that the problem was > occurring without Gitextensions (uninstalled at the time the traces were > taken). > > > I ran procmon.exe to monitor the activity of the process and I found out that > git.exe was spawning itself 385 times in total in the trace I took, until all > processes exit with error code 1, here is an extract of the procmon trace, > showing thread and process activity for git.exe (I have more exhaustive trace > but the file is quite big), I included the environment variables on the first > call: > > Time of Day Process NamePID Operation PathResult Detail > 8:57:20.7 git.exe 12744 Process Start SUCCESS "Parent PID: > 8412, Command line: ""C:\Program Files\Git\bin\git.exe"" --version, Current > directory: C:\Users\gpierre\Desktop\, Environment: > ; =::=::\ > ; =C:=C:\Users\gpierre\Desktop > ; =ExitCode=0001 > ; ALLUSERSPROFILE=C:\ProgramData > ; CommonProgramFiles=C:\Program Files\Common Files > ; CommonProgramFiles(x86)=C:\Program Files (x86)\Common Files > ; CommonProgramW6432=C:\Program Files\Common Files > ; ComSpec=C:\windows\system32\cmd.exe > ; DEFLOGDIR=C:\ProgramData\McAfee\Endpoint Security\Logs > ; FP_NO_HOST_CHECK=NO > ; GTK_BASEPATH=C:\Program Files (x86)\GtkSharp\2.12\ > ; HasSCCM2012Client=1 > ; HOME=C:\Users\gpierre > ; HOMEDRIVE=C: > ; HOMEPATH=\Users\gpierre > ; HOMESHARE=C:\Users\gpierre > ; LOCALAPPDATA=C:\Users\gpierre\AppData\Local > ; NUMBER_OF_PROCESSORS=4 > ; OS=Windows_NT > ; > Path=C:\ProgramData\Oracle\Java\javapath;C:\windows\system32;C:\windows;C:\windows\System32\Wbem;C:\windows\System32\WindowsPowerShell\v1.0\;C:\Program > Files\Dell\Dell Data Protection\Encryption\;C:\Program Files (x86)\NVIDIA > Corporation\PhysX\Common;C:\Program Files\Microsoft SQL > Server\130\Tools\Binn\;C:\Program Files\dotnet\;C:\Program Files > (x86)\GtkSharp\2.12\bin;C:\windows\system32\config\systemprofile\.dnx\bin;C:\Program > Files\Microsoft > DNX\Dnvm\;C:\EASE\APEX12-4.4.0\bin;C:\EASE\APEX12-4.4.0\platforms\default\DEOS;C:\Program > > Files\PuTTY\;C:\windows\System32\WindowsPowerShell\v1.0\;C:\windows\System32\WindowsPowerShell\v1.0\;C:\Program > > Files\1E\NomadBranch\;C:\HashiCorp\Vagrant\bin;C:\windows\System32\WindowsPowerShell\v1.0\ > ; PATHEXT=.COM;.EXE;.BAT;.CMD;.VBS;.VBE;.JS;.JSE;.WSF;.WSH;.MSC > ; PROCESSOR_ARCHITECTURE=AMD64 > ; PROCESSOR_IDENTIFIER=Intel64 Family 6 Model 94 Stepping 3, GenuineIntel > ; PROCESSOR_LEVEL=6 > ; PROCESSOR_REVISION=5e03 > ; ProgramData=C:\ProgramData > ; ProgramFiles=C:\Program Files > ; ProgramFiles(x86)=C:\Program Files (x86) > ; ProgramW6432=C:\Program Files > ; PROMPT=$P$G > ; PSModulePath=C:\Program > Files\WindowsPowerShell\Modules;C:\windows\system32\WindowsPowerShell\v1.0\Modules > ; PUBLIC=C:\Users\Public > ; SESSIONNAME=Console > ; SNC_LIB=gsskrb5.dll > ; SOEDataPartition=C: > ; SOEDesktopAdminModel=User > ; SOESystemPartitionLabel=SOE-Disk > ; SystemDrive=C: > ; SystemRoot=C:\windows > ; TEMP=C:\Users\gpierre\AppData\Local\Temp > ; TMP=C:\Users\gpierre\AppData\Local\Temp > ; USERNAME=gpierre > ; USERPROFILE=C:\Users\gpierre > ; VBOX_MSI_INSTALL_PATH=C:\Program Files\Oracle\VirtualBox\ > ; VS140COMNTOOLS=C:\Program Files (x86)\Microsoft Visual Studio > 14.0\Common7\Tools\ > ; VS80COMNTOOLS=C:\Program Files (x86)\Microsoft Visual Studio > 8\Common7\Tools\ > ; windir=C:\windows > ; windows_tracing_flags=3 > ; windows_tracing_logfile=C:\BVTBin\Tests\installpackage\csilogfile.l
[PATCH v2 1/8] http: use --stdin when getting dumb HTTP pack
When Git fetches a pack using dumb HTTP, it reuses the server's name for the packfile (which incorporates a hash), which is different from the behavior of fetch-pack and receive-pack. A subsequent patch will allow downloading packs over HTTP(S) as part of a fetch. These downloads will not necessarily be from a Git repository, and thus may not have a hash as part of its name. Thus, teach http to pass --stdin to index-pack, so that we have no reliance on the server's name for the packfile. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- http.c | 33 +++-- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/http.c b/http.c index a32ad36ddf..34f82af87c 100644 --- a/http.c +++ b/http.c @@ -2204,9 +2204,9 @@ int finish_http_pack_request(struct http_pack_request *preq) { struct packed_git **lst; struct packed_git *p = preq->target; - char *tmp_idx; - size_t len; struct child_process ip = CHILD_PROCESS_INIT; + int tmpfile_fd; + int ret = 0; close_pack_index(p); @@ -2218,35 +2218,24 @@ int finish_http_pack_request(struct http_pack_request *preq) lst = &((*lst)->next); *lst = (*lst)->next; - if (!strip_suffix(preq->tmpfile.buf, ".pack.temp", &len)) - BUG("pack tmpfile does not end in .pack.temp?"); - tmp_idx = xstrfmt("%.*s.idx.temp", (int)len, preq->tmpfile.buf); + tmpfile_fd = xopen(preq->tmpfile.buf, O_RDONLY); argv_array_push(&ip.args, "index-pack"); - argv_array_pushl(&ip.args, "-o", tmp_idx, NULL); - argv_array_push(&ip.args, preq->tmpfile.buf); + argv_array_push(&ip.args, "--stdin"); ip.git_cmd = 1; - ip.no_stdin = 1; + ip.in = tmpfile_fd; ip.no_stdout = 1; if (run_command(&ip)) { - unlink(preq->tmpfile.buf); - unlink(tmp_idx); - free(tmp_idx); - return -1; - } - - unlink(sha1_pack_index_name(p->sha1)); - - if (finalize_object_file(preq->tmpfile.buf, sha1_pack_name(p->sha1)) -|| finalize_object_file(tmp_idx, sha1_pack_index_name(p->sha1))) { - free(tmp_idx); - return -1; + ret = -1; + goto cleanup; } install_packed_git(the_repository, p); - free(tmp_idx); - return 0; +cleanup: + close(tmpfile_fd); + unlink(preq->tmpfile.buf); + return ret; } struct http_pack_request *new_http_pack_request( -- 2.19.0.271.gfe8321ec05.dirty
[PATCH v2 0/8] CDN offloading of fetch response
Here's my current progress - the only thing that is lacking is more tests, maybe, so I think it's ready for review. I wrote in version 1: > I know > that there are some implementation details that could be improved (e.g. > parallelization of the CDN downloads, starting CDN downloads *after* > closing the first HTTP request, holding on to the .keep locks until > after the refs are set), but will work on those once the overall design > is more or less finalized. This code now starts CDN downloads after closing the first HTTP request, and it holds on to the .keep files until after the refs are set. I'll leave parallelization of the CDN downloads for later work. One relatively significant change: someone pointed out that the issue fixed by 50d3413740 ("http: make redirects more obvious", 2016-12-06) could also occur here, so I have changed it so that the server is required to send the packfile's hash along with the URI. This does mean that Ævar's workflow described in [1] would not work. Quoting Ævar: > More concretely, I'd like to have a setup where a server can just dumbly > point to some URL that probably has most of the data, without having any > idea what OIDs are in it. So that e.g. some machine entirely > disconnected from the server (and with just a regular clone) can > continually generating an up-to-date-enough packfile. With 50d3413740, it seems to me that it's important for the server to know details about the URIs that it points to, so such a disconnection would not work. [1] https://public-inbox.org/git/87k1hv6eel@evledraar.gmail.com/ Jonathan Tan (8): http: use --stdin when getting dumb HTTP pack http: improve documentation of http_pack_request http-fetch: support fetching packfiles by URL Documentation: order protocol v2 sections Documentation: add Packfile URIs design doc upload-pack: refactor reading of pack-objects out fetch-pack: support more than one pack lockfile upload-pack: send part of packfile response as uri Documentation/git-http-fetch.txt | 8 +- Documentation/technical/packfile-uri.txt | 78 Documentation/technical/protocol-v2.txt | 44 +-- builtin/fetch-pack.c | 17 ++- builtin/pack-objects.c | 76 +++ connected.c | 8 +- fetch-pack.c | 129 +-- fetch-pack.h | 2 +- http-fetch.c | 64 -- http.c | 82 +++- http.h | 32 - t/t5550-http-fetch-dumb.sh | 25 t/t5702-protocol-v2.sh | 57 + transport-helper.c | 5 +- transport.c | 14 +- transport.h | 6 +- upload-pack.c| 155 +-- 17 files changed, 670 insertions(+), 132 deletions(-) create mode 100644 Documentation/technical/packfile-uri.txt Range-diff against v1: 1: 987c9b686b < -: -- http: use --stdin and --keep when downloading pack -: -- > 1: 87173d0ad1 http: use --stdin when getting dumb HTTP pack 2: 4b53a6f48c ! 2: 66d31720a0 http: improve documentation of http_pack_request @@ -45,4 +45,4 @@ + */ extern struct http_pack_request *new_http_pack_request( struct packed_git *target, const char *base_url); - int finish_http_pack_request(struct http_pack_request *preq, char **lockfile); + extern int finish_http_pack_request(struct http_pack_request *preq); 3: afe73a282d ! 3: 02373f6afe http-fetch: support fetching packfiles by URL @@ -31,7 +31,8 @@ +--packfile:: + Instead of a commit id on the command line (which is not expected in + this case), 'git http-fetch' fetches the packfile directly at the given -+ URL and generates the corresponding .idx file. ++ URL and uses index-pack to generate corresponding .idx and .keep files. ++ The output of index-pack is printed to stdout. + --recover:: Verify that everything reachable from target is fetched. Used after @@ -101,12 +102,12 @@ + struct http_pack_request *preq; + struct slot_results results; + int ret; -+ char *lockfile; + + preq = new_http_pack_request(NULL, url); + if (preq == NULL) + die("couldn't create http pack request"); + preq->slot->results = &results; ++ preq->generate_keep = 1; + + if (start_active_slot(preq->slot)) { + run_active_slot(preq->slot); @@ -118,9 +119,8 @@ + die("Unable to start request"); + } + -+ if ((ret = finish_http_pack_request(preq, &lockfile))) ++ if ((ret = finish_http_pack_request(preq)))
[PATCH v2 6/8] upload-pack: refactor reading of pack-objects out
Subsequent patches will change how the output of pack-objects is processed, so extract that processing into its own function. Currently, at most 1 character can be buffered (in the "buffered" local variable). One of those patches will require a larger buffer, so replace that "buffered" local variable with a buffer array. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- upload-pack.c | 81 ++- 1 file changed, 47 insertions(+), 34 deletions(-) diff --git a/upload-pack.c b/upload-pack.c index d098ef5982..987d2e139b 100644 --- a/upload-pack.c +++ b/upload-pack.c @@ -102,14 +102,52 @@ static int write_one_shallow(const struct commit_graft *graft, void *cb_data) return 0; } +struct output_state { + char buffer[8193]; + int used; +}; + +static int relay_pack_data(int pack_objects_out, struct output_state *os) +{ + /* +* We keep the last byte to ourselves +* in case we detect broken rev-list, so that we +* can leave the stream corrupted. This is +* unfortunate -- unpack-objects would happily +* accept a valid packdata with trailing garbage, +* so appending garbage after we pass all the +* pack data is not good enough to signal +* breakage to downstream. +*/ + ssize_t readsz; + + readsz = xread(pack_objects_out, os->buffer + os->used, + sizeof(os->buffer) - os->used); + if (readsz < 0) { + return readsz; + } + os->used += readsz; + + if (os->used > 1) { + send_client_data(1, os->buffer, os->used - 1); + os->buffer[0] = os->buffer[os->used - 1]; + os->used = 1; + } else { + send_client_data(1, os->buffer, os->used); + os->used = 0; + } + + return readsz; +} + static void create_pack_file(const struct object_array *have_obj, const struct object_array *want_obj) { struct child_process pack_objects = CHILD_PROCESS_INIT; - char data[8193], progress[128]; + struct output_state output_state = {0}; + char progress[128]; char abort_msg[] = "aborting due to possible repository " "corruption on the remote side."; - int buffered = -1; ssize_t sz; int i; FILE *pipe_fd; @@ -239,39 +277,15 @@ static void create_pack_file(const struct object_array *have_obj, continue; } if (0 <= pu && (pfd[pu].revents & (POLLIN|POLLHUP))) { - /* Data ready; we keep the last byte to ourselves -* in case we detect broken rev-list, so that we -* can leave the stream corrupted. This is -* unfortunate -- unpack-objects would happily -* accept a valid packdata with trailing garbage, -* so appending garbage after we pass all the -* pack data is not good enough to signal -* breakage to downstream. -*/ - char *cp = data; - ssize_t outsz = 0; - if (0 <= buffered) { - *cp++ = buffered; - outsz++; - } - sz = xread(pack_objects.out, cp, - sizeof(data) - outsz); - if (0 < sz) - ; - else if (sz == 0) { + int result = relay_pack_data(pack_objects.out, +&output_state); + + if (result == 0) { close(pack_objects.out); pack_objects.out = -1; - } - else + } else if (result < 0) { goto fail; - sz += outsz; - if (1 < sz) { - buffered = data[sz-1] & 0xFF; - sz--; } - else - buffered = -1; - send_client_data(1, data, sz); } /* @@ -296,9 +310,8 @@ static void create_pack_file(const struct object_array *have_obj, } /* flush the data */ - if (0 <= buffered) { - data[0] = buffered; - send_client_data(1, data, 1); + if (output_state.used > 0) { + send_client_data(1, output_state.buffer, output_state.used); fprintf(stderr, "flushed.\n"); } if (use_sideband) -- 2.19.0.271.gfe8321ec05.dirty
[PATCH v2 7/8] fetch-pack: support more than one pack lockfile
Whenever a fetch results in a packfile being downloaded, a .keep file is generated, so that the packfile can be preserved (from, say, a running "git repack") until refs are written referring to the contents of the packfile. In a subsequent patch, a successful fetch using protocol v2 may result in more than one .keep file being generated. Therefore, teach fetch_pack() and the transport mechanism to support multiple .keep files. Implementation notes: - builtin/fetch-pack.c normally does not generate .keep files, and thus is unaffected by this or future changes. However, it has an undocumented "--lock-pack" feature, used by remote-curl.c when implementing the "fetch" remote helper command. In keeping with the remote helper protocol, only one "lock" line will ever be written; the rest will result in warnings to stderr. However, in practice, warnings will never be written because the remote-curl.c "fetch" is only used for protocol v0/v1 (which will not generate multiple .keep files). (Protocol v2 uses the "stateless-connect" command, not the "fetch" command.) - connected.c has an optimization in that connectivity checks on a ref need not be done if the target object is in a pack known to be self-contained and connected. If there are multiple packfiles, this optimization can no longer be done. Signed-off-by: Jonathan Tan --- builtin/fetch-pack.c | 17 +++-- connected.c | 8 +--- fetch-pack.c | 23 --- fetch-pack.h | 2 +- transport-helper.c | 5 +++-- transport.c | 14 -- transport.h | 6 +++--- 7 files changed, 43 insertions(+), 32 deletions(-) diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c index 153a2bd282..709fc4c44b 100644 --- a/builtin/fetch-pack.c +++ b/builtin/fetch-pack.c @@ -48,8 +48,8 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) struct ref **sought = NULL; int nr_sought = 0, alloc_sought = 0; int fd[2]; - char *pack_lockfile = NULL; - char **pack_lockfile_ptr = NULL; + struct string_list pack_lockfiles = STRING_LIST_INIT_DUP; + struct string_list *pack_lockfiles_ptr = NULL; struct child_process *conn; struct fetch_pack_args args; struct oid_array shallow = OID_ARRAY_INIT; @@ -134,7 +134,7 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) } if (!strcmp("--lock-pack", arg)) { args.lock_pack = 1; - pack_lockfile_ptr = &pack_lockfile; + pack_lockfiles_ptr = &pack_lockfiles; continue; } if (!strcmp("--check-self-contained-and-connected", arg)) { @@ -235,10 +235,15 @@ int cmd_fetch_pack(int argc, const char **argv, const char *prefix) } ref = fetch_pack(&args, fd, conn, ref, dest, sought, nr_sought, -&shallow, pack_lockfile_ptr, version); - if (pack_lockfile) { - printf("lock %s\n", pack_lockfile); +&shallow, pack_lockfiles_ptr, version); + if (pack_lockfiles.nr) { + int i; + + printf("lock %s\n", pack_lockfiles.items[0].string); fflush(stdout); + for (i = 1; i < pack_lockfiles.nr; i++) + warning(_("Lockfile created but not reported: %s"), + pack_lockfiles.items[i].string); } if (args.check_self_contained_and_connected && args.self_contained_and_connected) { diff --git a/connected.c b/connected.c index 1bba888eff..a866fcf9e8 100644 --- a/connected.c +++ b/connected.c @@ -40,10 +40,12 @@ int check_connected(oid_iterate_fn fn, void *cb_data, if (transport && transport->smart_options && transport->smart_options->self_contained_and_connected && - transport->pack_lockfile && - strip_suffix(transport->pack_lockfile, ".keep", &base_len)) { + transport->pack_lockfiles.nr == 1 && + strip_suffix(transport->pack_lockfiles.items[0].string, +".keep", &base_len)) { struct strbuf idx_file = STRBUF_INIT; - strbuf_add(&idx_file, transport->pack_lockfile, base_len); + strbuf_add(&idx_file, transport->pack_lockfiles.items[0].string, + base_len); strbuf_addstr(&idx_file, ".idx"); new_pack = add_packed_git(idx_file.buf, idx_file.len, 1); strbuf_release(&idx_file); diff --git a/fetch-pack.c b/fetch-pack.c index 812be15d7e..cf89af21d9 100644 --- a/fetch-pack.c +++ b/fetch-pack.c @@ -755,7 +755,7 @@ static int sideband_demux(int in, int out, void *data) } static int get_pack(struct fetch_pack_args *args, - int xd[2], char **pack_lockfile) + int
[PATCH v2 2/8] http: improve documentation of http_pack_request
struct http_pack_request and the functions that use it will be modified in a subsequent patch. Using it is complicated (to use, call the initialization function, then set some but not all fields in the returned struct), so add some documentation to help future users. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- http.h | 19 ++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/http.h b/http.h index 4eb4e808e5..ded1edcca4 100644 --- a/http.h +++ b/http.h @@ -202,14 +202,31 @@ extern int http_get_info_packs(const char *base_url, struct packed_git **packs_head); struct http_pack_request { + /* +* Initialized by new_http_pack_request(). +*/ char *url; struct packed_git *target; + struct active_request_slot *slot; + + /* +* After calling new_http_pack_request(), point lst to the head of the +* pack list that target is in. finish_http_pack_request() will remove +* target from lst and call install_packed_git() on target. +*/ struct packed_git **lst; + + /* +* State managed by functions in http.c. +*/ FILE *packfile; struct strbuf tmpfile; - struct active_request_slot *slot; }; +/* + * target must be an element in a pack list obtained from + * http_get_info_packs(). + */ extern struct http_pack_request *new_http_pack_request( struct packed_git *target, const char *base_url); extern int finish_http_pack_request(struct http_pack_request *preq); -- 2.19.0.271.gfe8321ec05.dirty
[PATCH v2 3/8] http-fetch: support fetching packfiles by URL
Teach http-fetch the ability to download packfiles directly, given a URL, and to verify them. The http_pack_request suite of functions have been modified to support a NULL target. When target is NULL, the given URL is downloaded directly instead of being treated as the root of a repository. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- Documentation/git-http-fetch.txt | 8 +++- http-fetch.c | 64 +--- http.c | 49 +--- http.h | 19 -- t/t5550-http-fetch-dumb.sh | 25 + 5 files changed, 135 insertions(+), 30 deletions(-) diff --git a/Documentation/git-http-fetch.txt b/Documentation/git-http-fetch.txt index 666b042679..8357359a9b 100644 --- a/Documentation/git-http-fetch.txt +++ b/Documentation/git-http-fetch.txt @@ -9,7 +9,7 @@ git-http-fetch - Download from a remote Git repository via HTTP SYNOPSIS [verse] -'git http-fetch' [-c] [-t] [-a] [-d] [-v] [-w filename] [--recover] [--stdin] +'git http-fetch' [-c] [-t] [-a] [-d] [-v] [-w filename] [--recover] [--stdin | --packfile | ] DESCRIPTION --- @@ -40,6 +40,12 @@ commit-id:: ['\t'] +--packfile:: + Instead of a commit id on the command line (which is not expected in + this case), 'git http-fetch' fetches the packfile directly at the given + URL and uses index-pack to generate corresponding .idx and .keep files. + The output of index-pack is printed to stdout. + --recover:: Verify that everything reachable from target is fetched. Used after an earlier fetch is interrupted. diff --git a/http-fetch.c b/http-fetch.c index a32ac118d9..a9764d6f96 100644 --- a/http-fetch.c +++ b/http-fetch.c @@ -5,7 +5,7 @@ #include "walker.h" static const char http_fetch_usage[] = "git http-fetch " -"[-c] [-t] [-a] [-v] [--recover] [-w ref] [--stdin] commit-id url"; +"[-c] [-t] [-a] [-v] [--recover] [-w ref] [--stdin | --packfile | commit-id] url"; int cmd_main(int argc, const char **argv) { @@ -19,6 +19,7 @@ int cmd_main(int argc, const char **argv) int rc = 0; int get_verbosely = 0; int get_recover = 0; + int packfile = 0; while (arg < argc && argv[arg][0] == '-') { if (argv[arg][1] == 't') { @@ -35,43 +36,80 @@ int cmd_main(int argc, const char **argv) get_recover = 1; } else if (!strcmp(argv[arg], "--stdin")) { commits_on_stdin = 1; + } else if (!strcmp(argv[arg], "--packfile")) { + packfile = 1; } arg++; } - if (argc != arg + 2 - commits_on_stdin) + if (argc != arg + 2 - (commits_on_stdin || packfile)) usage(http_fetch_usage); if (commits_on_stdin) { commits = walker_targets_stdin(&commit_id, &write_ref); + } else if (packfile) { + /* URL will be set later */ } else { commit_id = (char **) &argv[arg++]; commits = 1; } - if (argv[arg]) - str_end_url_with_slash(argv[arg], &url); + if (packfile) { + url = xstrdup(argv[arg]); + } else { + if (argv[arg]) + str_end_url_with_slash(argv[arg], &url); + } setup_git_directory(); git_config(git_default_config, NULL); http_init(NULL, url, 0); - walker = get_http_walker(url); - walker->get_verbosely = get_verbosely; - walker->get_recover = get_recover; - rc = walker_fetch(walker, commits, commit_id, write_ref, url); + if (packfile) { + struct http_pack_request *preq; + struct slot_results results; + int ret; + + preq = new_http_pack_request(NULL, url); + if (preq == NULL) + die("couldn't create http pack request"); + preq->slot->results = &results; + preq->generate_keep = 1; + + if (start_active_slot(preq->slot)) { + run_active_slot(preq->slot); + if (results.curl_result != CURLE_OK) { + die("Unable to get pack file %s\n%s", preq->url, + curl_errorstr); + } + } else { + die("Unable to start request"); + } + + if ((ret = finish_http_pack_request(preq))) + die("finish_http_pack_request gave result %d", ret); + release_http_pack_request(preq); + rc = 0; + } else { + walker = get_http_walker(url); + walker->get_verbosely = get_verbosely; + walker->get_recover = get_recover; + + rc = walker_
[PATCH v2 8/8] upload-pack: send part of packfile response as uri
Teach upload-pack to send part of its packfile response as URIs. An administrator may configure a repository with one or more "uploadpack.blobpackfileuri" lines, each line containing an OID, a pack hash, and a URI. A client may configure fetch.uriprotocols to be a comma-separated list of protocols that it is willing to use to fetch additional packfiles - this list will be sent to the server. Whenever an object with one of those OIDs would appear in the packfile transmitted by upload-pack, the server may exclude that object, and instead send the URI. The client will then download the packs referred to by those URIs before performing the connectivity check. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- builtin/pack-objects.c | 76 fetch-pack.c | 112 +++-- t/t5702-protocol-v2.sh | 57 + upload-pack.c | 78 +--- 4 files changed, 312 insertions(+), 11 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index a9fac7c128..2fa962c87d 100644 --- a/builtin/pack-objects.c +++ b/builtin/pack-objects.c @@ -110,6 +110,8 @@ static unsigned long window_memory_limit = 0; static struct list_objects_filter_options filter_options; +static struct string_list uri_protocols = STRING_LIST_INIT_NODUP; + enum missing_action { MA_ERROR = 0, /* fail if any missing objects are encountered */ MA_ALLOW_ANY, /* silently allow ALL missing objects */ @@ -118,6 +120,15 @@ enum missing_action { static enum missing_action arg_missing_action; static show_object_fn fn_show_object; +struct configured_exclusion { + struct oidmap_entry e; + char *pack_hash_hex; + char *uri; +}; +static struct oidmap configured_exclusions; + +static struct oidset excluded_by_config; + /* * stats */ @@ -832,6 +843,25 @@ static off_t write_reused_pack(struct hashfile *f) return reuse_packfile_offset - sizeof(struct pack_header); } +static void write_excluded_by_configs(void) +{ + struct oidset_iter iter; + const struct object_id *oid; + + oidset_iter_init(&excluded_by_config, &iter); + while ((oid = oidset_iter_next(&iter))) { + struct configured_exclusion *ex = + oidmap_get(&configured_exclusions, oid); + + if (!ex) + BUG("configured exclusion wasn't configured"); + write_in_full(1, ex->pack_hash_hex, strlen(ex->pack_hash_hex)); + write_in_full(1, " ", 1); + write_in_full(1, ex->uri, strlen(ex->uri)); + write_in_full(1, "\n", 1); + } +} + static const char no_split_warning[] = N_( "disabling bitmap writing, packs are split due to pack.packSizeLimit" ); @@ -1125,6 +1155,25 @@ static int want_object_in_pack(const struct object_id *oid, } } + if (uri_protocols.nr) { + struct configured_exclusion *ex = + oidmap_get(&configured_exclusions, oid); + int i; + const char *p; + + if (ex) { + for (i = 0; i < uri_protocols.nr; i++) { + if (skip_prefix(ex->uri, + uri_protocols.items[i].string, + &p) && + *p == ':') { + oidset_insert(&excluded_by_config, oid); + return 0; + } + } + } + } + return 1; } @@ -2726,6 +2775,29 @@ static int git_pack_config(const char *k, const char *v, void *cb) pack_idx_opts.version); return 0; } + if (!strcmp(k, "uploadpack.blobpackfileuri")) { + struct configured_exclusion *ex = xmalloc(sizeof(*ex)); + const char *oid_end, *pack_end; + /* +* Stores the pack hash. This is not a true object ID, but is +* of the same form. +*/ + struct object_id pack_hash; + + if (parse_oid_hex(v, &ex->e.oid, &oid_end) || + *oid_end != ' ' || + parse_oid_hex(oid_end + 1, &pack_hash, &pack_end) || + *pack_end != ' ') + die(_("value of uploadpack.blobpackfileuri must be " + "of the form ' ' (got '%s')"), v); + if (oidmap_get(&configured_exclusions, &ex->e.oid)) + die(_("object already configured in another " + "uploadpack.blobpackfileuri (got '%s')"), v); + ex->pack_hash_hex = xcalloc(1, pack_end - oid_end); + memcpy(ex->pack_hash_hex, oid_end + 1, pack_end - oid_e
[PATCH v2 5/8] Documentation: add Packfile URIs design doc
Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- Documentation/technical/packfile-uri.txt | 78 Documentation/technical/protocol-v2.txt | 28 - 2 files changed, 105 insertions(+), 1 deletion(-) create mode 100644 Documentation/technical/packfile-uri.txt diff --git a/Documentation/technical/packfile-uri.txt b/Documentation/technical/packfile-uri.txt new file mode 100644 index 00..6a5a6440d5 --- /dev/null +++ b/Documentation/technical/packfile-uri.txt @@ -0,0 +1,78 @@ +Packfile URIs += + +This feature allows servers to serve part of their packfile response as URIs. +This allows server designs that improve scalability in bandwidth and CPU usage +(for example, by serving some data through a CDN), and (in the future) provides +some measure of resumability to clients. + +This feature is available only in protocol version 2. + +Protocol + + +The server advertises `packfile-uris`. + +If the client then communicates which protocols (HTTPS, etc.) it supports with +a `packfile-uris` argument, the server MAY send a `packfile-uris` section +directly before the `packfile` section (right after `wanted-refs` if it is +sent) containing URIs of any of the given protocols. The URIs point to +packfiles that use only features that the client has declared that it supports +(e.g. ofs-delta and thin-pack). See protocol-v2.txt for the documentation of +this section. + +Clients then should understand that the returned packfile could be incomplete, +and that it needs to download all the given URIs before the fetch or clone is +complete. + +Server design +- + +The server can be trivially made compatible with the proposed protocol by +having it advertise `packfile-uris`, tolerating the client sending +`packfile-uris`, and never sending any `packfile-uris` section. But we should +include some sort of non-trivial implementation in the Minimum Viable Product, +at least so that we can test the client. + +This is the implementation: a feature, marked experimental, that allows the +server to be configured by one or more `uploadpack.blobPackfileUri= +` entries. Whenever the list of objects to be sent is assembled, a blob +with the given sha1 can be replaced by the given URI. This allows, for example, +servers to delegate serving of large blobs to CDNs. + +Client design +- + +While fetching, the client needs to remember the list of URIs and cannot +declare that the fetch is complete until all URIs have been downloaded as +packfiles. + +The division of work (initial fetch + additional URIs) introduces convenient +points for resumption of an interrupted clone - such resumption can be done +after the Minimum Viable Product (see "Future work"). + +The client can inhibit this feature (i.e. refrain from sending the +`packfile-uris` parameter) by passing --no-packfile-uris to `git fetch`. + +Future work +--- + +The protocol design allows some evolution of the server and client without any +need for protocol changes, so only a small-scoped design is included here to +form the MVP. For example, the following can be done: + + * On the server, a long-running process that takes in entire requests and + outputs a list of URIs and the corresponding inclusion and exclusion sets of + objects. This allows, e.g., signed URIs to be used and packfiles for common + requests to be cached. + * On the client, resumption of clone. If a clone is interrupted, information + could be recorded in the repository's config and a "clone-resume" command + can resume the clone in progress. (Resumption of subsequent fetches is more + difficult because that must deal with the user wanting to use the repository + even after the fetch was interrupted.) + +There are some possible features that will require a change in protocol: + + * Additional HTTP headers (e.g. authentication) + * Byte range support + * Different file formats referenced by URIs (e.g. raw object) diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt index 36239ec7e9..7b63c26ecd 100644 --- a/Documentation/technical/protocol-v2.txt +++ b/Documentation/technical/protocol-v2.txt @@ -323,13 +323,26 @@ included in the client's request: indicating its sideband (1, 2, or 3), and the server may send "0005\2" (a PKT-LINE of sideband 2 with no payload) as a keepalive packet. +If the 'packfile-uris' feature is advertised, the following argument +can be included in the client's request as well as the potential +addition of the 'packfile-uris' section in the server's response as +explained below. + +packfile-uris + Indicates to the server that the client is willing to receive + URIs of any of the given protocols in place of objects in the + sent packfile. Before performing the connectivity check, the + client should download from all given URIs. Currently, the + protocols supported are "http" and "https". + Th
[PATCH v2 4/8] Documentation: order protocol v2 sections
The current C Git implementation expects Git servers to follow a specific order of sections when transmitting protocol v2 responses, but this is not explicit in the documentation. Make the order explicit. Signed-off-by: Jonathan Tan Signed-off-by: Junio C Hamano --- Documentation/technical/protocol-v2.txt | 18 -- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/Documentation/technical/protocol-v2.txt b/Documentation/technical/protocol-v2.txt index ead85ce35c..36239ec7e9 100644 --- a/Documentation/technical/protocol-v2.txt +++ b/Documentation/technical/protocol-v2.txt @@ -325,11 +325,11 @@ included in the client's request: The response of `fetch` is broken into a number of sections separated by delimiter packets (0001), with each section beginning with its section -header. +header. Most sections are sent only when the packfile is sent. -output = *section -section = (acknowledgments | shallow-info | wanted-refs | packfile) - (flush-pkt | delim-pkt) +output = acknowledgements flush-pkt | +[acknowledgments delim-pkt] [shallow-info delim-pkt] +[wanted-refs delim-pkt] packfile flush-pkt acknowledgments = PKT-LINE("acknowledgments" LF) (nak | *ack) @@ -351,9 +351,10 @@ header. *PKT-LINE(%x01-03 *%x00-ff) acknowledgments section - * If the client determines that it is finished with negotiations - by sending a "done" line, the acknowledgments sections MUST be - omitted from the server's response. + * If the client determines that it is finished with negotiations by + sending a "done" line (thus requiring the server to send a packfile), + the acknowledgments sections MUST be omitted from the server's + response. * Always begins with the section header "acknowledgments" @@ -404,9 +405,6 @@ header. which the client has not indicated was shallow as a part of its request. - * This section is only included if a packfile section is also - included in the response. - wanted-refs section * This section is only included if the client has requested a ref using a 'want-ref' line and if a packfile section is also -- 2.19.0.271.gfe8321ec05.dirty
bitmaps by default? [was: prune: use bitmaps for reachability traversal]
Jeff King wrote: > Pruning generally has to traverse the whole commit graph in order to > see which objects are reachable. This is the exact problem that > reachability bitmaps were meant to solve, so let's use them (if they're > available, of course). Perhaps this is good impetus for doing bitmaps by default? It would make life easier for people new to hosting git servers (and hopefully reduce centralization :) I started working on it, but t0410-partial-clone.sh fails with "Failed to write bitmap index. Packfile doesn't have full closure"; so more work needs to be done w.r.t. default behavior on partial clones... Here's my WIP: diff --git a/builtin/repack.c b/builtin/repack.c index 67f8978043..ca98d32715 100644 --- a/builtin/repack.c +++ b/builtin/repack.c @@ -14,7 +14,7 @@ static int delta_base_offset = 1; static int pack_kept_objects = -1; -static int write_bitmaps; +static int write_bitmaps = -1; static int use_delta_islands; static char *packdir, *packtmp; @@ -344,10 +344,14 @@ int cmd_repack(int argc, const char **argv, const char *prefix) die(_("--keep-unreachable and -A are incompatible")); if (pack_kept_objects < 0) - pack_kept_objects = write_bitmaps; + pack_kept_objects = write_bitmaps > 0; - if (write_bitmaps && !(pack_everything & ALL_INTO_ONE)) - die(_(incremental_bitmap_conflict_error)); + if (!(pack_everything & ALL_INTO_ONE)) { + if (write_bitmaps > 0) + die(_(incremental_bitmap_conflict_error)); + } else if (write_bitmaps < 0) { + write_bitmaps = 1; + } packdir = mkpathdup("%s/pack", get_object_directory()); packtmp = mkpathdup("%s/.tmp-%d-pack", packdir, (int)getpid()); @@ -368,7 +372,7 @@ int cmd_repack(int argc, const char **argv, const char *prefix) argv_array_push(&cmd.args, "--indexed-objects"); if (repository_format_partial_clone) argv_array_push(&cmd.args, "--exclude-promisor-objects"); - if (write_bitmaps) + if (write_bitmaps > 0) argv_array_push(&cmd.args, "--write-bitmap-index"); if (use_delta_islands) argv_array_push(&cmd.args, "--delta-islands");