-
-
Notifications
You must be signed in to change notification settings - Fork 313
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
Comments
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 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. |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
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. v0.35.0...v0.36.0#diff-33ecbfd10725c10171160eb137e962270ecf1ab29012fc23356d71e6ce026a35 The above is a fix to 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 If not, then… it definitely works now and the issue can indeed be closed :). |
gix clone
sets core.symlinks
to false
on Windows even if globally true
gix clone
ignores global core.symlinks
on Windows
You're right. It is still ignoring the global value of With
With
The command scope with
The command scope with
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 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 |
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 |
Unless they are differentiated elsewhere, it looks like The variables whose associated capabilities are gauged there and that are reasonable to ignore from the global scope seem to be:
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, 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):
Yes, Git actually does set it in this scenario.
|
Thanks for all the research!
This probably means one will have to check how the logic is implemented in Git itself so
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. |
Of each of these configuration variables, 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. |
I still think fixing the 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 :). |
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.
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.) |
Indeed, there was the Somehow I feel that this might get harder than before as |
A few of the gix-worktree-state checkout tests have been checking if the filesystem supports symlinks, while one was skipped (marked ignored) on Windows based on the idea that symlinks would not be created, and also had an assertion that assumed symlinks would not be successfully created. This commit changes such tests so that they run on all platforms, including Windows, and so that, on all platforms, they assert that the filesystem supports symlinks, and assert that expected symlinks are created after attempts to do so. (The reason to assert that the filesystem supports symlinks is so that if this is not detected, either due to a failure or detection or due to the filesystem really not supporting symlinks, the test failures will be clear, rather than having a later assertion fail for unclear reasons.) Since GitoxideLabs#1444, tests involving symlinks are expected to work even on Windows. That PR included changes to the way fixtures were run, and to other parts of the test suite, to cause symlinks to be created in cases where they had previously had not (GitoxideLabs#1443). A number of tests had been assumed not to work due to limitations of Windows, MSYS, or Git: - Although Windows will not allow all users to create symlinks under all configurations, the test suite expects to be run on a Windows system configured to permit this. - Although `ln -s` and similar commands in MSYS environments, including Git Bash, do not by default attempt to create actual symlinks, this does happen when symlink creation is enabled using the `MSYS` environment variable, as done in 0899c2e (GitoxideLabs#1444). (This situation differs from that of Unix-style executable bits, which do not have filesystem support on Windows. While `chmod +x` and `chmod -x` commands do not take effect on Windows, which slightly limits the ability to test such metadata and requires that a number of fixtures set the mode directly in the ndex, with symlinks there is no such inherent restriction. Provided that the `MSYS` environment variable is set to allow it, which gix-testtools takes care of since GitoxideLabs#1444, and that Windows permits the user running the test suite to create symlinks, which is already needed to properly run the test suite on Windows, the same `ln -s` commands in fixture scripts that work on Unix-like systems will also work on Windows.) - Although `git` commands will not check out symlinks as actual symlinks on Windows unless `core.symlinks` is set to `true`, this is not typically required for the way symlinks are used in the gitoixde test suite. Instead, we usually test the presence of expected symlink metadata in repository data structures such as an index and trees, as well as the ability of gitoxide to check out symlinks. (We do not intentionally test the ability to run `ln -s` in Git Bash, but this is needed in order to create a number of the repositories for testing. Having `git` check out symlinks is not typically needed for this.) In addition, since we are requiring that Windows test environments permit the user running the test suite to create symlinks, any failures that arise in the future due to greater sensitivity to `core.symlinks` (see GitoxideLabs#1353 for context) could be worked around by setting that configuration variable for the tests, either in gix-testtools via `GIT_CONFIG_{COUNT,KEY,VALUE}` or in the specifically affected fixture scripts. While GitoxideLabs#1444 updated a number of tests to reflect the ability to create symlinks in fixture scripts and the wish to test them on all platforms including Windows, some tests remain to be updated. This commit covers the gix-worktree-statte checkout tests. This does not cover even their associated fixtures, which can already create symlinks (given the above described conditions), but that should be updated so they can set intended executable permissions (see above on `chmod`). This will be done separately.
Current behavior 😯
On Windows,
core.symlinks
is intended to tofalse
when not set, and in some installations is even set asfalse
in the installation scope. Changing it, if done, is most often done in the global scope, such as by runninggit 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 ifcore.symlinks
was globally set totrue
. It furthermore would setcore.symlinks
tofalse
explicitly in the newly cloned repository's.git/config
.Starting in
gitoxide
0.36.0, the behavior has changed, but the bug of disregardingcore.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 whencore.symlinks
is globally set tofalse
.(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 honorcore.symlinks
in the command scope, both with-c core.symlinks=true
and withGIT_{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 totrue
for the installation, at installation time. This coincides with the behaviorgix
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 totrue
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 tofalse
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 causegix 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 withsymlinks
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.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
tofalse
globally and/or check that it is setOn 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 showfalse
.Clone with
gix clone
and inspect the repositoryOn 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
: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 globalcore.symlinks
value offalse
taken together with the absence of any other more narrowly scoped configuration that would override it.In PowerShell, that shows output like:
To verify that
gix
setcore.symlinks
totrue
in the local configuration, inspecthas-symlink/.git/config
, or run this command, which outputstrue
:Redo the clone with
core.symlinks
set tofalse
in the commandTo see how this differs from the behavior of
gix
whencore.symlinks
is provided on the command line, first delete the cloned repository:rm -rf has-symlink
rm -r -fo has-symlink
Then try the clone again, but this time use the
-c
option togix
:Check the
symlink
entry again:This time, that shows that it is a regular file on disk. For example, in PowerShell, it looks like:
However, it has still set
core.symlinks
totrue
in the newly cloned repository's local configuration. This still outputsfalse
:Thus the reason
-c
behaved differently was that it overrode the wrongly setfalse
value thatgix
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 thangix 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
tofalse
on Windows even when it is unset and the user is capable of creating symbolic links. My guess is that, whencore.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 offalse
forcore.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 breakingssh
forgit
andgix
by running:Then I attempted to clone a repository that I know I can otherwise clone with
gix
:As expected, this attempted to use the bogus
foo
implementation of SSH:To undo that, simply run:
The text was updated successfully, but these errors were encountered: