Galan Rémi <remi.galan-alfo...@ensimag.grenoble-inp.fr> writes:
> Check if commits were removed (i.e. a line was deleted) and print
> warnings or stop git rebase depending on the value of the
> configuration variable rebase.missingCommitsCheck.
>
> This patch gives the user the possibility to avoid silent loss of
> information (losing a commit through deleting the line in this case)
> if he wants.
>
> Add the configuration variable rebase.missingCommitsCheck.
> - When unset or set to "ignore", no checking is done.
> - When set to "warn", the commits are checked, warnings are
> displayed but git rebase still proceeds.
> - When set to "error", the commits are checked, warnings are
> displayed and the rebase is stopped.
> (The user can then use 'git rebase --edit-todo' and
> 'git rebase --continue', or 'git rebase --abort')
>
> rebase.missingCommitsCheck defaults to "ignore".
>
> Signed-off-by: Galan Rémi <remi.galan-alfo...@ensimag.grenoble-inp.fr>
> ---
> In git-rebase--interactive, in the error case of check_todo_list, I
> added 'git checkout $onto' so that using 'die' for the error allows
> to use 'git rebase --edit-todo' (otherwise HEAD would not have been
> changed and it still would be placed after the commits of the
> rebase).
> Since now it doesn't abort the rebase, the documentation and the
> messages in the case error have changed.
> I moved the error case away from the initial test case for missing
> commits as to prepare for 3/3 part of the patch. It is something that
> was advised by Eric Sunshine when I checked both missing and
> duplicated commits, but that I removed it when removing the checking
> for duplicated commits since there was only one test. However I add
> it again since 3/3 will add more checking.
> I use the variable raiseError that I affect if the error must be
> raised instead of testing directly because I think it makes more
> sense with 3/3 and if we add other check in the future since it adds
> more possible errors (the test for the error case if not something
> like 'if (test checkLevel = error && test -s todo.miss) || test cond2
> || test cond3 || ...').
> I am wondering if a check_todo_list call should be added in the
> '--continue' part of the code: with this patch, the checking is only
> done once, if the user doesn't edit correctly with 'git rebase
> --edit-todo', it won't be picked by this.
> In the tests in t3404 I now also test that the warning/error messages
> are correct.
> I tried to not change this patch too much since it was already
> heavily reviewed, but there are some changes (mostly the ones
> mentionned above).
>
> Documentation/config.txt | 11 +++++
> Documentation/git-rebase.txt | 6 +++
> git-rebase--interactive.sh | 96
> +++++++++++++++++++++++++++++++++++++++++++
> t/t3404-rebase-interactive.sh | 66 +++++++++++++++++++++++++++++
> 4 files changed, 179 insertions(+)
>
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 4d21ce1..25b2a04 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -2160,6 +2160,17 @@ rebase.autoStash::
> successful rebase might result in non-trivial conflicts.
> Defaults to false.
>
> +rebase.missingCommitsCheck::
> + If set to "warn", git rebase -i will print a warning if some
> + commits are removed (e.g. a line was deleted), however the
> + rebase will still proceed. If set to "error", it will print
> + the previous warning and stop the rebase, 'git rebase
> + --edit-todo' can then be used to correct the error. If set to
> + "ignore", no checking is done.
> + To drop a commit without warning or error, use the `drop`
> + command in the todo-list.
> + Defaults to "ignore".
> +
> receive.advertiseAtomic::
> By default, git-receive-pack will advertise the atomic push
> capability to its clients. If you don't want to this capability
> diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
> index 34bd070..2ca3b8d 100644
> --- a/Documentation/git-rebase.txt
> +++ b/Documentation/git-rebase.txt
> @@ -213,6 +213,12 @@ rebase.autoSquash::
> rebase.autoStash::
> If set to true enable '--autostash' option by default.
>
> +rebase.missingCommitsCheck::
> + If set to "warn", print warnings about removed commits in
> + interactive mode. If set to "error", print the warnings and
> + stop the rebase. If set to "ignore", no checking is
> + done. "ignore" by default.
> +
> OPTIONS
> -------
> --onto <newbase>::
> diff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh
> index 72abf90..68a71d0 100644
> --- a/git-rebase--interactive.sh
> +++ b/git-rebase--interactive.sh
> @@ -834,6 +834,100 @@ add_exec_commands () {
> mv "$1.new" "$1"
> }
>
> +# Print the list of the SHA-1 of the commits
> +# from stdin to stdout
> +todo_list_to_sha_list () {
> + git stripspace --strip-comments |
> + while read -r command sha1 rest
> + do
> + case $command in
> + "$comment_char"*|''|noop|x|"exec")
> + ;;
> + *)
> + printf "%s\n" "$sha1"
> + ;;
> + esac
> + done
> +}
> +
> +# Use warn for each line of a file
> +# $1: file
> +warn_file () {
> + while read -r line
> + do
> + warn " - $line"
> + done <"$1"
> +}
> +
> +# Check if the user dropped some commits by mistake
> +# Behaviour determined by rebase.missingCommitsCheck.
> +check_todo_list () {
> + raiseError=f
> +
> + checkLevel=$(git config --get rebase.missingCommitsCheck)
> + checkLevel=${checkLevel:-ignore}
> + # Don't be case sensitive
> + checkLevel=$(echo "$checkLevel" | tr 'A-Z' 'a-z')
Avoid echo on user-supplied data. If $checkLevel starts with a "-", the
behavior is platform-dependant. Should not happen if the user is
sensible, but using
printf '%s' "$checkLevel"
is safer.
> + opt="--no-walk=sorted --format=oneline --abbrev-commit
> --stdin"
> + git rev-list $opt <"$todo".miss >"$todo".miss+
> + mv "$todo".miss+ "$todo".miss
> +
> + warn "Warning: some commits may have been dropped" \
> + "accidentally."
> + warn "Dropped commits (newer to older):"
> + warn_file "$todo".miss
I would find it more elegant with less intermediate files, like
git rev-list $opt <"$todo".miss | while read -r line
do
warn " - $line"
done
> + if test $raiseError = t
> + then
> + # Checkout before the first commit of the
> + # rebase: this way git rebase --continue
> + # will work correctly as it expects HEAD to be
> + # placed before the commit of the next action
> + GIT_REFLOG_ACTION="$GIT_REFLOG_ACTION: checkout $onto_name"
> + output git checkout $onto || die_abort "could not detach HEAD"
> + git update-ref ORIG_HEAD $orig_head
This is cut-and-pasted from below in the same file. It would deserve a
helper function I think.
--
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
