lib: remove usage of winreg crate

[thoughtpolice]

This pulls in a whole library just for what is morally 3 function calls and can be done easily in the Windows API. Remove it.

This uses unsafe; the usage of windows-rs might be more preferrable since we already (and will likely always) depend on it, but it would probably remain unsafe...

Checklist

If applicable:

• I have updated CHANGELOG.md
• I have updated the documentation (README.md, docs/, demos/)
• I have updated the config schema (cli/src/config-schema.json)
• I have added tests to cover my changes

[thoughtpolice]

I don't know if we actually want this change but I was taking a first crack at thinning up the dependency tree a little and seeing what impacts the features have, how much we actually need the crate, etc.

[martinvonz]

How much smaller does the binary become? How much faster does the build become?

[emilazy]

I would much prefer windows-rs over this, but I am not sure it is worth it anyway. winreg does not itself have any dependencies that we would be able to remove by dropping it, and outsourcing the Windows domain knowledge and verification of the unsafe stuff to them feels worth it. Maybe the unsafe stuff would be sufficiently simple with windows-rs that I’d feel differently, but I lean towards this not being a fruit worth picking.

If we only use it for check_symlink_support, can’t we just do a feature test instead – try to create a symlink and see if it works? Looking at a registry entry that might not correlate perfectly with our ability to do symlinks seems worth avoiding anyway.

[thoughtpolice]

My main reasoning was simpler than binary size or anything: we just literally don't need this crate, and implementing this is very straightforward. For something that would see continuous evolution I can see a lot of value in a crate. Standard formats or protocols, performance sensitive stuff, certain kinds of 3rd party integrations. If you're going to write lots of Windows-related code, these crates also have high value. But we are testing one thing, and these APIs are incredibly stable and have remained almost unchanged (the parts we're using, anyway) for nearly 30 years at this point. No, it's not a huge dent. But at the end of the day we're bringing in a whole crate for like, what, 9 lines of code that would work as far back as Windows 95?

As far as the compatibility check goes, my understanding is that it's more or less correct; Windows 10 has had symlinks-for-devmode on for at least a couple years at this point so they are basically synonymous. I suppose we could also try just creating them and handling the error too, but I haven't thought to deeply about it.

---

Mostly unrelated, there was a similar example I had recently in a private repository where I ended up vendoring some rust crates to make some small mods, and out of curiosity I looked up the final transitive line counts, and it was nearly 50,000 lines of code, all because it used a proc macro (and thus pulled in syn and quote, etc). The actual crate itself was like 4,000 lines, and the proc-macroverse crates were about 35,000 LOC IIRC. That macro was basically used only to derive some accessors for a flag that was defined like FLAG_FEATURE_XYZ_EXISTS = 0x123. So, I had to compile almost 40,000 lines of code to implement the moral equivalent of const FLAG_EXISTS: u64 = 0x123; or whatever in a single module. That is the kind of thing I think we should try to avoid.

[thoughtpolice]

(And to be clear I don't think we have to merge this, the alternative suggested by Emily might be just as good. But I think this is a good example of where I differ from a lot of people on this topic; this is the kind of stuff that I think we can and should evaluate and take some responsibility for and be just willing to decide: yes, it's a little extra work, but we don't need that.)

[emilazy]

As far as the compatibility check goes, my understanding is that it's more or less correct; Windows 10 has had symlinks-for-devmode on for at least a couple years at this point so they are basically synonymous. I suppose we could also try just creating them and handling the error too, but I haven't thought to deeply about it.

I think that the toggle determinates whether non‐administrators can create symlinks, though. I think administrators can do it regardless and there’s some specific permission you can assign even without developer mode. Whether we’d want to take advantage of running as an administrator in that way, I don’t know, but we’re at least not quite checking what the function name implies.

FWIW, I’d be more inclined to just bite the bullet on the dependency here if not for the unsafe, or if the crate was bigger, or pulled in other new dependencies. In this case the crate is less than 2,000 lines of Rust, some of which is serde stuff that we’re not pulling in, and the dependencies are cfg-if and windows-sys, neither of which I see leaving our graph any time soon. So it seems low‐cost as dependencies go to me.

But probably just doing an actual runtime check would be the best of both worlds? The ability to create them might also depend on the filesystem, not just the OS. Like the samefile checks we do to detect case‐insensitivity clashes on the specific FS we’re operating on, it seems like we’d be better off just always trying to create symlinks and then falling back if we can’t (with a cache, if it’s performance‐relevant to keep trying somehow).

[thoughtpolice]

I think that the toggle determinates whether non‐administrators can create symlinks, though. I think administrators can do it regardless and there’s some specific permission you can assign even without developer mode. Whether we’d want to take advantage of running as an administrator in that way, I don’t know, but we’re at least not quite checking what the function name implies.

Doesn't modern Windows even have "adminless" mode? Practically speaking they've done insane amounts to basically mean you never need it. I just don't think this case is worth worrying about, but if it is, it's like 9 more lines of code to check if you're running with NTSYSTEM privileges...

FWIW, I’d be more inclined to just bite the bullet on the dependency here if not for the unsafe, or if the crate was bigger, or pulled in other new dependencies. In this case the crate is less than 2,000 lines of Rust, some of which is serde stuff that we’re not pulling in, and the dependencies are cfg-if and windows-sys, neither of which I see leaving our graph any time soon. So it seems low‐cost as dependencies go to me.

This is kind of the thing I want to push back against, though. Like this idea "We already have 5,000 lines of dependencies that are sticking around, so just throw like 2,000 more on top so you can reduce 9 lines of code to 3 lines of code" is exactly what I'm not a fan of. It's like a reverse form of code golfing. Just using the lines of code isn't a perfect metric, but reducing the dependency footprint is a process that has to be continuously applied, not just given free passes. You don't get to skip leg day.

Part of the problem is really structural and unsolvable in a broad sense and that's due to Rust lacking some features needed to make crate graphs more modular, and that is largely due to trait impl coherence requirements. Haskell had this same problem (and Backpack, despite being good was too late to fix it, so it's probably too late for Rust to ever fix it at this rate so we're fucked forever I guess.) Like we're never going to be able to get rid of stuff like rand or whatever, but I can live with that. But this stuff? I mean, this is just... not a necessary dependency...

But probably just doing an actual runtime check would be the best of both worlds? The ability to create them might also depend on the filesystem, not just the OS. Like the samefile checks we do to detect case‐insensitivity clashes on the specific FS we’re operating on, it seems like we’d be better off just always trying to create symlinks and then falling back if we can’t (with a cache, if it’s performance‐relevant to keep trying somehow).

Frankly I'd almost rather do it the other way around: require developer mode and just require tested ReFS/NTFS for the JJ repo, but that isn't feasible practically speaking because, among other things, in this particular case BOFH's in your IT department will make up a reason why Johnanathan McProgrammer doesn't deserve it or something and also their job is too hard and they're too busy to click 4 buttons in their MDM setup (don't ask about the specifics of this scenario.) I have more thoughts about this particular setup, but I catch your drift here. If we want to get rid of it that way I'm happy to file a different change.