From 023e37c37780d6a56f2870a979c8eb3a9ee9a44d Mon Sep 17 00:00:00 2001 From: Matthieu Moy Date: Mon, 18 Jun 2012 20:18:21 +0200 Subject: verify_filename(): ask the caller to chose the kind of diagnosis verify_filename() can be called in two different contexts. Either we just tried to interpret a string as an object name, and it fails, so we try looking for a working tree file (i.e. we finished looking at revs that come earlier on the command line, and the next argument must be a pathname), or we _know_ that we are looking for a pathname, and shouldn't even try interpreting the string as an object name. For example, with this change, we get: $ git log COPYING HEAD:inexistant fatal: HEAD:inexistant: no such path in the working tree. Use '-- ...' to specify paths that do not exist locally. $ git log HEAD:inexistant fatal: Path 'inexistant' does not exist in 'HEAD' Signed-off-by: Matthieu Moy Signed-off-by: Junio C Hamano --- builtin/reset.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 8c2c1d52a2..4cc34c9084 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -285,7 +285,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) rev = argv[i++]; } else { /* Otherwise we treat this as a filename */ - verify_filename(prefix, argv[i]); + verify_filename(prefix, argv[i], 1); } } -- cgit v1.2.1 From 13243c2c7a758bb6510ba26d0372f384b5f422ce Mon Sep 17 00:00:00 2001 From: Junio C Hamano Date: Tue, 3 Jul 2012 10:04:22 -0700 Subject: reset: the command takes committish This is not strictly correct, in that resetting selected index entries from corresponding paths out of a given tree without moving HEAD is a valid operation, and in such case a tree-ish would suffice. But the existing code already requires a committish in the codepath, so let's be consistent with it for now. Signed-off-by: Junio C Hamano --- builtin/reset.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 8c2c1d52a2..392fb6361e 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -276,7 +276,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) * Otherwise, argv[i] could be either or and * has to be unambiguous. */ - else if (!get_sha1(argv[i], sha1)) { + else if (!get_sha1_committish(argv[i], sha1)) { /* * Ok, argv[i] looks like a rev; it should not * be a filename. @@ -289,9 +289,15 @@ int cmd_reset(int argc, const char **argv, const char *prefix) } } - if (get_sha1(rev, sha1)) + if (get_sha1_committish(rev, sha1)) die(_("Failed to resolve '%s' as a valid ref."), rev); + /* + * NOTE: As "git reset $treeish -- $path" should be usable on + * any tree-ish, this is not strictly correct. We are not + * moving the HEAD to any commit; we are merely resetting the + * entries in the index to that of a treeish. + */ commit = lookup_commit_reference(sha1); if (!commit) die(_("Could not parse object '%s'."), rev); -- cgit v1.2.1 From c1e9c2a73dedf79179ee30fd6a552c998d0d4ec7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nguy=E1=BB=85n=20Th=C3=A1i=20Ng=E1=BB=8Dc=20Duy?= Date: Mon, 20 Aug 2012 19:32:39 +0700 Subject: i18n: reset: mark parseopt strings for translation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Nguyễn Thái Ngọc Duy Signed-off-by: Junio C Hamano --- builtin/reset.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 74442bd766..915cc9f86f 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -22,9 +22,9 @@ #include "cache-tree.h" static const char * const git_reset_usage[] = { - "git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []", - "git reset [-q] [--] ...", - "git reset --patch [] [--] [...]", + N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []"), + N_("git reset [-q] [--] ..."), + N_("git reset --patch [] [--] [...]"), NULL }; @@ -235,17 +235,17 @@ int cmd_reset(int argc, const char **argv, const char *prefix) struct commit *commit; struct strbuf msg = STRBUF_INIT; const struct option options[] = { - OPT__QUIET(&quiet, "be quiet, only report errors"), + OPT__QUIET(&quiet, N_("be quiet, only report errors")), OPT_SET_INT(0, "mixed", &reset_type, - "reset HEAD and index", MIXED), - OPT_SET_INT(0, "soft", &reset_type, "reset only HEAD", SOFT), + N_("reset HEAD and index"), MIXED), + OPT_SET_INT(0, "soft", &reset_type, N_("reset only HEAD"), SOFT), OPT_SET_INT(0, "hard", &reset_type, - "reset HEAD, index and working tree", HARD), + N_("reset HEAD, index and working tree"), HARD), OPT_SET_INT(0, "merge", &reset_type, - "reset HEAD, index and working tree", MERGE), + N_("reset HEAD, index and working tree"), MERGE), OPT_SET_INT(0, "keep", &reset_type, - "reset HEAD but keep local changes", KEEP), - OPT_BOOLEAN('p', "patch", &patch_mode, "select hunks interactively"), + N_("reset HEAD but keep local changes"), KEEP), + OPT_BOOLEAN('p', "patch", &patch_mode, N_("select hunks interactively")), OPT_END() }; -- cgit v1.2.1 From 10746a361689aaa1aa98b8d4e7fb3b8463391864 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:33 -0800 Subject: reset $pathspec: no need to discard index Since 34110cd (Make 'unpack_trees()' have a separate source and destination index, 2008-03-06), the index no longer gets clobbered by do_diff_cache() and we can remove the code for discarding and re-reading it. There are two paths to update_index_refresh() from cmd_reset(), but on both paths, either read_cache() or read_cache_unmerged() will have been called, so the call to read_cache() in this method is redundant (although practically free). This speeds up "git reset -- ." a little on the linux-2.6 repo (best of five, warm cache): Before After real 0m0.093s 0m0.080s user 0m0.040s 0m0.020s sys 0m0.050s 0m0.050s Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 915cc9f86f..8cc7c722ef 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -126,9 +126,6 @@ static int update_index_refresh(int fd, struct lock_file *index_lock, int flags) fd = hold_locked_index(index_lock, 1); } - if (read_cache() < 0) - return error(_("Could not read index")); - result = refresh_index(&the_index, (flags), NULL, NULL, _("Unstaged changes after reset:")) ? 1 : 0; if (write_cache(fd, active_cache, active_nr) || @@ -141,12 +138,6 @@ static void update_index_from_diff(struct diff_queue_struct *q, struct diff_options *opt, void *data) { int i; - int *discard_flag = data; - - /* do_diff_cache() mangled the index */ - discard_cache(); - *discard_flag = 1; - read_cache(); for (i = 0; i < q->nr; i++) { struct diff_filespec *one = q->queue[i]->one; @@ -179,17 +170,15 @@ static int read_from_tree(const char *prefix, const char **argv, unsigned char *tree_sha1, int refresh_flags) { struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); - int index_fd, index_was_discarded = 0; + int index_fd; struct diff_options opt; memset(&opt, 0, sizeof(opt)); diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt); opt.output_format = DIFF_FORMAT_CALLBACK; opt.format_callback = update_index_from_diff; - opt.format_callback_data = &index_was_discarded; index_fd = hold_locked_index(lock, 1); - index_was_discarded = 0; read_cache(); if (do_diff_cache(tree_sha1, &opt)) return 1; @@ -197,9 +186,6 @@ static int read_from_tree(const char *prefix, const char **argv, diff_flush(&opt); diff_tree_release_paths(&opt); - if (!index_was_discarded) - /* The index is still clobbered from do_diff_cache() */ - discard_cache(); return update_index_refresh(index_fd, lock, refresh_flags); } -- cgit v1.2.1 From d94c5e2fa24dce13a3dc1ba178f381cb09bb0853 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:34 -0800 Subject: reset $pathspec: exit with code 0 if successful "git reset $pathspec" currently exits with a non-zero exit code if the worktree is dirty after resetting, which is inconsistent with reset without pathspec, and it makes it harder to know whether the command really failed. Change it to exit with code 0 regardless of whether the worktree is dirty so that non-zero indicates an error. This makes the 4 "disambiguation" test cases in t7102 clearer since they all used to "fail", 3 of which "failed" due to changes in the work tree. Now only the ambiguous one fails. Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 8cc7c722ef..65413d0a67 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -119,19 +119,17 @@ static void print_new_head_line(struct commit *commit) static int update_index_refresh(int fd, struct lock_file *index_lock, int flags) { - int result; - if (!index_lock) { index_lock = xcalloc(1, sizeof(struct lock_file)); fd = hold_locked_index(index_lock, 1); } - result = refresh_index(&the_index, (flags), NULL, NULL, - _("Unstaged changes after reset:")) ? 1 : 0; + refresh_index(&the_index, (flags), NULL, NULL, + _("Unstaged changes after reset:")); if (write_cache(fd, active_cache, active_nr) || commit_locked_index(index_lock)) return error ("Could not refresh index"); - return result; + return 0; } static void update_index_from_diff(struct diff_queue_struct *q, -- cgit v1.2.1 From 18648e89e78662e3a6a9a38adfca3e6001cfaf85 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:35 -0800 Subject: reset.c: pass pathspec around instead of (prefix, argv) pair We use the path arguments in two places in reset.c: in interactive_reset() and read_from_tree(). Both of these call get_pathspec(), so we pass the (prefix, argv) pair to both functions. Move the call to get_pathspec() out of these methods, for two reasons: 1) One argument is simpler than two. 2) It lets us use the (arguably clearer) "if (pathspec)" in place of "if (i < argc)". Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 65413d0a67..045c9609f2 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -153,26 +153,15 @@ static void update_index_from_diff(struct diff_queue_struct *q, } } -static int interactive_reset(const char *revision, const char **argv, - const char *prefix) -{ - const char **pathspec = NULL; - - if (*argv) - pathspec = get_pathspec(prefix, argv); - - return run_add_interactive(revision, "--patch=reset", pathspec); -} - -static int read_from_tree(const char *prefix, const char **argv, - unsigned char *tree_sha1, int refresh_flags) +static int read_from_tree(const char **pathspec, unsigned char *tree_sha1, + int refresh_flags) { struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); int index_fd; struct diff_options opt; memset(&opt, 0, sizeof(opt)); - diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt); + diff_tree_setup_paths(pathspec, &opt); opt.output_format = DIFF_FORMAT_CALLBACK; opt.format_callback = update_index_from_diff; @@ -216,6 +205,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) const char *rev = "HEAD"; unsigned char sha1[20], *orig = NULL, sha1_orig[20], *old_orig = NULL, sha1_old_orig[20]; + const char **pathspec = NULL; struct commit *commit; struct strbuf msg = STRBUF_INIT; const struct option options[] = { @@ -287,22 +277,25 @@ int cmd_reset(int argc, const char **argv, const char *prefix) die(_("Could not parse object '%s'."), rev); hashcpy(sha1, commit->object.sha1); + if (i < argc) + pathspec = get_pathspec(prefix, argv + i); + if (patch_mode) { if (reset_type != NONE) die(_("--patch is incompatible with --{hard,mixed,soft}")); - return interactive_reset(rev, argv + i, prefix); + return run_add_interactive(rev, "--patch=reset", pathspec); } /* git reset tree [--] paths... can be used to * load chosen paths from the tree into the index without * affecting the working tree nor HEAD. */ - if (i < argc) { + if (pathspec) { if (reset_type == MIXED) warning(_("--mixed with paths is deprecated; use 'git reset -- ' instead.")); else if (reset_type != NONE) die(_("Cannot do %s reset with paths."), _(reset_type_names[reset_type])); - return read_from_tree(prefix, argv + i, sha1, + return read_from_tree(pathspec, sha1, quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); } if (reset_type == NONE) -- cgit v1.2.1 From 4f4ad3d938beafd785c319664348268bbef11a00 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:36 -0800 Subject: reset: don't allow "git reset -- $pathspec" in bare repo Running e.g. "git reset ." in a bare repo results in an index file being created from the HEAD commit. The differences compared to the index are then printed as usual, but since there is no worktree, it will appear as if all files are deleted. For example, in a bare clone of git.git: Unstaged changes after reset: D .gitattributes D .gitignore D .mailmap ... This happens because the check for is_bare_repository() happens after we branch off into read_from_tree() to reset with paths. Fix by moving the branching point after the check. Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 045c9609f2..664fad9fc5 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -295,8 +295,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix) else if (reset_type != NONE) die(_("Cannot do %s reset with paths."), _(reset_type_names[reset_type])); - return read_from_tree(pathspec, sha1, - quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); } if (reset_type == NONE) reset_type = MIXED; /* by default */ @@ -308,6 +306,10 @@ int cmd_reset(int argc, const char **argv, const char *prefix) die(_("%s reset is not allowed in a bare repository"), _(reset_type_names[reset_type])); + if (pathspec) + return read_from_tree(pathspec, sha1, + quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); + /* Soft reset does not touch the index file nor the working tree * at all, but requires them in a good order. Other resets reset * the index file to the tree object we are switching to. */ -- cgit v1.2.1 From 39ea722d829ddc5bb253fbe12333d768965f4fe7 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:37 -0800 Subject: reset.c: extract function for parsing arguments Declutter cmd_reset() a bit by moving out the argument parsing to its own function. Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 70 +++++++++++++++++++++++++++++++-------------------------- 1 file changed, 38 insertions(+), 32 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 664fad9fc5..58f0f6116b 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -198,36 +198,11 @@ static void die_if_unmerged_cache(int reset_type) } -int cmd_reset(int argc, const char **argv, const char *prefix) +static const char **parse_args(int argc, const char **argv, const char *prefix, const char **rev_ret) { - int i = 0, reset_type = NONE, update_ref_status = 0, quiet = 0; - int patch_mode = 0; + int i = 0; const char *rev = "HEAD"; - unsigned char sha1[20], *orig = NULL, sha1_orig[20], - *old_orig = NULL, sha1_old_orig[20]; - const char **pathspec = NULL; - struct commit *commit; - struct strbuf msg = STRBUF_INIT; - const struct option options[] = { - OPT__QUIET(&quiet, N_("be quiet, only report errors")), - OPT_SET_INT(0, "mixed", &reset_type, - N_("reset HEAD and index"), MIXED), - OPT_SET_INT(0, "soft", &reset_type, N_("reset only HEAD"), SOFT), - OPT_SET_INT(0, "hard", &reset_type, - N_("reset HEAD, index and working tree"), HARD), - OPT_SET_INT(0, "merge", &reset_type, - N_("reset HEAD, index and working tree"), MERGE), - OPT_SET_INT(0, "keep", &reset_type, - N_("reset HEAD but keep local changes"), KEEP), - OPT_BOOLEAN('p', "patch", &patch_mode, N_("select hunks interactively")), - OPT_END() - }; - - git_config(git_default_config, NULL); - - argc = parse_options(argc, argv, prefix, options, git_reset_usage, - PARSE_OPT_KEEP_DASHDASH); - + unsigned char unused[20]; /* * Possible arguments are: * @@ -250,7 +225,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) * Otherwise, argv[i] could be either or and * has to be unambiguous. */ - else if (!get_sha1_committish(argv[i], sha1)) { + else if (!get_sha1_committish(argv[i], unused)) { /* * Ok, argv[i] looks like a rev; it should not * be a filename. @@ -262,6 +237,40 @@ int cmd_reset(int argc, const char **argv, const char *prefix) verify_filename(prefix, argv[i], 1); } } + *rev_ret = rev; + return i < argc ? get_pathspec(prefix, argv + i) : NULL; +} + +int cmd_reset(int argc, const char **argv, const char *prefix) +{ + int reset_type = NONE, update_ref_status = 0, quiet = 0; + int patch_mode = 0; + const char *rev; + unsigned char sha1[20], *orig = NULL, sha1_orig[20], + *old_orig = NULL, sha1_old_orig[20]; + const char **pathspec = NULL; + struct commit *commit; + struct strbuf msg = STRBUF_INIT; + const struct option options[] = { + OPT__QUIET(&quiet, N_("be quiet, only report errors")), + OPT_SET_INT(0, "mixed", &reset_type, + N_("reset HEAD and index"), MIXED), + OPT_SET_INT(0, "soft", &reset_type, N_("reset only HEAD"), SOFT), + OPT_SET_INT(0, "hard", &reset_type, + N_("reset HEAD, index and working tree"), HARD), + OPT_SET_INT(0, "merge", &reset_type, + N_("reset HEAD, index and working tree"), MERGE), + OPT_SET_INT(0, "keep", &reset_type, + N_("reset HEAD but keep local changes"), KEEP), + OPT_BOOLEAN('p', "patch", &patch_mode, N_("select hunks interactively")), + OPT_END() + }; + + git_config(git_default_config, NULL); + + argc = parse_options(argc, argv, prefix, options, git_reset_usage, + PARSE_OPT_KEEP_DASHDASH); + pathspec = parse_args(argc, argv, prefix, &rev); if (get_sha1_committish(rev, sha1)) die(_("Failed to resolve '%s' as a valid ref."), rev); @@ -277,9 +286,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix) die(_("Could not parse object '%s'."), rev); hashcpy(sha1, commit->object.sha1); - if (i < argc) - pathspec = get_pathspec(prefix, argv + i); - if (patch_mode) { if (reset_type != NONE) die(_("--patch is incompatible with --{hard,mixed,soft}")); -- cgit v1.2.1 From dca48cf520933c028f807f65175c29482a209f23 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:38 -0800 Subject: reset.c: remove unnecessary variable 'i' Throughout most of parse_args(), the variable 'i' remains at 0. Many references are still made to the variable even when it could only have the value 0. This made at least me, who has relatively little experience with C programming styles, think that parts of the function was meant to be part of a loop. To avoid such confusion, remove the variable and also the 'argc' parameter and check for NULL trailing argv instead. Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 58f0f6116b..d89cf4d113 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -198,9 +198,8 @@ static void die_if_unmerged_cache(int reset_type) } -static const char **parse_args(int argc, const char **argv, const char *prefix, const char **rev_ret) +static const char **parse_args(const char **argv, const char *prefix, const char **rev_ret) { - int i = 0; const char *rev = "HEAD"; unsigned char unused[20]; /* @@ -211,34 +210,34 @@ static const char **parse_args(int argc, const char **argv, const char *prefix, * git reset [-opts] -- ... * git reset [-opts] ... * - * At this point, argv[i] points immediately after [-opts]. + * At this point, argv points immediately after [-opts]. */ - if (i < argc) { - if (!strcmp(argv[i], "--")) { - i++; /* reset to HEAD, possibly with paths */ - } else if (i + 1 < argc && !strcmp(argv[i+1], "--")) { - rev = argv[i]; - i += 2; + if (argv[0]) { + if (!strcmp(argv[0], "--")) { + argv++; /* reset to HEAD, possibly with paths */ + } else if (argv[1] && !strcmp(argv[1], "--")) { + rev = argv[0]; + argv += 2; } /* - * Otherwise, argv[i] could be either or and + * Otherwise, argv[0] could be either or and * has to be unambiguous. */ - else if (!get_sha1_committish(argv[i], unused)) { + else if (!get_sha1_committish(argv[0], unused)) { /* - * Ok, argv[i] looks like a rev; it should not + * Ok, argv[0] looks like a rev; it should not * be a filename. */ - verify_non_filename(prefix, argv[i]); - rev = argv[i++]; + verify_non_filename(prefix, argv[0]); + rev = *argv++; } else { /* Otherwise we treat this as a filename */ - verify_filename(prefix, argv[i], 1); + verify_filename(prefix, argv[0], 1); } } *rev_ret = rev; - return i < argc ? get_pathspec(prefix, argv + i) : NULL; + return argv[0] ? get_pathspec(prefix, argv) : NULL; } int cmd_reset(int argc, const char **argv, const char *prefix) @@ -270,7 +269,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) argc = parse_options(argc, argv, prefix, options, git_reset_usage, PARSE_OPT_KEEP_DASHDASH); - pathspec = parse_args(argc, argv, prefix, &rev); + pathspec = parse_args(argv, prefix, &rev); if (get_sha1_committish(rev, sha1)) die(_("Failed to resolve '%s' as a valid ref."), rev); -- cgit v1.2.1 From 7bca0e451b8e502fa882b458d6fc825da80034cb Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:39 -0800 Subject: reset.c: extract function for updating {ORIG_,}HEAD By extracting the code for updating the HEAD and ORIG_HEAD symbolic references to a separate function, we declutter cmd_reset() a bit and we make it clear that e.g. the four variables {,sha1_}{,old_}orig are only used by this code. Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index d89cf4d113..2187d6453d 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -240,16 +240,35 @@ static const char **parse_args(const char **argv, const char *prefix, const char return argv[0] ? get_pathspec(prefix, argv) : NULL; } +static int update_refs(const char *rev, const unsigned char *sha1) +{ + int update_ref_status; + struct strbuf msg = STRBUF_INIT; + unsigned char *orig = NULL, sha1_orig[20], + *old_orig = NULL, sha1_old_orig[20]; + + if (!get_sha1("ORIG_HEAD", sha1_old_orig)) + old_orig = sha1_old_orig; + if (!get_sha1("HEAD", sha1_orig)) { + orig = sha1_orig; + set_reflog_message(&msg, "updating ORIG_HEAD", NULL); + update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR); + } else if (old_orig) + delete_ref("ORIG_HEAD", old_orig, 0); + set_reflog_message(&msg, "updating HEAD", rev); + update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, MSG_ON_ERR); + strbuf_release(&msg); + return update_ref_status; +} + int cmd_reset(int argc, const char **argv, const char *prefix) { int reset_type = NONE, update_ref_status = 0, quiet = 0; int patch_mode = 0; const char *rev; - unsigned char sha1[20], *orig = NULL, sha1_orig[20], - *old_orig = NULL, sha1_old_orig[20]; + unsigned char sha1[20]; const char **pathspec = NULL; struct commit *commit; - struct strbuf msg = STRBUF_INIT; const struct option options[] = { OPT__QUIET(&quiet, N_("be quiet, only report errors")), OPT_SET_INT(0, "mixed", &reset_type, @@ -333,17 +352,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) /* Any resets update HEAD to the head being switched to, * saving the previous head in ORIG_HEAD before. */ - if (!get_sha1("ORIG_HEAD", sha1_old_orig)) - old_orig = sha1_old_orig; - if (!get_sha1("HEAD", sha1_orig)) { - orig = sha1_orig; - set_reflog_message(&msg, "updating ORIG_HEAD", NULL); - update_ref(msg.buf, "ORIG_HEAD", orig, old_orig, 0, MSG_ON_ERR); - } - else if (old_orig) - delete_ref("ORIG_HEAD", old_orig, 0); - set_reflog_message(&msg, "updating HEAD", rev); - update_ref_status = update_ref(msg.buf, "HEAD", sha1, orig, 0, MSG_ON_ERR); + update_ref_status = update_refs(rev, sha1); switch (reset_type) { case HARD: @@ -360,7 +369,5 @@ int cmd_reset(int argc, const char **argv, const char *prefix) remove_branch_state(); - strbuf_release(&msg); - return update_ref_status; } -- cgit v1.2.1 From 352f58a57ba3050adbdc2dcdcd3839e584f1431b Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:40 -0800 Subject: reset.c: share call to die_if_unmerged_cache() Use a single condition to guard the call to die_if_unmerged_cache for both --soft and --keep. This avoids the small distraction of the precondition check from the logic following it. Also change an instance of if (e) err = err || f(); to the almost as short, but clearer if (e && !err) err = f(); (which is equivalent since we only care whether exit code is 0) Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 2187d6453d..4e341950a1 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -337,15 +337,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix) /* Soft reset does not touch the index file nor the working tree * at all, but requires them in a good order. Other resets reset * the index file to the tree object we are switching to. */ - if (reset_type == SOFT) + if (reset_type == SOFT || reset_type == KEEP) die_if_unmerged_cache(reset_type); - else { - int err; - if (reset_type == KEEP) - die_if_unmerged_cache(reset_type); - err = reset_index_file(sha1, reset_type, quiet); - if (reset_type == KEEP) - err = err || reset_index_file(sha1, MIXED, quiet); + + if (reset_type != SOFT) { + int err = reset_index_file(sha1, reset_type, quiet); + if (reset_type == KEEP && !err) + err = reset_index_file(sha1, MIXED, quiet); if (err) die(_("Could not reset index file to revision '%s'."), rev); } -- cgit v1.2.1 From b7099a06e8ffe61a06d3e32632e832e59f23bd4d Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:41 -0800 Subject: reset --keep: only write index file once "git reset --keep" calls reset_index_file() twice, first doing a two-way merge to the target revision, updating the index and worktree, and then resetting the index. After each call, we write the index file. In the unlikely event that the second call to reset_index_file() fails, the index will have been merged to the target revision, but HEAD will not be updated, leaving the user with a dirty index. By moving the locking, writing and committing out of reset_index_file() and into the caller, we can avoid writing the index twice, thereby making the sure we don't end up in the half-way reset state. As a bonus, we speed up "git reset --keep" a little on the linux-2.6 repo (best of five, warm cache): Before After real 0m0.315s 0m0.296s user 0m0.290s 0m0.280s sys 0m0.020s 0m0.010s Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 4e341950a1..7c440add07 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -38,14 +38,12 @@ static inline int is_merge(void) return !access(git_path("MERGE_HEAD"), F_OK); } -static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet) +static int reset_index(const unsigned char *sha1, int reset_type, int quiet) { int nr = 1; - int newfd; struct tree_desc desc[2]; struct tree *tree; struct unpack_trees_options opts; - struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); memset(&opts, 0, sizeof(opts)); opts.head_idx = 1; @@ -67,8 +65,6 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet opts.reset = 1; } - newfd = hold_locked_index(lock, 1); - read_cache_unmerged(); if (reset_type == KEEP) { @@ -91,10 +87,6 @@ static int reset_index_file(const unsigned char *sha1, int reset_type, int quiet prime_cache_tree(&active_cache_tree, tree); } - if (write_cache(newfd, active_cache, active_nr) || - commit_locked_index(lock)) - return error(_("Could not write new index file.")); - return 0; } @@ -341,9 +333,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix) die_if_unmerged_cache(reset_type); if (reset_type != SOFT) { - int err = reset_index_file(sha1, reset_type, quiet); + struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); + int newfd = hold_locked_index(lock, 1); + int err = reset_index(sha1, reset_type, quiet); if (reset_type == KEEP && !err) - err = reset_index_file(sha1, MIXED, quiet); + err = reset_index(sha1, MIXED, quiet); + if (!err && + (write_cache(newfd, active_cache, active_nr) || + commit_locked_index(lock))) { + err = error(_("Could not write new index file.")); + } if (err) die(_("Could not reset index file to revision '%s'."), rev); } -- cgit v1.2.1 From 1ca38f85860b33ddc79b1494baf29aecde8616cd Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:42 -0800 Subject: reset: avoid redundant error message If writing or committing the new index file fails, we print "Could not write new index file." followed by "Could not reset index file to revision $rev.". The first message seems to imply the second, so print only the first message. Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 7c440add07..97fa9f7859 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -338,13 +338,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix) int err = reset_index(sha1, reset_type, quiet); if (reset_type == KEEP && !err) err = reset_index(sha1, MIXED, quiet); - if (!err && - (write_cache(newfd, active_cache, active_nr) || - commit_locked_index(lock))) { - err = error(_("Could not write new index file.")); - } if (err) die(_("Could not reset index file to revision '%s'."), rev); + if (write_cache(newfd, active_cache, active_nr) || + commit_locked_index(lock)) + die(_("Could not write new index file.")); } /* Any resets update HEAD to the head being switched to, -- cgit v1.2.1 From b489097e1dbff7764e35ee260f8397d62a70fbb5 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:43 -0800 Subject: reset.c: replace switch by if-else The switch statement towards the end of reset.c is missing case arms for KEEP and MERGE for no obvious reason, and soon the only non-empty case arm will be the one for HARD. So let's proactively replace it by if-else, which will let us move one if statement out without leaving funny-looking left-overs. Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 97fa9f7859..c3eb2eb48d 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -349,18 +349,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix) * saving the previous head in ORIG_HEAD before. */ update_ref_status = update_refs(rev, sha1); - switch (reset_type) { - case HARD: - if (!update_ref_status && !quiet) - print_new_head_line(commit); - break; - case SOFT: /* Nothing else to do. */ - break; - case MIXED: /* Report what has not been updated. */ + if (reset_type == HARD && !update_ref_status && !quiet) + print_new_head_line(commit); + else if (reset_type == MIXED) /* Report what has not been updated. */ update_index_refresh(0, NULL, quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); - break; - } remove_branch_state(); -- cgit v1.2.1 From bf883f3006f719ba1ade0180f1764f42cc20b529 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:44 -0800 Subject: reset.c: move update_index_refresh() call out of read_from_tree() The final part of cmd_reset() essentially looks like: if (pathspec) { ... read_from_tree(...); } else { ... reset_index(...); update_index_refresh(...); ... } where read_from_tree() internally also calls update_index_refresh(). Move the call to update_index_refresh() out of read_from_tree for symmetry with the 'else' block, making read_from_tree() and reset_index() closer in functionality. Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index c3eb2eb48d..70733c271d 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -145,11 +145,8 @@ static void update_index_from_diff(struct diff_queue_struct *q, } } -static int read_from_tree(const char **pathspec, unsigned char *tree_sha1, - int refresh_flags) +static int read_from_tree(const char **pathspec, unsigned char *tree_sha1) { - struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); - int index_fd; struct diff_options opt; memset(&opt, 0, sizeof(opt)); @@ -157,7 +154,6 @@ static int read_from_tree(const char **pathspec, unsigned char *tree_sha1, opt.output_format = DIFF_FORMAT_CALLBACK; opt.format_callback = update_index_from_diff; - index_fd = hold_locked_index(lock, 1); read_cache(); if (do_diff_cache(tree_sha1, &opt)) return 1; @@ -165,7 +161,7 @@ static int read_from_tree(const char **pathspec, unsigned char *tree_sha1, diff_flush(&opt); diff_tree_release_paths(&opt); - return update_index_refresh(index_fd, lock, refresh_flags); + return 0; } static void set_reflog_message(struct strbuf *sb, const char *action, @@ -322,9 +318,13 @@ int cmd_reset(int argc, const char **argv, const char *prefix) die(_("%s reset is not allowed in a bare repository"), _(reset_type_names[reset_type])); - if (pathspec) - return read_from_tree(pathspec, sha1, - quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); + if (pathspec) { + struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); + int index_fd = hold_locked_index(lock, 1); + return read_from_tree(pathspec, sha1) || + update_index_refresh(index_fd, lock, + quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); + } /* Soft reset does not touch the index file nor the working tree * at all, but requires them in a good order. Other resets reset -- cgit v1.2.1 From 01a19dfc1ac53011deef492b21e52bf7840cef49 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:45 -0800 Subject: reset.c: move lock, write and commit out of update_index_refresh() In preparation for the/a following patch, move the locking, writing and committing of the index file out of update_index_refresh(). The code duplication caused will soon be taken care of. What remains of update_index_refresh() is just one line, but it is still called from two places, so let's leave it for now. In the process, we expose and fix the minor UI bug that makes us print "Could not refresh index" when we fail to write the index file when invoked with a pathspec. Copy the error message from the pathspec-less codepath ("Could not write new index file."). Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 70733c271d..c1d6ef2609 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -109,19 +109,10 @@ static void print_new_head_line(struct commit *commit) printf("\n"); } -static int update_index_refresh(int fd, struct lock_file *index_lock, int flags) +static void update_index_refresh(int flags) { - if (!index_lock) { - index_lock = xcalloc(1, sizeof(struct lock_file)); - fd = hold_locked_index(index_lock, 1); - } - refresh_index(&the_index, (flags), NULL, NULL, _("Unstaged changes after reset:")); - if (write_cache(fd, active_cache, active_nr) || - commit_locked_index(index_lock)) - return error ("Could not refresh index"); - return 0; } static void update_index_from_diff(struct diff_queue_struct *q, @@ -321,9 +312,14 @@ int cmd_reset(int argc, const char **argv, const char *prefix) if (pathspec) { struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); int index_fd = hold_locked_index(lock, 1); - return read_from_tree(pathspec, sha1) || - update_index_refresh(index_fd, lock, - quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); + if (read_from_tree(pathspec, sha1)) + return 1; + update_index_refresh( + quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); + if (write_cache(index_fd, active_cache, active_nr) || + commit_locked_index(lock)) + return error("Could not write new index file."); + return 0; } /* Soft reset does not touch the index file nor the working tree @@ -351,9 +347,15 @@ int cmd_reset(int argc, const char **argv, const char *prefix) if (reset_type == HARD && !update_ref_status && !quiet) print_new_head_line(commit); - else if (reset_type == MIXED) /* Report what has not been updated. */ - update_index_refresh(0, NULL, - quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); + else if (reset_type == MIXED) { /* Report what has not been updated. */ + struct lock_file *index_lock = xcalloc(1, sizeof(struct lock_file)); + int fd = hold_locked_index(index_lock, 1); + update_index_refresh( + quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); + if (write_cache(fd, active_cache, active_nr) || + commit_locked_index(index_lock)) + error("Could not refresh index"); + } remove_branch_state(); -- cgit v1.2.1 From bc41bf422e15209860bfc6c898dbd3cd89d5da34 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:46 -0800 Subject: reset [--mixed]: only write index file once When doing a mixed reset without paths, the index is locked, read, reset, and written back as part of the actual reset operation (in reset_index()). Then, when showing the list of worktree modifications, we lock the index again, refresh it, and write it. Change this so we only write the index once, making "git reset" a little faster. It does mean that the index lock will be held a little longer, but the difference is small compared to the time spent refreshing the index. There is one minor functional difference: We used to say "Could not write new index file." if the first write failed, and "Could not refresh index" if the second write failed. Now, we will only use the first message. This speeds up "git reset" a little on the linux-2.6 repo (best of five, warm cache): Before After real 0m0.239s 0m0.214s user 0m0.160s 0m0.130s sys 0m0.070s 0m0.080s Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index c1d6ef2609..e8a3e41531 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -336,6 +336,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix) err = reset_index(sha1, MIXED, quiet); if (err) die(_("Could not reset index file to revision '%s'."), rev); + + if (reset_type == MIXED) /* Report what has not been updated. */ + update_index_refresh( + quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); + if (write_cache(newfd, active_cache, active_nr) || commit_locked_index(lock)) die(_("Could not write new index file.")); @@ -347,15 +352,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix) if (reset_type == HARD && !update_ref_status && !quiet) print_new_head_line(commit); - else if (reset_type == MIXED) { /* Report what has not been updated. */ - struct lock_file *index_lock = xcalloc(1, sizeof(struct lock_file)); - int fd = hold_locked_index(index_lock, 1); - update_index_refresh( - quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); - if (write_cache(fd, active_cache, active_nr) || - commit_locked_index(index_lock)) - error("Could not refresh index"); - } remove_branch_state(); -- cgit v1.2.1 From 3bbf2f20f62ec6aa7998d9f7acc942ded6639870 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:47 -0800 Subject: reset.c: finish entire cmd_reset() whether or not pathspec is given By not returning from inside the "if (pathspec)" block, we can let the pathspec-aware and pathspec-less code share a bit more, making it easier to make future changes that should affect both cases. This also highlights the similarity between read_from_tree() and reset_index(). Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 42 ++++++++++++++++++------------------------ 1 file changed, 18 insertions(+), 24 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index e8a3e41531..c316d9b589 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -309,19 +309,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix) die(_("%s reset is not allowed in a bare repository"), _(reset_type_names[reset_type])); - if (pathspec) { - struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); - int index_fd = hold_locked_index(lock, 1); - if (read_from_tree(pathspec, sha1)) - return 1; - update_index_refresh( - quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); - if (write_cache(index_fd, active_cache, active_nr) || - commit_locked_index(lock)) - return error("Could not write new index file."); - return 0; - } - /* Soft reset does not touch the index file nor the working tree * at all, but requires them in a good order. Other resets reset * the index file to the tree object we are switching to. */ @@ -331,11 +318,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix) if (reset_type != SOFT) { struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); int newfd = hold_locked_index(lock, 1); - int err = reset_index(sha1, reset_type, quiet); - if (reset_type == KEEP && !err) - err = reset_index(sha1, MIXED, quiet); - if (err) - die(_("Could not reset index file to revision '%s'."), rev); + if (pathspec) { + if (read_from_tree(pathspec, sha1)) + return 1; + } else { + int err = reset_index(sha1, reset_type, quiet); + if (reset_type == KEEP && !err) + err = reset_index(sha1, MIXED, quiet); + if (err) + die(_("Could not reset index file to revision '%s'."), rev); + } if (reset_type == MIXED) /* Report what has not been updated. */ update_index_refresh( @@ -346,14 +338,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix) die(_("Could not write new index file.")); } - /* Any resets update HEAD to the head being switched to, - * saving the previous head in ORIG_HEAD before. */ - update_ref_status = update_refs(rev, sha1); + if (!pathspec) { + /* Any resets without paths update HEAD to the head being + * switched to, saving the previous head in ORIG_HEAD before. */ + update_ref_status = update_refs(rev, sha1); - if (reset_type == HARD && !update_ref_status && !quiet) - print_new_head_line(commit); + if (reset_type == HARD && !update_ref_status && !quiet) + print_new_head_line(commit); - remove_branch_state(); + remove_branch_state(); + } return update_ref_status; } -- cgit v1.2.1 From 7637df131ec9939b737ca284874478cb5c899348 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:48 -0800 Subject: reset.c: inline update_index_refresh() Now that there is only one caller left to the single-line method update_index_refresh(), inline it. Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index c316d9b589..520c1a56ea 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -109,12 +109,6 @@ static void print_new_head_line(struct commit *commit) printf("\n"); } -static void update_index_refresh(int flags) -{ - refresh_index(&the_index, (flags), NULL, NULL, - _("Unstaged changes after reset:")); -} - static void update_index_from_diff(struct diff_queue_struct *q, struct diff_options *opt, void *data) { @@ -329,9 +323,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix) die(_("Could not reset index file to revision '%s'."), rev); } - if (reset_type == MIXED) /* Report what has not been updated. */ - update_index_refresh( - quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN); + if (reset_type == MIXED) { /* Report what has not been updated. */ + int flags = quiet ? REFRESH_QUIET : REFRESH_IN_PORCELAIN; + refresh_index(&the_index, flags, NULL, NULL, + _("Unstaged changes after reset:")); + } if (write_cache(newfd, active_cache, active_nr) || commit_locked_index(lock)) -- cgit v1.2.1 From 2f328c3d2e88230a236e3d84d2bd6de59aea578d Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:49 -0800 Subject: reset $sha1 $pathspec: require $sha1 only to be treeish Resetting with paths does not update HEAD and there is nothing else that a commit should be needed for. Relax the argument parsing so only a tree is required. The sha1 is only passed to read_from_tree(), which already only requires a tree. The "rev" variable we pass to run_add_interactive() will resolve to a tree. This is fine since interactive_reset only needs the parameter to be a treeish and doesn't use it for display purposes. Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 48 +++++++++++++++++++++++++++--------------------- 1 file changed, 27 insertions(+), 21 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 520c1a56ea..b776867ad2 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -178,9 +178,10 @@ static const char **parse_args(const char **argv, const char *prefix, const char /* * Possible arguments are: * - * git reset [-opts] ... - * git reset [-opts] -- ... - * git reset [-opts] -- ... + * git reset [-opts] [] + * git reset [-opts] [...] + * git reset [-opts] -- [...] + * git reset [-opts] -- [...] * git reset [-opts] ... * * At this point, argv points immediately after [-opts]. @@ -195,11 +196,13 @@ static const char **parse_args(const char **argv, const char *prefix, const char } /* * Otherwise, argv[0] could be either or and - * has to be unambiguous. + * has to be unambiguous. If there is a single argument, it + * can not be a tree */ - else if (!get_sha1_committish(argv[0], unused)) { + else if ((!argv[1] && !get_sha1_committish(argv[0], unused)) || + (argv[1] && !get_sha1_treeish(argv[0], unused))) { /* - * Ok, argv[0] looks like a rev; it should not + * Ok, argv[0] looks like a commit/tree; it should not * be a filename. */ verify_non_filename(prefix, argv[0]); @@ -241,7 +244,6 @@ int cmd_reset(int argc, const char **argv, const char *prefix) const char *rev; unsigned char sha1[20]; const char **pathspec = NULL; - struct commit *commit; const struct option options[] = { OPT__QUIET(&quiet, N_("be quiet, only report errors")), OPT_SET_INT(0, "mixed", &reset_type, @@ -263,19 +265,23 @@ int cmd_reset(int argc, const char **argv, const char *prefix) PARSE_OPT_KEEP_DASHDASH); pathspec = parse_args(argv, prefix, &rev); - if (get_sha1_committish(rev, sha1)) - die(_("Failed to resolve '%s' as a valid ref."), rev); - - /* - * NOTE: As "git reset $treeish -- $path" should be usable on - * any tree-ish, this is not strictly correct. We are not - * moving the HEAD to any commit; we are merely resetting the - * entries in the index to that of a treeish. - */ - commit = lookup_commit_reference(sha1); - if (!commit) - die(_("Could not parse object '%s'."), rev); - hashcpy(sha1, commit->object.sha1); + if (!pathspec) { + struct commit *commit; + if (get_sha1_committish(rev, sha1)) + die(_("Failed to resolve '%s' as a valid revision."), rev); + commit = lookup_commit_reference(sha1); + if (!commit) + die(_("Could not parse object '%s'."), rev); + hashcpy(sha1, commit->object.sha1); + } else { + struct tree *tree; + if (get_sha1_treeish(rev, sha1)) + die(_("Failed to resolve '%s' as a valid tree."), rev); + tree = parse_tree_indirect(sha1); + if (!tree) + die(_("Could not parse object '%s'."), rev); + hashcpy(sha1, tree->object.sha1); + } if (patch_mode) { if (reset_type != NONE) @@ -340,7 +346,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) update_ref_status = update_refs(rev, sha1); if (reset_type == HARD && !update_ref_status && !quiet) - print_new_head_line(commit); + print_new_head_line(lookup_commit_reference(sha1)); remove_branch_state(); } -- cgit v1.2.1 From 166ec2e96e31bac913f44823dadd6c732d353172 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:50 -0800 Subject: reset: allow reset on unborn branch Some users seem to think, knowingly or not, that being on an unborn branch is like having a commit with an empty tree checked out, but when run on an unborn branch, "git reset" currently fails with: fatal: Failed to resolve 'HEAD' as a valid ref. Instead of making users figure out that they should run git rm --cached -r . , let's teach "git reset" without a revision argument, when on an unborn branch, to behave as if the user asked to reset to an empty tree. Don't take the analogy with an empty commit too far, though, but still disallow explictly referring to HEAD in "git reset HEAD". Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index b776867ad2..45b01ebbcc 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -240,7 +240,7 @@ static int update_refs(const char *rev, const unsigned char *sha1) int cmd_reset(int argc, const char **argv, const char *prefix) { int reset_type = NONE, update_ref_status = 0, quiet = 0; - int patch_mode = 0; + int patch_mode = 0, unborn; const char *rev; unsigned char sha1[20]; const char **pathspec = NULL; @@ -265,7 +265,11 @@ int cmd_reset(int argc, const char **argv, const char *prefix) PARSE_OPT_KEEP_DASHDASH); pathspec = parse_args(argv, prefix, &rev); - if (!pathspec) { + unborn = !strcmp(rev, "HEAD") && get_sha1("HEAD", sha1); + if (unborn) { + /* reset on unborn branch: treat as reset to empty tree */ + hashcpy(sha1, EMPTY_TREE_SHA1_BIN); + } else if (!pathspec) { struct commit *commit; if (get_sha1_committish(rev, sha1)) die(_("Failed to resolve '%s' as a valid revision."), rev); @@ -286,7 +290,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) if (patch_mode) { if (reset_type != NONE) die(_("--patch is incompatible with --{hard,mixed,soft}")); - return run_add_interactive(rev, "--patch=reset", pathspec); + return run_add_interactive(sha1_to_hex(sha1), "--patch=reset", pathspec); } /* git reset tree [--] paths... can be used to @@ -340,16 +344,16 @@ int cmd_reset(int argc, const char **argv, const char *prefix) die(_("Could not write new index file.")); } - if (!pathspec) { + if (!pathspec && !unborn) { /* Any resets without paths update HEAD to the head being * switched to, saving the previous head in ORIG_HEAD before. */ update_ref_status = update_refs(rev, sha1); if (reset_type == HARD && !update_ref_status && !quiet) print_new_head_line(lookup_commit_reference(sha1)); - - remove_branch_state(); } + if (!pathspec) + remove_branch_state(); return update_ref_status; } -- cgit v1.2.1 From 3fde386a40f38dbaa684c17603e71909b862d021 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Mon, 14 Jan 2013 21:47:51 -0800 Subject: reset [--mixed]: use diff-based reset whether or not pathspec was given Thanks to b65982b (Optimize "diff-index --cached" using cache-tree, 2009-05-20), resetting with paths is much faster than resetting without paths. Some timings for the linux-2.6 repo to illustrate this (best of five, warm cache): reset reset . real 0m0.219s 0m0.080s user 0m0.140s 0m0.040s sys 0m0.070s 0m0.030s These two commands should do the same thing, so instead of having the user type the trailing " ." to get the faster do_diff_cache()-based implementation, always use it when doing a mixed reset, with or without paths (so "git reset $rev" would also be faster). Timing "git reset" shows that it indeed becomes as fast as "git reset ." after this patch. Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 45b01ebbcc..921afbe62e 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -322,7 +322,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix) if (reset_type != SOFT) { struct lock_file *lock = xcalloc(1, sizeof(struct lock_file)); int newfd = hold_locked_index(lock, 1); - if (pathspec) { + if (reset_type == MIXED) { if (read_from_tree(pathspec, sha1)) return 1; } else { -- cgit v1.2.1 From bf44142f5464e913d99f9544787e59fc630f6cc9 Mon Sep 17 00:00:00 2001 From: Martin von Zweigbergk Date: Wed, 16 Jan 2013 10:00:35 -0800 Subject: reset: update documentation to require only tree-ish with paths When resetting with paths, we no longer require a commit argument, but only a tree-ish. Update the documentation and synopsis accordingly. Signed-off-by: Martin von Zweigbergk Signed-off-by: Junio C Hamano --- builtin/reset.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 921afbe62e..6032131a90 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -23,8 +23,8 @@ static const char * const git_reset_usage[] = { N_("git reset [--mixed | --soft | --hard | --merge | --keep] [-q] []"), - N_("git reset [-q] [--] ..."), - N_("git reset --patch [] [--] [...]"), + N_("git reset [-q] [--] ..."), + N_("git reset --patch [] [--] [...]"), NULL }; -- cgit v1.2.1 From ecaee8050cec23eb4cf082512e907e3e52c20b57 Mon Sep 17 00:00:00 2001 From: Alexey Shumkin Date: Wed, 26 Jun 2013 14:19:50 +0400 Subject: pretty: --format output should honor logOutputEncoding One can set an alias $ git config [--global] alias.lg "log --graph --pretty=format:'%Cred%h%Creset -%C(yellow)%d%Creset %s %Cgreen(%cd) %C(bold blue)<%an>%Creset' --abbrev-commit --date=local" to see the log as a pretty tree (like *gitk* but in a terminal). However, log messages written in an encoding i18n.commitEncoding which differs from terminal encoding are shown corrupted even when i18n.logOutputEncoding and terminal encoding are the same (e.g. log messages committed on a Cygwin box with Windows-1251 encoding seen on a Linux box with a UTF-8 encoding and vice versa). To simplify an example we can say the following two commands are expected to give the same output to a terminal: $ git log --oneline --no-color $ git log --pretty=format:'%h %s' However, the former pays attention to i18n.logOutputEncoding configuration, while the latter does not when it formats "%s". The same corruption is true for $ git diff --submodule=log and $ git rev-list --pretty=format:%s HEAD and $ git reset --hard This patch makes pretty --format honor logOutputEncoding when it formats log message. Signed-off-by: Alexey Shumkin Signed-off-by: Junio C Hamano --- builtin/reset.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'builtin/reset.c') diff --git a/builtin/reset.c b/builtin/reset.c index 6032131a90..afa6e020e8 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -93,10 +93,12 @@ static int reset_index(const unsigned char *sha1, int reset_type, int quiet) static void print_new_head_line(struct commit *commit) { const char *hex, *body; + char *msg; hex = find_unique_abbrev(commit->object.sha1, DEFAULT_ABBREV); printf(_("HEAD is now at %s"), hex); - body = strstr(commit->buffer, "\n\n"); + msg = logmsg_reencode(commit, NULL, get_log_output_encoding()); + body = strstr(msg, "\n\n"); if (body) { const char *eol; size_t len; @@ -107,6 +109,7 @@ static void print_new_head_line(struct commit *commit) } else printf("\n"); + logmsg_free(msg, commit); } static void update_index_from_diff(struct diff_queue_struct *q, -- cgit v1.2.1