Re: [PATCH v7 9/9] ref: add symlink ref content check for files backend

From: Patrick Steinhardt <ps@pks.im>
Date: Nov 13, 2024, 07:36 UTC
Message-ID: <ZzRW8M39-hw1E9-h@pks.im>
In-Reply-To: <ZzCis8E49O10O1zr@ArchLinux>

On Sun, Nov 10, 2024 at 08:10:27PM +0800, shejialuo wrote:

Show 7 quoted lines

> @@ -3572,8 +3579,30 @@ static int files_fsck_refs_content(struct ref_store *ref_store,
>  
>  	report.path = target_name;
>  
> -	if (S_ISLNK(iter->st.st_mode))
> +	if (S_ISLNK(iter->st.st_mode)) {
> +		const char* relative_referent_path = NULL;

Nit: the asterisk should stick with the variable name.

Show 21 quoted lines

> +		ret = fsck_report_ref(o, &report,
> +				      FSCK_MSG_SYMLINK_REF,
> +				      "use deprecated symbolic link for symref");
> +
> +		strbuf_add_absolute_path(&abs_gitdir, ref_store->gitdir);
> +		strbuf_normalize_path(&abs_gitdir);
> +		if (!is_dir_sep(abs_gitdir.buf[abs_gitdir.len - 1]))
> +			strbuf_addch(&abs_gitdir, '/');
> +
> +		strbuf_add_real_path(&ref_content, iter->path.buf);
> +		skip_prefix(ref_content.buf, abs_gitdir.buf,
> +			    &relative_referent_path);
> +
> +		if (relative_referent_path)
> +			strbuf_addstr(&referent, relative_referent_path);
> +		else
> +			strbuf_addbuf(&referent, &ref_content);
> +
> +		ret |= files_fsck_symref_target(o, &report, &referent, 1);
>  		goto cleanup;
> +	}

I wonder whether this logic works as expected with per-worktree symbolic refs which are a symlink. On the other hand I wonder whether those work as expected in the first place. Probably not. *shrug*

In any case, it would be nice to have a test for this.

Patrick