Skip to content

aws-cloudformation-languageserver: init at 1.7.0#489310

Open
mbarneyjr wants to merge 2 commits into
NixOS:masterfrom
mbarneyjr:add-aws-cloudformation-languageserver
Open

aws-cloudformation-languageserver: init at 1.7.0#489310
mbarneyjr wants to merge 2 commits into
NixOS:masterfrom
mbarneyjr:add-aws-cloudformation-languageserver

Conversation

@mbarneyjr

Copy link
Copy Markdown
Contributor

This inits aws-cloudformation-languageserver at version 1.4.0 (latest as of creating this PR).

The AWS CloudFormation LanguageServer is an LSP server that works on CloudFormation files. I've been using this (packaged almost identically) in my personal Neovim configuration successfully.

Other notes:

  • The actual INSTALLATION.md guide recommends unzipping the release archive and running node /path/to/install-location/cfn-lsp-server-standalone.js --stdio. This derivation uses makeWrapper to use node from nixpkgs to run the cfn-lsp-server-standalone.js file.
  • There is an update.sh script to update the sources.json that the package.nix uses to pass the hashes into fetchzip and version to mkDerivation, etc. I've pointed passthru.updateScript to it too.

This is my first new package contribution, if I'm missing anything I'm more than happy to receive any feedback!

Things done

  • Built on platform:
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested, as applicable:
  • Ran nixpkgs-review on this PR. See nixpkgs-review usage.
  • Tested basic functionality of all binary files, usually in ./result/bin/.
  • Nixpkgs Release Notes
    • Package update: when the change is major or breaking.
  • NixOS Release Notes
    • Module addition: when adding a new NixOS module.
    • Module update: when the change is significant.
  • Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.

@nixpkgs-ci nixpkgs-ci Bot added 8.has: package (new) This PR adds a new package 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 9.needs: reviewer This PR currently has no reviewers requested and needs attention. 8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` labels Feb 11, 2026
@mbarneyjr

Copy link
Copy Markdown
Contributor Author

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 489310
Commit: 1e3d9fdb0f42b34140d074a1866466eec8db9d90


aarch64-darwin

✅ 1 package built:
  • aws-cloudformation-languageserver

@nixos-discourse

Copy link
Copy Markdown

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/6506

@SuperSandro2000 SuperSandro2000 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why are we not building the software from source?

Comment thread pkgs/by-name/aw/aws-cloudformation-languageserver/update.sh Outdated
Comment thread pkgs/by-name/aw/aws-cloudformation-languageserver/update.sh Outdated
Comment thread pkgs/by-name/aw/aws-cloudformation-languageserver/package.nix Outdated
Comment thread pkgs/by-name/aw/aws-cloudformation-languageserver/package.nix Outdated
Comment thread pkgs/by-name/aw/aws-cloudformation-languageserver/package.nix Outdated
Comment thread pkgs/by-name/aw/aws-cloudformation-languageserver/package.nix Outdated
@nixpkgs-ci nixpkgs-ci Bot removed the 9.needs: reviewer This PR currently has no reviewers requested and needs attention. label Mar 5, 2026
@mbarneyjr

Copy link
Copy Markdown
Contributor Author

Why are we not building the software from source?

This is the method documented in the aws-cloudformation-languageserver repository to install it: INSTALLATION.md

Thanks for the review! I'll incorporate your suggestions into this PR

@mbarneyjr mbarneyjr force-pushed the add-aws-cloudformation-languageserver branch from 1e3d9fd to fbf5f72 Compare March 7, 2026 00:56
@mbarneyjr

Copy link
Copy Markdown
Contributor Author

I've incorporated all of the suggested changes, it builds and the update script still runs. Let me know if there's anything else you'd like me to change, thanks again! 🙏

@mbarneyjr mbarneyjr force-pushed the add-aws-cloudformation-languageserver branch from fbf5f72 to 7a447ef Compare March 7, 2026 01:13
@SuperSandro2000

Copy link
Copy Markdown
Member

This is the method documented in the aws-cloudformation-languageserver repository to install it: INSTALLATION.md

Yeah, because building it from source for the average consumer is probably a bit overkill and takes a bit to long, but we, as the distro, could do it, right? Can you try that? If it doesn't work out we can keep the binary download but I think we should really try it at least.

Comment thread pkgs/by-name/aw/aws-cloudformation-languageserver/package.nix
@mbarneyjr mbarneyjr force-pushed the add-aws-cloudformation-languageserver branch from 7a447ef to 80a13ca Compare March 8, 2026 00:01
@mbarneyjr

Copy link
Copy Markdown
Contributor Author

Can you try that? If it doesn't work out we can keep the binary download but I think we should really try it at least.

That makes sense to me, I'll give it an effort to build from source as part of the derivation and keep you posted

@mbarneyjr mbarneyjr force-pushed the add-aws-cloudformation-languageserver branch from 80a13ca to 90589cf Compare March 8, 2026 19:31
@mbarneyjr

mbarneyjr commented Mar 8, 2026

Copy link
Copy Markdown
Contributor Author

The current commit has a build-from-source approach. The upstream project build involves a webpack config that tries to run its own npm ci command to a temp node_modules directory that would break when running in the sandbox, so I've patched the webpack config to skip that step and just copy the node_modules that gets produced from buildNpmPackage itself. The build-from-source approach makes this package more accessible to those that prefer/require fromSource, but the upstream build process is a bit atypical so this package isn't as "pure" to the upstream in that sense. I'm happy to maintain the package under either approach that we feel would be best for nixpkgs

I've tested the build and it still seems to function on my macos and nixos machines, verified with my neovim config running it

(if we're happy with the from-source approach I'll remove the update.sh and sources.json)

@mbarneyjr

Copy link
Copy Markdown
Contributor Author

@SuperSandro2000 I don't mean to bug you, are you able to continue reviewing this?

@nixos-discourse

Copy link
Copy Markdown

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/6808

Comment thread pkgs/by-name/aw/aws-cloudformation-languageserver/sources.json Outdated
Comment thread pkgs/by-name/aw/aws-cloudformation-languageserver/update.sh Outdated
Comment thread pkgs/by-name/aw/aws-cloudformation-languageserver/package.nix
@mbarneyjr mbarneyjr force-pushed the add-aws-cloudformation-languageserver branch 2 times, most recently from 93ed47e to 2c31944 Compare April 29, 2026 00:27
@mbarneyjr

Copy link
Copy Markdown
Contributor Author

Fixed the __structuredAttrs issue from the CI check

@mbarneyjr mbarneyjr force-pushed the add-aws-cloudformation-languageserver branch from 2c31944 to 7c997ec Compare June 2, 2026 17:23
@mbarneyjr mbarneyjr changed the title aws-cloudformation-languageserver: init at 1.4.0 aws-cloudformation-languageserver: init at 1.7.0 Jun 2, 2026
@mbarneyjr mbarneyjr force-pushed the add-aws-cloudformation-languageserver branch 2 times, most recently from 66db536 to 1c96c08 Compare June 2, 2026 17:45
@mbarneyjr

Copy link
Copy Markdown
Contributor Author

Updated to CloudFormation LanguageServer 1.7.0, rebased master to resolve conflicts, and tested to make sure it still works

@mbarneyjr mbarneyjr force-pushed the add-aws-cloudformation-languageserver branch 2 times, most recently from 4183807 to d655cf8 Compare June 19, 2026 03:13
@mbarneyjr

Copy link
Copy Markdown
Contributor Author

Per a review I've gotten on a separate PR:

The finalAttrs pattern is recommended for all new packages

we can use tag here for a slightly more efficient download from GitHub

updating this package to use finalAttrs instead of rec and tag instead of rev

@mbarneyjr mbarneyjr force-pushed the add-aws-cloudformation-languageserver branch from d655cf8 to f7495d0 Compare June 19, 2026 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

8.has: maintainer-list (update) This PR changes `maintainers/maintainer-list.nix` 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants