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.

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

gix clone ignores global core.symlinks on Windows #1353

Open
EliahKagan opened this issue May 6, 2024 · 12 comments
Open

gix clone ignores global core.symlinks on Windows #1353

EliahKagan opened this issue May 6, 2024 · 12 comments
Assignees
Labels
acknowledged an issue is accepted as shortcoming to be fixed C-Windows Windows-specific issues help wanted Extra attention is needed

Comments

@EliahKagan
Copy link
Contributor

EliahKagan commented May 6, 2024

Current behavior 😯

On Windows, core.symlinks is intended to to false when not set, and in some installations is even set as false in the installation scope. Changing it, if done, is most often done in the global scope, such as by running git config --global core.symlinks true.

But gix clone never honors this global configuration.

Prior to gitoxide 0.36.0, it checked out regular-file stubs rather than symlinks, even on a Windows system where the user running it is capable of creating symlinks in the directory being checked out to, even if core.symlinks was globally set to true. It furthermore would set core.symlinks to false explicitly in the newly cloned repository's .git/config.

Starting in gitoxide 0.36.0, the behavior has changed, but the bug of disregarding core.symlinks in the global scope has remained. The current behavior is to check out symlinks as symlinks on a Windows system where the user running it is capable of creating symlinks in the directory being checked out to, even when core.symlinks is globally set to false.

(This also now happens when core.symlinks is globally unset and not otherwise specified, but even though that differs from the Git behavior, it is not itself obviously a bug or undesirable.)

gix does honor core.symlinks in the command scope, both with -c core.symlinks=true and with GIT_{CONFIG_KEY_VALUE}. But even in that case, gix clone sets a value in the local scope (in the reposiitory's .git/config) reflecting its determination of whether symlinks are supported.

This is not readily checkable on CI without tests specifically for it or other special effort, since the Windows runners for GitHub Actions customize core.symlinks, setting it to true for the installation, at installation time. This coincides with the behavior gix detects is available, in in any case technically involves the installation or system scope rather than the global scope.

Expected behavior 🤔

When core.symlinks is set to true explicitly in the global scope and it has not been overridden in the command scope, gix clone should attempt to check out symbolic links as symbolic links rather than as regular files. I believe it will still not do this as often as it should, though in most cases where it is desired that does now happen, since it creates symlinks when it detects it can.

When core.symlinks is set to false explicitly in the global scope and it has not been overridden in the command scope, gix clone should not attempt to check out symbolic links as symbolic links, but should instead create regular files. This is the expected behavior that, currently, is decisively not satisfied: the global configuration scope cannot currently be used to cause gix clone not to check out symlinks.

Git behavior

Git will check out symlinks as actual Windows symbolic links rather than as regular-file stubs, including in commands such as git clone.

Git also does not set this variable in the local scope. Local repository .git/config are not created with symlinks set in their [core] sections.

Steps to reproduce 🕹

Make the test repository

Create a repository with a symbolic link. For simplicity, it should be a symbolic link to a regular file in the repository, and thus not dangling/broken. (Besides simplicity, this also avoids confusion relating to the separate issue #1354.) This can be done with Git either on Windows or on a Unix-like system; I used Ubuntu, pushed to a remote, and cloned from a remote, to ensure that this bug was not specific to file:// remotes.

git init has-symlink
cd has-symlink
echo 'This is a regular file.' >target
ln -s target symlink
git commit -m 'Initial commit'

Alternatively, the has-symlink repository may be used. It was created that way (then minimal documentation was added in a second commit). It is used in the instructions below, which assume that either PowerShell or Git Bash is used.

Set core.symlinks to false globally and/or check that it is set

On a Windows system, run this command, at least if that has not already been done in your user account:

git config --global core.symlinks false

Checking that it is set in the global scope by running git config core.symlinks should show false.

Clone with gix clone and inspect the repository

On a Windows system in which you have the ability to create symlinks, in a location on a volume that supports them, clone the repository with gix:

gix clone git@github.com:EliahKagan/has-symlink.git

The clone succeeds, but inspecting the checked out files reveals that symlink is a symbolic link, even though it should have been created as a regular file due to the global core.symlinks value of false taken together with the absence of any other more narrowly scoped configuration that would override it.

ls -l has-symlink/symlink

In PowerShell, that shows output like:

    Directory: C:\Users\ek\src\has-symlink

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
la---           6/23/2024  1:31 AM              0 symlink -> target

To verify that gix set core.symlinks to true in the local configuration, inspect has-symlink/.git/config, or run this command, which outputs true:

git -C has-symlink config --local core.symlinks

Redo the clone with core.symlinks set to false in the command

To see how this differs from the behavior of gix when core.symlinks is provided on the command line, first delete the cloned repository:

  • In Git Bash, run: rm -rf has-symlink
  • In PowerShell, run: rm -r -fo has-symlink

Then try the clone again, but this time use the -c option to gix:

gix -c core.symlinks=false clone git@github.com:EliahKagan/has-symlink.git

Check the symlink entry again:

ls -l has-symlink/symlink

This time, that shows that it is a regular file on disk. For example, in PowerShell, it looks like:

    Directory: C:\Users\ek\src\has-symlink

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
-a---           6/23/2024  1:04 PM              6 symlink

However, it has still set core.symlinks to true in the newly cloned repository's local configuration. This still outputs false:

git -C has-symlink config --local core.symlinks

Thus the reason -c behaved differently was that it overrode the wrongly set false value that gix placed at repository level. Furthermore, using -c is an insufficient workaround because it is still set otherwise locally (though unsetting it locally after cloning it that way should be a sufficient workaround until a fix is available).

Optionally, compare to the behavior of Git

Deleting the cloned repository again and cloning it with git clone rather than gix clone shows the expected behavior, and -c does not need to be used.

The behavior of Git may go further than desired for gitoxide, however, since it defaults core.symlinks to false on Windows even when it is unset and the user is capable of creating symbolic links. My guess is that, when core.symlinks is unset in all scopes originally, then operating based on capabilities may be the intended behavior of gitoxide.

The key behavior of Git that gitoxide should but does not follow is that an explicit core.symlinks setting should be followed, at least whenever that is possible to do. Following an explicit value of false for core.symlinks is always possible to do.

Optionally, verify that other global configuration is honored

The problem is not that gix doesn't use global configuration in general. I verified this by temporarily breaking ssh for git and gix by running:

git config --global core.sshCommand foo   # Warning: breaks ssh in git until undone!

Then I attempted to clone a repository that I know I can otherwise clone with gix:

gix clone git@github.com:EliahKagan/gitoxide.git

As expected, this attempted to use the bogus foo implementation of SSH:

Error: Failed to invoke program "foo"

Caused by:
    program not found

To undo that, simply run:

git config --global --unset core.sshCommand
@Byron
Copy link
Owner

Byron commented May 6, 2024

The issue might originate here which is the only spot where it writes the local configuration.

It's a well-known limitation that in-memory configuration can't be written back yet, merely because I didn't work on the API for that yet, which also explains why it doesn't change.

However, this also means that gix_fs::Capabilities::probe() is unable to figure out that the filesystem supports symlinks on Windows, or in this particular situation.

Finally, another bug seems to be that it doesn't correctly pick up global configuration for this and is probably quite none-conforming with how Git does it.

@Byron Byron added help wanted Extra attention is needed acknowledged an issue is accepted as shortcoming to be fixed labels May 6, 2024
@EliahKagan

This comment was marked as resolved.

@EliahKagan

This comment was marked as resolved.

@Byron
Copy link
Owner

Byron commented Jun 23, 2024

That's…interesting, as I also don't know how I would have fixed that. There were a few changes related to cloning, but nothing that would have affected the configuration that is written.
Oh… wait, I realize what changed.

v0.35.0...v0.36.0#diff-33ecbfd10725c10171160eb137e962270ecf1ab29012fc23356d71e6ce026a35

The above is a fix to gix-fs, which was racy before which could cause test failures. With a single clone, I'd think it can't be racy, but it's definitely more proper now.

It's likely that the fix isn't really a fix, but that it now detects that symlinks are possible, so it sets the value to true. You can put that to the test by turning the global symlink support off - it will probably still set it to true locally.

If not, then… it definitely works now and the issue can indeed be closed :).

@EliahKagan EliahKagan changed the title gix clone sets core.symlinks to false on Windows even if globally true gix clone ignores global core.symlinks on Windows Jun 23, 2024
@EliahKagan
Copy link
Contributor Author

EliahKagan commented Jun 23, 2024

You're right. It is still ignoring the global value of core.symlinks, just by creating symlinks as symlinks even if core.symlinks is set to false globally, rather than by creating them as regular files even if core.symlinks is set to true globally, as had been done before gitoxide 0.36.0.

With gitoxide 0.36.0, with core.symlinks unset globally, symlinks are created:

C:\Users\ek\src> rm -r -fo has-symlink
C:\Users\ek\src> git config --global --unset core.symlinks
C:\Users\ek\src> gix clone [email protected]:EliahKagan/has-symlink.git
 12:57:25 indexing done 8.0 objects in 0.00s (22.6k objects/s)
 12:57:25 decompressing done 1.5KB in 0.00s (4.2MB/s)
 12:57:25     Resolving done 8.0 objects in 0.05s (158.0 objects/s)
 12:57:25      Decoding done 1.5KB in 0.05s (29.9KB/s)
 12:57:25 writing index file done 1.3KB in 0.00s (3.0MB/s)
 12:57:25  create index file done 8.0 objects in 0.06s (140.0 objects/s)
 12:57:25          read pack done 1.1KB in 0.06s (18.5KB/s)
 12:57:25           checkout done 4.0 files in 0.00s (1.4k files/s)
 12:57:25            writing done 848B in 0.00s (298.3KB/s)
HEAD:refs/remotes/origin/HEAD (implicit)
        778c750e6ce28bde1aedd8c7d4c4decc312f45d9 HEAD symref-target:refs/heads/main -> refs/remotes/origin/HEAD [new]
+refs/heads/*:refs/remotes/origin/*
        778c750e6ce28bde1aedd8c7d4c4decc312f45d9 refs/heads/main -> refs/remotes/origin/main [new]
C:\Users\ek\src> ls has-symlink

    Directory: C:\Users\ek\src\has-symlink

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
d----           6/23/2024 12:57 PM                .git
-a---           6/23/2024 12:57 PM            617 COPYING
-a---           6/23/2024 12:57 PM            200 README.md
la---           6/23/2024 12:57 PM              0 symlink -> target
-a---           6/23/2024 12:57 PM             25 target

With gitoxide 0.36.0, with core.symlinks set globally to false, symlinks are created:

C:\Users\ek\src> rm -r -fo has-symlink
C:\Users\ek\src> git config --global core.symlinks false
C:\Users\ek\src> gix clone [email protected]:EliahKagan/has-symlink.git
 12:57:59 indexing done 8.0 objects in 0.00s (34.4k objects/s)
 12:57:59 decompressing done 1.5KB in 0.00s (6.3MB/s)
 12:57:59     Resolving done 8.0 objects in 0.05s (157.0 objects/s)
 12:57:59      Decoding done 1.5KB in 0.05s (29.6KB/s)
 12:57:59 writing index file done 1.3KB in 0.00s (5.0MB/s)
 12:57:59  create index file done 8.0 objects in 0.05s (152.0 objects/s)
 12:57:59          read pack done 1.1KB in 0.05s (21.3KB/s)
 12:57:59           checkout done 4.0 files in 0.00s (1.6k files/s)
 12:57:59            writing done 848B in 0.00s (339.2KB/s)
HEAD:refs/remotes/origin/HEAD (implicit)
        778c750e6ce28bde1aedd8c7d4c4decc312f45d9 HEAD symref-target:refs/heads/main -> refs/remotes/origin/HEAD [new]
+refs/heads/*:refs/remotes/origin/*
        778c750e6ce28bde1aedd8c7d4c4decc312f45d9 refs/heads/main -> refs/remotes/origin/main [new]
C:\Users\ek\src> ls has-symlink

    Directory: C:\Users\ek\src\has-symlink

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
d----           6/23/2024 12:57 PM                .git
-a---           6/23/2024 12:57 PM            617 COPYING
-a---           6/23/2024 12:57 PM            200 README.md
la---           6/23/2024 12:57 PM              0 symlink -> target
-a---           6/23/2024 12:57 PM             25 target

The command scope with -c does work (as before):

C:\Users\ek\src> rm -r -fo has-symlink
C:\Users\ek\src> gix -c core.symlinks=false clone [email protected]:EliahKagan/has-symlink.git
 12:58:44 indexing done 8.0 objects in 0.00s (30.0k objects/s)
 12:58:44 decompressing done 1.5KB in 0.00s (5.5MB/s)
 12:58:45     Resolving done 8.0 objects in 0.05s (158.0 objects/s)
 12:58:45      Decoding done 1.5KB in 0.05s (29.9KB/s)
 12:58:45 writing index file done 1.3KB in 0.00s (7.8MB/s)
 12:58:45  create index file done 8.0 objects in 0.05s (154.0 objects/s)
 12:58:45          read pack done 1.1KB in 0.05s (21.4KB/s)
 12:58:45           checkout done 4.0 files in 0.00s (1.8k files/s)
 12:58:45            writing done 848B in 0.00s (375.6KB/s)
HEAD:refs/remotes/origin/HEAD (implicit)
        778c750e6ce28bde1aedd8c7d4c4decc312f45d9 HEAD symref-target:refs/heads/main -> refs/remotes/origin/HEAD [new]
+refs/heads/*:refs/remotes/origin/*
        778c750e6ce28bde1aedd8c7d4c4decc312f45d9 refs/heads/main -> refs/remotes/origin/main [new]
C:\Users\ek\src> ls has-symlink

    Directory: C:\Users\ek\src\has-symlink

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
d----           6/23/2024 12:58 PM                .git
-a---           6/23/2024 12:58 PM            617 COPYING
-a---           6/23/2024 12:58 PM            200 README.md
-a---           6/23/2024 12:58 PM              6 symlink
-a---           6/23/2024 12:58 PM             25 target

The command scope with GIT_CONFIG_{COUNT,KEY,VALUE} likewise works (as I presume it did before, though I did not test it at that time):

C:\Users\ek\src> rm -r -fo has-symlink
C:\Users\ek\src> $env:GIT_CONFIG_COUNT = '1'
C:\Users\ek\src> $env:GIT_CONFIG_KEY_0 = 'core.symlinks'
C:\Users\ek\src> $env:GIT_CONFIG_VALUE_0 = 'false'
C:\Users\ek\src> gix clone [email protected]:EliahKagan/has-symlink.git
 13:04:53 indexing done 8.0 objects in 0.00s (19.1k objects/s)
 13:04:53 decompressing done 1.5KB in 0.00s (3.5MB/s)
 13:04:53     Resolving done 8.0 objects in 0.05s (158.0 objects/s)
 13:04:53      Decoding done 1.5KB in 0.05s (29.9KB/s)
 13:04:53 writing index file done 1.3KB in 0.00s (8.6MB/s)
 13:04:53  create index file done 8.0 objects in 0.05s (154.0 objects/s)
 13:04:53          read pack done 1.1KB in 0.06s (20.6KB/s)
 13:04:53           checkout done 4.0 files in 0.00s (1.8k files/s)
 13:04:53            writing done 848B in 0.00s (389.0KB/s)
HEAD:refs/remotes/origin/HEAD (implicit)
        778c750e6ce28bde1aedd8c7d4c4decc312f45d9 HEAD symref-target:refs/heads/main -> refs/remotes/origin/HEAD [new]
+refs/heads/*:refs/remotes/origin/*
        778c750e6ce28bde1aedd8c7d4c4decc312f45d9 refs/heads/main -> refs/remotes/origin/main [new]
C:\Users\ek\src> ls has-symlink

    Directory: C:\Users\ek\src\has-symlink

Mode                 LastWriteTime         Length Name
----                 -------------         ------ ----
d----           6/23/2024  1:04 PM                .git
-a---           6/23/2024  1:04 PM            617 COPYING
-a---           6/23/2024  1:04 PM            200 README.md
-a---           6/23/2024  1:04 PM              6 symlink
-a---           6/23/2024  1:04 PM             25 target

I've updated the issue title here to account for how the bug of disregarding that configuration variable is still present. I will update the issue description with details about what has and has not changed (as well as to fix the missing --global option).

Edit: I have revised the issue description to reflect the current situation. I have also checked the effect on what is written into local scope configuration to verify that the current description is accurate with respect to that, since checking .git/config was not otherwise part of my recent experiments.

@Byron
Copy link
Owner

Byron commented Jun 23, 2024

Thanks for the re-test and the update to the issue!

This means that once the global configuration for symlinks is respected, along with other values that it wants to put into the local config file, this issue will be fixed.

This could be a easy entrypoint for you if you wanted to give it a try. This is the entrypoint, and from there it gets to initializing a new repository without using information in the global configuration at all - it's merely writing down actual filesystem capabilities.

It would also be interesting to see if git places -c core.symlink=false into the configuration file, or if that remains true even though it's ignored for just that invocation (but not persisted).

@EliahKagan
Copy link
Contributor Author

This is the entrypoint, and from there it gets to initializing a new repository without using information in the global configuration at all - it's merely writing down actual filesystem capabilities.

Unless they are differentiated elsewhere, it looks like core.symlinks is not the only configuration variable where the global scope should take precedence over the result of auto-detection.

The variables whose associated capabilities are gauged there and that are reasonable to ignore from the global scope seem to be:

  • core.repositoryFormatVersion, because git-config(1) documents it as an "internal variable."
  • core.bare, because when cloning it should be determined by whether the clone is specified to be a bare clone.
  • core.ignoreCase, because git-config(1) documents it as an "internal variable."

I believe users should not set those variables in the global scope. They seem reasonable not to honor from the global scope, but I am unsure if that is necessarily the best behavior. My feeling is that, when present in the global scope, core.ignoreCase should be honored, while the others should not. But this is just a feeling.

In contrast, the variables whose associated capabilities are gauged there but where the global scope value should take precedence if set seem to be the others, which are not documented as internal implementation details and that make sense to set in any scope (even though setting some of them in the global scope might be unusual):

It would also be interesting to see if git places -c core.symlink=false into the configuration file, or if that remains true even though it's ignored for just that invocation (but not persisted).

Yes, Git actually does set it in this scenario.

C:\Users\ek\src> git config --global core.symlinks
true
C:\Users\ek\src> git -c core.symlinks=false clone [email protected]:EliahKagan/has-symlink.git
Cloning into 'has-symlink'...
remote: Enumerating objects: 8, done.
remote: Counting objects: 100% (8/8), done.
remote: Compressing objects: 100% (6/6), done.
remote: Total 8 (delta 0), reused 8 (delta 0), pack-reused 0
Receiving objects: 100% (8/8), done.
C:\Users\ek\src> cat has-symlink/.git/config
[core]
        repositoryformatversion = 0
        filemode = false
        bare = false
        logallrefupdates = true
        symlinks = false
        ignorecase = true
[remote "origin"]
        url = [email protected]:EliahKagan/has-symlink.git
        fetch = +refs/heads/*:refs/remotes/origin/*
[branch "main"]
        remote = origin
        merge = refs/heads/main

@Byron
Copy link
Owner

Byron commented Jun 24, 2024

Thanks for all the research!

I believe users should not set those variables in the global scope. They seem reasonable not to honor from the global scope, but I am unsure if that is necessarily the best behavior. My feeling is that, when present in the global scope, core.ignoreCase should be honored, while the others should not. But this is just a feeling.

This probably means one will have to check how the logic is implemented in Git itself so gitoxide can act similarly to Git.

Yes, Git actually does set it in this scenario.

That is also very interesting, it's probably one of the few (if not the only) situations where command-line overrides are persisted. And here it' svery understandable as well.

@EliahKagan
Copy link
Contributor Author

Of each of these configuration variables, core.symlinks seems like the main one where not using the global value, if present and not overridden in a narrower scope, is unexpected and potentially undesirable.

So I wonder if your original idea that I could fix that specifically might be the best way to start, and then the behavior can be further improved with respect to the other affected variables and also in terms of when, if ever, configuration originating in some other scope is to be written into the newly created repository's local configuration.

If I end up working on this soon, would it get in the way of any other changes being worked on, such as in #1427? There are several different things I'm researching in gitoxide right now, so I could wait until a more convenient time to work on this, if applicable.

@Byron
Copy link
Owner

Byron commented Jun 27, 2024

I still think fixing the core.symlinks handling is a good point to start and I don't believe there will be any interference of #1427 . There is definitely no rush in fixing this either.

Regarding the other standard variables that are written into newly initialized repositories, I think it would be worth handling these like Git does at the very same time if it comes easily. Somehow I don't expect Git to do anything special there, it's probably handling each value similarly. However, please scope the issue like you see fit once you get to it.

Thanks again for offering a fix - I can't wait to see you edit plain Rust :).

@EliahKagan
Copy link
Contributor Author

EliahKagan commented Jun 27, 2024

Thanks again for offering a fix

No problem, I look forward to it. I may do this while other stuff such as #1429 is also in progress, or I may do this afterwards. I'll let you know if I run into any problems.

I can't wait to see you edit plain Rust :).

Thanks, however I'm not sure what you mean, since I would consider #1342 to have been a case of editing plain Rust, and I also expect that the change for this may be less extensive than there.

But I am eager to write more Rust code, having not been doing much of that recently. (And perhaps that is also what you mean.)

@Byron
Copy link
Owner

Byron commented Jun 27, 2024

Indeed, there was the gix-url changes that I already forgot about, while all the shell-related work is stronger in memory for some reason.

Somehow I feel that this might get harder than before as gix is so much more complex than a well isolated plumbing crate like gix-url. And of course, I hope one day it will all fall into place in gix as well, but having a 'super-crate' definitely is 'super-hard' sometimes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
acknowledged an issue is accepted as shortcoming to be fixed C-Windows Windows-specific issues help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants