| Message ID | 20240918232825.2627999-3-gitster@pobox.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | deprecating core.preferSymlinkRefs | expand |
On Wed, Sep 18, 2024 at 04:28:23PM -0700, Junio C Hamano wrote: > diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh > index d369330562..4e517cdc13 100755 > --- a/t/t0600-reffiles-backend.sh > +++ b/t/t0600-reffiles-backend.sh > [...] > @@ -468,26 +468,40 @@ test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' ' > esac > ' > > -test_expect_success SYMLINKS 'symref transaction supports symlinks' ' > +test_expect_success SYMLINKS 'symlinks used as symrefs are still supported' ' > test_when_finished "git symbolic-ref -d TEST_SYMREF_HEAD || :" && > git update-ref refs/heads/new HEAD && > - test_config core.prefersymlinkrefs true && > - cat >stdin <<-EOF && > - start > - symref-create TEST_SYMREF_HEAD refs/heads/new > - prepare > - commit > - EOF > - git update-ref --no-deref --stdin <stdin 2>stderr && > + # manually do this, as core.prefersymlinkrefs no longer > + # affects how a symref is created (as a textual symref). > + ln -f -s refs/heads/new .git/TEST_SYMREF_HEAD && There are two other tests that probably should get the same treatment. In the patch below I've just deleted them, but I think since they are really about the reading side, they'd probably want a similar manual setup. diff --git a/t/t7201-co.sh b/t/t7201-co.sh index 2d984eb4c6..72eb5f62e7 100755 --- a/t/t7201-co.sh +++ b/t/t7201-co.sh @@ -407,18 +407,6 @@ test_expect_success 'checkout w/--track from tag fails' ' test "z$(git rev-parse main^0)" = "z$(git rev-parse HEAD)" ' -test_expect_success 'detach a symbolic link HEAD' ' - git checkout main && - git config --bool core.prefersymlinkrefs yes && - git checkout side && - git checkout main && - it=$(git symbolic-ref HEAD) && - test "z$it" = zrefs/heads/main && - here=$(git rev-parse --verify refs/heads/main) && - git checkout side^ && - test "z$(git rev-parse --verify refs/heads/main)" = "z$here" -' - test_expect_success 'checkout with --track fakes a sensible -b <name>' ' git config remote.origin.fetch "+refs/heads/*:refs/remotes/origin/*" && git update-ref refs/remotes/origin/koala/bear renamer && diff --git a/t/t9903-bash-prompt.sh b/t/t9903-bash-prompt.sh index 95e9955bca..b4d7206bea 100755 --- a/t/t9903-bash-prompt.sh +++ b/t/t9903-bash-prompt.sh @@ -50,15 +50,6 @@ test_expect_success 'prompt - branch name' ' test_cmp expected "$actual" ' -test_expect_success SYMLINKS 'prompt - branch name - symlink symref' ' - printf " (main)" >expected && - test_when_finished "git checkout main" && - test_config core.preferSymlinkRefs true && - git checkout main && - __git_ps1 >"$actual" && - test_cmp expected "$actual" -' - test_expect_success 'prompt - unborn branch' ' printf " (unborn)" >expected && git checkout --orphan unborn && I noticed these because I had a similar proposal long ago, which never got merged (I don't think because anybody particularly disagreed, but it fell through the cracks and never got picked up again): https://lore.kernel.org/git/20151230065343.GA26964@sigill.intra.peff.net/ What you have here is (modulo the two hunks above) more complete than what I have, so I don't think there's anything else to try to salvage from it. A little bit of the history in the linked commit message is interesting as to how we ended up here, but ultimately not really that important. -Peff
Jeff King <peff@peff.net> writes: > I noticed these because I had a similar proposal long ago, which never > got merged (I don't think because anybody particularly disagreed, but it > fell through the cracks and never got picked up again): > > https://lore.kernel.org/git/20151230065343.GA26964@sigill.intra.peff.net/ > > What you have here is (modulo the two hunks above) more complete than > what I have, so I don't think there's anything else to try to salvage > from it. A little bit of the history in the linked commit message is > interesting as to how we ended up here, but ultimately not really that > important. It was useful to find out another reason (which I failed to mention in this series) why symbolic links will be misleading: when it points at a ref that is in packed-refs. Thanks.
diff --git a/Documentation/config/core.txt b/Documentation/config/core.txt index f0245f5050..a6f67cab27 100644 --- a/Documentation/config/core.txt +++ b/Documentation/config/core.txt @@ -285,10 +285,10 @@ CIFS/Microsoft Windows. + False by default. -core.preferSymlinkRefs (deprecated):: +core.preferSymlinkRefs (removed):: Instead of the default "symref" format for HEAD and other symbolic reference files, use symbolic links. The support - for this variable will be dropped in Git 3.0. + for this variable was dropped in Git 3.0. core.alternateRefsCommand:: When advertising tips of available history from an alternate, use the shell to diff --git a/refs/files-backend.c b/refs/files-backend.c index c40a248b9f..1296272252 100644 --- a/refs/files-backend.c +++ b/refs/files-backend.c @@ -2011,26 +2011,6 @@ static int commit_ref_update(struct files_ref_store *refs, return 0; } -#ifdef NO_SYMLINK_HEAD -#define create_ref_symlink(a, b) (-1) -#else -static int create_ref_symlink(struct ref_lock *lock, const char *target) -{ - int ret = -1; - - char *ref_path = get_locked_file_path(&lock->lk); - unlink(ref_path); - ret = symlink(target, ref_path); - free(ref_path); - - if (ret) - fprintf(stderr, "no symlink - falling back to symbolic ref\n"); - else - warning("core.preferSymlinkRefs will be removed in Git 3.0"); - return ret; -} -#endif - static int create_symref_lock(struct ref_lock *lock, const char *target, struct strbuf *err) { @@ -3003,13 +2983,10 @@ static int files_transaction_finish(struct ref_store *ref_store, } } - /* - * We try creating a symlink, if that succeeds we continue to the - * next update. If not, we try and create a regular symref. - */ + /* Warn against core.preferSymlinkRefs set to true */ if (update->new_target && prefer_symlink_refs) - if (!create_ref_symlink(lock, update->new_target)) - continue; + /* we used to, but no longer, create a symlink here */ + warning("core.preferSymlinkRefs was removed in Git 3.0"); if (update->flags & REF_NEEDS_COMMIT) { clear_loose_ref_cache(refs); diff --git a/t/t0600-reffiles-backend.sh b/t/t0600-reffiles-backend.sh index d369330562..4e517cdc13 100755 --- a/t/t0600-reffiles-backend.sh +++ b/t/t0600-reffiles-backend.sh @@ -468,26 +468,40 @@ test_expect_success POSIXPERM 'git reflog expire honors core.sharedRepository' ' esac ' -test_expect_success SYMLINKS 'symref transaction supports symlinks' ' +test_expect_success SYMLINKS 'symlinks used as symrefs are still supported' ' test_when_finished "git symbolic-ref -d TEST_SYMREF_HEAD || :" && git update-ref refs/heads/new HEAD && - test_config core.prefersymlinkrefs true && - cat >stdin <<-EOF && - start - symref-create TEST_SYMREF_HEAD refs/heads/new - prepare - commit - EOF - git update-ref --no-deref --stdin <stdin 2>stderr && + # manually do this, as core.prefersymlinkrefs no longer + # affects how a symref is created (as a textual symref). + ln -f -s refs/heads/new .git/TEST_SYMREF_HEAD && test_path_is_symlink .git/TEST_SYMREF_HEAD && - test "$(test_readlink .git/TEST_SYMREF_HEAD)" = refs/heads/new && - test_grep "core\.preferSymlinkRefs will be removed" stderr + echo refs/heads/new >expect && + git symbolic-ref TEST_SYMREF_HEAD >actual && + test_cmp actual expect +' + +test_expect_success 'core.prefersymlinkrefs gets a warning' ' + test_when_finished "git symbolic-ref -d TEST_SYMREF_HEAD || :" && + git update-ref refs/heads/new HEAD && + + test_config core.prefersymlinkrefs true && + git symbolic-ref TEST_SYMREF_HEAD refs/heads/new 2>stderr && + test_grep "core\.preferSymlinkRefs was removed" stderr && + + git symbolic-ref -d TEST_SYMREF_HEAD && + git config core.prefersymlinkrefs false && + git symbolic-ref TEST_SYMREF_HEAD refs/heads/new 2>stderr && + test_grep ! "core\.preferSymlinkRefs was removed" stderr && + + git symbolic-ref -d TEST_SYMREF_HEAD && + git config --unset core.prefersymlinkrefs && + git symbolic-ref TEST_SYMREF_HEAD refs/heads/new 2>stderr && + test_grep ! "core\.preferSymlinkRefs was removed" stderr ' -test_expect_success 'symref transaction supports false symlink config' ' +test_expect_success 'symref transaction' ' test_when_finished "git symbolic-ref -d TEST_SYMREF_HEAD || :" && git update-ref refs/heads/new HEAD && - test_config core.prefersymlinkrefs false && cat >stdin <<-EOF && start symref-create TEST_SYMREF_HEAD refs/heads/new @@ -499,7 +513,7 @@ test_expect_success 'symref transaction supports false symlink config' ' git symbolic-ref TEST_SYMREF_HEAD >actual && echo refs/heads/new >expect && test_cmp expect actual && - test_grep ! "core\.preferSymlinkRefs will be removed" stderr + test_grep ! "core\.preferSymlinkRefs was removed" stderr ' test_done
This step and the next step are for Git 3.0. Now we are preparing for a big Git 3.0 transition after warning against use of this configuration variable to create a new symlink that represents a symbolic ref for N months, it is time to actually remove the support of the configuration variable. In this patch, we mostly ignore core.preferSymlinkRefs and always create a textual symref when we are asked to create a symref, but when core.preferSymlinkRefs would have caused us to use a symbolic link in an older version of Git, we will issue a warning. We also have in our build infrastructure to selectively set the CPP macro NO_SYMLINK_HEAD, but an accompanying patch will remove them. The final warning is meant to help users who set the configuration variable and expected it to create a symlink, but instead got a textual symref. They may not even recognise the configuration after they set it long time ago and forgot about it by now, so we keep the "git config --help" entry for the variable to help them recall what it was about. After a few releases, we will get rid of this warning and the codebase will look as if such a configuration variable never existed, but we are not quite there yet. Signed-off-by: Junio C Hamano <gitster@pobox.com> --- Documentation/config/core.txt | 4 ++-- refs/files-backend.c | 29 +++--------------------- t/t0600-reffiles-backend.sh | 42 +++++++++++++++++++++++------------ 3 files changed, 33 insertions(+), 42 deletions(-)