Skip to content

prisma-engines: add missing dependency#211099

Merged
winterqt merged 1 commit intoNixOS:masterfrom
johnrichardrinehart:johnrichardrinehart/fix-prisma-4.8.0
Jan 19, 2023
Merged

prisma-engines: add missing dependency#211099
winterqt merged 1 commit intoNixOS:masterfrom
johnrichardrinehart:johnrichardrinehart/fix-prisma-4.8.0

Conversation

@johnrichardrinehart
Copy link
Contributor

Description of changes
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, 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/)
  • 23.05 Release Notes (or backporting 22.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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

prisma 4.8.0 seems to have introduced a dependency on git (https://github.com/prisma/prisma-engines/blob/df68a3303adc647325a8743cbca118079e22dee7/introspection-engine/core/build.rs#L4) which is acknowledged in the upstream nix files (https://github.com/prisma/prisma-engines/blob/df68a3303adc647325a8743cbca118079e22dee7/nix/all-engines.nix#L24-L25). They had these in nativeBuildInputs so I added them to this derivation's nativeBuildInputs. However, some of the upstream nativeBuildInputs are here in buildInputs, so if that needs to be changed for some reason then let me know (and let me know why, please).

Note that this project maintain their own flake.nix and upstream nix files so if it's possible/recommended to set up a dependency on their nix derivations directly then let's try to do that!

Copy link
Member

@winterqt winterqt left a comment

Choose a reason for hiding this comment

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

The commit message isn't in line with our guidelines. I'd suggest something like prisma-engines: add missing dependency or something better if you can think of one (just need to include that prefix).

Also: just in case you were wondering, this was caused by #207804.

@johnrichardrinehart johnrichardrinehart force-pushed the johnrichardrinehart/fix-prisma-4.8.0 branch from f98f510 to c1d76ba Compare January 17, 2023 16:22
@johnrichardrinehart
Copy link
Contributor Author

The commit message isn't in line with our guidelines.

I'm always screwing that up. Sorry about that.

I'd suggest something like prisma-engines: add missing dependency or something better if you can think of one (just need to include that prefix).

That was a good suggestion. I stole it.

Also: just in case you were wondering, this was caused by #207804.

Ahhhh. Thanks! It's a shame that managed to get merged without detecting build failures for dependent packages like this one :/ .

@johnrichardrinehart johnrichardrinehart force-pushed the johnrichardrinehart/fix-prisma-4.8.0 branch from c1d76ba to b8f3e7b Compare January 17, 2023 16:27
@winterqt winterqt changed the title fix(prisma): add build-time deps prisma-engines: add missing dependency Jan 17, 2023
@ofborg ofborg bot requested review from Pamplemousse and tomhoule January 17, 2023 17:17
@ofborg ofborg bot added 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. labels Jan 17, 2023
@johnrichardrinehart johnrichardrinehart force-pushed the johnrichardrinehart/fix-prisma-4.8.0 branch from b8f3e7b to 245fd10 Compare January 18, 2023 02:01
@winterqt
Copy link
Member

(Was there a conflict or something that made you rebase? Or was that not the only change between those force pushes? GitHub doesn't let me see it easily.)

Copy link
Contributor

@tomhoule tomhoule left a comment

Choose a reason for hiding this comment

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

looks good to me 👍

@johnrichardrinehart
Copy link
Contributor Author

johnrichardrinehart commented Jan 18, 2023

(Was there a conflict or something that made you rebase? Or was that not the only change between those force pushes? GitHub doesn't let me see it easily.)

Sorry for hiding the history - it wasn't intentional. No conflict. I could have committed on top or just squashed the changes on this branch. I have a habit of updating my local base branch and rebasing my PR branch and then force-pushing. So, just muscle memory.

@mguentner
Copy link
Contributor

It would be nice to merge this prior to #211411 so that a revision exists where 4.8.0 of prisma-engines is build-able in nixpkgs.

@tomhoule
Copy link
Contributor

tomhoule commented Jan 19, 2023

It would be nice to merge this prior to #211411 so that a revision exists where 4.8.0 of prisma-engines is build-able in nixpkgs.

I agree.

Copy link
Member

@winterqt winterqt left a comment

Choose a reason for hiding this comment

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

Thanks!

@winterqt winterqt merged commit 82bb67a into NixOS:master Jan 19, 2023
@johnrichardrinehart johnrichardrinehart deleted the johnrichardrinehart/fix-prisma-4.8.0 branch January 19, 2023 22:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

4 participants

Comments