Skip to content

Conversation

@christopherfujino
Copy link
Contributor

@christopherfujino christopherfujino commented Jun 17, 2022

Fixes #96568

Throw a tool exit with paths to the where symlinking from and to (most likely the pubcache doesn't have read permissions).

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 17, 2022
@christopherfujino christopherfujino changed the title pipe through more details to handleSymlinkException [flutter_tools] tool exit access denied during symlinking Jun 17, 2022
@christopherfujino christopherfujino marked this pull request as ready for review June 17, 2022 22:48
@christopherfujino christopherfujino force-pushed the tool-exit-on-errno-5-during-symlink branch from 7aef7bd to 8d8d42e Compare June 17, 2022 22:52

expect(() => handleSymlinkException(e, platform: platform, os: os), returnsNormally);
expect(
() => handleSymlinkException(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add 2 more tests that hits this two different instructions? final String instructions = (version != null && version >= Version(10, 0, 14972)) .... Or let me know if I am reading this tests wrong and you are already catching that

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't really update this behavior or test, I just updated the function signature and made the fields required (the only real new test here is Symlink ERROR_ACCESS_DENIED failures show developers paths that were used.

Note the test on line 1614 covers newer windows versions.

Copy link
Contributor

@Jasguerrero Jasguerrero left a comment

Choose a reason for hiding this comment

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

LGTM

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • This commit is not mergeable and has conflicts. Please rebase your PR and fix all the conflicts.

@fluttergithubbot
Copy link
Contributor

This pull request is not suitable for automatic merging in its current state.

  • The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@fluttergithubbot fluttergithubbot merged commit 788c8b8 into flutter:master Jun 23, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Jun 24, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jun 24, 2022
camsim99 pushed a commit to camsim99/flutter that referenced this pull request Aug 10, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 30, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Aug 30, 2022
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

Development

Successfully merging this pull request may close these issues.

[tool_crash] FileSystemException: Cannot create link, OS Error: Access Denied., errno = 5

3 participants