Skip to content

Comments

flutter: Separate cache and unwrapped derivations #2#240715

Merged
Kranzes merged 1 commit intoNixOS:masterfrom
FlafyDev:flutter-cache-drv-2
Jul 5, 2023
Merged

flutter: Separate cache and unwrapped derivations #2#240715
Kranzes merged 1 commit intoNixOS:masterfrom
FlafyDev:flutter-cache-drv-2

Conversation

@FlafyDev
Copy link
Contributor

This PR is a continuation of 238177 (Sorry for the trouble)

flutter-unwrapped will now not come with engine artifacts in its cache directory.
To specify a different cache directory, set FLUTTER_CACHE_DIR

Before this PR, changing which artifacts Flutter will have required rebuilding Flutter. This is because Flutter's unwrapped derivation required symlinking the engine artifacts to the cache directory($out/bin/cache).

So this:

pkgs.flutter.override {
  supportsAndroid = false;
  supportsLinuxDesktop = true;
}

And this:

pkgs.flutter.override {
  supportsAndroid = true;
  supportsLinuxDesktop = true;
}

Would build Flutter twice, even though it's not necessary.

To fix this, this PR moved the cache directory to an environment variable(when specified). This allows changing which artifacts
Flutter sees at runtime, so no need to rebuild.

Additionally, the wrapper has been adjusted to this change and will now provide the engine artifacts through FLUTTER_CACHE_DIR.

Note

The sh file $out/bin/internal/shared.sh runs when launching Flutter and calls $FLUTTER_ROOT/bin/cache/ instead of our environment variable FLUTTER_CACHE_DIR.
I decided not to patch it since the script doesn't require engine artifacts(which are the only thing not added by the unwrapped derivation), so it shouldn't fail, and patching it will just be harder to maintain.

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.11 Release Notes (or backporting 23.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.

flutter-unwrapped will now not come with engine artifacts in its cache directory(`$out/bin/cache`).

To specify a different cache directory, set FLUTTER_CACHE_DIR.

Flutter's wrapper now sets FLUTTER_CACHE_DIR to set engine artifacts.

The sh file `$out/bin/internal/shared.sh` runs when launching Flutter and calls `"$FLUTTER_ROOT/bin/cache/` instead of our environment variable `FLUTTER_CACHE_DIR`.
I decided not to patch it since the script doesn't require engine artifacts(which are the only thing not added by the unwrapped derivation), so it shouldn't fail, and patching it will just be harder to maintain.
@FlafyDev FlafyDev force-pushed the flutter-cache-drv-2 branch from d81c367 to 570f3ef Compare June 30, 2023 12:23
@FlafyDev
Copy link
Contributor Author

CC @hacker1024 @gilice @SuperSandro2000


Please add lndir to nativeBuildInputs

Done


Can you try to reduce the nested let ins here? Having more variables in a higher scope would be probably preferable.

Done. I reduced most of the let ins.


Can we move this out of the ?? otherwise changing one value here requires copying all of this.

I'm not sure where to move the default value to.
And like @hacker1024 said:

I don't think it's such a big deal. Someone who chooses to configure this will probably not need half of these defaults anyway and simply choose exactly what they need without complex conditionals.

you would usually edit supportsLinuxDesktop or supportsAndroid. If you need more control over the artifacts, you edit includedEngineArtifacts

@ofborg ofborg bot added 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. labels Jun 30, 2023
@FlafyDev
Copy link
Contributor Author

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

16 packages built:
  • firmware-updater
  • firmware-updater.debug
  • fluffychat
  • fluffychat.debug
  • flutter
  • flutter-unwrapped (flutterPackages.stable)
  • flutter-unwrapped.cache (flutterPackages.stable.cache)
  • flutter2
  • flutter2-unwrapped (flutterPackages.v2)
  • flutter2-unwrapped.cache (flutterPackages.v2.cache)
  • flutter37
  • flutter37-unwrapped (flutterPackages.v37)
  • flutter37-unwrapped.cache (flutterPackages.v37.cache)
  • hover
  • yubioath-flutter
  • yubioath-flutter.debug

@figsoda figsoda added 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages. labels Jul 1, 2023
@Kranzes Kranzes merged commit d625c36 into NixOS:master Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 11-100 This PR causes between 11 and 100 packages to rebuild on Linux. 11.by: package-maintainer This PR was created by a maintainer of all the package it changes. 12.approvals: 1 This PR was reviewed and approved by one person. 12.approved-by: package-maintainer This PR was reviewed and approved by a maintainer listed in any of the changed packages.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants