Skip to content

Create plugin symlinks for Windows and Linux#50599

Merged
stuartmorgan-g merged 4 commits intoflutter:masterfrom
stuartmorgan-g:symlink-windows-linux-plugins
Feb 13, 2020
Merged

Create plugin symlinks for Windows and Linux#50599
stuartmorgan-g merged 4 commits intoflutter:masterfrom
stuartmorgan-g:symlink-windows-linux-plugins

Conversation

@stuartmorgan-g
Copy link
Copy Markdown
Contributor

@stuartmorgan-g stuartmorgan-g commented Feb 11, 2020

Description

This makes ephemeral symlinks to each plugin, for use by build systems.
This is similar to the logic implemented in the Podfile on iOS and
macOS, but managed internally to the Flutter tool.

This allows for build system integrations (e.g., .sln references to
projects) not to have developer-specific paths, as with iOS/macOS.

Related Issues

Exploration for addressing #32719 and #32720
Related to #41146

Tests

I added the following tests: createPluginSymlink test group

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • 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.

This makes ephemeral symlinks to each plugin, for use by build systems.
This is similar to the logic implemented in the Podfile on iOS and
macOS, but managed internally to the Flutter tool.

Exploration for addressing flutter#32719 and flutter#32720
Related to flutter#41146
@fluttergithubbot fluttergithubbot added the tool Affects the "flutter" command-line tool. See also t: labels. label Feb 11, 2020
@stuartmorgan-g stuartmorgan-g changed the title WIP: Create plugin symlinks for Windows and Linux Create plugin symlinks for Windows and Linux Feb 12, 2020
@stuartmorgan-g
Copy link
Copy Markdown
Contributor Author

I tested this branch on FDE's CI, and it passed on both AppVeyor and GitHub actions, so fortunately it looks like it's normal for CI setups to have the necessary permissions to make symlinks on Windows (which was my primary concern with doing this).

Copy link
Copy Markdown
Contributor

@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.

LGTM with some nits

Comment thread packages/flutter_tools/lib/src/plugins.dart
Comment thread packages/flutter_tools/lib/src/plugins.dart Outdated
Comment thread packages/flutter_tools/lib/src/plugins.dart
Comment thread packages/flutter_tools/lib/src/plugins.dart
/// Replaces [symlinkDirectory] with a directory containing symlinks to each plugin
/// listed in [platformPlugins].
void _createPlatformPluginSymlinks(Directory symlinkDirectory, List<dynamic> platformPlugins) {
// Start fresh each time to avoid stale links.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there some data we could track to avoid blowing the whole thing away? It seems like this might come back to bite us if we induce dependent build systems to do more work than necessary because of it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed to only fully recreate when the plugin list actually changes, and to do repairs (in case someone nukes ephemeral/) on each build. In practice I don't think it would affect build systems since it's a containing directory, not the project or sources, that is the symlink, but this version is still pretty low risk in terms of things getting out of sync.

@stuartmorgan-g stuartmorgan-g merged commit 7bdd475 into flutter:master Feb 13, 2020
@stuartmorgan-g stuartmorgan-g deleted the symlink-windows-linux-plugins branch February 13, 2020 00:23
@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Aug 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants