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

gotify: 2.0.21 -> 2.1.0 #140139

Merged
merged 1 commit into from
Oct 1, 2021
Merged

gotify: 2.0.21 -> 2.1.0 #140139

merged 1 commit into from
Oct 1, 2021

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Oct 1, 2021

Motivation for this change

ChangeLog: https://github.com/gotify/server/releases/tag/v2.0.22
ChangeLog: https://github.com/gotify/server/releases/tag/v2.0.23
ChangeLog: https://github.com/gotify/server/releases/tag/v2.1.0

While the update only contains a few small features and a few bugfixes,
the change was rather messy for us unfortunately:

  • It seems as if npmjs.org-packages can't be transformed into
    pkg___pkg-x.y.z for Yarn's offline cache. The name
    https___registry.npmjs.org_caniuse_lite___caniuse_lite_1.0.30001237.tgz
    isn't the problem because when changing the URL "parser" of yarn2nix
    to transform this into org_caniuse_lite___caniuse_lite_1.0.30001237
    this doesn't help either.

    Instead, I derived the fix from gitlab[1] where yarn.lock gets
    patched to make sure that it detects the package in the offline-cache
    properly.

  • The frontend is now built with react-scripts. This is a problem for
    us because it tries to write into node_modules/.cache even though
    node_modules is a store-path in the context of yarn2nix[2].

    The change isn't pretty, but solves the issue for us.

[1]

${replace}/bin/replace-literal -f -e '"https://codeload.github.com/dagrejs/dagre-d3/tar.gz/e1a00e5cb518f5d2304a35647e024f31d178e55b"' \
'"https___codeload.github.com_dagrejs_dagre_d3_tar.gz_e1a00e5cb518f5d2304a35647e024f31d178e55b"' yarn.lock

[2] facebook/create-react-app#11263

Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.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.

ChangeLog: https://github.com/gotify/server/releases/tag/v2.0.22
ChangeLog: https://github.com/gotify/server/releases/tag/v2.0.23
ChangeLog: https://github.com/gotify/server/releases/tag/v2.1.0

While the update only contains a few small features and a few bugfixes,
the change was rather messy for us unfortunately:

* It seems as if `npmjs.org`-packages can't be transformed into
  `pkg___pkg-x.y.z` for Yarn's offline cache. The name
  `https___registry.npmjs.org_caniuse_lite___caniuse_lite_1.0.30001237.tgz`
  isn't the problem because when changing the URL "parser" of `yarn2nix`
  to transform this into `org_caniuse_lite___caniuse_lite_1.0.30001237`
  this doesn't help either.

  Instead, I derived the fix from `gitlab`[1] where `yarn.lock` gets
  patched to make sure that it detects the package in the offline-cache
  properly.

* The frontend is now built with `react-scripts`. This is a problem for
  us because it tries to write into `node_modules/.cache` even though
  `node_modules` is a store-path in the context of `yarn2nix`[2].

  The change isn't pretty, but solves the issue for us.

[1] https://github.com/NixOS/nixpkgs/blob/f007b794c758000a275b00dd0695d2fb155195f0/pkgs/applications/version-management/gitlab/default.nix#L85-L86
[2] facebook/create-react-app#11263
Copy link
Contributor

@doronbehar doronbehar 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 maintaining this package @Ma27 !

pkgs/servers/gotify/ui.nix Show resolved Hide resolved
Comment on lines +34 to +36
# react-scripts requires a writable node_modules/.cache, so we have to copy the symlink's contents back
# into `node_modules/`.
# See https://github.com/facebook/create-react-app/issues/11263
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is good but perhaps should be at the beginning of the buildPhase.

Copy link
Member Author

Choose a reason for hiding this comment

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

There are a few more comments related to the blocks separated by whitespace, so I think it's OK to to leave it as-is, right? :)

@Ma27 Ma27 merged commit 107a594 into NixOS:master Oct 1, 2021
@Ma27 Ma27 deleted the bump-gotify branch October 1, 2021 16:45
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.

2 participants