building on windows fails due to symlinks in working tree

[fbstj]

Current behavior 😯

error: failed to load manifest for workspace member `C:\code\joe\gitoxide\gix-diff`

Caused by:
  failed to load manifest for dependency `gix-filter`

Caused by:
  failed to load manifest for dependency `gix-packetline-blocking`

Caused by:
  failed to parse manifest at `C:\code\joe\gitoxide\gix-packetline-blocking\Cargo.toml`

Caused by:
  can't find library `gix-packetline-blocking`, rename file to `src/lib.rs` or specify lib.path

Expected behavior 🤔

I can compile

Git behavior

No response

Steps to reproduce 🕹

1. I have checked out the byron/gitoxide repository (using vscode)
2. I have run any cargo command anywhere in the repository

I suspect this is because the repo uses symlinks to reduce copying.

[Byron]

Indeed, this is an issue I also suffer from whenever trying things in my Windows VM. To fix it, I remove the symlink in gix-packetline-blocking/src and replace it with a copy of gix-packetline/src.

@EliahKagan shared that he was able to get it to work with after setting core.symlinks = true on the user-level before cloning.

As of now, I don't have a fix except to give in and not use a symlink. This could be feasible in terms of maintenance burden, but I don't like that this implies these crates should be independent, even though they are not. The problem this is solving is that gix-filter needs access to a blocking version of the packetline crate, but can't set it or else it would force everyone to use the blocking version. And all this is to keep the sync and async versions swappable, it's a bit of a mess and I hope that one day Rust will have this capability out of the box, I think they are designing solutions already.

[EliahKagan]

@EliahKagan shared that he was able to get it to work with after setting core.symlinks = true

Yes, I have found that and some variations (one that I prefer is detailed below) to work for me.

This variable is false by default in Windows (it is changed to true explicitly for the GitHub-hosted GHA runners, which is why git clones symlinks there) because symlink creation is traditionally a privileged operation on Windows, such that git will fail to create them if it is run without the necessary privilege.

These days, though, I think it's increasingly common that people creating symlinks on Windows have his privilege, even if not running as an administrator, because Windows 10 and higher have a "developer mode" setting that Microsoft suggests using that includes this effect.

(Even if running as an administrator, UAC can get in the way of symlink creation, since commands like git and gh don't elevate to check out symlinks. So "developer mode" is not only for users running with limited accounts.)

on the user-level before cloning.

It can also be set as a local repository variable after cloning, if the clone is done without checking out a working tree. This is what I've used lately on Windows:

gh repo clone gitoxide -- --no-checkout
cd gitoxide
git config --local core.symlinks true
git switch main

gh repo clone could of course be replaced with git clone while still using this technique (the -- separator would be removed).

As of now, I don't have a fix except to give in and not use a symlink.

Would it be reasonable for the justfile to gain a target that fixes this up on Windows? If so, it could copy the files, or attempt to create symlinks and, if that fails, then copy the files. But I'm not sure this is at all the sort of thing that belongs there.

Edit: Added the missing git config command to actually have git make symlinks!

[Byron]

Thanks so much for chiming in - your account has much more detail than I could ever have produced.

The justfile unfortunately doesn't support the notion of groups or section at all so running just will only show a flat list of targets, sorted lexicographically. Having a target, despite not being very discoverable, would definitely help though and I would see myself using this (in my setup, symlinks won't ever work as the checkout is on the host with a 'shared' drive in the guest).

[EliahKagan]

I've noticed that the problem also happens--as one would expect--when attempting to install gitoxide globally (for use of the gix and ein commands) from source via cargo.

A workaround, which can be modified according to the specific cargo command one would have used, is:

1. Clone and check out the repository with symbolic links intact as described above. (On systems where that cannot be done, copying the files manually should work for this as for everything, but I have not tested that.)

2. From the cloned directory, build and install it:

   cargo install --path . --locked

I am curious if there are important differences in effect that I am unaware of and that should be noted. I am also curious if there is a single cargo command sufficient to achieve this, without previously cloning or changing core.symlinks to true for the user or otherwise globally.

[Byron]

I am curious if there are important differences in effect that I am unaware of and that should be noted. I am also curious if there is a single cargo command sufficient to achieve this, without previously cloning or changing core.symlinks to true for the user or otherwise globally.

Cargo does download a compressed tar archive with the source of each crate, with all files within the crate being created by following symlinks, effectively resolving them. Thus I would assume the following works on windows:

cargo install gitoxide

But cargo install --git https://github.com/Byron/gitoxide gitoxide most certainly won't work as symlinks might not be 'creatable' on Windows at all without the right permissions.

As Cargo uses libgit2 for cloning and checkout, but can hopefully sometime this year also use gitoxide for that, I could imagine implementing a special checkout mode which resolves symlinks manually. This would lead to a dirty worktree in the clone as a symlink was just replaced with a directory (its target), but Cargo wouldn't necessarily care about that I think.

I have thought about a solution to this a lot and thought maybe it would be best if there was a justfile target that calls a script or Rust program that copies all files from gix-packetline/src to gix-packetline-blocking/src and adds a comment at the top of each copy to indicate where it's coming from, and what to execute to update the copied file. That way, accidental edits can be avoided, while it's reasonably easy to update the copy. Lastly, a CI check could be added that executes the script to update, and validates that there is no change after that or else, fail, as there are edits to gix-packetline which haven't been brought to the copy.

All that sounds pretty straightforward to do, what do you think, @EliahKagan? There even is space for little utilities that can easily be run with cargo run -p gix-testtools, so one wouldn't have to write yet another shell script for it.

[EliahKagan]

Cargo does download a compressed tar archive with the source of each crate, with all files within the crate being created by following symlinks, effectively resolving them. Thus I would assume the following works on windows:

cargo install gitoxide

Yes, that is working for me Windows. That can't be made to install the version from the tip of the main branch in the GitHub repository, though, can it?

But cargo install --git https://github.com/Byron/gitoxide gitoxide most certainly won't work as symlinks might not be 'creatable' on Windows at all without the right permissions.

I would expect it to work if core.symlinks is turned on globally and the user has permissions to create them, but otherwise not. Does it actually work even less often than that?

I have thought about a solution to this a lot and thought maybe it would be best if there was a justfile target that calls a script or Rust program that copies all files from gix-packetline/src to gix-packetline-blocking/src and adds a comment at the top of each copy to indicate where it's coming from, and what to execute to update the copied file. That way, accidental edits can be avoided, while it's reasonably easy to update the copy.

This seems to me like a good idea, and it looks like all the files in there are .rs source code files, so it should be straightforward to add the correct kind of comment in the correct place (unlike if, for example, a test directory were also included with shell script fixture files with shebangs).

Lastly, a CI check could be added that executes the script to update, and validates that there is no change after that or else, fail, as there are edits to gix-packetline which
haven't been brought to the copy.

Ah, I see, the copy would actually be tracked in source control, as automatically generated files, and the justfile target would only have to be run after modifying the contents of gix-packetline/src. So this would replace the symlink entirely, and symlinks would from that point forward only be used for individual crates' license files (as they are).

Should this be a new CI check, or should the check go into the test suite in some way? Of course the tests should not modify tracked source code files, but the script/tool that generates the files could have a dry-run mode, which a test could use and check.

If it goes into the test suite, it technically applies across crates, but I wonder if it could just go in the gix-packetline tests, since git-packetline-blocking doesn't seem to have its own test directory anyway. (It seems to me that this is not really an integration test.)

Also, for comparing the files--and I believe this applies whichever approach is taken--do you have an opinion as to whether line ending mismatches, where lines of one file end in Unix-style "\n" and lines in the corresponding file end in Windows-style "\r\n", should be treated specially and permitted? (In some languages' source code files, the distinction matters when multiline string literals are involved. But it looks like this is not an issue for Rust raw string literals at least since rust-lang/rust#62865, so I feel like we could go either way.)

[Byron]

Cargo does download a compressed tar archive with the source of each crate, with all files within the crate being created by following symlinks, effectively resolving them. Thus I would assume the following works on windows:

cargo install gitoxide

Yes, that is working for me Windows. That can't be made to install the version from the tip of the main branch in the GitHub repository, though, can it?

No, sorry for the confusion. I mentioned it before I fully understood you were referring to the --git … install option, but left it because I thought it's good to have mentioned it after I asked myself what becomes of the symlink after publishing.

But cargo install --git https://github.com/Byron/gitoxide gitoxide most certainly won't work as symlinks might not be 'creatable' on Windows at all without the right permissions.

I would expect it to work if core.symlinks is turned on globally and the user has permissions to create them, but otherwise not. Does it actually work even less often than that?

Ah, you mean that also doesn't work while the core.symlinks flag is enabled? I wouldn't be surprised as libgit2 typically ignores most configuration flags, or maybe doesn't completely implement them. I'd expect gitoxide to be able to do that once checkout is also implemented. It's the track I am currently working on, git reset provides git checkout essentially. When done, it will be integrated into Cargo which should make this work.

This reminds me that you can tell cargo to use git when fetching, but I presume it would still not use git for the checkout, meaning symlinks probably still don't work.

I have thought about a solution to this a lot and thought maybe it would be best if there was a justfile target that calls a script or Rust program that copies all files from gix-packetline/src to gix-packetline-blocking/src and adds a comment at the top of each copy to indicate where it's coming from, and what to execute to update the copied file. That way, accidental edits can be avoided, while it's reasonably easy to update the copy.

This seems to me like a good idea, and it looks like all the files in there are .rs source code files, so it should be straightforward to add the correct kind of comment in the correct place (unlike if, for example, a test directory were also included with shell script fixture files with shebangs).

Yes, src/ should be enough, but it's probably a good idea to have the tool fail if it encounters unexpected files.

Lastly, a CI check could be added that executes the script to update, and validates that there is no change after that or else, fail, as there are edits to gix-packetline which
haven't been brought to the copy.

Ah, I see, the copy would actually be tracked in source control, as automatically generated files, and the justfile target would only have to be run after modifying the contents of gix-packetline/src. So this would replace the symlink entirely, and symlinks would from that point forward only be used for individual crates' license files (as they are).

Yes, the current symlink at gix-packetline-blocking would be removed in favor of 'managed copies'. Writing the tool for that in Rust and integrating it into the justfile seems like a nice warmup activity with high value attached to it - this symlink simply has to go.

Should this be a new CI check, or should the check go into the test suite in some way? Of course the tests should not modify tracked source code files, but the script/tool that generates the files could have a dry-run mode, which a test could use and check.

If it goes into the test suite, it technically applies across crates, but I wonder if it could just go in the gix-packetline tests, since git-packetline-blocking doesn't seem to have its own test directory anyway. (It seems to me that this is not really an integration test.)

I would go with a CI-only test that can be a simple as executing the just target, as I found myself never executing all tests locally anymore before pushing. The reason is… they take too long, and for most tasks there is no intricate interdependencies to worry about so running specific tests locally is a good indication of expected success.
It's also special-case enough to keep it simpler, by not having to worry about a a local dry-run mode.

Also, for comparing the files--and I believe this applies whichever approach is taken--do you have an opinion as to whether line ending mismatches, where lines of one file end in Unix-style "\n" and lines in the corresponding file end in Windows-style "\r\n", should be treated specially and permitted? (In some languages' source code files, the distinction matters when multiline string literals are involved. But it looks like this is not an issue for Rust raw string literals at least since rust-lang/rust#62865, so I feel like we could go either way.)

I'd not worry about this as Git will do so for you. The idea is to execute the just target to update the gix-packetline-blocking source - it should be reasonably safe to do that by checking in advance that there are no local modifications there. Then the update should run, and afterwards, there is still supposed to be no local modification. As the tool only copies, it should be fairly straightforward to deduct the line-ending style. But you are right, the style needs to be deducted to match the rest of the file.

I don't know what the string literals would have to do with it though, my assumption was that the header of each file would be prefixed with something like:

//! DO NOT EDIT - this is a copy of <worktree-relative/path/to/src/*.rs>. Run `just <the-target>` to update it. <NL>

[EliahKagan]

I would expect it to work if core.symlinks is turned on globally and the user has permissions to create them, but otherwise not. Does it actually work even less often than that?

Ah, you mean that also doesn't work while the core.symlinks flag is enabled? I wouldn't be surprised as libgit2 typically ignores most configuration flags, or maybe doesn't completely implement them. I'd expect gitoxide to be able to do that once checkout is also implemented. It's the track I am currently working on, git reset provides git checkout essentially. When done, it will be integrated into Cargo which should make this work.

It does work when core.symlinks is set to true and the user is capable of creating symlinks. I had meant I expected it would work in that situation. I've checked and verified that it does.

A notable exception is that if previously attempted unsuccessfully, such as with core.symlinks set to false, the cached clone will be automatically reused, resulting in the same failure again. This can be solved by clearing that out, such as with cargo cache -a.

Yes, the current symlink at gix-packetline-blocking would be removed in favor of 'managed copies'. Writing the tool for that in Rust and integrating it into the justfile seems like a nice warmup activity with high value attached to it - this symlink simply has to go.

I would be pleased to do this.

I'm a bit worried about the case where gix-packetline-blocking/src has for some reason been deleted altogether and the tool is run. It feels like the tool should work in such a case. That is the natural situation in the first ever run of the tool (though working around it that one time is of course not too much trouble). It also seems like it could come up in the future.

But if it is implemented as a subcommand in tests/tools/src/main.rs, I believe that will not be supported, at least with the convenient cargo run syntax for running it, because cargo relies on the entire project having the appropriate structure already:

> cargo run -p gix-testtools copy-packetline
error: failed to load manifest for workspace member `C:\Users\ek\source\repos\gitoxide\gix-diff`

Caused by:
  failed to load manifest for dependency `gix-filter`

Caused by:
  failed to load manifest for dependency `gix-packetline-blocking`

Caused by:
  failed to parse manifest at `C:\Users\ek\source\repos\gitoxide\gix-packetline-blocking\Cargo.toml`

Caused by:
  can't find library `gix-packetline-blocking`, rename file to `src/lib.rs` or specify lib.path

(This happens before the copy-packetline subcommand is handled and is independent of what subcommand is used or even of whether or not a subcommand name is passed.)

If it is instead implemented as a shell script, then that is avoided.

But a shell script may have rarer but weirder failure modes on Windows where the interpreter used to run it is unexpected. But then again maybe that would already be revealed by any use of just in such a situation. Anyway, I am inclined to think it's reasonable for this to be implemented in Rust at least initially.

It's also special-case enough to keep it simpler, by not having to worry about a a local dry-run mode.

Yes, that's true. After posting my previous comment, I thought a bit about a dry-run mode and realized that it may either constrain the approach taken or complicate the implementation.

I don't know what the string literals would have to do with it though

To clarify, I wasn't thinking about the material added to the top of the file, but to the issue of line endings changing (even perhaps due to local git configuration) inside the file. The situation that had briefly concerned me was

let s = r#"first line
second line"

because in some languages such literals would be different depending on the line ending "physically" appearing inside the raw string. But that's not the case in Rust, so I'm not worried about it. I only mentioned that to say why I wasn't worried about it--sorry about the confusion there.

[Byron]

A notable exception is that if previously attempted unsuccessfully, such as with core.symlinks set to false, the cached clone will be automatically reused, resulting in the same failure again. This can be solved by clearing that out, such as with cargo cache -a.

That is interesting! From memory I can tell that indeed, git dependencies have no notion of cache-invalidation beyond the repository being updated on the remote.

I would be pleased to do this.

Great!

And it's true, there is a bootstrap issue that I didn't think of. Maybe the copy should be done by a script and in a clean way: assure destination isn't dirty, delete the destination, copy the files over to the destination, and then run the tool on the destination folder insert the docstrings.

And even thought this involves a shell script, this approach seems to have none of the issues. Also, I'd feel particularly comfortable about the said shell script knowing that you are writing it.