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

Detect permissions issues running flutter on Windows #17975

Merged
merged 3 commits into from
May 30, 2018
Merged

Detect permissions issues running flutter on Windows #17975

merged 3 commits into from
May 30, 2018

Conversation

DanTup
Copy link
Contributor

@DanTup DanTup commented May 28, 2018

I think this is a reasonable fix for #17972 (/ #17967 / #17742). Currently Windows users just get stuck in an infinite loop if they put Flutter in a folder they don't have write access to (such as Program Files and then run as non-Admin).

@devoncarew
Copy link
Member

@goderbauer, are you the best one to review this PR?

@devoncarew
Copy link
Member

@DanTup, @mit-mit, what about updating the getting started page for windows to specifically call out not installing flutter into Program Files?

@mit-mit
Copy link
Member

mit-mit commented May 29, 2018

Sure, but there are many other folders with similar issues.

@DanTup
Copy link
Contributor Author

DanTup commented May 29, 2018

I think there could be value calling it out given how common it seems to be (probably because people are used to install apps there). I think Program Files, Program Files (x86) and Windows are the main ones (I can't find a full list on the MS site though). It's better to avoid the issue altogether than have someone put it in the wrong place and have to move it?

bin/flutter.bat Outdated
ECHO.
ECHO This may because you do not have sufficient permissions to write to
ECHO this folder and is often caused by installing Flutter into a
ECHO folder like C:\Program Files and then running as a non-Admin.
Copy link
Member

Choose a reason for hiding this comment

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

This may encourage users to run flutter ad admin, which is something we shouldn't do IMHO. Actually, on Linux we explicitly disallow that. Can we rephrase this to something that doesn't push users towards running things as admin? E.g. "This may be because flutter doesn't have write permissions for %FLUTTER_ROOT%. Consider moving flutter to a writable path, e.g. your home directory."

@goderbauer
Copy link
Member

+1 to warning people about extracting Flutter into directories that are known to be problematic.

It might be even better to give users an explicit example path in the instructions that they should use. The instructions currently say:

Extract the zip-file and place the contained flutter in the desired installation location for the Flutter SDK

This could be changed to: "Extract the zip-file and place the contained flutter directory in the desired installation location for the Flutter SDK (e.g. C:\src\flutter)"

@DanTup
Copy link
Contributor Author

DanTup commented May 29, 2018

I've tweaked the text, let me know if this looks better. I can also tweak the website tomorrow if there are no objections to your suggestion.

Copy link
Member

@goderbauer goderbauer left a comment

Choose a reason for hiding this comment

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

LGTM

bin/flutter.bat Outdated
ECHO %cache_dir%
ECHO.
ECHO This may be because flutter doesn't have write permissions for
ECHO this path. Try moving flutter to a writable folder, such as
Copy link
Member

Choose a reason for hiding this comment

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

nit: maybe say "try moving the flutter directory..." since we don't want people to just move the flutter executable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. I've also changed the next instance of folder to location since it seemed weird to mix directory and folder and if I made them all the same there were three instances of directory in that line (move your flutter directory to a writeable directory such as your home directory).

Now reads:

This may be because flutter doesn't have write permissions for
this path. Try moving the flutter directory to a writable location,
such as within your home directory.

I'll land this, but feel free to request changes (or just change it) if you're not sold on it :)

@DanTup DanTup merged commit 5ce672b into flutter:master May 30, 2018
@DanTup DanTup deleted the add-permissions-check branch May 30, 2018 07:14
@DanTup
Copy link
Contributor Author

DanTup commented May 30, 2018

Extract the zip-file and place the contained flutter directory in the desired installation location for the Flutter SDK (e.g. C:\src\flutter)

I was going to add to this:

Do not extract Flutter to a folder that requires elevated permissions (like C:\Program Files\).

However this could be misinterpreted as that being a suggestion of somewhere good. Is the first change above enough, or should we include something like the second line (but maybe word it in a less ambiguous way)?

christopherfujino added a commit to christopherfujino/flutter that referenced this pull request Apr 28, 2020
0c35a34 Roll src/third_party/skia f5132a05c893..4baa7326ccfb (1 commits) (flutter#18003)
f07712b Roll src/third_party/skia 5f56cb1d3b4f..f5132a05c893 (6 commits) (flutter#17999)
103c9c7 Bumped up the timeout for testAccessibilityFocusOnTextSemanticsProducesCorrectIosViews (flutter#17988)
a1fdff6 Roll src/third_party/skia 81ef385c1fcd..5f56cb1d3b4f (14 commits) (flutter#17991)
22479ca Roll Dart to 726d3c772554924f62db0b9e0d4c280dbbddc824 (flutter#17993)
494a63c Trial PR to enable null safety unfork in the Dart SDK. (flutter#17818)
887efcb Roll fuchsia/sdk/core/linux-amd64 from eapIV... to 3h-X9... (flutter#17987)
50ae2b9 Set SkSL asset manager in RunConfiguration ctor (flutter#17948)
5f437fb Started ignoring remote keyboard notifications. (flutter#17981)
027eff8 Reenable flutter scenic test to identify crashes and follow up on fixes. (flutter#17979)
992a55c Update buildroot (flutter#17978)
d5587df Roll src/third_party/skia 78debd6f6d83..81ef385c1fcd (1 commits) (flutter#17976)
aa00d50 [web] Don't allow empty initial route (flutter#17936)
3b0e415 Roll fuchsia/sdk/core/mac-amd64 from 9O3Ef... to arZdZ... (flutter#17975)
66af9ea Roll src/third_party/skia 981d590e8eba..78debd6f6d83 (5 commits) (flutter#17972)
732bf22 Manual roll of Dart to 03429b20cd67f85d65cc589b529ab8c1a4780912...a53d336b9fd4bbb415d2f1e3f4c653aa107f31c7 (flutter#17971)
cad81c7 Roll src/third_party/skia 1ae3e75a0b4c..981d590e8eba (1 commits) (flutter#17968)
eed05dd Add initial unit tests for the android embedding (flutter#17921)
168a65f Roll src/third_party/dart 2e438d1baffc..a53d336b9fd4 (4 commits) (flutter#17967)
742adb8 Roll src/third_party/skia 97cfb05aabe4..1ae3e75a0b4c (2 commits) (flutter#17966)
805ef7c Roll fuchsia/sdk/core/mac-amd64 from 2CE6x... to 9O3Ef... (flutter#17963)
11c6a18 Roll src/third_party/skia c12aad9485a9..97cfb05aabe4 (3 commits) (flutter#17957)
4e29e57 Roll fuchsia/sdk/core/mac-amd64 from 9-v-E... to 2CE6x... (flutter#17955)
4f3b929 Roll src/third_party/dart 216e3df4526c..2e438d1baffc (7 commits) (flutter#17950)
4f888d6 Change the repo fetch script used in integration tests (flutter#17943)
3999ef9 Roll src/third_party/skia 1e21d14f2b8b..c12aad9485a9 (20 commits) (flutter#17942)
d01de3b Roll src/third_party/dart a69cb6d700f5..216e3df4526c (16 commits) (flutter#17945)
2589d07 Fix accessibility focus loss when first focusing on text field (flutter#17803)
3af2b1a Roll fuchsia/sdk/core/linux-amd64 from _dAFU... to G4HpJ... (flutter#17938)
d132ac5 [web] Fix exception when getting boxes for rich text range (flutter#17933)
cade0e9 [web] Batch systemFontChange messages (flutter#17885)
christopherfujino added a commit that referenced this pull request Apr 28, 2020
0c35a34 Roll src/third_party/skia f5132a05c893..4baa7326ccfb (1 commits) (#18003)
f07712b Roll src/third_party/skia 5f56cb1d3b4f..f5132a05c893 (6 commits) (#17999)
103c9c7 Bumped up the timeout for testAccessibilityFocusOnTextSemanticsProducesCorrectIosViews (#17988)
a1fdff6 Roll src/third_party/skia 81ef385c1fcd..5f56cb1d3b4f (14 commits) (#17991)
22479ca Roll Dart to 726d3c772554924f62db0b9e0d4c280dbbddc824 (#17993)
494a63c Trial PR to enable null safety unfork in the Dart SDK. (#17818)
887efcb Roll fuchsia/sdk/core/linux-amd64 from eapIV... to 3h-X9... (#17987)
50ae2b9 Set SkSL asset manager in RunConfiguration ctor (#17948)
5f437fb Started ignoring remote keyboard notifications. (#17981)
027eff8 Reenable flutter scenic test to identify crashes and follow up on fixes. (#17979)
992a55c Update buildroot (#17978)
d5587df Roll src/third_party/skia 78debd6f6d83..81ef385c1fcd (1 commits) (#17976)
aa00d50 [web] Don't allow empty initial route (#17936)
3b0e415 Roll fuchsia/sdk/core/mac-amd64 from 9O3Ef... to arZdZ... (#17975)
66af9ea Roll src/third_party/skia 981d590e8eba..78debd6f6d83 (5 commits) (#17972)
732bf22 Manual roll of Dart to 03429b20cd67f85d65cc589b529ab8c1a4780912...a53d336b9fd4bbb415d2f1e3f4c653aa107f31c7 (#17971)
cad81c7 Roll src/third_party/skia 1ae3e75a0b4c..981d590e8eba (1 commits) (#17968)
eed05dd Add initial unit tests for the android embedding (#17921)
168a65f Roll src/third_party/dart 2e438d1baffc..a53d336b9fd4 (4 commits) (#17967)
742adb8 Roll src/third_party/skia 97cfb05aabe4..1ae3e75a0b4c (2 commits) (#17966)
805ef7c Roll fuchsia/sdk/core/mac-amd64 from 2CE6x... to 9O3Ef... (#17963)
11c6a18 Roll src/third_party/skia c12aad9485a9..97cfb05aabe4 (3 commits) (#17957)
4e29e57 Roll fuchsia/sdk/core/mac-amd64 from 9-v-E... to 2CE6x... (#17955)
4f3b929 Roll src/third_party/dart 216e3df4526c..2e438d1baffc (7 commits) (#17950)
4f888d6 Change the repo fetch script used in integration tests (#17943)
3999ef9 Roll src/third_party/skia 1e21d14f2b8b..c12aad9485a9 (20 commits) (#17942)
d01de3b Roll src/third_party/dart a69cb6d700f5..216e3df4526c (16 commits) (#17945)
2589d07 Fix accessibility focus loss when first focusing on text field (#17803)
3af2b1a Roll fuchsia/sdk/core/linux-amd64 from _dAFU... to G4HpJ... (#17938)
d132ac5 [web] Fix exception when getting boxes for rich text range (#17933)
cade0e9 [web] Batch systemFontChange messages (#17885)
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants