4.7 Exercise 7
From git repo, checkout revision 4284497396 and merge
cdb5330a9b .
Solve all conflicts.
CB in diff.h
There’s one API conflict in diff.h
Listing 4.21:Exercise 7 - CB in diff.h443<<<<<<< HEAD 444void diff_no_index(struct rev_info *, int, const char **); 445||||||| 125dcea963 446void diff_no_index(struct repository *, struct rev_info *, int, const char **); 447======= 448int diff_no_index(struct repository *, struct rev_info *, 449 int implicit_no_index, int, const char **); 450>>>>>>> cdb5330a9b
Looks very straight forward:
dMU
Removed struct repository * as an argument to function diff_no_index.
dML
Added (or rather, inserted) int implicit_no_index as an argument to function
diff_no_index.
Resolution
Given that it is much simpler to delete things than moving things around, we will
work from LB.
Listing 4.22:Exercise 7 - Step 1 LB in diff.h447======= 448int diff_no_index(struct repository *, struct rev_info *, 449 int implicit_no_index, int, const char **); 450>>>>>>> cdb5330a9b
Then we delete struct repository * as an argument.
Listing 4.23:Exercise 7 - Step 2 LB in diff.h447======= 448int diff_no_index(struct rev_info *, 449 int implicit_no_index, int, const char **); 450>>>>>>> cdb5330a9b
Listing 4.24:Exercise 7 - Resolution CB in diff.h441int diff_result_code(struct diff_options *, int); 442 443int diff_no_index(struct rev_info *, int implicit_no_index, int, 444 const char **); 445 446int index_differs_from(struct repository *r, const char *def, 447 const struct diff_flags *flags, 448 int ita_invisible_in_index);
Notice how I repositioned the arguments to match a more git-project standards
formatting.
This of course means that all existing calls to diff_no_index will be broken. It’s
likely that there will be some conflicting ones, but we already know that
there might be some that might get away with it that we will have to hunt
down.
CB in builtin/diff.c
This one will make us sweat a little bit.
Listing 4.25:Exercise 7 - CB in builtin/diff.c324<<<<<<< HEAD 325 if (no_index && argc != i + 2) { 326 if (no_index == DIFF_NO_INDEX_IMPLICIT) { 327 /* 328 * There was no --no-index and there were not two 329 * paths. It is possible that the user intended 330 * to do an inside-repository operation. 331 */ 332 fprintf(stderr, "Not a git repository\n"); 333 fprintf(stderr, 334 "To compare two paths outside a working tree:\n"); 335 } 336 /* Give the usage message for non-repository usage and exit. */ 337 usagef("git diff %s <path> <path>", 338 no_index == DIFF_NO_INDEX_EXPLICIT ? 339 "--no-index" : "[--no-index]"); 340 341 } 342||||||| 125dcea963 343 if (no_index && argc != i + 2) { 344 if (no_index == DIFF_NO_INDEX_IMPLICIT) { 345 /* 346 * There was no --no-index and there were not two 347 * paths. It is possible that the user intended 348 * to do an inside-repository operation. 349 */ 350 fprintf(stderr, "Not a git repository\n"); 351 fprintf(stderr, 352 "To compare two paths outside a working tree:\n"); 353 } 354 /* Give the usage message for non-repository usage and exit. */ 355 usagef("git diff %s <path> <path>", 356 no_index == DIFF_NO_INDEX_EXPLICIT ? 357 "--no-index" : "[--no-index]"); 358 359 } 360 if (no_index) 361 /* If this is a no-index diff, just run it and exit there. */ 362 diff_no_index(the_repository, &rev, argc, argv); 363 364 /* Otherwise, we are doing the usual "git" diff */ 365 rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index; 366======= 367 if (no_index) 368 /* If this is a no-index diff, just run it and exit there. */ 369 exit(diff_no_index(the_repository, &rev, 370 no_index == DIFF_NO_INDEX_IMPLICIT, 371 argc, argv)); 372 373 /* Otherwise, we are doing the usual "git" diff */ 374 rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index; 375>>>>>>> cdb5330a9b
This one looks like each one of the blocks has taken care of removing one part of
the MB. Let’s go through with the whole analysis process.
dMU
The second conditional from MB (lines 360-362) along with the code that followed
have been removed.
dML
The first conditional from MB (lines 343-359) has been removed. The call to
diff_no_index got a new parameter (from the expression no_index ==
DIFF_NO_INDEX_IMPLICIT ) and it is now enclosed inside one exit call
(lines 360-362). At this point I want you to be very cautious about what will happen
with conflict resolution because this is the call that we had to work for API
conflicts in the previous CB.
Resolution
Always wanting to do a thorough analysis, let’s check when/how the code was
deleted on each branch.
Code deleted toward HEAD
First, let’s try a reverse blame to see when the code was present last
time:
Listing 4.26:Exercise 7 - git blame reverse$ git blame -s --reverse MERGE_HEAD..HEAD builtin/diff.c . . . ^cdb5330a9b 321) repo_init_revisions(the_repository, &rev, prefix); ^cdb5330a9b 322) ^cdb5330a9b 323) if (no_index) ^cdb5330a9b 324) /* If this is a no-index diff, just run it and exit there. */ ^cdb5330a9b 325) exit(diff_no_index(the_repository, &rev, ^cdb5330a9b 326) no_index == DIFF_NO_INDEX_IMPLICIT, ^cdb5330a9b 327) argc, argv)); ^cdb5330a9b 328) ^cdb5330a9b 329) /* Otherwise, we are doing the usual "git" diff */ ^cdb5330a9b 330) rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index; ^cdb5330a9b 331) ^cdb5330a9b 332) /* Scale to real terminal size and respect statGraphWidth config */
Last time those lines were visible toward HEAD was on revision cdb5330a9b
and by checking the history of the file we will be able to find where those lines were
removed. There are ony a handful of revisions to check:
Listing 4.27:Exercise 7 - revisions to check$ git log --oneline cdb5330a9b..HEAD -- builtin/diff.c 12e5bdd9c4 Merge branch ’jk/diff-no-index-initialize’ 287ab28bfa diff: reuse diff setup for --no-index case 3a14fdec88 Merge branch ’sl/const’ 33de80b1d5 various: tighten constness of some local variables f8adbec9fe cache.h: flip NO_THE_INDEX_COMPATIBILITY_MACROS switch 1b0d968b34 read-cache.c: replace update_index_if_able with repo_&
And by checking them one by one we find our culprit in 287ab28bfae14:
Listing 4.28:Exercise 7 - revision where code was moved$ git show --pretty= 287ab28bfae14 -- builtin/diff.c diff --git a/builtin/diff.c b/builtin/diff.c index f0393bba23..777ca87156 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -337,21 +337,23 @@ int cmd_diff(int argc, const char **argv, const char *prefix) "--no-index" : "[--no-index]"); } - if (no_index) - /* If this is a no-index diff, just run it and exit there. */ - diff_no_index(the_repository, &rev, argc, argv); - - /* Otherwise, we are doing the usual "git" diff */ - rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index; - /* Scale to real terminal size and respect statGraphWidth config */ + /* Set up defaults that will apply to both no-index and regular diffs. */ rev.diffopt.stat_width = -1; rev.diffopt.stat_graph_width = -1; - - /* Default to let external and textconv be used */ rev.diffopt.flags.allow_external = 1; rev.diffopt.flags.allow_textconv = 1; + /* If this is a no-index diff, just run it and exit there. */ + if (no_index) + diff_no_index(&rev, argc, argv); + + /* + * Otherwise, we are doing the usual "git" diff; set up any + * further defaults that apply to regular diffs. + */ + rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index; + /* * Default to intent-to-add entries invisible in the * index. This makes them show up as new files in diff-files
We can see that the code was placed a little bit further down the file. And the
lines are there currently in our conflicted file, outside of the CB:
Listing 4.29:Exercise 7 - section of builtin/diff.c383 /* If this is a no-index diff, just run it and exit there. */ 384 if (no_index) 385 diff_no_index(&rev, argc, argv); 386 387 /* 388 * Otherwise, we are doing the usual "git" diff; set up any 389 * further defaults that apply to regular diffs. 390 */ 391 rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
And, just to make sure, we can blame the lines to see if they showed up on this
revision we are talking about:
Listing 4.30:Exercise 7 - revision where code was moved$ git blame -s -L 383,391 -- builtin/diff.c 287ab28bfae 383) /* If this is a no-index diff, just run it and exit there. */ 287ab28bfae 384) if (no_index) 287ab28bfae 385) diff_no_index(&rev, argc, argv); 287ab28bfae 386) 287ab28bfae 387) /* 287ab28bfae 388) * Otherwise, we are doing the usual "git" diff; set up any 287ab28bfae 389) * further defaults that apply to regular diffs. 287ab28bfae 390) */ 287ab28bfae 391) rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
And this is precisely one of the spots where we have to consider the API change.
Remember the function we had to adjust? diff_no_index. Exactly what we have in
line 385. You can see that in the patch where the call is moved, it originally had
the_repository as one argument and in the final position that parameter is gone, so
that part of our original API conflict is done. Coming back to the deleted
code analysis, it seems like we can safely assume that all adjustments have
been taken care of properly. Now let’s check the deleted code on the other
branch.
Code deleted toward the other branch
Listing 4.31:Exercise 7 - git blame reverse$ git blame -s --reverse HEAD..MERGE_HEAD builtin/diff.c . . . ^4284497396 322) repo_init_revisions(the_repository, &rev, prefix); ^4284497396 323) ^4284497396 324) if (no_index && argc != i + 2) { ^4284497396 325) if (no_index == DIFF_NO_INDEX_IMPLICIT) { ^4284497396 326) /* ^4284497396 327) * There was no --no-index and there were not two ^4284497396 328) * paths. It is possible that the user intended ^4284497396 329) * to do an inside-repository operation. ^4284497396 330) */ ^4284497396 331) fprintf(stderr, "Not a git repository\n"); ^4284497396 332) fprintf(stderr, ^4284497396 333) "To compare two paths outside a working tree:\n"); ^4284497396 334) } ^4284497396 335) /* Give the usage message for non-repository usage and exit. */ ^4284497396 336) usagef("git diff %s <path> <path>", ^4284497396 337) no_index == DIFF_NO_INDEX_EXPLICIT ? ^4284497396 338) "--no-index" : "[--no-index]"); ^4284497396 339) ^4284497396 340) }
From 4284497396, let’s see what revisions are following it:
Listing 4.32:Exercise 7 - revisions to check$ git log 4284497396..MERGE_HEAD --oneline -- builtin/diff.c 16bb3d714d diff --no-index: use parse_options() instead of diff_opt_parse()
Not that many revisions, huh? Let’s check:
Listing 4.33:Exercise 7 - revision where code was deleted and call adjusted$ git show 16bb3d714d --pretty= -- builtin/diff.c diff --git a/builtin/diff.c b/builtin/diff.c index f0393bba23..52dc3e136f 100644 --- a/builtin/diff.c +++ b/builtin/diff.c @@ -320,26 +320,11 @@ int cmd_diff(int argc, const char **argv, const char *prefix) repo_init_revisions(the_repository, &rev, prefix); - if (no_index && argc != i + 2) { - if (no_index == DIFF_NO_INDEX_IMPLICIT) { - /* - * There was no --no-index and there were not two - * paths. It is possible that the user intended - * to do an inside-repository operation. - */ - fprintf(stderr, "Not a git repository\n"); - fprintf(stderr, - "To compare two paths outside a working tree:\n"); - } - /* Give the usage message for non-repository usage and exit. */ - usagef("git diff %s <path> <path>", - no_index == DIFF_NO_INDEX_EXPLICIT ? - "--no-index" : "[--no-index]"); - - } if (no_index) /* If this is a no-index diff, just run it and exit there. */ - diff_no_index(the_repository, &rev, argc, argv); + exit(diff_no_index(the_repository, &rev, + no_index == DIFF_NO_INDEX_IMPLICIT, + argc, argv)); /* Otherwise, we are doing the usual "git" diff */ rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
And here we can see how the code was indeed deleted, and we got an adjustment
in the call to diff_no_index. If we are so kind to adjust the call that we saw is
still in line 385, we should be fine. As given that the MB was completely
deleted by what both branches did, that points to this as the ultimate final
resolution:
Listing 4.34:Exercise 7 - CB in builtin/diff.c318 init_diff_ui_defaults(); 319 git_config(git_diff_ui_config, NULL); 320 precompose_argv(argc, argv); 321 322 repo_init_revisions(the_repository, &rev, prefix); 323 324 /* Set up defaults that will apply to both no-index and regular diffs. */ 325 rev.diffopt.stat_width = -1; 326 rev.diffopt.stat_graph_width = -1; 327 rev.diffopt.flags.allow_external = 1; 328 rev.diffopt.flags.allow_textconv = 1; 329 330 /* If this is a no-index diff, just run it and exit there. */ 331 if (no_index) 332 exit(diff_no_index(&rev, no_index == DIFF_NO_INDEX_IMPLICIT, 333 argc, argv)); 334 335 /* 336 * Otherwise, we are doing the usual "git" diff; set up any 337 * further defaults that apply to regular diffs. 338 */ 339 rev.diffopt.skip_stat_unmatch = !!diff_auto_refresh_index;
Notice how the call to diff_no_index is enclosed in a call to exit and
how we inserted the new parameter with the value coming from the other
branch.
CB#1 in diff-no-index.c
There are 2 CBs in diff-no-index.c. The first one is directly related to the signature
of the function that we worked on diff.h. Now we are dealing with the signature in
the implementation.
Listing 4.35:Exercise 7 - CB#1 in diff-no-index.c237<<<<<<< HEAD 238void diff_no_index(struct rev_info *revs, 239 int argc, const char **argv) 240||||||| 125dcea963 241void diff_no_index(struct repository *r, 242 struct rev_info *revs, 243 int argc, const char **argv) 244======= 245static const char * const diff_no_index_usage[] = { 246 N_("git diff --no-index [<options>] <path> <path>"), 247 NULL 248}; 249 250int diff_no_index(struct repository *r, 251 struct rev_info *revs, 252 int implicit_no_index, 253 int argc, const char **argv) 254>>>>>>> cdb5330a9b
dMU
Removed struct repository *r as an argument to diff_no_index.
dML
Added the definition of diff_no_index_usage and also inserted implicit_no_index as
an argument to the function diff_no_index.
Resolution
It’s more than obvious we need to work from LB, right?
Listing 4.36:Exercise 7 - Step 1 - LB#1 in diff-no-index.c244======= 245static const char * const diff_no_index_usage[] = { 246 N_("git diff --no-index [<options>] <path> <path>"), 247 NULL 248}; 249 250int diff_no_index(struct repository *r, 251 struct rev_info *revs, 252 int implicit_no_index, 253 int argc, const char **argv) 254>>>>>>> cdb5330a9b
We remove struct the repository *r as an argument, per dMU.
Listing 4.37:Exercise 7 - Step 2 - LB#1 in diff-no-index.c244======= 245static const char * const diff_no_index_usage[] = { 246 N_("git diff --no-index [<options>] <path> <path>"), 247 NULL 248}; 249 250int diff_no_index(struct rev_info *revs, 251 int implicit_no_index, 252 int argc, const char **argv) 253>>>>>>> cdb5330a9b
Done with CB#1:
Listing 4.38:Exercise 7 - Resolution of CB#1 in diff-no-index.c234 } 235} 236 237static const char * const diff_no_index_usage[] = { 238 N_("git diff --no-index [<options>] <path> <path>"), 239 NULL 240}; 241 242int diff_no_index(struct rev_info *revs, 243 int implicit_no_index, 244 int argc, const char **argv) 245{ 246 int i, no_index; 247 const char *paths[2];
Can you take care of solving the remaning conflicts in the project? Here’s what
has to be done, so you can check once you are finished:
CB#2 in diff-no-index.c
Listing 4.39:Exercise 7 - Resolution of CB#2 in diff-no-index.c255 struct option *options; 256 257 options = parse_options_concat(no_index_options, 258 revs->diffopt.parseopts); 259 argc = parse_options(argc, argv, revs->prefix, options, 260 diff_no_index_usage, 0); 261 if (argc != 2) { 262 if (implicit_no_index) 263 warning(_("Not a git repository. Use --no-index to " 264 "compare two paths outside a working tree")); 265 usage_with_options(diff_no_index_usage, options); 266 } 267 FREE_AND_NULL(options); 268 for (i = 0; i < 2; i++) {
No more need for the call to repo_diff_setup because it is removed in one of the
branches and, just in case, r (the repository) is no longer a part of the parameters of
the function we are working on.
CB in diff.c
Listing 4.40:Exercise 7 - Resolution of CB in diff.c5530 ac = parse_options(ac, av, prefix, options->parseopts, NULL, 5531 PARSE_OPT_KEEP_DASHDASH | 5532 PARSE_OPT_KEEP_UNKNOWN | 5533 PARSE_OPT_NO_INTERNAL_HELP | 5534 PARSE_OPT_ONE_SHOT | 5535 PARSE_OPT_STOP_AT_NON_OPTION); 5536 5537 return ac; 5538}
We keep LB as is because the new definition to handle color-moved-ws per
revision 8ce2020ff07995 will also take care of –no-color-moved-ws.
What’s missing?
Last step would be checkout for any other calls to the conflicted function:
Listing 4.41:Exercise 7 - calls to diff_no_index$ git grep -n "diff_no_index(" builtin/diff.c:332: diff_no_index(&rev, no_index == DIFF_NO_INDEX_IMPLICIT, diff-no-index.c:242:int diff_no_index(struct rev_info *revs, diff.h:443:int diff_no_index(struct rev_info *, int implicit_no_index, int,
And it’s all related to the code that we worked on while working on the conflicts
so we are all good.
Finally, if you compare with dcd6a8c09a, you should get no meaningful
differences.