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

feat(aft): Adds link, clean, bootstrap, pub commands #1883

Merged
merged 18 commits into from
Aug 3, 2022

Conversation

dnys1
Copy link
Contributor

@dnys1 dnys1 commented Jul 13, 2022

Adds the following commands to aft:

  • bootstrap/bs: Sets up repo for development work (link + pub get)
  • clean: Cleans temporary files and build artifacts for all packages
  • link: Links all packages together using pubspec_overrides.yaml
  • pub: Run pub commands for all packages in the repo
    • get: Runs dart pub get/flutter pub get for all packages
    • upgrade: Runs dart pub upgrade/flutter pub upgrade for all packages
    • publish: Runs dart pub publish/flutter pub publish for all packages which need publishing

@dnys1 dnys1 requested a review from a team as a code owner July 13, 2022 23:49
@dnys1 dnys1 force-pushed the feat/next/aft-cmds branch 7 times, most recently from af36ceb to c4ebd22 Compare July 14, 2022 12:50
@@ -36,8 +36,14 @@ commands:
- run:
name: Install and set up melos
command: |
flutter pub global activate melos 1.3.0
melos bootstrap
flutter pub global activate melos
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there any value in pinning this anymore?

flutter_tools:
git:
url: https://github.com/flutter/flutter.git
ref: master
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we pin to a commit?

Copy link
Member

Choose a reason for hiding this comment

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

For flutter-tools maybe, but I'd think for pub there might potentially be more chance of security updates that we'd want to get automatically?

pub:
git:
url: https://github.com/dart-lang/pub.git
ref: master
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we pin to a commit?

@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2022

Codecov Report

Merging #1883 (0d995bc) into next (9adf82d) will increase coverage by 10.04%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             next    #1883       +/-   ##
===========================================
+ Coverage   34.14%   44.18%   +10.04%     
===========================================
  Files         107      122       +15     
  Lines        6274     7505     +1231     
===========================================
+ Hits         2142     3316     +1174     
- Misses       4132     4189       +57     
Flag Coverage Δ
android-unit-tests ∅ <ø> (∅)
flutter-unit-tests 26.86% <ø> (ø)
ios-unit-tests 89.08% <ø> (+9.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...os/unit_tests/QueryPredicateBuilderUnitTests.swift 100.00% <0.00%> (ø)
...os/unit_tests/ModelSchemaEquatableExtensions.swift 89.83% <0.00%> (ø)
...plify_flutter/example/ios/Runner/AppDelegate.swift 100.00% <0.00%> (ø)
...it_tests/DataStoreHubEventStreamHandlerTests.swift 97.87% <0.00%> (ø)
.../ios/unit_tests/amplify_flutter_exampleTests.swift 94.59% <0.00%> (ø)
...ple/ios/unit_tests/QuerySortBuilderUnitTests.swift 100.00% <0.00%> (ø)
...e/example/ios/unit_tests/GetJsonFromFileUtil.swift 57.14% <0.00%> (ø)
...e/ios/unit_tests/MockAnalyticsCategoryPlugin.swift 15.38% <0.00%> (ø)
...e/ios/unit_tests/AmplifyModelSchemaUnitTests.swift 97.70% <0.00%> (ø)
.../example/ios/unit_tests/resources/SchemaData.swift 100.00% <0.00%> (ø)
... and 5 more

@dnys1 dnys1 force-pushed the feat/next/aft-cmds branch 3 times, most recently from e4eb7b2 to f5b8a52 Compare July 20, 2022 16:58
Dillon Nys added 11 commits July 20, 2022 10:26
commit-id:54696df6

Revert "chore: Remove `pubspec_overrides` from tracking"

This reverts commit 37914fb.

This is needed by `mono_repo` until a better solution is available besides prefixing every mono_repo command with `aft link`.

commit-id:d2fcc064
Links packages together by creating `pubspec_overrides.yaml`

commit-id:43ac3346
Cleans the repo by removing temporary files created by link/build commands.

commit-id:7807b7ff
Adds aliases of `pub get` and `pub upgrade` which can be run for all packages in the repo. Embeds `pub` and `flutter` tools for a significant speed-up over spawning subprocesses.

commit-id:5f697643
Adds command to publish packages which need publishing. A check is made to `pub.dev` to see which version is the latest.

commit-id:ac025581
Adds command to wrap `link` + `pub get` commands for the repo to replicate `melos bs`.

commit-id:1546a8c5
Clean up README and library files for new commands.

commit-id:e0e6dead
commit-id:58c2392c
Instead of having `deps.yaml`, have a single `aft.yaml` which will manage all the configuration for the repo, including for example ignored packages.

commit-id:d309c42c
Adds pre-publish checks to prevent package publishing issues, esp. related to generated `build_runner` artifacts.

commit-id:12c8b511
commit-id:89a88d5f
Dillon Nys added 3 commits August 3, 2022 07:32
…t/boostrap

Since we no longer track generated JS files, in order for the packages to be fully bootstrapped/usuable requires running `build_runner` for those with generated outputs. By default, now, `aft bootstrap` will run the generation - the idea being that it will only be run once to initialize a fresh repository. After that `aft pub get` accomplishes the same thing, although by default it will not run `build_runner`.

`aft boostrap` can be run with the `--no-build` flag to disable this behavior, while `aft pub get/upgrade` can be run with the `--build` flag to enable it.

commit-id:6c54b4df
Comment on lines 98 to 104
if (path.basename(pkgB.path) == 'example') {
return 1;
}
if (path.basename(pkgA.path) == 'example') {
return -1;
}
return pkgA.compareTo(pkgB);
Copy link
Member

Choose a reason for hiding this comment

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

nit: Not sure it really matters in this case, but the final order of the example packages is going to depend on the original order. If you sort ['example/foo', 'example/bar'] and ['example/bar', 'example/foo'], they will not be the same after sort. if both are example packages, you probably just want to return pkgA.compareTo(pkgB).

Suggested change
if (path.basename(pkgB.path) == 'example') {
return 1;
}
if (path.basename(pkgA.path) == 'example') {
return -1;
}
return pkgA.compareTo(pkgB);
if (path.basename(pkgB.path) == 'example' && !path.basename(pkgA.path) == 'example') {
return 1;
}
if (path.basename(pkgA.path) == 'example' && !path.basename(pkgB.path) == 'example' ) {
return -1;
}
return pkgA.compareTo(pkgB);

Or alternatively

Suggested change
if (path.basename(pkgB.path) == 'example') {
return 1;
}
if (path.basename(pkgA.path) == 'example') {
return -1;
}
return pkgA.compareTo(pkgB);
if (path.basename(pkgB.path) == 'example' && path.basename(pkgA.path) == 'example') {
return pkgA.compareTo(pkgB);
}
if (path.basename(pkgB.path) == 'example') {
return 1;
}
if (path.basename(pkgA.path) == 'example') {
return -1;
}
return pkgA.compareTo(pkgB);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, yeah it was just to get examples before their parent packages. I ended up removing this since I switched to using flutter_tools logic for pub get. Before, we were overriding the logic which checked for an example package.

aws_common:
path: ../aws_common
aws_signature_v4:
path: ../aws_signature_v4
amplify_lints:
Copy link
Member

Choose a reason for hiding this comment

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

This is not in alphabetical order. When I run aft link this is updated to be in alphabetical order. You might need to re-run link?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, good catch

import 'package:async/async.dart';
import 'package:path/path.dart' as p;

const _deletePaths = [
Copy link
Member

Choose a reason for hiding this comment

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

What is the motivation for deleting these? Should they be regenerated after being deleted? I found it surprising that aft clean deleted files that were committed to source control.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, I think I had envisioned them not being tracked originally, but this needs to be fixed in mono_repo first.

import 'package:path/path.dart' as path;

/// Command to bootstrap/link Dart/Flutter packages in the repo.
class BootstrapCommand extends AmplifyCommand {
Copy link
Member

Choose a reason for hiding this comment

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

Bootstrap is failing for the authenticator for me. See stack trace below. Are you not seeing that?

amplify_authenticator type '_InternalLinkedHashMap' is not a subtype of type 'Map' in type cast #0 GenerateLocalizationsTarget.build (package:flutter_tools/src/build_system/targets/localizations.dart:66:7) #1 _BuildInstance._invokeInternal (package:flutter_tools/src/build_system/build_system.dart:847:27) #2 FlutterBuildSystem.build (package:flutter_tools/src/build_system/build_system.dart:614:16) #3 generateLocalizationsSyntheticPackage (package:flutter_tools/src/dart/generate_synthetic_packages.dart:58:30) #4 PackagesGetCommand._runPubGet (package:flutter_tools/src/commands/packages.dart:132:7) #5 PackagesGetCommand.runCommand (package:flutter_tools/src/commands/packages.dart:181:7) #6 FlutterCommand.run. (package:flutter_tools/src/runner/flutter_command.dart:1209:27) #7 AppContext.run. (package:flutter_tools/src/base/context.dart:150:19) #8 CommandRunner.runCommand (package:args/command_runner.dart:209:13) #9 FlutterCommandRunner.runCommand. (package:flutter_tools/src/runner/flutter_command_runner.dart:281:9) #10 AppContext.run. (package:flutter_tools/src/base/context.dart:150:19) #11 FlutterCommandRunner.runCommand (package:flutter_tools/src/runner/flutter_command_runner.dart:229:5) #12 runFlutterPub. (package:aft/src/pub/pub_runner.dart:90:7) #13 AppContext.run. (package:flutter_tools/src/base/context.dart:150:19) #14 runFlutterPub (package:aft/src/pub/pub_runner.dart:84:3) #15 Result.capture. (package:async/src/result/result.dart:87:24)

amplify_authenticator_example
type '_InternalLinkedHashMap<String, dynamic>' is not a subtype of type 'Map<String, Object>' in type cast
#0 GenerateLocalizationsTarget.build (package:flutter_tools/src/build_system/targets/localizations.dart:66:7)
#1 _BuildInstance._invokeInternal (package:flutter_tools/src/build_system/build_system.dart:847:27)

#2 FlutterBuildSystem.build (package:flutter_tools/src/build_system/build_system.dart:614:16)

#3 generateLocalizationsSyntheticPackage (package:flutter_tools/src/dart/generate_synthetic_packages.dart:58:30)

#4 PackagesGetCommand._runPubGet (package:flutter_tools/src/commands/packages.dart:132:7)

#5 PackagesGetCommand.runCommand (package:flutter_tools/src/commands/packages.dart:175:5)

#6 FlutterCommand.run. (package:flutter_tools/src/runner/flutter_command.dart:1209:27)

#7 AppContext.run. (package:flutter_tools/src/base/context.dart:150:19)

#8 CommandRunner.runCommand (package:args/command_runner.dart:209:13)

#9 FlutterCommandRunner.runCommand. (package:flutter_tools/src/runner/flutter_command_runner.dart:281:9)

#10 AppContext.run. (package:flutter_tools/src/base/context.dart:150:19)

#11 FlutterCommandRunner.runCommand (package:flutter_tools/src/runner/flutter_command_runner.dart:229:5)

#12 runFlutterPub. (package:aft/src/pub/pub_runner.dart:90:7)

#13 AppContext.run. (package:flutter_tools/src/base/context.dart:150:19)

#14 runFlutterPub (package:aft/src/pub/pub_runner.dart:84:3)

#15 Result.capture. (package:async/src/result/result.dart:87:24)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I have an open PR for it, it's just not merged yet. There's a TODO in there somewhere to make failures in pub get fatal, but right now it just prints them.

Dillon Nys added 2 commits August 3, 2022 10:53
Do not delete `pubspec_overrides` since these are being published now

commit-id:008c0acd
@dnys1 dnys1 merged commit 0e210f4 into aws-amplify:next Aug 3, 2022
@dnys1 dnys1 deleted the feat/next/aft-cmds branch August 3, 2022 20:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants