Merge branch 'gt/add-u-commit-i-pathspec-check'
"git add -u <pathspec>" and "git commit [-i] <pathspec>" did not diagnose a pathspec element that did not match any files in certain situations, unlike "git add <pathspec>" did. * gt/add-u-commit-i-pathspec-check: builtin/add: error out when passing untracked path with -u builtin/commit: error out when passing untracked path with -i revision: optionally record matches with pathspec elementsmaint
						commit
						d75ec4c627
					
				|  | @ -368,6 +368,7 @@ int cmd_add(int argc, const char **argv, const char *prefix) | ||||||
| 	int add_new_files; | 	int add_new_files; | ||||||
| 	int require_pathspec; | 	int require_pathspec; | ||||||
| 	char *seen = NULL; | 	char *seen = NULL; | ||||||
|  | 	char *ps_matched = NULL; | ||||||
| 	struct lock_file lock_file = LOCK_INIT; | 	struct lock_file lock_file = LOCK_INIT; | ||||||
|  |  | ||||||
| 	git_config(add_config, NULL); | 	git_config(add_config, NULL); | ||||||
|  | @ -545,12 +546,17 @@ int cmd_add(int argc, const char **argv, const char *prefix) | ||||||
|  |  | ||||||
| 	begin_odb_transaction(); | 	begin_odb_transaction(); | ||||||
|  |  | ||||||
|  | 	ps_matched = xcalloc(pathspec.nr, 1); | ||||||
| 	if (add_renormalize) | 	if (add_renormalize) | ||||||
| 		exit_status |= renormalize_tracked_files(&pathspec, flags); | 		exit_status |= renormalize_tracked_files(&pathspec, flags); | ||||||
| 	else | 	else | ||||||
| 		exit_status |= add_files_to_cache(the_repository, prefix, | 		exit_status |= add_files_to_cache(the_repository, prefix, | ||||||
| 						  &pathspec, include_sparse, | 						  &pathspec, ps_matched, | ||||||
| 						  flags); | 						  include_sparse, flags); | ||||||
|  |  | ||||||
|  | 	if (take_worktree_changes && !add_renormalize && !ignore_add_errors && | ||||||
|  | 	    report_path_error(ps_matched, &pathspec)) | ||||||
|  | 		exit(128); | ||||||
|  |  | ||||||
| 	if (add_new_files) | 	if (add_new_files) | ||||||
| 		exit_status |= add_files(&dir, flags); | 		exit_status |= add_files(&dir, flags); | ||||||
|  | @ -564,6 +570,7 @@ finish: | ||||||
| 			       COMMIT_LOCK | SKIP_IF_UNCHANGED)) | 			       COMMIT_LOCK | SKIP_IF_UNCHANGED)) | ||||||
| 		die(_("unable to write new index file")); | 		die(_("unable to write new index file")); | ||||||
|  |  | ||||||
|  | 	free(ps_matched); | ||||||
| 	dir_clear(&dir); | 	dir_clear(&dir); | ||||||
| 	clear_pathspec(&pathspec); | 	clear_pathspec(&pathspec); | ||||||
| 	return exit_status; | 	return exit_status; | ||||||
|  |  | ||||||
|  | @ -882,7 +882,8 @@ static int merge_working_tree(const struct checkout_opts *opts, | ||||||
| 			 * entries in the index. | 			 * entries in the index. | ||||||
| 			 */ | 			 */ | ||||||
|  |  | ||||||
| 			add_files_to_cache(the_repository, NULL, NULL, 0, 0); | 			add_files_to_cache(the_repository, NULL, NULL, NULL, 0, | ||||||
|  | 					   0); | ||||||
| 			init_merge_options(&o, the_repository); | 			init_merge_options(&o, the_repository); | ||||||
| 			o.verbosity = 0; | 			o.verbosity = 0; | ||||||
| 			work = write_in_core_index_as_tree(the_repository); | 			work = write_in_core_index_as_tree(the_repository); | ||||||
|  |  | ||||||
|  | @ -441,16 +441,21 @@ static const char *prepare_index(const char **argv, const char *prefix, | ||||||
| 	 * (B) on failure, rollback the real index. | 	 * (B) on failure, rollback the real index. | ||||||
| 	 */ | 	 */ | ||||||
| 	if (all || (also && pathspec.nr)) { | 	if (all || (also && pathspec.nr)) { | ||||||
|  | 		char *ps_matched = xcalloc(pathspec.nr, 1); | ||||||
| 		repo_hold_locked_index(the_repository, &index_lock, | 		repo_hold_locked_index(the_repository, &index_lock, | ||||||
| 				       LOCK_DIE_ON_ERROR); | 				       LOCK_DIE_ON_ERROR); | ||||||
| 		add_files_to_cache(the_repository, also ? prefix : NULL, | 		add_files_to_cache(the_repository, also ? prefix : NULL, | ||||||
| 				   &pathspec, 0, 0); | 				   &pathspec, ps_matched, 0, 0); | ||||||
|  | 		if (!all && report_path_error(ps_matched, &pathspec)) | ||||||
|  | 			exit(128); | ||||||
|  |  | ||||||
| 		refresh_cache_or_die(refresh_flags); | 		refresh_cache_or_die(refresh_flags); | ||||||
| 		cache_tree_update(&the_index, WRITE_TREE_SILENT); | 		cache_tree_update(&the_index, WRITE_TREE_SILENT); | ||||||
| 		if (write_locked_index(&the_index, &index_lock, 0)) | 		if (write_locked_index(&the_index, &index_lock, 0)) | ||||||
| 			die(_("unable to write new index file")); | 			die(_("unable to write new index file")); | ||||||
| 		commit_style = COMMIT_NORMAL; | 		commit_style = COMMIT_NORMAL; | ||||||
| 		ret = get_lock_file_path(&index_lock); | 		ret = get_lock_file_path(&index_lock); | ||||||
|  | 		free(ps_matched); | ||||||
| 		goto out; | 		goto out; | ||||||
| 	} | 	} | ||||||
|  |  | ||||||
|  |  | ||||||
							
								
								
									
										11
									
								
								diff-lib.c
								
								
								
								
							
							
						
						
									
										11
									
								
								diff-lib.c
								
								
								
								
							|  | @ -127,7 +127,16 @@ void run_diff_files(struct rev_info *revs, unsigned int option) | ||||||
| 		if (diff_can_quit_early(&revs->diffopt)) | 		if (diff_can_quit_early(&revs->diffopt)) | ||||||
| 			break; | 			break; | ||||||
|  |  | ||||||
| 		if (!ce_path_match(istate, ce, &revs->prune_data, NULL)) | 		/* | ||||||
|  | 		 * NEEDSWORK: | ||||||
|  | 		 * Here we filter with pathspec but the result is further | ||||||
|  | 		 * filtered out when --relative is in effect.  To end-users, | ||||||
|  | 		 * a pathspec element that matched only to paths outside the | ||||||
|  | 		 * current directory is like not matching anything at all; | ||||||
|  | 		 * the handling of ps_matched[] here may become problematic | ||||||
|  | 		 * if/when we add the "--error-unmatch" option to "git diff". | ||||||
|  | 		 */ | ||||||
|  | 		if (!ce_path_match(istate, ce, &revs->prune_data, revs->ps_matched)) | ||||||
| 			continue; | 			continue; | ||||||
|  |  | ||||||
| 		if (revs->diffopt.prefix && | 		if (revs->diffopt.prefix && | ||||||
|  |  | ||||||
|  | @ -480,8 +480,8 @@ extern int verify_ce_order; | ||||||
| int cmp_cache_name_compare(const void *a_, const void *b_); | int cmp_cache_name_compare(const void *a_, const void *b_); | ||||||
|  |  | ||||||
| int add_files_to_cache(struct repository *repo, const char *prefix, | int add_files_to_cache(struct repository *repo, const char *prefix, | ||||||
| 		       const struct pathspec *pathspec, int include_sparse, | 		       const struct pathspec *pathspec, char *ps_matched, | ||||||
| 		       int flags); | 		       int include_sparse, int flags); | ||||||
|  |  | ||||||
| void overlay_tree_on_index(struct index_state *istate, | void overlay_tree_on_index(struct index_state *istate, | ||||||
| 			   const char *tree_name, const char *prefix); | 			   const char *tree_name, const char *prefix); | ||||||
|  |  | ||||||
|  | @ -3958,8 +3958,8 @@ static void update_callback(struct diff_queue_struct *q, | ||||||
| } | } | ||||||
|  |  | ||||||
| int add_files_to_cache(struct repository *repo, const char *prefix, | int add_files_to_cache(struct repository *repo, const char *prefix, | ||||||
| 		       const struct pathspec *pathspec, int include_sparse, | 		       const struct pathspec *pathspec, char *ps_matched, | ||||||
| 		       int flags) | 		       int include_sparse, int flags) | ||||||
| { | { | ||||||
| 	struct update_callback_data data; | 	struct update_callback_data data; | ||||||
| 	struct rev_info rev; | 	struct rev_info rev; | ||||||
|  | @ -3971,8 +3971,10 @@ int add_files_to_cache(struct repository *repo, const char *prefix, | ||||||
|  |  | ||||||
| 	repo_init_revisions(repo, &rev, prefix); | 	repo_init_revisions(repo, &rev, prefix); | ||||||
| 	setup_revisions(0, NULL, &rev, NULL); | 	setup_revisions(0, NULL, &rev, NULL); | ||||||
| 	if (pathspec) | 	if (pathspec) { | ||||||
| 		copy_pathspec(&rev.prune_data, pathspec); | 		copy_pathspec(&rev.prune_data, pathspec); | ||||||
|  | 		rev.ps_matched = ps_matched; | ||||||
|  | 	} | ||||||
| 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK; | 	rev.diffopt.output_format = DIFF_FORMAT_CALLBACK; | ||||||
| 	rev.diffopt.format_callback = update_callback; | 	rev.diffopt.format_callback = update_callback; | ||||||
| 	rev.diffopt.format_callback_data = &data; | 	rev.diffopt.format_callback_data = &data; | ||||||
|  |  | ||||||
|  | @ -142,6 +142,7 @@ struct rev_info { | ||||||
| 	/* Basic information */ | 	/* Basic information */ | ||||||
| 	const char *prefix; | 	const char *prefix; | ||||||
| 	const char *def; | 	const char *def; | ||||||
|  | 	char *ps_matched; /* optionally record matches of prune_data */ | ||||||
| 	struct pathspec prune_data; | 	struct pathspec prune_data; | ||||||
|  |  | ||||||
| 	/* | 	/* | ||||||
|  |  | ||||||
|  | @ -65,6 +65,16 @@ test_expect_success 'update did not touch untracked files' ' | ||||||
| 	test_must_be_empty out | 	test_must_be_empty out | ||||||
| ' | ' | ||||||
|  |  | ||||||
|  | test_expect_success 'error out when passing untracked path' ' | ||||||
|  | 	git reset --hard && | ||||||
|  | 	echo content >>baz && | ||||||
|  | 	echo content >>top && | ||||||
|  | 	test_must_fail git add -u baz top 2>err && | ||||||
|  | 	test_grep -e "error: pathspec .baz. did not match any file(s) known to git" err && | ||||||
|  | 	git diff --cached --name-only >actual && | ||||||
|  | 	test_must_be_empty actual | ||||||
|  | ' | ||||||
|  |  | ||||||
| test_expect_success 'cache tree has not been corrupted' ' | test_expect_success 'cache tree has not been corrupted' ' | ||||||
|  |  | ||||||
| 	git ls-files -s | | 	git ls-files -s | | ||||||
|  |  | ||||||
|  | @ -101,22 +101,8 @@ test_expect_success 'fail to commit untracked file (even with --include/--only)' | ||||||
| 	test_must_fail git commit --only -m "baz" baz 2>err && | 	test_must_fail git commit --only -m "baz" baz 2>err && | ||||||
| 	test_grep -e "$error" err && | 	test_grep -e "$error" err && | ||||||
|  |  | ||||||
| 	# TODO: as for --include, the below command will fail because |  | ||||||
| 	# nothing is staged. If something was staged, it would not fail |  | ||||||
| 	# even though the provided pathspec does not match any tracked |  | ||||||
| 	# path. (However, the untracked paths that match the pathspec are |  | ||||||
| 	# not committed and only the staged changes get committed.) |  | ||||||
| 	# In either cases, no error is returned to stderr like in (--only |  | ||||||
| 	# and without --only/--include) cases. In a similar manner, |  | ||||||
| 	# "git add -u baz" also does not error out. |  | ||||||
| 	# |  | ||||||
| 	# Therefore, the below test is just to document the current behavior |  | ||||||
| 	# and is not an endorsement to the current behavior, and we may |  | ||||||
| 	# want to fix this. And when that happens, this test should be |  | ||||||
| 	# updated accordingly. |  | ||||||
|  |  | ||||||
| 	test_must_fail git commit --include -m "baz" baz 2>err && | 	test_must_fail git commit --include -m "baz" baz 2>err && | ||||||
| 	test_must_be_empty err | 	test_grep -e "$error" err | ||||||
| ' | ' | ||||||
|  |  | ||||||
| test_expect_success 'setup: non-initial commit' ' | test_expect_success 'setup: non-initial commit' ' | ||||||
|  |  | ||||||
		Loading…
	
		Reference in New Issue
	
	 Junio C Hamano
						Junio C Hamano