very_good_workflows icon indicating copy to clipboard operation
very_good_workflows copied to clipboard

fix: resolving dependencies takes a relatively long time

Open orestesgaolin opened this issue 1 year ago • 1 comments

Description

Typically the Install dependencies step takes somewhere between 2-3 min. I was wondering if there was a way to cache dependencies across branches? I'm not sure if pubspec.lock is taken into account when caching, but I think it should not be the only indicator of reusing cache as usually most of the dependencies stays at the same version.

One idea I had was that once the first very_good packages get is run for the top package (app), then all the subsequent runs could use --offline flag to not refetch dependencies via network.

My workflow

name: app

on:
  - pull_request

jobs:
  build:
    uses: VeryGoodOpenSource/very_good_workflows/.github/workflows/flutter_package.yml@v1
    with:
      coverage_excludes: "*.g.dart"
      flutter_channel: "stable"
      flutter_version: "3.3.4"
      working_directory: "."
      test_recursion: false
      min_coverage: 0

Expected Behavior

Resolving time should be close to the local time e.g. below 1:30 min.

Screenshots

GitHub Action:

CleanShot 2022-10-10 at 09 42 50@2x

When running locally on M1 Pro the total time is 41.75 s

time very_good packages get --recursive
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/[REDACTED] (3.3s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app (5.0s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (2.7s)6s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (3.0s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (2.6s)6s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (2.8s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (2.5s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (2.0s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (4.3s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (2.1s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (2.6s)6s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (3.9s)9s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (3.0s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (1.7s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (2.8s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (3.0s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (2.7s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (2.9s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (3.1s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (1.7s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (0.9s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (1.8s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (3.2s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (2.9s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (2.2s)1s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (0.9s)
✓ Running "flutter packages get" in /Users/dominik/Projects/[REDACTED]/[REDACTED]-app/packages/[REDACTED] (2.8s)7s)

When running flutter packages get --offline for the main app the total time is around 2 seconds vs 3.3 seconds

orestesgaolin avatar Oct 10 '22 07:10 orestesgaolin

Hi @orestesgaolin 👋 Thanks for opening an issue!

We'll take a look shortly and see what we can do to improve the dependency installation step. I think it makes sense to cache the installed dependencies and we can probably use the pubspec.lock to invalidate the cache -- thanks for bringing this up!

felangel avatar Oct 11 '22 15:10 felangel

Hello everybody. I spent some time looking into this.

Seems like the flutter action takes care of caching everything under ./pub-cache, the problem is that the cache key never considers anything except the flutter version/arch/channel as keys. Subsequent changes on .pub_cache won't be saved to the cache.

We can fix that by providing a custom cache key with a hash to all lock files in the repo. But that will take no effect if the repo has all lock files ignored.

One idea I had was that once the first very_good packages get is run for the top package (app), then all the subsequent runs could use --offline flag to not refetch dependencies via network.

That would be risky and would break in most cases. In a scenario where the cache was mistakenly hit, any new dependencies would break the workflow, and since sub-packages tend to be libraries, their lock is almost always ignored. We cannot rely on the main apps lock since dev dependencies on the sub-packages will not be declared as transitive dependencies on the main apps lock.

An alternative is to consider the pubspec.yaml files as cache breakers.

So here is my proposal:

  • Pass cache-key: flutter-:os:-:channel:-:version:-:arch:-:hash:-${{ hashFiles('**/pubspec.yaml') }} to the flutter action.
  • Include an option to override that key for very good workflow users.
  • Propose the addition of an option to pass "restore-keys" to flutter actions which then would be passed to the cache action. Here: https://github.com/subosito/flutter-action/issues/195

renancaraujo avatar Oct 26 '22 12:10 renancaraujo