From 338bc8d81897d19f4795114e4e6a59d6ca44d1db Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 7 Oct 2016 18:08:30 +0200 Subject: pull: drop confusing prefix parameter of die_on_unclean_work_tree() In cmd_pull(), when verifying that there are no changes preventing a rebasing pull, we diligently pass the prefix parameter to the die_on_unclean_work_tree() function which in turn diligently passes it to the has_unstaged_changes() and has_uncommitted_changes() functions. The casual reader might now be curious (as this developer was) whether that means that calling `git pull --rebase` in a subdirectory will ignore unstaged changes in other parts of the working directory. And be puzzled that `git pull --rebase` (correctly) complains about those changes outside of the current directory. The puzzle is easily resolved: while we take pains to pass around the prefix and even pass it to init_revisions(), the fact that no paths are passed to init_revisions() ensures that the prefix is simply ignored. That, combined with the fact that we will *always* want a *full* working directory check before running a rebasing pull, is reason enough to simply do away with the actual prefix parameter and to pass NULL instead, as if we were running this from the top-level working directory anyway. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/builtin/pull.c b/builtin/pull.c index 398aae1..d4bd635 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -328,12 +328,12 @@ static int git_pull_config(const char *var, const char *value, void *cb) /** * Returns 1 if there are unstaged changes, 0 otherwise. */ -static int has_unstaged_changes(const char *prefix) +static int has_unstaged_changes(void) { struct rev_info rev_info; int result; - init_revisions(&rev_info, prefix); + init_revisions(&rev_info, NULL); DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); DIFF_OPT_SET(&rev_info.diffopt, QUICK); diff_setup_done(&rev_info.diffopt); @@ -344,7 +344,7 @@ static int has_unstaged_changes(const char *prefix) /** * Returns 1 if there are uncommitted changes, 0 otherwise. */ -static int has_uncommitted_changes(const char *prefix) +static int has_uncommitted_changes(void) { struct rev_info rev_info; int result; @@ -352,7 +352,7 @@ static int has_uncommitted_changes(const char *prefix) if (is_cache_unborn()) return 0; - init_revisions(&rev_info, prefix); + init_revisions(&rev_info, NULL); DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); DIFF_OPT_SET(&rev_info.diffopt, QUICK); add_head_to_pending(&rev_info); @@ -365,7 +365,7 @@ static int has_uncommitted_changes(const char *prefix) * If the work tree has unstaged or uncommitted changes, dies with the * appropriate message. */ -static void die_on_unclean_work_tree(const char *prefix) +static void die_on_unclean_work_tree(void) { struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file)); int do_die = 0; @@ -375,12 +375,12 @@ static void die_on_unclean_work_tree(const char *prefix) update_index_if_able(&the_index, lock_file); rollback_lock_file(lock_file); - if (has_unstaged_changes(prefix)) { + if (has_unstaged_changes()) { error(_("Cannot pull with rebase: You have unstaged changes.")); do_die = 1; } - if (has_uncommitted_changes(prefix)) { + if (has_uncommitted_changes()) { if (do_die) error(_("Additionally, your index contains uncommitted changes.")); else @@ -875,7 +875,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix) die(_("Updating an unborn branch with changes added to the index.")); if (!autostash) - die_on_unclean_work_tree(prefix); + die_on_unclean_work_tree(); if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs)) hashclr(rebase_fork_point); -- cgit v0.10.2-6-g49f6 From ea63b393ec76484690733d6f589c9e67fedbaa78 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 7 Oct 2016 18:08:34 +0200 Subject: pull: make code more similar to the shell script again When converting the pull command to a builtin, the require_clean_work_tree() function was renamed and the pull-specific parts hard-coded. This makes it impossible to reuse the code, so let's modify the code to make it more similar to the original shell script again. Note: when the hint "Please commit or stash them" was introduced first, Git did not have the convention of continuing error messages in lower case, but now we do have that convention, therefore we reintroduce this hint down-cased, obeying said convention. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/builtin/pull.c b/builtin/pull.c index d4bd635..58fc176 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -365,10 +365,11 @@ static int has_uncommitted_changes(void) * If the work tree has unstaged or uncommitted changes, dies with the * appropriate message. */ -static void die_on_unclean_work_tree(void) +static int require_clean_work_tree(const char *action, const char *hint, + int gently) { struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file)); - int do_die = 0; + int err = 0; hold_locked_index(lock_file, 0); refresh_cache(REFRESH_QUIET); @@ -376,20 +377,28 @@ static void die_on_unclean_work_tree(void) rollback_lock_file(lock_file); if (has_unstaged_changes()) { - error(_("Cannot pull with rebase: You have unstaged changes.")); - do_die = 1; + /* TRANSLATORS: the action is e.g. "pull with rebase" */ + error(_("Cannot %s: You have unstaged changes."), _(action)); + err = 1; } if (has_uncommitted_changes()) { - if (do_die) + if (err) error(_("Additionally, your index contains uncommitted changes.")); else - error(_("Cannot pull with rebase: Your index contains uncommitted changes.")); - do_die = 1; + error(_("Cannot %s: Your index contains uncommitted changes."), + _(action)); + err = 1; } - if (do_die) - exit(1); + if (err) { + if (hint) + error("%s", hint); + if (!gently) + exit(128); + } + + return err; } /** @@ -875,7 +884,8 @@ int cmd_pull(int argc, const char **argv, const char *prefix) die(_("Updating an unborn branch with changes added to the index.")); if (!autostash) - die_on_unclean_work_tree(); + require_clean_work_tree(N_("pull with rebase"), + _("please commit or stash them."), 0); if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs)) hashclr(rebase_fork_point); -- cgit v0.10.2-6-g49f6 From fd84986f467b2556e0675d1df00f83b3a323cf2e Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 7 Oct 2016 18:08:38 +0200 Subject: wt-status: make the require_clean_work_tree() function reusable The function used by "git pull" to stop the user when the working tree has changes is useful in other places. Let's move it into a more prominent (and into an actually reusable) spot: wt-status.[ch]. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/builtin/pull.c b/builtin/pull.c index 58fc176..01b6465 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -17,6 +17,7 @@ #include "revision.h" #include "tempfile.h" #include "lockfile.h" +#include "wt-status.h" enum rebase_type { REBASE_INVALID = -1, @@ -326,82 +327,6 @@ static int git_pull_config(const char *var, const char *value, void *cb) } /** - * Returns 1 if there are unstaged changes, 0 otherwise. - */ -static int has_unstaged_changes(void) -{ - struct rev_info rev_info; - int result; - - init_revisions(&rev_info, NULL); - DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); - DIFF_OPT_SET(&rev_info.diffopt, QUICK); - diff_setup_done(&rev_info.diffopt); - result = run_diff_files(&rev_info, 0); - return diff_result_code(&rev_info.diffopt, result); -} - -/** - * Returns 1 if there are uncommitted changes, 0 otherwise. - */ -static int has_uncommitted_changes(void) -{ - struct rev_info rev_info; - int result; - - if (is_cache_unborn()) - return 0; - - init_revisions(&rev_info, NULL); - DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); - DIFF_OPT_SET(&rev_info.diffopt, QUICK); - add_head_to_pending(&rev_info); - diff_setup_done(&rev_info.diffopt); - result = run_diff_index(&rev_info, 1); - return diff_result_code(&rev_info.diffopt, result); -} - -/** - * If the work tree has unstaged or uncommitted changes, dies with the - * appropriate message. - */ -static int require_clean_work_tree(const char *action, const char *hint, - int gently) -{ - struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file)); - int err = 0; - - hold_locked_index(lock_file, 0); - refresh_cache(REFRESH_QUIET); - update_index_if_able(&the_index, lock_file); - rollback_lock_file(lock_file); - - if (has_unstaged_changes()) { - /* TRANSLATORS: the action is e.g. "pull with rebase" */ - error(_("Cannot %s: You have unstaged changes."), _(action)); - err = 1; - } - - if (has_uncommitted_changes()) { - if (err) - error(_("Additionally, your index contains uncommitted changes.")); - else - error(_("Cannot %s: Your index contains uncommitted changes."), - _(action)); - err = 1; - } - - if (err) { - if (hint) - error("%s", hint); - if (!gently) - exit(128); - } - - return err; -} - -/** * Appends merge candidates from FETCH_HEAD that are not marked not-for-merge * into merge_heads. */ diff --git a/wt-status.c b/wt-status.c index 539aac1..4f9b513 100644 --- a/wt-status.c +++ b/wt-status.c @@ -16,6 +16,7 @@ #include "strbuf.h" #include "utf8.h" #include "worktree.h" +#include "lockfile.h" static const char cut_line[] = "------------------------ >8 ------------------------\n"; @@ -2209,3 +2210,78 @@ void wt_status_print(struct wt_status *s) break; } } + +/** + * Returns 1 if there are unstaged changes, 0 otherwise. + */ +static int has_unstaged_changes(void) +{ + struct rev_info rev_info; + int result; + + init_revisions(&rev_info, NULL); + DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); + DIFF_OPT_SET(&rev_info.diffopt, QUICK); + diff_setup_done(&rev_info.diffopt); + result = run_diff_files(&rev_info, 0); + return diff_result_code(&rev_info.diffopt, result); +} + +/** + * Returns 1 if there are uncommitted changes, 0 otherwise. + */ +static int has_uncommitted_changes(void) +{ + struct rev_info rev_info; + int result; + + if (is_cache_unborn()) + return 0; + + init_revisions(&rev_info, NULL); + DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); + DIFF_OPT_SET(&rev_info.diffopt, QUICK); + add_head_to_pending(&rev_info); + diff_setup_done(&rev_info.diffopt); + result = run_diff_index(&rev_info, 1); + return diff_result_code(&rev_info.diffopt, result); +} + +/** + * If the work tree has unstaged or uncommitted changes, dies with the + * appropriate message. + */ +int require_clean_work_tree(const char *action, const char *hint, int gently) +{ + struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file)); + int err = 0; + + hold_locked_index(lock_file, 0); + refresh_cache(REFRESH_QUIET); + update_index_if_able(&the_index, lock_file); + rollback_lock_file(lock_file); + + if (has_unstaged_changes()) { + /* TRANSLATORS: the action is e.g. "pull with rebase" */ + error(_("Cannot %s: You have unstaged changes."), _(action)); + err = 1; + } + + if (has_uncommitted_changes()) { + if (err) + error(_("Additionally, your index contains uncommitted changes.")); + else + error(_("Cannot %s: Your index contains uncommitted changes."), + _(action)); + err = 1; + } + + if (err) { + if (hint) + error("%s", hint); + if (!gently) + exit(128); + } + + return err; +} diff --git a/wt-status.h b/wt-status.h index e401837..68b4709 100644 --- a/wt-status.h +++ b/wt-status.h @@ -128,4 +128,7 @@ void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, . __attribute__((format (printf, 3, 4))) void status_printf(struct wt_status *s, const char *color, const char *fmt, ...); +/* The following function expects that the caller took care of reading the index. */ +int require_clean_work_tree(const char *action, const char *hint, int gently); + #endif /* STATUS_H */ -- cgit v0.10.2-6-g49f6 From 41a5dd6d8622a67216978131e394148c7fa58b19 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 7 Oct 2016 18:08:56 +0200 Subject: wt-status: export also the has_un{staged,committed}_changes() functions They will be used in the upcoming rebase helper. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/wt-status.c b/wt-status.c index 4f9b513..6ad8fb6 100644 --- a/wt-status.c +++ b/wt-status.c @@ -2214,7 +2214,7 @@ void wt_status_print(struct wt_status *s) /** * Returns 1 if there are unstaged changes, 0 otherwise. */ -static int has_unstaged_changes(void) +int has_unstaged_changes(void) { struct rev_info rev_info; int result; @@ -2230,7 +2230,7 @@ static int has_unstaged_changes(void) /** * Returns 1 if there are uncommitted changes, 0 otherwise. */ -static int has_uncommitted_changes(void) +int has_uncommitted_changes(void) { struct rev_info rev_info; int result; diff --git a/wt-status.h b/wt-status.h index 68b4709..68e367a 100644 --- a/wt-status.h +++ b/wt-status.h @@ -128,7 +128,9 @@ void status_printf_ln(struct wt_status *s, const char *color, const char *fmt, . __attribute__((format (printf, 3, 4))) void status_printf(struct wt_status *s, const char *color, const char *fmt, ...); -/* The following function expects that the caller took care of reading the index. */ +/* The following functions expect that the caller took care of reading the index. */ +int has_unstaged_changes(void); +int has_uncommitted_changes(void); int require_clean_work_tree(const char *action, const char *hint, int gently); #endif /* STATUS_H */ -- cgit v0.10.2-6-g49f6 From d8cc92ab13e438f225770843868ae5a58c6bb357 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 7 Oct 2016 18:09:00 +0200 Subject: wt-status: teach has_{unstaged,uncommitted}_changes() about submodules Sometimes we are *actually* interested in those changes... For example when an interactive rebase wants to continue with a staged submodule update. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/builtin/pull.c b/builtin/pull.c index 01b6465..d6e46ee 100644 --- a/builtin/pull.c +++ b/builtin/pull.c @@ -810,7 +810,7 @@ int cmd_pull(int argc, const char **argv, const char *prefix) if (!autostash) require_clean_work_tree(N_("pull with rebase"), - _("please commit or stash them."), 0); + _("please commit or stash them."), 1, 0); if (get_rebase_fork_point(rebase_fork_point, repo, *refspecs)) hashclr(rebase_fork_point); diff --git a/wt-status.c b/wt-status.c index 6ad8fb6..1a27057 100644 --- a/wt-status.c +++ b/wt-status.c @@ -2214,13 +2214,14 @@ void wt_status_print(struct wt_status *s) /** * Returns 1 if there are unstaged changes, 0 otherwise. */ -int has_unstaged_changes(void) +int has_unstaged_changes(int ignore_submodules) { struct rev_info rev_info; int result; init_revisions(&rev_info, NULL); - DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); + if (ignore_submodules) + DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); DIFF_OPT_SET(&rev_info.diffopt, QUICK); diff_setup_done(&rev_info.diffopt); result = run_diff_files(&rev_info, 0); @@ -2230,7 +2231,7 @@ int has_unstaged_changes(void) /** * Returns 1 if there are uncommitted changes, 0 otherwise. */ -int has_uncommitted_changes(void) +int has_uncommitted_changes(int ignore_submodules) { struct rev_info rev_info; int result; @@ -2239,7 +2240,8 @@ int has_uncommitted_changes(void) return 0; init_revisions(&rev_info, NULL); - DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); + if (ignore_submodules) + DIFF_OPT_SET(&rev_info.diffopt, IGNORE_SUBMODULES); DIFF_OPT_SET(&rev_info.diffopt, QUICK); add_head_to_pending(&rev_info); diff_setup_done(&rev_info.diffopt); @@ -2251,7 +2253,7 @@ int has_uncommitted_changes(void) * If the work tree has unstaged or uncommitted changes, dies with the * appropriate message. */ -int require_clean_work_tree(const char *action, const char *hint, int gently) +int require_clean_work_tree(const char *action, const char *hint, int ignore_submodules, int gently) { struct lock_file *lock_file = xcalloc(1, sizeof(*lock_file)); int err = 0; @@ -2261,13 +2263,13 @@ int require_clean_work_tree(const char *action, const char *hint, int gently) update_index_if_able(&the_index, lock_file); rollback_lock_file(lock_file); - if (has_unstaged_changes()) { + if (has_unstaged_changes(ignore_submodules)) { /* TRANSLATORS: the action is e.g. "pull with rebase" */ error(_("Cannot %s: You have unstaged changes."), _(action)); err = 1; } - if (has_uncommitted_changes()) { + if (has_uncommitted_changes(ignore_submodules)) { if (err) error(_("Additionally, your index contains uncommitted changes.")); else diff --git a/wt-status.h b/wt-status.h index 68e367a..54fec77 100644 --- a/wt-status.h +++ b/wt-status.h @@ -129,8 +129,9 @@ __attribute__((format (printf, 3, 4))) void status_printf(struct wt_status *s, const char *color, const char *fmt, ...); /* The following functions expect that the caller took care of reading the index. */ -int has_unstaged_changes(void); -int has_uncommitted_changes(void); -int require_clean_work_tree(const char *action, const char *hint, int gently); +int has_unstaged_changes(int ignore_submodules); +int has_uncommitted_changes(int ignore_submodules); +int require_clean_work_tree(const char *action, const char *hint, + int ignore_submodules, int gently); #endif /* STATUS_H */ -- cgit v0.10.2-6-g49f6 From 4777e175de439b7ea0c435de4bb0de9ab1663c38 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 7 Oct 2016 18:09:04 +0200 Subject: wt-status: begin error messages with lower-case The previous code still followed the old git-pull.sh code which did not adhere to our new convention. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano diff --git a/wt-status.c b/wt-status.c index 1a27057..7dbd1e1 100644 --- a/wt-status.c +++ b/wt-status.c @@ -2265,15 +2265,15 @@ int require_clean_work_tree(const char *action, const char *hint, int ignore_sub if (has_unstaged_changes(ignore_submodules)) { /* TRANSLATORS: the action is e.g. "pull with rebase" */ - error(_("Cannot %s: You have unstaged changes."), _(action)); + error(_("cannot %s: You have unstaged changes."), _(action)); err = 1; } if (has_uncommitted_changes(ignore_submodules)) { if (err) - error(_("Additionally, your index contains uncommitted changes.")); + error(_("additionally, your index contains uncommitted changes.")); else - error(_("Cannot %s: Your index contains uncommitted changes."), + error(_("cannot %s: Your index contains uncommitted changes."), _(action)); err = 1; } -- cgit v0.10.2-6-g49f6