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

vale-ls: init at 0.3.7-unstable-2024-03-13 #304375

Merged
merged 4 commits into from
May 2, 2024
Merged

Conversation

jansol
Copy link
Contributor

@jansol jansol commented Apr 15, 2024

Description of changes

Supersedes #288450 which seems to have stalled. This addresses most of the comment left in the old PR by @oliviacrain . I didn't find any decent way to pass the installVale init parameter -- it seems that has to be sent by the LSP client as part of the handshake process? Either way, although the documentation says it's enabled by default, the actual code seems to never initialize the field, meaning it is None which is treated as disabled?

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Apr 15, 2024
@ofborg ofborg bot requested a review from foo-dogsquared April 15, 2024 21:50
@oliviacrain
Copy link
Contributor

Will find time to review this later today, but at first glance, it looks like all my feedback has been addressed!

@jansol jansol force-pushed the vale-ls branch 2 times, most recently from a854fc4 to 54dfd83 Compare April 20, 2024 22:17
Copy link
Contributor

@oliviacrain oliviacrain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested with helix on aarch64-linux, appears to work for me! Thanks for picking this up!

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/1600

pkgs/by-name/va/vale-ls/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/va/vale-ls/package.nix Outdated Show resolved Hide resolved
pkgs/by-name/va/vale-ls/package.nix Outdated Show resolved Hide resolved
@Aleksanaa
Copy link
Member


Running phase: installPhase
Running phase: fixupPhase
shrinking RPATHs of ELF executables and libraries in /nix/store/gwq9a7lqlm8i190bhlszindvibjr5xwb-vale-ls-0.3.7-unstable-2024-03-13-vendor.tar.gz
checking for references to /build/ in /nix/store/gwq9a7lqlm8i190bhlszindvibjr5xwb-vale-ls-0.3.7-unstable-2024-03-13-vendor.tar.gz...
patching script interpreter paths in /nix/store/gwq9a7lqlm8i190bhlszindvibjr5xwb-vale-ls-0.3.7-unstable-2024-03-13-vendor.tar.gz
error: hash mismatch in fixed-output derivation '/nix/store/mixk6dkkv8013hv0lh8plbsbrz4lfl2f-vale-ls-0.3.7-unstable-2024-03-13-vendor.tar.gz.drv':
         specified: sha256-JDsveGMQaDLFOba6oKYdm14IYDUf4+FIG4dPm09zWog=
            got:    sha256-ifKdSTmVWfDZF5Kn9b5JpzDxa160oRTfzjvxeL9POBg=
error: 1 dependencies of derivation '/nix/store/ilaza77a8zxradx3hsdizfkpj4gxsflc-vale-ls-0.3.7-unstable-2024-03-13.drv' failed to build

@ofborg build vale-ls

@jansol
Copy link
Contributor Author

jansol commented May 1, 2024

Getting the same result here, I wonder if something got yanked & updated from crates.io? Or does the nix package version change also affect cargoHash?

@Aleksanaa
Copy link
Member

Getting the same result here, I wonder if something got yanked & updated from crates.io?

Maybe some upstream did something very unscrupulous like moving a tag.

Or does the nix package version change also affect cargoHash?

That's unlikely to happen, don't worry

@jansol
Copy link
Contributor Author

jansol commented May 1, 2024

(Rebased on latest master)

Okay this is interesting. When I build from my local copy I get the cargoHash specified in the derivation, but when I do nix build github:jansol/nixpkgs/vale-ls#vale-ls I get the one in your error message.

jansol and others added 2 commits May 1, 2024 21:10
@jansol
Copy link
Contributor Author

jansol commented May 1, 2024

Nvm looks like the hash change was indeed caused by addressing the review comments (which I'd forgot to pull into my local branch). Now they match up as expected.

@jansol jansol changed the title vale-ls: init at unstable-2024-03-13 vale-ls: init at 0.3.7-unstable-2024-03-13 May 1, 2024
@SuperSandro2000 SuperSandro2000 merged commit 18b0191 into NixOS:master May 2, 2024
25 of 26 checks passed
@jansol jansol deleted the vale-ls branch May 2, 2024 09:45
pull bot pushed a commit to auxolotl/nixpkgs that referenced this pull request May 2, 2024
@jansol jansol mentioned this pull request May 14, 2024
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants