Hi Junio On 21/03/2019 04:21, Junio C Hamano wrote:
Phillip Wood <phillip.wood123@xxxxxxxxx> writes:+static struct replay_opts get_replay_opts(const struct rebase_options *opts) +{ + struct replay_opts replay = REPLAY_OPTS_INIT; + + sequencer_init_config(&replay); + + replay.action = REPLAY_INTERACTIVE_REBASE; + replay.signoff = opts->signoff; + replay.allow_ff = !(opts->flags & REBASE_FORCE); + if (opts->allow_rerere_autoupdate) + replay.allow_rerere_auto = opts->allow_rerere_autoupdate; + replay.allow_empty = 1; + replay.allow_empty_message = opts->allow_empty_message; + replay.verbose = opts->flags & REBASE_VERBOSE; + replay.reschedule_failed_exec = opts->reschedule_failed_exec; + replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt); + replay.strategy = opts->strategy; + + return replay; +}This calls init_config() and then sets .action; does it revert to what dl/merge-cleanup-scissors-fix wants to do, which flipped the order to fix some bug? It is a bit hard to tell.
dl/merge-cleanup-scissors-fix changes sequencer_init_config() to depend on the value of action so action should to be set first. What is in pu at the moment in not quite right (though I'm not sure what the practical implications are as it looks like the rebase tests are passing[1]).
[1] https://travis-ci.org/git/git/jobs/509332448
Unfortunately because of the earlier huge code movement the changes to _this_ file does not conflict and cleanly merges, but because the other file is removed by this series while a topic in flight updates it, the semantic conflict like this luckily gets discovered. Especially since this is still an RFC, I'd preferred to see it without moving around the code too much (instead, exporting some symbols that need to be visible with each other after renaming them to more appropriate names that are fit in the global namespace).
I'm happy to try doing that, though the textual conflict would be in a different place in the file to the semantic conflict but at least they'd both be in the same file. I think it would only need to share the definitions of struct rebase_options, enum rebase_action and the declaration of run_rebase_interactive(). Would you be happy with the addition of builtin/rebase.h (there don't seem to be another headers in that directory). We could leave rebase--interactive.c around until rebase--preserve-merges.sh is finally removed.
Best Wishes Phillip
