dart: Use Nix instead of Pub #263345
dart: Use Nix instead of Pub #263345
Conversation
ofborg
bot
requested review from
FlafyDev,
gilice,
lukegb,
lelgenio,
RossComputerGuy,
babariviere,
ericdallo and
mkg20001
hacker1024
force-pushed
the
feature/pub2nix
branch
2 times, most recently
from
0f2abbd
to
325ff81
Compare
|
Result of 4 packages marked as broken and skipped:
26 packages built:
|
|
I have added a mechanism to customise Dart package derivations. If any packages require patches, they can now be easily added. Setup hooks have replaced the old package override mechanism, and can be used to do things like set environment variables and add to the FFI runtime dependency list. We should have probably done this from the beginning, but now it's even easier as the setup hooks can be built into the package derivations themselves. These changes allow the dependency list JSON file to be removed. |
hacker1024
force-pushed
the
feature/pub2nix
branch
from
81c7044
to
6fd1e80
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
|
Result of 30 packages built:
|
There was a problem hiding this comment.
Nice! this solution is way nicer to use than a vendor hash.
comments:
-
pub2nix still requires git hashes. But currently we have to specify a separate hash for every git dependency. Maybe it'd be better to have a single fixed-output derivation to get all the git dependencies and thus use only a single hash?
-
Maybe add instructions to convert the pubspec.lock to json? Not sure when Nix would say that, though. And it's quite simple to do anyway.
|
|
||
| mkHostedDependencySource = name: details: | ||
| let | ||
| archive = fetchurl { |
There was a problem hiding this comment.
why not use fetchTarBall?
There was a problem hiding this comment.
The other reason is that the hash in the pubspec.lock is for the archive itself. There's no way to determine the recursive hash of the contents in advance.
There was a problem hiding this comment.
Ah I see. Is this correct then?
# The hash in `pubspec.lock` is for the archive itself.
# If we use `fetchTarball` instead of `fetchurl`, we'll need to input the hash for the files after extraction, which we don't have.
# Additionally, `fetchTarball` fails with "tarball contains an unexpected number of top-level files".
# https://discourse.nixos.org/t/fetchtarball-with-multiple-top-level-directories-fails/20556
|
@FlafyDev do you want to rebase this to another branch? I can then force push it to this one. |
This still fails to build as it uses an old version of the protobuf package, though.
… in extraPackageConfigSetup
They are only needed at build time, after all.
This allows setup hooks to add runtime dependencies.
…tead of package overrides
We get a dependency list with pub2nix now. We can no longer easily distinguish between development dependency dependencies and regular dependency dependencies, but we weren't doing this anyway.
|
@mkg20001 I've rebased this PR on this branch
This is the nixpkgs review for 32e3ea1: Result of 2 packages failed to build:
32 packages built:
EDIT: Added a few more commits to that branch based on my suggestions. Shouldn't impact nixpkgs-review's result. |
mkg20001
force-pushed
the
feature/pub2nix
branch
from
35900eb
to
5a5db92
Compare
There was a problem hiding this comment.
pub2nix still requires git hashes. But currently we have to specify a separate hash for every git dependency. Maybe it'd be better to have a single fixed-output derivation to get all the git dependencies and thus use only a single hash?
I think this would be a bit complicated to do. Probably better to keep it to a future PR if necessary. Also if we make an update script for buildDartApplication and buildFlutterApplication it wouldn't really matter.
Maybe add instructions to convert the pubspec.lock to json? Not sure when Nix would say that, though. And it's quite simple to do anyway.
I still think this is necessary, but better to keep it to a different PR.
I'm not too keen in the idea of reducing granularity for just a little convenience. IIRC, running a build with blank hashes and This would also require custom fetching logic, as I don't think there's any off-the-shelf solution to make a FOD out of multiple Git repositories. |
|
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/3200 |
|
Result of 2 packages failed to build:
32 packages built:
|
|
This PR seems to break dart compile on Darwin. #279772 |
|
I think the SDK version parsing in pubspec.yaml is broken: 
|
|
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Description of changes
This PR removes
fetchDartDepsin favour of a newpub2nixlibrary. Instead of using Pub to generate a large fixed-output derivation, thepubspec.lockfile is parsed, and a derivation is made for each individual package.This improves the user experience significantly:
vendorHashis no longer needed (though Git dependencies still do need manual hashes)This PR builds on #262789, so for review, this comparison between the branches is appropriate. This can be changed if we decide against #262789, though - nothing relies on it directly.
All existing Dart and Flutter packages build and run, with the exception of
dart-sass-embedded, which was already broken with Dart 3. I've removed it in this PR, as it has been discontinued upstream and merged into the regulardart-sasspackage.Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)@NixOS/flutter