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

en-croissant: init at 0.9.2 #271493

Closed
wants to merge 10 commits into from
Closed

en-croissant: init at 0.9.2 #271493

wants to merge 10 commits into from

Conversation

lcscosta
Copy link

@lcscosta lcscosta commented Dec 1, 2023

Description of changes

Added the en-croissant a Modern Chess Database .
This is an app built with Tauri.

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.

Priorities

Add a 👍 reaction to pull requests you find important.

@NixOSInfra NixOSInfra added the 12. first-time contribution This PR is the author's first one; please be gentle! label Dec 1, 2023

src = fetchurl {
url = "https://github.com/franciscoBSalgueiro/en-croissant/releases/download/v${version}/en-croissant_${version}_amd64.deb";
sha256 = "1lrl0pg0aihx87yqygfailbf9n9ra1l0a7zg40j36c92h8k0zmcb";

This comment was marked as outdated.

@ofborg ofborg bot added 8.has: package (new) This PR adds a new package 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 labels Dec 2, 2023
@lcscosta lcscosta changed the title en-croissant: init at 0.7.1 en-croissant: init at 0.8.0 Jan 11, 2024
@lcscosta
Copy link
Author

The CI is being extremely cruel to me, perhaps the reviewers can help me in this battle.


dontFixup = true;
outputHashMode = "recursive";
outputHash = "sha256-DTsT30o2BcXqqQW7uKtIexGoPT1KTxqPgDxOh+16olY=";
Copy link
Member

Choose a reason for hiding this comment

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

you should use something like fetchNpmDeps then you wont need to deal with fixed output derivations yourself

Copy link
Author

Choose a reason for hiding this comment

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

Hi @kirillrdy, we need to create a fetchPnpmDeps. Unfortunately, this project uses only pnpm as its node-modules manager. The nix-packages with pnpm as node manager will be a little bit hard to read/maintain with this necessary workaround for downloading node-modules.

Another examples with the same implementation logic:

pkgs/applications/misc/pot/default.nix
pkgs/applications/networking/geph/default.nix
pkgs/by-name/ve/vesktop/package.nix

Copy link
Member

Choose a reason for hiding this comment

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

ok i see

as you can see from ofborg, the hash is different. The difficulty with fixed output derivations is that they must always produce same output. I get same hash as ofborg ( this is on x86_64-linux ) we'll need to figure out why content is different and try to fix it.

Copy link
Author

Choose a reason for hiding this comment

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

ok i see

as you can see from ofborg, the hash is different. The difficulty with fixed output derivations is that they must always produce same output. I get same hash as ofborg ( this is on x86_64-linux ) we'll need to figure out why content is different and try to fix it.

Hmmmm, I believe that this problem are from the optional dependencies, like rollup and esbuild. I will work on a way to use the nodePackages.<package-name> in the build, and in this way we can ignore the download of optional dependencies with pnpm.

Copy link
Member

@Misterio77 Misterio77 left a comment

Choose a reason for hiding this comment

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

Builds as expected and the package seems to work nicely on my end.

image

No hash mismatch at all (tried both x86-64-linux and aarch64-linux):

$ nix build github:nixos/nixpkgs/pull/271493/head#en-croissant.pnpm-deps --print-out-paths 
/nix/store/59nsarxf4sly419as0b9sgd0fn2cvv8q-en-croissant-pnpm-deps-0.8.0

$ nix build github:nixos/nixpkgs/pull/271493/head#legacyPackages.aarch64-linux.en-croissant.pnpm-deps --print-out-paths
/nix/store/59nsarxf4sly419as0b9sgd0fn2cvv8q-en-croissant-pnpm-deps-0.8.0

Comment on lines 37741 to 37742
en-croissant = callPackage ../by-name/en/en-croissant/package.nix { };

Copy link
Member

Choose a reason for hiding this comment

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

AFAIK, you won't need this when using by-name

Copy link
Member

Choose a reason for hiding this comment

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

correct, this is not needed

@lcscosta
Copy link
Author

@ofborg build en-croissant

@kirillrdy
Copy link
Member

@lcscosta to get CI passing "pkgs/by-name" check you need to rebase on latest master

@lcscosta
Copy link
Author

@lcscosta to get CI passing "pkgs/by-name" check you need to rebase on latest master

@kirillrdy, I fixed the 'pkgs/by-name' task and am currently awaiting the evaluation by ofborg. @Misterio77 helped me in testing the hash mismatch (aarch64 not worked on his machine), I am not confident that I have completely resolved this problem. If the pnpm patch does not work, I plan to try removing the optional dependencies, as I mentioned in the comment above.

@kirillrdy
Copy link
Member

@lcscosta can you please either squash commits or update commit messages to match contribution guidelines ?

@Misterio77
Copy link
Member

Hey @lcscosta,

Getting hash mismatches in my aarch64 machine:

error: hash mismatch in fixed-output derivation '/nix/store/pnlis9cxbi5a0wbmybzrhf63r9cz9ism-en-croissant-pnpm-deps-0.8.0.drv':
         specified: sha256-VfX5333MYqjCI2+8cTlr6313TojDFSYlytYb0Us2olA=
            got:    sha256-H8oWgAo8TRkgNnmAAsV9923BbLiUbOAJoW7x/Vzml20=

nix-info:

  • system: "aarch64-linux"
  • host os: Linux 6.1.67, NixOS, 24.05 (Uakari), 24.05.20240108.317484b
  • multi-user?: yes
  • sandbox: yes
  • version: nix-env (Nix) 2.18.1

@kirillrdy
Copy link
Member

@Misterio77 if you have access to both fixed output derivations you can just diff -r them to see whats different

@Misterio77
Copy link
Member

Hey @lcscosta and @kirillrdy.

Here's the x86_64-linux -> aarch64-linux diff:

diff '--color=auto' -r /nix/store/ggnzrp25am692b887qcidb5bb7qbc6x2-en-croissant-pnpm-deps-0.8.0/v3/files/05/eaedbfcc4e89510bcf9ada0761655c33cab7e9bde0c4372600dfb156eba8da4b53df94136135f062a32cac3e7775db7983ea835a73c90286b2bcdbc3ea4f6b-index.json /nix/store/s8xvcm8n4dk2qi6p4hrpcr07k0sx1wkr-en-croissant-pnpm-deps-0.8.0/v3/files/05/eaedbfcc4e89510bcf9ada0761655c33cab7e9bde0c4372600dfb156eba8da4b53df94136135f062a32cac3e7775db7983ea835a73c90286b2bcdbc3ea4f6b-index.json
60c60
<     "linux-x64-node-v18-{\"/@fortawesome/fontawesome-common-types/6.4.0\":{}}": {
---
>     "linux-arm64-node-v18-{\"/@fortawesome/fontawesome-common-types/6.4.0\":{}}": {
diff '--color=auto' -r /nix/store/ggnzrp25am692b887qcidb5bb7qbc6x2-en-croissant-pnpm-deps-0.8.0/v3/files/1c/d8a2d77db17e89a0e50559c341f05d7a4ddb6b3b5411d006ff4278ebbdb5f26786c59035dda5993362f024ab9f0e41fb960e9b3bb55283c0cb1697b09e9181-index.json /nix/store/s8xvcm8n4dk2qi6p4hrpcr07k0sx1wkr-en-croissant-pnpm-deps-0.8.0/v3/files/1c/d8a2d77db17e89a0e50559c341f05d7a4ddb6b3b5411d006ff4278ebbdb5f26786c59035dda5993362f024ab9f0e41fb960e9b3bb55283c0cb1697b09e9181-index.json
30c30
<     "linux-x64-node-v18-{}": {
---
>     "linux-arm64-node-v18-{}": {
diff '--color=auto' -r /nix/store/ggnzrp25am692b887qcidb5bb7qbc6x2-en-croissant-pnpm-deps-0.8.0/v3/files/4c/a151a7d4f1ac90dd4567d2b12111284a2f9babf7d221e7ae395c1a12c13ad45e136f18a755520517cedc64da385328c6c35eaf1e138eb8734ae99af616bcdd-index.json /nix/store/s8xvcm8n4dk2qi6p4hrpcr07k0sx1wkr-en-croissant-pnpm-deps-0.8.0/v3/files/4c/a151a7d4f1ac90dd4567d2b12111284a2f9babf7d221e7ae395c1a12c13ad45e136f18a755520517cedc64da385328c6c35eaf1e138eb8734ae99af616bcdd-index.json
40c40
<     "linux-x64-node-v18-{\"/@esbuild/linux-arm64/0.17.6\":{},\"/@esbuild/linux-x64/0.17.6\":{}}": {
---
>     "linux-arm64-node-v18-{\"/@esbuild/linux-arm64/0.17.6\":{},\"/@esbuild/linux-x64/0.17.6\":{}}": {
52c52
<         "integrity": "sha512-20Aodjh/lyF1R1hZL2+UgmNTJFYS2QjdX2xzq+tfpAoUXFy3xAP9eXjU6k9TnYwo3/UUrGf1/3iJZQv0UgmjCQ==",
---
>         "integrity": "sha512-RKT4I4swW4DYYbzIMS7anpJ9G4YErmoIi498d4bFY8E4xTeyQp6mtzwpzS0OtKXeY7Kn2TcJTvLiFXXkLqvOTw==",
54c54
<         "size": 8994816
---
>         "size": 8323072
diff '--color=auto' -r /nix/store/ggnzrp25am692b887qcidb5bb7qbc6x2-en-croissant-pnpm-deps-0.8.0/v3/files/71/eab1a1e754adc6b287b63b657e8d75b6c3cc644e8b2541802e0fae225384129254f5a79c51d90247c8d65253f101643b01f8a8e4b6489912e30be91a5f01a4-index.json /nix/store/s8xvcm8n4dk2qi6p4hrpcr07k0sx1wkr-en-croissant-pnpm-deps-0.8.0/v3/files/71/eab1a1e754adc6b287b63b657e8d75b6c3cc644e8b2541802e0fae225384129254f5a79c51d90247c8d65253f101643b01f8a8e4b6489912e30be91a5f01a4-index.json
40c40
<     "linux-x64-node-v18-{\"/@esbuild/linux-arm64/0.18.20\":{},\"/@esbuild/linux-x64/0.18.20\":{}}": {
---
>     "linux-arm64-node-v18-{\"/@esbuild/linux-arm64/0.18.20\":{},\"/@esbuild/linux-x64/0.18.20\":{}}": {
52c52
<         "integrity": "sha512-VhbLe+ls+gVl6qq+U5OjKNAJP/0019tO9M0eFdaBiwSrwAq0Hv3jNjq3MYAqDGo1yzl59eh9CDRzShH80qPDRw==",
---
>         "integrity": "sha512-1YG98Tqp0Je/JnIVh8CNiw7IYikWAd0iQAiAuV1MrSZs5ZIrvSGElgEkDwH70yp3VKWlWQp1pRRX1/93qYYgzg==",
54c54
<         "size": 9347072
---
>         "size": 8650752
diff '--color=auto' -r /nix/store/ggnzrp25am692b887qcidb5bb7qbc6x2-en-croissant-pnpm-deps-0.8.0/v3/files/92/eb4f7911969bc579765b453f5cc68d038403368b663e12b5d4215956677c6142bc03d5bc787caa4e5b46b5382979d14513678b9b4a408f4f040748e8c3fc9d-index.json /nix/store/s8xvcm8n4dk2qi6p4hrpcr07k0sx1wkr-en-croissant-pnpm-deps-0.8.0/v3/files/92/eb4f7911969bc579765b453f5cc68d038403368b663e12b5d4215956677c6142bc03d5bc787caa4e5b46b5382979d14513678b9b4a408f4f040748e8c3fc9d-index.json
19540c19540
<     "linux-x64-node-v18-{\"/@fortawesome/fontawesome-common-types/6.4.0\":{}}": {
---
>     "linux-arm64-node-v18-{\"/@fortawesome/fontawesome-common-types/6.4.0\":{}}": {
diff '--color=auto' -r /nix/store/ggnzrp25am692b887qcidb5bb7qbc6x2-en-croissant-pnpm-deps-0.8.0/v3/files/97/b89f7d0a53d8ead97c7dab5c8a7bfc592669e64cd2fc5bc4df4a7e20b0198b99ab3320fc976f5f6e91df56719ff3f1becae0dd8f4de974dc9e49ea0f2cafe3-index.json /nix/store/s8xvcm8n4dk2qi6p4hrpcr07k0sx1wkr-en-croissant-pnpm-deps-0.8.0/v3/files/97/b89f7d0a53d8ead97c7dab5c8a7bfc592669e64cd2fc5bc4df4a7e20b0198b99ab3320fc976f5f6e91df56719ff3f1becae0dd8f4de974dc9e49ea0f2cafe3-index.json
40c40
<     "linux-x64-node-v18-{\"/@esbuild/linux-arm64/0.19.8\":{},\"/@esbuild/linux-x64/0.19.8\":{}}": {
---
>     "linux-arm64-node-v18-{\"/@esbuild/linux-arm64/0.19.8\":{},\"/@esbuild/linux-x64/0.19.8\":{}}": {
52c52
<         "integrity": "sha512-j6UwdVufgcjslI7IRBq/yBXBqTlx4BPf6uhcceCAh2yq0W2nV04LwRyHMybczyLLuSm219JYPnzriJW7i5nvxg==",
---
>         "integrity": "sha512-beuCuXDhkQAo6N4WSp5BV2Aod7h19MQMeLZYjtbYL6rpWBhDosaO6kykoQZNZohM2DuavRvUBopRiH1KHnzZtg==",
54c54
<         "size": 9486336
---
>         "size": 8781824
diff '--color=auto' -r /nix/store/ggnzrp25am692b887qcidb5bb7qbc6x2-en-croissant-pnpm-deps-0.8.0/v3/files/ed/d2a04f2c498e5accc056586f56ae8f75ead03f1905efb9a1e2207cc951534e5da17672a5825ed0b024d328e3dd675bc18ce917e4f53cbc704d68b3234a5c93-index.json /nix/store/s8xvcm8n4dk2qi6p4hrpcr07k0sx1wkr-en-croissant-pnpm-deps-0.8.0/v3/files/ed/d2a04f2c498e5accc056586f56ae8f75ead03f1905efb9a1e2207cc951534e5da17672a5825ed0b024d328e3dd675bc18ce917e4f53cbc704d68b3234a5c93-index.json
90c90
<     "linux-x64-node-v18-{\"/@swc/core-linux-arm64-gnu/1.3.100\":{},\"/@swc/core-linux-arm64-musl/1.3.100\":{},\"/@swc/core-linux-x64-gnu/1.3.100\":{},\"/@swc/core-linux-x64-musl/1.3.100\":{},\"/@swc/counter/0.1.2\":{},\"/@swc/types/0.1.5\":{}}": {
---
>     "linux-arm64-node-v18-{\"/@swc/core-linux-arm64-gnu/1.3.100\":{},\"/@swc/core-linux-arm64-musl/1.3.100\":{},\"/@swc/core-linux-x64-gnu/1.3.100\":{},\"/@swc/core-linux-x64-musl/1.3.100\":{},\"/@swc/counter/0.1.2\":{},\"/@swc/types/0.1.5\":{}}": {

@Misterio77
Copy link
Member

Misterio77 commented Feb 16, 2024

It seems that it's caused by esbuild. Maybe a good enough solution is per-platform hashes, like this:

outputHash = {
x86_64-linux = "sha256-LJPjWNpVfdUu8F5BMhAzpTo/h6ax7lxY2EESHj5P390=";
aarch64-linux = "sha256-N1K4pV5rbWmO/KonvYegzBoWa6TYQIqhQyxH/sWjOJQ=";
i686-linux = "sha256-/Q7VZahYhLdKVFB25CanROYxD2etQOcRg+4bXZUMqTc=";
x86_64-darwin = "sha256-9biFAbFD7Bva7KPKztgCvcaoX8E6AlJBKkjlDQdP6Zw=";
aarch64-darwin = "sha256-to5Y0R9tm9b7jUQAK3eBylLhpu+w5oDd63FbBCBAvd8=";
}.${stdenv.system} or (throw "Unsupported system: ${stdenv.system}");

@lcscosta lcscosta changed the title en-croissant: init at 0.8.0 en-croissant: init at 0.9.2 Feb 18, 2024
@lcscosta
Copy link
Author

It seems that it's caused by esbuild. Maybe a good enough solution is per-platform hashes, like this:

outputHash = {
x86_64-linux = "sha256-LJPjWNpVfdUu8F5BMhAzpTo/h6ax7lxY2EESHj5P390=";
aarch64-linux = "sha256-N1K4pV5rbWmO/KonvYegzBoWa6TYQIqhQyxH/sWjOJQ=";
i686-linux = "sha256-/Q7VZahYhLdKVFB25CanROYxD2etQOcRg+4bXZUMqTc=";
x86_64-darwin = "sha256-9biFAbFD7Bva7KPKztgCvcaoX8E6AlJBKkjlDQdP6Zw=";
aarch64-darwin = "sha256-to5Y0R9tm9b7jUQAK3eBylLhpu+w5oDd63FbBCBAvd8=";
}.${stdenv.system} or (throw "Unsupported system: ${stdenv.system}");

Yes, I have the same doubts about esbuild. Another way to deal with this is by building esbuild from source and using it, but I failed to implement this solution. One example is:

https://github.com/NixOS/nixpkgs/blob/master/pkgs/applications/misc/pot/default.nix#L101-L112

@Misterio77 I attempted to implement a per-platform solution using the hash that worked on my machine, but it failed on ofborg.

My machine metadata

 - system: `"x86_64-linux"`
 - host os: `Linux 6.1.78, NixOS, 23.11 (Tapir), 23.11.20240217.84d981b`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.18.1`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`

@kirillrdy
Copy link
Member

$ nix build --nix-path nixpkgs=/home/kirillvr/.cache/nixpkgs-review/pr-271493-1/nixpkgs nixpkgs-overlays=/tmp/tmpxdr1gzfs --extra-experimental-features nix-command no-url-literals --no-link --keep-going --no-allow-import-from-derivation --option build-use-sandbox relaxed -f /home/kirillvr/.cache/nixpkgs-review/pr-271493-1/build.nix
error: hash mismatch in fixed-output derivation '/nix/store/nn0lnxdkia0b6dlp4rvnyilygkfqv249-en-croissant-pnpm-deps-0.9.2.drv':
         specified: sha256-Q1VyEUFeUrcyGkMDl360839QHfzSbbWthqj6akYaP+k=
            got:    sha256-FFVkz/oSXJPB03lCkysGTSkrpdvEly15Y7JTrGKPp/I=
error: 1 dependencies of derivation '/nix/store/n8941dkgx0ds5cai9d86vyjnlivg2c4a-en-croissant-0.9.2.drv' failed to build
error: 1 dependencies of derivation '/nix/store/3cfvb5qn8dfcz0fmy6qhn2aaw40y4hnf-review-shell.drv' failed to build

Link to currently reviewing PR:
https://github.com/NixOS/nixpkgs/pull/271493

1 package failed to build:
en-croissant

error: build log of '/nix/store/n8941dkgx0ds5cai9d86vyjnlivg2c4a-en-croissant-0.9.2.drv^*' is not available
error: build log of '/nix/store/njn6hg4g2y3r346aca7dby5j6xfi5r2n-en-croissant-0.9.2' is not available

@lcscosta are you able to diff old FOD and current ?

Copy link
Contributor

@eclairevoyant eclairevoyant left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution and your work so far! I've provided some feedback below:


src = fetchFromGitHub {
owner = "franciscoBSalgueiro";
repo = pname;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
repo = pname;
repo = "en-croissant";

jq --raw-output ". * $pnpmPatch" package.json.orig > package.json
'';

installPhase = ''
Copy link
Contributor

Choose a reason for hiding this comment

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

description = "A Modern Chess Database";
homepage = "https://github.com/franciscoBSalgueiro/en-croissant/";
maintainers = [ maintainers.lcscosta ];
license = licenses.gpl3;
Copy link
Contributor

Choose a reason for hiding this comment

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

this is a deprecated identifier; please use gpl3Plus or gpl3Only in accordance with whichever upstream went with

homepage = "https://github.com/franciscoBSalgueiro/en-croissant/";
maintainers = [ maintainers.lcscosta ];
license = licenses.gpl3;
sourceProvenance = with sourceTypes; [ binaryNativeCode ];
Copy link
Contributor

Choose a reason for hiding this comment

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

as it's now built from source, should we remove this now?

outputHashMode = "recursive";
outputHash = {
x86_64-linux = "sha256-Q1VyEUFeUrcyGkMDl360839QHfzSbbWthqj6akYaP+k=";
}.${stdenv.system} or (throw "Unsupported system: ${stdenv.system}");
Copy link
Contributor

Choose a reason for hiding this comment

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

does this mean only x86_64-linux is supported for this package?

@9glenda
Copy link
Member

9glenda commented Apr 20, 2024

Result of nixpkgs-review pr 271493 run on x86_64-linux 1

1 package failed to build:
  • en-croissant
specified: sha256-Q1VyEUFeUrcyGkMDl360839QHfzSbbWthqj6akYaP+k= got: sha256-FFVkz/oSXJPB03lCkysGTSkrpdvEly15Y7JTrGKPp/I=

@SuperSandro2000
Copy link
Member

#290715 got now merged

@wegank wegank added the 2.status: merge conflict This PR has merge conflicts with the target branch label Sep 27, 2024
@gepbird
Copy link
Contributor

gepbird commented Oct 1, 2024

Superseded by #341451.

@gepbird gepbird closed this Oct 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.status: merge conflict This PR has merge conflicts with the target branch 8.has: package (new) This PR adds a new package 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin 10.rebuild-linux: 1-10 10.rebuild-linux: 1 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12. first-time contribution This PR is the author's first one; please be gentle!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants