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

dprint-plugins: init #369415

Merged
merged 1 commit into from
Jan 2, 2025
Merged

dprint-plugins: init #369415

merged 1 commit into from
Jan 2, 2025

Conversation

phanirithvij
Copy link
Member

The plan is to keep track of official dprint plugins here, utilising nixpkgs' update infra. see numtide/treefmt-nix#278 (comment)

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/)
  • 25.05 Release Notes (or backporting 24.11 and 25.05 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.

cc: @zimbatm @kachick

@phanirithvij phanirithvij force-pushed the dprint-plugins branch 4 times, most recently from df2add1 to b96de3e Compare December 30, 2024 21:24
@phanirithvij
Copy link
Member Author

phanirithvij commented Dec 30, 2024

dprint requires the .wasm at the end. if not, it is asking for a checksum along with the file path eg. /nix/store/63fgj93wdnkav6nn266cvg5yy899ccr3-g-plane-pretty_graphql-0.2.1@aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa...

$ nix build -f default.nix dprint-plugins --print-out-paths --no-link
/nix/store/63fgj93wdnkav6nn266cvg5yy899ccr3-g-plane-pretty_graphql-0.2.1.wasm
/nix/store/091iqd1y4rs0ma8q3h601b5wjwdzd3ml-dprint-plugin-jupyter-0.1.5.wasm
/nix/store/6ps6p7jrpkhfsqdljhbln6x7lpq0mqf3-dprint-plugin-biome-0.7.1.wasm
/nix/store/9kvjnc47sd1hcrs1fy8sb11lijk2rs8j-dprint-plugin-toml-0.6.3.wasm
/nix/store/1zjqlkb8rzp1gpx66n3n310jpc9isg8l-dprint-plugin-json-0.19.4.wasm
/nix/store/bwrzsr7kb2rwggpysxi9vl0h1qkz3v7h-g-plane-markup_fmt-0.18.0.wasm
/nix/store/g2niwwr2ygpdkkzsijfj33napxzakgs9-dprint-plugin-markdown-0.17.8.wasm
/nix/store/i2ngikhn7zlayjbh4fn6ibfpg49ghr4p-g-plane-pretty_yaml-0.5.0.wasm
/nix/store/gsc5xd8vylrnvnsvr71rq8b6qbvdilh0-dprint-plugin-dockerfile-0.3.2.wasm
/nix/store/0h56sj4kmyxl6vb4ahfw21fn8cpc63dd-g-plane-malva-0.11.1.wasm
/nix/store/ygkhhlpl0yxf2jymnkcpksblzpfnlxzp-dprint-plugin-typescript-0.93.3.wasm
/nix/store/j5aqsds6bjywx9fsknfak6wzg89cclg0-dprint-plugin-ruff-0.3.9.wasm

@phanirithvij phanirithvij changed the title dprint-plugins: init WIP dprint-plugins: init Dec 30, 2024
@phanirithvij

This comment was marked as outdated.

@kachick
Copy link
Member

kachick commented Dec 31, 2024

In my understanding, dprint and the plugins have WASM schema versions. If there is a mismatch between them, dprint CLI does not run the plugin.

As far as I know, it occurs when dprint < 0.47 and the plugin uses the crate dprint-core >= 0.67.
For example, when using an old dprint CLI and several latest plugins.

Use https://github.com/dprint/dprint/releases/download/0.46.3/dprint-x86_64-unknown-linux-gnu.zip

With dprint.json as below

{
 "biome": {
 },
 "plugins": [
   "https://github.com/dprint/dprint-plugin-biome/releases/download/0.7.1/plugin.wasm"
 ]
}

Then

> ./dprint check
Error resolving plugin https://github.com/dprint/dprint-plugin-biome/releases/download/0.7.1/plugin.wasm: Invalid schema version: 4 -- Expected: 3. Upgrade your dprint CLI (/tmp/tmp.0KZQ0xdk2Y/dprint).

As far as I know, the latest dprint CLI version 0.48.0 supports old schema versions. However, I think it is not a guaranteed specification.

How to keep the compatibility

A version is listed at https://github.com/dprint/plugins/blob/cc8cd4c35bb0685b0a599b8d3c30923e8f41b0a0/info.json#L2-L3, but it does not mean the JSON result always points correct schema versions.

So I suggest it. Adding a test that includes all packaged plugins in the dprint.json enhances the stability of the dprint package in combination with dprint-plugins.
Ideal state is one where an update-script updates the dprint and the plugins together, but I don't know how to realize it in nixpkgs.

@kachick
Copy link
Member

kachick commented Dec 31, 2024

So I suggest it. Adding a test that includes all packaged plugins in the dprint.json enhances the stability of the dprint package in combination with dprint-plugins.

Or would injecting the dprint CLI package for each plugin package's checkPhase be more fluent?

@phanirithvij phanirithvij force-pushed the dprint-plugins branch 2 times, most recently from e1048df to 66004d1 Compare December 31, 2024 08:01
@github-actions github-actions bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux and removed 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 labels Dec 31, 2024
@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 and removed 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux labels Dec 31, 2024
@phanirithvij phanirithvij force-pushed the dprint-plugins branch 2 times, most recently from bff840d to 88475cf Compare December 31, 2024 19:29
@phanirithvij phanirithvij requested a review from zimbatm December 31, 2024 19:30
@phanirithvij
Copy link
Member Author

phanirithvij commented Dec 31, 2024

@kachick I will look into adding compatibility tests once the code generation part is finalised

Copy link
Member

@zimbatm zimbatm left a comment

Choose a reason for hiding this comment

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

Thanks @phanirithvij , great work.

I would suggest leaving the schema compatibility issue for later, if it becomes a problem. We should also be mindful of not extending the scope of the PR too much. But up to you @phanirithvij .

@phanirithvij
Copy link
Member Author

Agreed, I will do that in a follow-up PR when I find the time.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Jan 1, 2025
Copy link
Member

@kachick kachick left a comment

Choose a reason for hiding this comment

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

Thanks for your work 👍

I don't have enough knowledge to review these Python code and usecase in treefmt-nix. However this discussion sounds good to me. 🙇

nixpkgs-review result

Generated using nixpkgs-review.

Command: nixpkgs-review pr 369415


x86_64-linux

✅ 12 packages built:
  • dprint-plugins.dprint-plugin-biome
  • dprint-plugins.dprint-plugin-dockerfile
  • dprint-plugins.dprint-plugin-json
  • dprint-plugins.dprint-plugin-jupyter
  • dprint-plugins.dprint-plugin-markdown
  • dprint-plugins.dprint-plugin-ruff
  • dprint-plugins.dprint-plugin-toml
  • dprint-plugins.dprint-plugin-typescript
  • dprint-plugins.g-plane-malva
  • dprint-plugins.g-plane-markup_fmt
  • dprint-plugins.g-plane-pretty_graphql
  • dprint-plugins.g-plane-pretty_yaml

x86_64-darwin

✅ 12 packages built:
  • dprint-plugins.dprint-plugin-biome
  • dprint-plugins.dprint-plugin-dockerfile
  • dprint-plugins.dprint-plugin-json
  • dprint-plugins.dprint-plugin-jupyter
  • dprint-plugins.dprint-plugin-markdown
  • dprint-plugins.dprint-plugin-ruff
  • dprint-plugins.dprint-plugin-toml
  • dprint-plugins.dprint-plugin-typescript
  • dprint-plugins.g-plane-malva
  • dprint-plugins.g-plane-markup_fmt
  • dprint-plugins.g-plane-pretty_graphql
  • dprint-plugins.g-plane-pretty_yaml

aarch64-darwin

✅ 12 packages built:
  • dprint-plugins.dprint-plugin-biome
  • dprint-plugins.dprint-plugin-dockerfile
  • dprint-plugins.dprint-plugin-json
  • dprint-plugins.dprint-plugin-jupyter
  • dprint-plugins.dprint-plugin-markdown
  • dprint-plugins.dprint-plugin-ruff
  • dprint-plugins.dprint-plugin-toml
  • dprint-plugins.dprint-plugin-typescript
  • dprint-plugins.g-plane-malva
  • dprint-plugins.g-plane-markup_fmt
  • dprint-plugins.g-plane-pretty_graphql
  • dprint-plugins.g-plane-pretty_yaml

pkgs/by-name/dp/dprint/package.nix Outdated Show resolved Hide resolved
fetchurl {
inherit hash url;
name = "${pname}-${version}.wasm";
meta.description = description;
Copy link
Member

Choose a reason for hiding this comment

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

Don't we need to set license for each plugin?

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

as of now, all the official plugin repos have mit license (even g-plane/*), so I added it as a default argument.

Copy link
Member Author

@phanirithvij phanirithvij Jan 2, 2025

Choose a reason for hiding this comment

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

Then, ideally won't we need to build the wasm binaries from source?

not for this pr, just saying.

Copy link
Member

@kachick kachick Jan 2, 2025

Choose a reason for hiding this comment

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

Then, ideally won't we need to build the wasm binaries from source?

Initially, I had the same question. However, as far as I know, at least for security reasons, the dprint CLI restricts behaviors for wasm plugins. This is a point that I prefer when selecting dprint WASM plugins for formatting.

https://github.com/dprint/dprint/blob/14eb074e9baf8d0533f04ad954e17a5a375e6385/crates/dprint/src/commands/formatting.rs#L2019

as of now, all the official plugin repos have mit license (even g-plane/*), so I added it as a default argument.

Sure. Bye the way, dprint project respects plugins license. I agree for that plugins may depend on the upstream crate license.

dprint license
https://github.com/dprint/dprint/blob/14eb074e9baf8d0533f04ad954e17a5a375e6385/docs/wasm-plugin-development.md?plain=1#L142

And I know a third-party plugin which choose another license of MIT https://github.com/RubixDev/dprint-plugin-stylua/blob/017360e78861dbef4d5c14087660525f9adc9f1a/LICENSE

Co-Authored-By: Jonas Chevalier <[email protected]>
Signed-off-by: phanirithvij <[email protected]>
@kachick kachick self-requested a review January 2, 2025 05:15
Copy link
Member

@kachick kachick left a comment

Choose a reason for hiding this comment

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

Thanks for update!

@phanirithvij phanirithvij added 12.approvals: 2 This PR was reviewed and approved by two reputable people and removed 12.approvals: 1 This PR was reviewed and approved by one reputable person labels Jan 2, 2025
@zimbatm zimbatm merged commit a5ca207 into NixOS:master Jan 2, 2025
24 of 25 checks passed
@phanirithvij phanirithvij deleted the dprint-plugins branch January 2, 2025 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 11-100 10.rebuild-linux: 11-100 12.approvals: 2 This PR was reviewed and approved by two reputable people
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants