-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Binary is not being patched on NixOS in the latest nightly #11695
Comments
What do you mean by regression? That code has been like this for almost |
IIRC NixOS detection were at first detecting /etc/nixos folder (which is cursed as well) |
Yeah, but that was 15 months ago. Maybe you updated your distro (is that a thing on Nix?) and they added this quotes recently. |
That's probable const idString = contents.split('\n').find((a) => a.startsWith("ID="))
return idString?.toLowerCase()?.indexOf("nixos") !== -1; should probably work? |
We could also look for the three ways of spelling it. Is that file even a shell script? |
It looks like it's a bit simpler than that https://www.linux.org/docs/man5/os-release.html |
I've tested it with the patch I suggested above — detection works correctly with it. |
11696: editors/code: fix nixos detection r=lnicola a=cab404 Problem: NixOS started using quotes around it's id field in /etc/os-release Solution: Parially parsing os-release, and detecting, whether `nixos` appears anywhere in "ID=" field\ See #11695 Closes #11695 Co-authored-by: Vladimir Serov <me@cab404.ru>
@lnicola This is cursed, and should never have been accepted:
https://github.com/rust-analyzer/rust-analyzer/blob/0182f7451600e1700873549c324f7d13585547f4/editors/code/src/main.ts#L272
This is a serious regression in nightly and it's important to fix it before the next release.
@matklad, please take a look.
The text was updated successfully, but these errors were encountered: