git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [RFC/PATCH 4/3] create_symref: drop support for writing symbolic links

From
Jeff King <peff@peff.net>
Date
Dec 30, 2015, 06:53 UTC
Message-ID
<20151230065343.GA26964@sigill.intra.peff.net>
In-Reply-To
<xmqqbn99hzrv.fsf@gitster.mtv.corp.google.com>
On Tue, Dec 29, 2015 at 10:32:04AM -0800, Junio C Hamano wrote:
Show 11 quoted lines
> Jeff King <peff@peff.net> writes:
> 
> > A conservative choice would probably be to issue a deprecation warning
> > when we see it defined, wait a few versions, and then apply the patch
> > below.
> 
> I agree with the analysis below.  And I agree that in the ideal
> world, it would have been better not to add "prefer symlink refs"
> configuration in the first place.  But we do not live in the ideal
> world, and we already have it, so deprecation would need the usual
> multi-step process.

Here's the first step of that multi-step process. The commit message will look familiar, as the rationale for deprecating is the same as for dropping.

This is on top of what I posted earlier (it obviously could be independent, but I assume we're planning to merge the other bits, and I don't mind holding this topic hostage for a little while to save some annoying merging).

-- >8 --
Subject: [PATCH] create_symref: deprecate support for writing symbolic links

Long ago, in 2fabd21 (Disable USE_SYMLINK_HEAD by default, 2005-11-15), we switched git's default behavior to writing text symrefs instead of symbolic links. Any scripts accustomed to looking directly at .git/HEAD were updated to use `rev-parse` instead. The Linux kernel's setlocalversion script was one, and it was fixed in 117a93d (kbuild: Use git in scripts/setlocalversion, 2006-01-04).

However, the problem still happened when bisecting the kernel; pre-117a93d would not build properly with a newer git, because they wanted to look directly at HEAD. To solve this, we added 9f0bb90 (core.prefersymlinkrefs: use symlinks for .git/HEAD, 2006-05-02), which lets the user turn on the old behavior, theoretically letting you bisect older kernel history.

But there are a few complications with this solution:
  - packed-refs means you are limited in what you can do
    with .git/HEAD. If it is a symlink, you may `readlink`
    it to see where it points, but you cannot necessarily
    `cat .git/HEAD` to get the sha1, as the pointed-to ref
    may be packed.
    In particular, this means that the pre-117a93d kbuild
    script would sometimes work and sometimes not.
  - These days, we bisect on a detached HEAD. So during
    bisection, .git/HEAD _is_ a regular file with the
    sha1, and it works to `cat` it, whether or not you set
    core.preferSymlinkRefs.

Such scripts will all be broken again if we move to alternate ref backends. They should have learned to use `rev-parse` long ago, and it is only bisecting ancient history that is a problem.

Now that almost ten years have passed, it seems less likely that developers will bisect so far back in history. And moreover, this is but one of many possible problems developers run into when trying to build versions. The standard solution is to apply a "fixup" patch or other workaround while test-building the project, and that would work here, too.

This patch therefore deprecates core.preferSymlinkRefs completely. There are a few reasons to want to do so:

  1. It drops some code that is probably exercised very
     rarely.
  2. The symlink code is not up to the same standards as the
     rest of the ref code. In particular, it is racy with
     respect to simultaneous readers and writers.
  3. If we want to eventually drop the symlink-reading code,
     this is a good first step to deprecating it.

We print a deprecation warning anytime a symlink is created using this code. That prevents us from spamming the user with multiple warnings from read-only operations, but catches major operations like init, clone, and checkout.

Signed-off-by: Jeff King <peff@peff.net>
---
There's no advice.* safety valve here. The solution is "fix your
script", and the last-ditch effort is "come to the mailing list and tell
us why it would be a bad thing to remove the feature".
 refs/files-backend.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
diff --git a/refs/files-backend.c b/refs/files-backend.c
index 180c837..22b7c11 100644
--- a/refs/files-backend.c
+++ b/refs/files-backend.c
@@ -2860,12 +2860,27 @@ static int create_symref_locked(struct ref_lock *lock, const char *refname,
 	return 0;
 }
 
+static const char symlink_deprecation_warning[] =
+"The core.preferSymlinkRefs configuration option has been\n"
+"deprecated and will be removed in a future version of Git. If your\n"
+"workflow or script depends on '.git/HEAD' being a symbolic link,\n"
+"it should be adjusted to use:\n"
+"\n"
+"        git rev-parse HEAD\n"
+"\n"
+"        git rev-parse --symbolic-full-name HEAD\n"
+"\n"
+"to get the sha1 or branch name, respectively.";
+
 int create_symref(const char *refname, const char *target, const char *logmsg)
 {
 	struct strbuf err = STRBUF_INIT;
 	struct ref_lock *lock;
 	int ret;
 
+	if (prefer_symlink_refs)
+		warning("%s", symlink_deprecation_warning);
+
 	lock = lock_ref_sha1_basic(refname, NULL, NULL, NULL, REF_NODEREF, NULL,
 				   &err);
 	if (!lock) {
-- 
2.7.0.rc3.367.g09631da
Previous: Junio C HamanoNext: Jeff King
Message 20 of 25 in “improve symbolic-ref robustness”
  1. 0/4 improve symbolic-ref robustnessJeff King, Dec 20, 2015
  2. 1/4 symbolic-ref: propagate error code from create_symref()Jeff King, Dec 20, 2015
  3. 2/4 t1401: test reflog creation for git-symbolic-refJeff King, Dec 20, 2015
  4. 3/4 create_symref: modernize variable namesJeff King, Dec 20, 2015
  5. Michael HaggertyDec 28, 2015
  6. Jeff KingDec 29, 2015
  7. 4/4 create_symref: use existing ref-lock codeJeff King, Dec 20, 2015
  8. Junio C HamanoDec 21, 2015
  9. Jeff KingDec 22, 2015
  10. Michael HaggertyDec 28, 2015
  11. Jeff KingDec 29, 2015
  12. Jeff KingDec 29, 2015
  13. 0/3 improve symbolic-ref robustnessJeff King, Dec 29, 2015
  14. 1/3 create_symref: modernize variable namesJeff King, Dec 29, 2015
  15. 2/3 create_symref: use existing ref-lock codeJeff King, Dec 29, 2015
  16. 3/3 create_symref: write reflog while holding lockJeff King, Dec 29, 2015
  17. 4/3 create_symref: drop support for writing symbolic linksJeff King, Dec 29, 2015
  18. Jeff KingDec 29, 2015
  19. Junio C HamanoDec 29, 2015
  20. Jeff KingDec 30, 2015
  21. Jeff KingDec 30, 2015
  22. Michael HaggertyDec 29, 2015
  23. Junio C HamanoDec 29, 2015
  24. Junio C HamanoDec 29, 2015
  25. Jeff KingDec 30, 2015

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.