Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Clarify API for dir.[ch] and unpack-trees.[ch] -- mark relevant fields as internal #1149

Closed
wants to merge 13 commits into from
Closed

Clarify API for dir.[ch] and unpack-trees.[ch] -- mark relevant fields as internal #1149

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
7bbc457
t2021: fix platform-specific leftover cruft
newren Feb 26, 2023
8ffdb6c
unpack-trees: heed requests to overwrite ignored files
newren Feb 25, 2023
879a93a
dir: separate public from internal portion of dir_struct
newren Oct 6, 2021
4ce9fae
dir: add a usage note to exclude_per_dir
newren Jan 17, 2022
1234440
dir: mark output only fields of dir_struct as such
newren Sep 2, 2022
4e86e39
unpack-trees: clean up some flow control
newren Feb 13, 2022
cd3d489
sparse-checkout: avoid using internal API of unpack-trees
newren Feb 13, 2022
09140cb
sparse-checkout: avoid using internal API of unpack-trees, take 2
newren Feb 23, 2023
27f2d47
unpack_trees: start splitting internal fields from public API
newren Jan 17, 2022
4236c0d
unpack-trees: mark fields only used internally as internal
newren Jan 17, 2022
76f4a54
unpack-trees: rewrap a few overlong lines from previous patch
newren Jan 17, 2022
ee36935
unpack-trees: special case read-tree debugging as internal usage
newren Jan 17, 2022
6575c00
unpack-trees: add usage notices around df_conflict_entry
newren Jan 17, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 15 additions & 1 deletion t/t2021-checkout-overwrite.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,10 +50,13 @@ test_expect_success 'checkout commit with dir must not remove untracked a/b' '

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Derrick Stolee wrote (reply to this):

On 2/27/2023 10:28 AM, Elijah Newren via GitGitGadget wrote:
> From: Elijah Newren <newren@gmail.com>

>  test_expect_success SYMLINKS 'the symlink remained' '
>  
> -	test_when_finished "rm a/b" &&
>  	test -h a/b
>  '
>  
> +test_expect_success 'cleanup after previous symlink tests' '
> +	rm a/b
> +'

I was confused why this worked without "rm -f a/b" and it seems
the path exists in all cases, it's just a symlink on the filesystem
in the case of the SYMLINKS prerequisite.

Thanks,
-Stolee

test_expect_success SYMLINKS 'the symlink remained' '

test_when_finished "rm a/b" &&
test -h a/b
'

test_expect_success 'cleanup after previous symlink tests' '
rm a/b
'

test_expect_success SYMLINKS 'checkout -f must not follow symlinks when removing entries' '
git checkout -f start &&
mkdir dir &&
Expand All @@ -66,4 +69,15 @@ test_expect_success SYMLINKS 'checkout -f must not follow symlinks when removing
test_path_is_file untracked/f
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the Git mailing list, Jonathan Tan wrote (reply to this):

"Elijah Newren via GitGitGadget" <gitgitgadget@gmail.com> writes:
> diff --git a/unpack-trees.c b/unpack-trees.c
> index 3d05e45a279..4518d33ed99 100644
> --- a/unpack-trees.c
> +++ b/unpack-trees.c
> @@ -2337,7 +2337,7 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,
>  
>  	memset(&d, 0, sizeof(d));
>  	if (o->dir)
> -		d.exclude_per_dir = o->dir->exclude_per_dir;
> +		setup_standard_excludes(&d);
>  	i = read_directory(&d, o->src_index, pathbuf, namelen+1, NULL);
>  	dir_clear(&d);
>  	free(pathbuf);

Thanks to the later patches in this patch set, I only needed to look at
unpack-trees.c to see how o->dir (later, o->internal.dir) is set. The
only place it is set is in unpack_trees(), in which a flag is set and
setup_standard_excludes() is called. So the RHS of the diff here does
effectively the same thing as the LHS. (As for the flag, it is not set
in the RHS, but it was not set in the LHS in the first place, so that's
fine.)

Thanks - all 13 patches in this patch set look good.

'

test_expect_success 'checkout --overwrite-ignore should succeed if only ignored files in the way' '
git checkout -b df_conflict &&
test_commit contents some_dir &&
git checkout start &&
mkdir some_dir &&
echo autogenerated information >some_dir/ignore &&
echo ignore >.git/info/exclude &&
git checkout --overwrite-ignore df_conflict &&
! test_path_is_dir some_dir
'

test_done
2 changes: 1 addition & 1 deletion unpack-trees.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2337,7 +2337,7 @@ static int verify_clean_subdirectory(const struct cache_entry *ce,

memset(&d, 0, sizeof(d));
if (o->dir)
d.exclude_per_dir = o->dir->exclude_per_dir;
setup_standard_excludes(&d);
i = read_directory(&d, o->src_index, pathbuf, namelen+1, NULL);
dir_clear(&d);
free(pathbuf);
Expand Down