Junio C Hamano <gitster@xxxxxxxxx> writes: > Junio C Hamano <gitster@xxxxxxxxx> writes: > >> Matthieu Moy <Matthieu.Moy@xxxxxxx> writes: >> >>> If after a failed "exec" instruction there are staged changes,... >> >> I have to wonder why whatever "exec" runs is mucking with the index in the >> first place. Shouldn't we forbid it? > > I suspect your patch amounts to the same thing of forbidding, but > detecting the lack of $author_script feels like it is covering up the > symptom and not directly going for the cause of the symptom. > > I wonder if doing something like this would be more direct approach to > achieve the same thing. Not the same thing, but both patches could go well together. Mine covers pick deadbeef exec make test # :-( make test failed, I'm going to fix it hack hack hack git add changes # OK, seems fixed. git rebase --continue # --> rebase tells me I forgot to commit my fixup patch i.e. the user changed the index interactively, not within exec. Yours covers the case where the command itself changes the index. > + # Run in subshell because require_clean_work_tree can die. > + dirty=f > + (require_clean_work_tree "rebase") || dirty=t This will display error messages like Cannot rebase: You have unstaged changes and right after > warn "Execution failed: $rest" > + test "$dirty" = f || > + warn "and made changes to the index and/or the working tree" which sounds redundant. This should probably be (require_clean_work_tree "rebase" 2>/dev/null) || dirty=t but looking more closely at the patch, you're not the one introducing this, it was already there since 92c62a3f. > fi > + > ;; > *) I think this one can be removed, there's usually no blank line before ;; in the code. -- 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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html