Re: [PATCH] refs/files: deprecate writing symrefs as symbolic links

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> +* Support for `core.preferSymlinkRefs=true` has been deprecated and will be
> +  removed in Git 3.0. If set, symbolic refs like "HEAD" would be written as
> +  symbolic links instead of as a plain file using the symref format.

The second sentence reads as if we are talking about a newly
introduced feature, but I cannot quite rephrase it to avoid that
impression myself.

> +Note that for now, only the writing side for such symbolic links is deprecated.
> +Reading such symbolic links is still supported for now.

Double "for now".  Let's start the above with "Note that only the ...".

> diff --git a/Documentation/config/core.adoc b/Documentation/config/core.adoc
> index 08739bb9d42..e9272bbc0bd 100644
> --- a/Documentation/config/core.adoc
> +++ b/Documentation/config/core.adoc
> @@ -290,6 +290,9 @@ core.preferSymlinkRefs::
>  	and other symbolic reference files, use symbolic links.
>  	This is sometimes needed to work with old scripts that
>  	expect HEAD to be a symbolic link.
> ++
> +This configuration is deprecated and will be removed in Git 3.0. Writing
> +symbolic links for symrefs will not be supported anymore.

Or spinning it a bit positively:

	Symbolic refs will always be written as textual symrefs.

> -#ifdef NO_SYMLINK_HEAD
> +#if defined(NO_SYMLINK_HEAD) || defined(WITH_BREAKING_CHANGES)
>  #define create_ref_symlink(a, b) (-1)

Perhaps we want to say NOT_CONSTANT(-1) here?

>  #else
>  static int create_ref_symlink(struct ref_lock *lock, const char *target)
>  {
> +	static int warn_once = 1;
> +	char *ref_path;
>  	int ret = -1;
>  
> -	char *ref_path = get_locked_file_path(&lock->lk);
> +	if (warn_once)
> +		warning(_("'core.preferSymlinkRefs=true' is nominated for removal.\n"
> +			  "hint: The use of symbolic links for symbolic refs is deprecated\n"
> +			  "hint: and will be removed in Git 3.0. The configuration that\n"
> +			  "hint: tells Git to use them is thus going away. You can unset\n"
> +			  "hint: it with:\n"
> +			  "hint:\n"
> +			  "hint:\tgit config unset core.preferSymlinkRefs\n"
> +			  "hint:\n"
> +			  "hint: Git will then use the symref format instead."));

I've always called them "textual symref" when I needed to
differenciate them from symbolic links.  I haven't seen "symref
format", though.  Do we have an official terminology?

> +	warn_once = 0;
> +
> +	ref_path = get_locked_file_path(&lock->lk);

Was there a reason why we want to first warn and then attempt to
lock?  We are afraid that we may die before we have a chance to
warn()?