@matheusbsbs / matheus-bagliano / commits / 76eab50f75

replay: remove dead code and rearrange

22d99f01 (replay: add --advance or 'cherry-pick' mode, 2023-11-24) both added `--advance` and made one of `--onto` or `--advance` mandatory. But `determine_replay_mode` claims that there is a third alternative; neither of `--onto` or `--advance` were given: if (onto_name) { ... } else if (*advance_name) { ... } else { ... } But this is false—the fallthrough else-block is dead code. Commit 22d99f01 was iterated upon by several people.[1] The initial author wrote code for a sort of *guess mode*, allowing for shorter commands when that was possible. But the next person instead made one of the aforementioned options mandatory. In turn this code was dead on arrival in git.git. [1]: https://lore.kernel.org/git/CABPp-BEcJqjD4ztsZo2FTZgWT5ZOADKYEyiZtda+d0mSd1quPQ@mail.gmail.com/ Let’s remove this code. We can also join the if-block with the condition `!*advance_name` into the `*onto` block since we do not set `*advance_name` in this function. It only looked like we might set it since the dead code has this line: *advance_name = xstrdup_or_null(last_key); Let’s also rename the function since we do not determine the replay mode here. We just set up `*onto` and refs to update. Note that there might be more dead code caused by this *guess mode*. We only concern ourselves with this function for now. Helped-by: Elijah Newren <newren@gmail.com> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Kristoffer Haugsbakk committed Jan 5, 2026 at 20:53 UTC 76eab50f756fedfa28388213d7fea209f86dfae6

1 file changed +16 -54

builtin/replay.c

+16 -54

@@ -162,12 +162,12 @@ static void get_ref_information(struct repository *repo,

162 }

163 }

164

165 -static void determine_replay_mode(struct repository *repo,

166 - struct rev_cmdline_info *cmd_info,

167 - const char *onto_name,

168 - char **advance_name,

169 - struct commit **onto,

170 - struct strset **update_refs)

165 +static void set_up_replay_mode(struct repository *repo,

166 + struct rev_cmdline_info *cmd_info,

167 + const char *onto_name,

168 + char **advance_name,

169 + struct commit **onto,

170 + struct strset **update_refs)

171 {

172 struct ref_info rinfo;

173

@@ -182,10 +182,16 @@ static void determine_replay_mode(struct repository *repo,

182 if (rinfo.positive_refexprs <

183 strset_get_size(&rinfo.positive_refs))

184 die(_("all positive revisions given must be references"));

185 - } else if (*advance_name) {

185 + *update_refs = xcalloc(1, sizeof(**update_refs));

186 + **update_refs = rinfo.positive_refs;

187 + memset(&rinfo.positive_refs, 0, sizeof(**update_refs));

188 + } else {

189 struct object_id oid;

190 char *fullname = NULL;

191

192 + if (!*advance_name)

193 + BUG("expected either onto_name or *advance_name in this function");

194 +

195 *onto = peel_committish(repo, *advance_name);

196 if (repo_dwim_ref(repo, *advance_name, strlen(*advance_name),

197 &oid, &fullname, 0) == 1) {

@@ -196,51 +202,6 @@ static void determine_replay_mode(struct repository *repo,

202 }

203 if (rinfo.positive_refexprs > 1)

204 die(_("cannot advance target with multiple sources because ordering would be ill-defined"));

199 - } else {

200 - int positive_refs_complete = (

201 - rinfo.positive_refexprs ==

202 - strset_get_size(&rinfo.positive_refs));

203 - int negative_refs_complete = (

204 - rinfo.negative_refexprs ==

205 - strset_get_size(&rinfo.negative_refs));

206 - /*

207 - * We need either positive_refs_complete or

208 - * negative_refs_complete, but not both.

209 - */

210 - if (rinfo.negative_refexprs > 0 &&

211 - positive_refs_complete == negative_refs_complete)

212 - die(_("cannot implicitly determine whether this is an --advance or --onto operation"));

213 - if (negative_refs_complete) {

214 - struct hashmap_iter iter;

215 - struct strmap_entry *entry;

216 - const char *last_key = NULL;

217 -

218 - if (rinfo.negative_refexprs == 0)

219 - die(_("all positive revisions given must be references"));

220 - else if (rinfo.negative_refexprs > 1)

221 - die(_("cannot implicitly determine whether this is an --advance or --onto operation"));

222 - else if (rinfo.positive_refexprs > 1)

223 - die(_("cannot advance target with multiple source branches because ordering would be ill-defined"));

224 -

225 - /* Only one entry, but we have to loop to get it */

226 - strset_for_each_entry(&rinfo.negative_refs,

227 - &iter, entry) {

228 - last_key = entry->key;

229 - }

230 -

231 - free(*advance_name);

232 - *advance_name = xstrdup_or_null(last_key);

233 - } else { /* positive_refs_complete */

234 - if (rinfo.negative_refexprs > 1)

235 - die(_("cannot implicitly determine correct base for --onto"));

236 - if (rinfo.negative_refexprs == 1)

237 - *onto = rinfo.onto;

238 - }

239 - }

240 - if (!*advance_name) {

241 - *update_refs = xcalloc(1, sizeof(**update_refs));

242 - **update_refs = rinfo.positive_refs;

243 - memset(&rinfo.positive_refs, 0, sizeof(**update_refs));

205 }

206 strset_clear(&rinfo.negative_refs);

207 strset_clear(&rinfo.positive_refs);

@@ -451,8 +412,9 @@ int cmd_replay(int argc,

412 revs.simplify_history = 0;

413 }

414

454 - determine_replay_mode(repo, &revs.cmdline, onto_name, &advance_name,

455 - &onto, &update_refs);

415 + set_up_replay_mode(repo, &revs.cmdline,

416 + onto_name, &advance_name,

417 + &onto, &update_refs);

418

419 if (!onto) /* FIXME: Should handle replaying down to root commit */

420 die("Replaying down to root commit is not supported yet!");