[go: nahoru, domu]

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

Add the ability to inject a bootstrap script #66897

Merged
merged 4 commits into from
Sep 29, 2020
Merged

Add the ability to inject a bootstrap script #66897

merged 4 commits into from
Sep 29, 2020

Conversation

MarcusTomlinson
Copy link
Contributor
@MarcusTomlinson MarcusTomlinson commented Sep 29, 2020

Description

This PR adds the ability to inject a custom bootstrap script on Mac, Linux (via bin/internal/bootstrap.sh), and Windows (via bin/internal/bootstrap.bat).

The primary purpose for this change is to enable wider IDE support for the flutter snap on Linux by having the snap inject a bootstrap script that sets up the environment required to ensure its build dependancies can be found (See issues linked below).

Related Issues

Resolves: #66414
Resolves: flutter/flutter-intellij#4746
Resolves: canonical/flutter-snap#8

Tests

Added 'will load bootstrap script before starting' test to command_output_test.dart.

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I signed the CLA.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

  • No, no existing tests failed, so this is not a breaking change.
  • Yes, this is a breaking change.

Copy link
Member
@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

I think this could be tested in packages/flutter_tools/test/integration.shard/command_output_test.dart

Create a test that write a bootstrap file that echos something, run flutter and then asset that the echo is in the stdout. For example

testWithoutContext('will load bootstrap script before starting', () async {
    final String flutterBin = fileSystem.path.join(getFlutterRoot(), 'bin', 'flutter');
    final File bootstrap = fileSystem.file(
      fileSystem.path.join(
        getFlutterRoot(),
        'bin',
        'internal',
        platform.isWindows ? 'boostrap.bat' : 'bootstap',
    ));
    // Something different per platform., make it executable.
    bootstrap.writeAsStringSync('contents');

    final ProcessResult result = await processManager.run(<String>[
      flutterBin,
      ...getLocalEngineArguments(),
    ]);

    // Development tools.
    expect(result.stdout, contains(''));
});

bin/internal/shared.bat Outdated Show resolved Hide resolved
@jonahwilliams
Copy link
Member

(Also make sure to delete the bootstrap after the test is done in a finally block

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 29, 2020
Copy link
Member
@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

Thanks for adding the test @MarcusTomlinson !

LGTM

@MarcusTomlinson MarcusTomlinson merged commit 5f76bfb into flutter:master Sep 29, 2020
@devoncarew
Copy link
Member

👍 thanks for the contribution - great to see this land!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
4 participants