kernel
/
git
mirror of https://git.kernel.org/pub/scm/git/git.git
1
0
Fork 0
Code Releases Activity

Merge branch 'jk/diff-result-code-cleanup' 

"git diff --no-such-option" and other corner cases around the exit
status of the "diff" command has been corrected.

* jk/diff-result-code-cleanup:
  diff: drop useless "status" parameter from diff_result_code()
  diff: drop useless return values in git-diff helpers
  diff: drop useless return from run_diff_{files,index} functions
  diff: die when failing to read index in git-diff builtin
  diff: show usage for unknown builtin_diff_files() options
  diff-files: avoid negative exit value
  diff: spell DIFF_INDEX_CACHED out when calling run_diff_index()
maint
Junio C Hamano 2023-09-01 11:26:28 -07:00
parent 3525f1dbc1 5cc6b2d70b
commit f137bd4358
17 changed files with 81 additions and 95 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
add-interactive.c
View File

@ -569,7 +569,7 @@ static int get_modified_files(struct repository *r,
copy_pathspec(&rev.prune_data, ps);

if (s.mode == FROM_INDEX)
run_diff_index(&rev, 1);
run_diff_index(&rev, DIFF_INDEX_CACHED);
else {
rev.diffopt.flags.ignore_dirty_submodules = 1;
run_diff_files(&rev, 0);

3
builtin/add.c
View File

@ -194,8 +194,7 @@ static int edit_patch(int argc, const char **argv, const char *prefix)
out = xopen(file, O_CREAT | O_WRONLY | O_TRUNC, 0666);
rev.diffopt.file = xfdopen(out, "w");
rev.diffopt.close_file = 1;
if (run_diff_files(&rev, 0))
die(_("Could not write patch"));
run_diff_files(&rev, 0);

if (launch_editor(file, NULL, NULL))
die(_("editing patch failed"));

4
builtin/am.c
View File

@ -1430,7 +1430,7 @@ static void write_index_patch(const struct am_state *state)
rev_info.diffopt.close_file = 1;
add_pending_object(&rev_info, &tree->object, "");
diff_setup_done(&rev_info.diffopt);
run_diff_index(&rev_info, 1);
run_diff_index(&rev_info, DIFF_INDEX_CACHED);
release_revisions(&rev_info);
}

@ -1593,7 +1593,7 @@ static int fall_back_threeway(const struct am_state *state, const char *index_pa
rev_info.diffopt.filter |= diff_filter_bit('M');
add_pending_oid(&rev_info, "HEAD", &our_tree, 0);
diff_setup_done(&rev_info.diffopt);
run_diff_index(&rev_info, 1);
run_diff_index(&rev_info, DIFF_INDEX_CACHED);
release_revisions(&rev_info);
}


6
builtin/describe.c
View File

@ -668,7 +668,7 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
struct lock_file index_lock = LOCK_INIT;
struct rev_info revs;
struct strvec args = STRVEC_INIT;
int fd, result;
int fd;

setup_work_tree();
prepare_repo_settings(the_repository);
@ -685,9 +685,9 @@ int cmd_describe(int argc, const char **argv, const char *prefix)
strvec_pushv(&args, diff_index_args);
if (setup_revisions(args.nr, args.v, &revs, NULL) != 1)
BUG("malformed internal diff-index command line");
result = run_diff_index(&revs, 0);
run_diff_index(&revs, 0);

if (!diff_result_code(&revs.diffopt, result))
if (!diff_result_code(&revs.diffopt))
suffix = NULL;
else
suffix = dirty;

12
builtin/diff-files.c
View File

@ -80,14 +80,10 @@ int cmd_diff_files(int argc, const char **argv, const char *prefix)
(rev.diffopt.output_format & DIFF_FORMAT_PATCH))
diff_merges_set_dense_combined_if_unset(&rev);

if (repo_read_index_preload(the_repository, &rev.diffopt.pathspec, 0) < 0) {
perror("repo_read_index_preload");
result = -1;
goto cleanup;
}
result = run_diff_files(&rev, options);
result = diff_result_code(&rev.diffopt, result);
cleanup:
if (repo_read_index_preload(the_repository, &rev.diffopt.pathspec, 0) < 0)
die_errno("repo_read_index_preload");
run_diff_files(&rev, options);
result = diff_result_code(&rev.diffopt);
release_revisions(&rev);
return result;
}

4
builtin/diff-index.c
View File

@ -72,8 +72,8 @@ int cmd_diff_index(int argc, const char **argv, const char *prefix)
perror("repo_read_index");
return -1;
}
result = run_diff_index(&rev, option);
result = diff_result_code(&rev.diffopt, result);
run_diff_index(&rev, option);
result = diff_result_code(&rev.diffopt);
release_revisions(&rev);
return result;
}

2
builtin/diff-tree.c
View File

@ -232,5 +232,5 @@ int cmd_diff_tree(int argc, const char **argv, const char *prefix)
diff_free(&opt->diffopt);
}

return diff_result_code(&opt->diffopt, 0);
return diff_result_code(&opt->diffopt);
}

79
builtin/diff.c
View File

@ -77,9 +77,9 @@ static void stuff_change(struct diff_options *opt,
diff_queue(&diff_queued_diff, one, two);
}

static int builtin_diff_b_f(struct rev_info *revs,
int argc, const char **argv UNUSED,
struct object_array_entry **blob)
static void builtin_diff_b_f(struct rev_info *revs,
int argc, const char **argv UNUSED,
struct object_array_entry **blob)
{
/* Blob vs file in the working tree*/
struct stat st;
@ -109,12 +109,11 @@ static int builtin_diff_b_f(struct rev_info *revs,
path);
diffcore_std(&revs->diffopt);
diff_flush(&revs->diffopt);
return 0;
}

static int builtin_diff_blobs(struct rev_info *revs,
int argc, const char **argv UNUSED,
struct object_array_entry **blob)
static void builtin_diff_blobs(struct rev_info *revs,
int argc, const char **argv UNUSED,
struct object_array_entry **blob)
{
const unsigned mode = canon_mode(S_IFREG | 0644);

@ -134,11 +133,10 @@ static int builtin_diff_blobs(struct rev_info *revs,
blob_path(blob[0]), blob_path(blob[1]));
diffcore_std(&revs->diffopt);
diff_flush(&revs->diffopt);
return 0;
}

static int builtin_diff_index(struct rev_info *revs,
int argc, const char **argv)
static void builtin_diff_index(struct rev_info *revs,
int argc, const char **argv)
{
unsigned int option = 0;
while (1 < argc) {
@ -163,20 +161,18 @@ static int builtin_diff_index(struct rev_info *revs,
setup_work_tree();
if (repo_read_index_preload(the_repository,
&revs->diffopt.pathspec, 0) < 0) {
perror("repo_read_index_preload");
return -1;
die_errno("repo_read_index_preload");
}
} else if (repo_read_index(the_repository) < 0) {
perror("repo_read_cache");
return -1;
die_errno("repo_read_cache");
}
return run_diff_index(revs, option);
run_diff_index(revs, option);
}

static int builtin_diff_tree(struct rev_info *revs,
int argc, const char **argv,
struct object_array_entry *ent0,
struct object_array_entry *ent1)
static void builtin_diff_tree(struct rev_info *revs,
int argc, const char **argv,
struct object_array_entry *ent0,
struct object_array_entry *ent1)
{
const struct object_id *(oid[2]);
struct object_id mb_oid;
@ -209,13 +205,12 @@ static int builtin_diff_tree(struct rev_info *revs,
}
diff_tree_oid(oid[0], oid[1], "", &revs->diffopt);
log_tree_diff_flush(revs);
return 0;
}

static int builtin_diff_combined(struct rev_info *revs,
int argc, const char **argv UNUSED,
struct object_array_entry *ent,
int ents, int first_non_parent)
static void builtin_diff_combined(struct rev_info *revs,
int argc, const char **argv UNUSED,
struct object_array_entry *ent,
int ents, int first_non_parent)
{
struct oid_array parents = OID_ARRAY_INIT;
int i;
@ -236,7 +231,6 @@ static int builtin_diff_combined(struct rev_info *revs,
}
diff_tree_combined(&ent[first_non_parent].item->oid, &parents, revs);
oid_array_clear(&parents);
return 0;
}

static void refresh_index_quietly(void)
@ -254,7 +248,7 @@ static void refresh_index_quietly(void)
repo_update_index_if_able(the_repository, &lock_file);
}

static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv)
static void builtin_diff_files(struct rev_info *revs, int argc, const char **argv)
{
unsigned int options = 0;

@ -269,8 +263,10 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv
options |= DIFF_SILENT_ON_REMOVED;
else if (!strcmp(argv[1], "-h"))
usage(builtin_diff_usage);
else
return error(_("invalid option: %s"), argv[1]);
else {
error(_("invalid option: %s"), argv[1]);
usage(builtin_diff_usage);
}
argv++; argc--;
}

@ -287,10 +283,9 @@ static int builtin_diff_files(struct rev_info *revs, int argc, const char **argv
setup_work_tree();
if (repo_read_index_preload(the_repository, &revs->diffopt.pathspec,
0) < 0) {
perror("repo_read_index_preload");
return -1;
die_errno("repo_read_index_preload");
}
return run_diff_files(revs, options);
run_diff_files(revs, options);
}

struct symdiff {
@ -404,7 +399,7 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
int blobs = 0, paths = 0;
struct object_array_entry *blob[2];
int nongit = 0, no_index = 0;
int result = 0;
int result;
struct symdiff sdiff;

/*
@ -583,17 +578,17 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
if (!ent.nr) {
switch (blobs) {
case 0:
result = builtin_diff_files(&rev, argc, argv);
builtin_diff_files(&rev, argc, argv);
break;
case 1:
if (paths != 1)
usage(builtin_diff_usage);
result = builtin_diff_b_f(&rev, argc, argv, blob);
builtin_diff_b_f(&rev, argc, argv, blob);
break;
case 2:
if (paths)
usage(builtin_diff_usage);
result = builtin_diff_blobs(&rev, argc, argv, blob);
builtin_diff_blobs(&rev, argc, argv, blob);
break;
default:
usage(builtin_diff_usage);
@ -602,18 +597,18 @@ int cmd_diff(int argc, const char **argv, const char *prefix)
else if (blobs)
usage(builtin_diff_usage);
else if (ent.nr == 1)
result = builtin_diff_index(&rev, argc, argv);
builtin_diff_index(&rev, argc, argv);
else if (ent.nr == 2) {
if (sdiff.warn)
warning(_("%s...%s: multiple merge bases, using %s"),
sdiff.left, sdiff.right, sdiff.base);
result = builtin_diff_tree(&rev, argc, argv,
&ent.objects[0], &ent.objects[1]);
builtin_diff_tree(&rev, argc, argv,
&ent.objects[0], &ent.objects[1]);
} else
result = builtin_diff_combined(&rev, argc, argv,
ent.objects, ent.nr,
first_non_parent);
result = diff_result_code(&rev.diffopt, result);
builtin_diff_combined(&rev, argc, argv,
ent.objects, ent.nr,
first_non_parent);
result = diff_result_code(&rev.diffopt);
if (1 < rev.diffopt.skip_stat_unmatch)
refresh_index_quietly();
release_revisions(&rev);

2
builtin/log.c
View File

@ -549,7 +549,7 @@ static int cmd_log_walk_no_free(struct rev_info *rev)
rev->diffopt.flags.check_failed) {
return 02;
}
return diff_result_code(&rev->diffopt, 0);
return diff_result_code(&rev->diffopt);
}

static int cmd_log_walk(struct rev_info *rev)

16
builtin/stash.c
View File

@ -973,7 +973,7 @@ static int show_stash(int argc, const char **argv, const char *prefix)
}
log_tree_diff_flush(&rev);

ret = diff_result_code(&rev.diffopt, 0);
ret = diff_result_code(&rev.diffopt);
cleanup:
strvec_clear(&stash_args);
free_stash_info(&info);
@ -1089,7 +1089,6 @@ static int get_untracked_files(const struct pathspec *ps, int include_untracked,
*/
static int check_changes_tracked_files(const struct pathspec *ps)
{
int result;
struct rev_info rev;
struct object_id dummy;
int ret = 0;
@ -1111,14 +1110,14 @@ static int check_changes_tracked_files(const struct pathspec *ps)
add_head_to_pending(&rev);
diff_setup_done(&rev.diffopt);

result = run_diff_index(&rev, 1);
if (diff_result_code(&rev.diffopt, result)) {
run_diff_index(&rev, DIFF_INDEX_CACHED);
if (diff_result_code(&rev.diffopt)) {
ret = 1;
goto done;
}

result = run_diff_files(&rev, 0);
if (diff_result_code(&rev.diffopt, result)) {
run_diff_files(&rev, 0);
if (diff_result_code(&rev.diffopt)) {
ret = 1;
goto done;
}
@ -1309,10 +1308,7 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps

add_pending_object(&rev, parse_object(the_repository, &info->b_commit),
"");
if (run_diff_index(&rev, 0)) {
ret = -1;
goto done;
}
run_diff_index(&rev, 0);

cp_upd_index.git_cmd = 1;
strvec_pushl(&cp_upd_index.args, "update-index",

7
builtin/submodule--helper.c
View File

@ -629,7 +629,6 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,
char *displaypath;
struct strvec diff_files_args = STRVEC_INIT;
struct rev_info rev = REV_INFO_INIT;
int diff_files_result;
struct strbuf buf = STRBUF_INIT;
const char *git_dir;
struct setup_revision_opt opt = {
@ -669,9 +668,9 @@ static void status_submodule(const char *path, const struct object_id *ce_oid,
repo_init_revisions(the_repository, &rev, NULL);
rev.abbrev = 0;
setup_revisions(diff_files_args.nr, diff_files_args.v, &rev, &opt);
diff_files_result = run_diff_files(&rev, 0);
run_diff_files(&rev, 0);

if (!diff_result_code(&rev.diffopt, diff_files_result)) {
if (!diff_result_code(&rev.diffopt)) {
print_status(flags, ' ', path, ce_oid,
displaypath);
} else if (!(flags & OPT_CACHED)) {
@ -1141,7 +1140,7 @@ static int compute_summary_module_list(struct object_id *head_oid,
}

if (diff_cmd == DIFF_INDEX)
run_diff_index(&rev, info->cached);
run_diff_index(&rev, info->cached ? DIFF_INDEX_CACHED : 0);
else
run_diff_files(&rev, 0);
prepare_submodule_summary(info, &list);

8
diff-lib.c
View File

@ -96,7 +96,7 @@ static int match_stat_with_submodule(struct diff_options *diffopt,
return changed;
}

int run_diff_files(struct rev_info *revs, unsigned int option)
void run_diff_files(struct rev_info *revs, unsigned int option)
{
int entries, i;
int diff_unmerged_stage = revs->max_count;
@ -272,7 +272,6 @@ int run_diff_files(struct rev_info *revs, unsigned int option)
diffcore_std(&revs->diffopt);
diff_flush(&revs->diffopt);
trace_performance_since(start, "diff-files");
return 0;
}

/*
@ -606,7 +605,7 @@ void diff_get_merge_base(const struct rev_info *revs, struct object_id *mb)
free_commit_list(merge_bases);
}

int run_diff_index(struct rev_info *revs, unsigned int option)
void run_diff_index(struct rev_info *revs, unsigned int option)
{
struct object_array_entry *ent;
int cached = !!(option & DIFF_INDEX_CACHED);
@ -640,7 +639,6 @@ int run_diff_index(struct rev_info *revs, unsigned int option)
diffcore_std(&revs->diffopt);
diff_flush(&revs->diffopt);
trace_performance_leave("diff-index");
return 0;
}

int do_diff_cache(const struct object_id *tree_oid, struct diff_options *opt)
@ -682,7 +680,7 @@ int index_differs_from(struct repository *r,
rev.diffopt.flags.ignore_submodules = flags->ignore_submodules;
}
rev.diffopt.ita_invisible_in_index = ita_invisible_in_index;
run_diff_index(&rev, 1);
run_diff_index(&rev, DIFF_INDEX_CACHED);
has_changes = rev.diffopt.flags.has_changes;
release_revisions(&rev);
return (has_changes != 0);

2
diff-no-index.c
View File

@ -364,7 +364,7 @@ int diff_no_index(struct rev_info *revs,
* The return code for --no-index imitates diff(1):
* 0 = no changes, 1 = changes, else error
*/
ret = diff_result_code(&revs->diffopt, 0);
ret = diff_result_code(&revs->diffopt);

out:
for (i = 0; i < ARRAY_SIZE(to_free); i++)

6
diff.c
View File

@ -6982,16 +6982,14 @@ void diffcore_std(struct diff_options *options)
options->found_follow = 0;
}

int diff_result_code(struct diff_options *opt, int status)
int diff_result_code(struct diff_options *opt)
{
int result = 0;

diff_warn_rename_limit("diff.renameLimit",
opt->needed_rename_limit,
opt->degraded_cc_to_c);
if (!opt->flags.exit_with_status &&
!(opt->output_format & DIFF_FORMAT_CHECKDIFF))
return status;

if (opt->flags.exit_with_status &&
opt->flags.has_changes)
result |= 01;

6
diff.h
View File

@ -637,17 +637,17 @@ void diff_get_merge_base(const struct rev_info *revs, struct object_id *mb);
#define DIFF_SILENT_ON_REMOVED 01
/* report racily-clean paths as modified */
#define DIFF_RACY_IS_MODIFIED 02
int run_diff_files(struct rev_info *revs, unsigned int option);
void run_diff_files(struct rev_info *revs, unsigned int option);

#define DIFF_INDEX_CACHED 01
#define DIFF_INDEX_MERGE_BASE 02
int run_diff_index(struct rev_info *revs, unsigned int option);
void run_diff_index(struct rev_info *revs, unsigned int option);

int do_diff_cache(const struct object_id *, struct diff_options *);
int diff_flush_patch_id(struct diff_options *, struct object_id *, int);
void flush_one_hunk(struct object_id *result, git_hash_ctx *ctx);

int diff_result_code(struct diff_options *, int);
int diff_result_code(struct diff_options *);

int diff_no_index(struct rev_info *,
int implicit_no_index, int, const char **);

5
t/t4017-diff-retval.sh
View File

@ -138,4 +138,9 @@ test_expect_success 'check honors conflict marker length' '
git reset --hard
'

test_expect_success 'option errors are not confused by --exit-code' '
test_must_fail git diff --exit-code --nonsense 2>err &&
grep '^usage:' err
'

test_done

12
wt-status.c
View File

@ -675,7 +675,7 @@ static void wt_status_collect_changes_index(struct wt_status *s)
rev.diffopt.flags.recursive = 1;

copy_pathspec(&rev.prune_data, &s->pathspec);
run_diff_index(&rev, 1);
run_diff_index(&rev, DIFF_INDEX_CACHED);
release_revisions(&rev);
}

@ -1156,7 +1156,7 @@ static void wt_longstatus_print_verbose(struct wt_status *s)
rev.diffopt.a_prefix = "c/";
rev.diffopt.b_prefix = "i/";
} /* else use prefix as per user config */
run_diff_index(&rev, 1);
run_diff_index(&rev, DIFF_INDEX_CACHED);
if (s->verbose > 1 &&
wt_status_check_worktree_changes(s, &dirty_submodules)) {
status_printf_ln(s, c,
@ -2580,8 +2580,8 @@ int has_unstaged_changes(struct repository *r, int ignore_submodules)
}
rev_info.diffopt.flags.quick = 1;
diff_setup_done(&rev_info.diffopt);
result = run_diff_files(&rev_info, 0);
result = diff_result_code(&rev_info.diffopt, result);
run_diff_files(&rev_info, 0);
result = diff_result_code(&rev_info.diffopt);
release_revisions(&rev_info);
return result;
}
@ -2614,8 +2614,8 @@ int has_uncommitted_changes(struct repository *r,
}

diff_setup_done(&rev_info.diffopt);
result = run_diff_index(&rev_info, 1);
result = diff_result_code(&rev_info.diffopt, result);
run_diff_index(&rev_info, DIFF_INDEX_CACHED);
result = diff_result_code(&rev_info.diffopt);
release_revisions(&rev_info);
return result;
}