we should detect the user's PATH env var and use that when we shell out to the flutter tool #601

devoncarew · 2017-01-09T16:09:41Z

In many OSes, when the user launches a GUI app from a launcher (OSX dock for example) the shell used is not necessarily the user's default shell. The env variables they had set up won't be visible to that GUI app. When we then shell out to something like the flutter tool, it won't be able to detect the location of the android deve tools, or other things that we'd find on a PATH.

See related (Dart) code here: https://github.com/dart-atom/atom.dart/blob/master/lib/node/process.dart#L190.

The text was updated successfully, but these errors were encountered:

skybrian · 2017-01-13T00:06:42Z

We should also handle the case where the Android dev tools aren't in the user's path at all.

For me, they're not, since I tend to use bash aliases for command I run regularly, instead of adding them to my path. It looks like I did set ANDROID_HOME, though.

devoncarew · 2017-01-13T03:05:17Z

The flutter command has some functionality for that - flutter config:

Usage: flutter config [arguments]
-h, --help                  Print this usage information.
    --[no-]analytics        Enable or disable reporting anonymously tool usage statistics and crash reports.
    --gradle-dir            The gradle install directory.
    --android-studio-dir    The Android Studio install directory.

skybrian · 2017-01-14T01:39:51Z

It looks like currently we don't use the PATH or environment variables at all. We find the Flutter SDK from the globally configured Dart SDK, under the assumption that the Dart SDK is in $FLUTTER_HOME/bin/cache/dart-sdk. If it's not, the getFlutterSdk() method returns null. Sometimes we handle that, other times it silently does nothing. [1]

It seems like we want the Flutter SDK and the Dart SDK to be consistent, so we should help the user set their Dart SDK correctly, and then everything works? At least, we need to show an error when the current Dart SDK is not within a Flutter directory, explaining what to change it to.

I'm not sure what the UI for this should look like. What are our options for displaying errors?

(Also, the DartSdk plugin currently doesn't have a per-project Dart SDK setting, but I guess that is fixed in 2017.1 [2])

[1] Code references:
FlutterSdk.getGlobalFlutterSdk() uses DartSdk.getGlobalDartSdk() and getFlutterSdkByDartSdk().

[2] https://youtrack.jetbrains.com/issue/WEB-20870

skybrian · 2017-01-14T02:59:34Z

It looks like this should be the job of: SdkConfigurationNotificationProvider.createNotificationPanel().

However, I set a breakpoint and changed the Dart SDK, and it didn't stop there.

devoncarew · 2017-01-14T20:01:24Z

At least, we need to show an error when the current Dart SDK is not within a Flutter directory, explaining what to change it to.

+1!

What are our options for displaying errors?

Some good UI options are showing a toast (an IntelliJ Notification), and displaying a butter bar at the top of the current editor.

Totally correct that IntelliJ isn't using PATH - we're locating relative to the sdk location. But when we shell out to the flutter tool, it using the PATH to discover other dev tools, like adb, the android sdk, ios tools, ... We want to set up PATH for the benefit of that tool, in order to make IntelliJ when launched from an app launcher similar to the behavior a user would see when they run the flutter cli tool or launch IntelliJ from the command-line.

ScottPierce · 2017-01-17T21:31:56Z

If what's being described here is an inability to access certain environment variables in IntellIj, I've worked around this for years for Android by using launchctl setenv in my ~/.bash_profile. This allows me to run the same commands in IntelliJ terminal as I run in my normal terminal for osx.

launchctl setenv PATH $PATH
launchctl setenv JAVA_HOME $JAVA_HOME
launchctl setenv ANDROID_HOME $ANDROID_HOME
launchctl setenv FLUTTER_ROOT $FLUTTER_ROOT

devoncarew · 2017-01-19T16:58:53Z

Just for context, many users are hitting this on gitter.

skybrian · 2017-01-19T21:30:20Z

What is gitter? How do I read more about this?

devoncarew · 2017-01-19T22:04:56Z

gitter: https://gitter.im/flutter/flutter :)

Just providing context for the amount of impact of the issue -

skybrian · 2017-01-20T00:41:09Z

I haven't reproduced this yet. I have a shell open with PATH='/usr/bin:/bin' and Flutter commands work fine so far, so I'm skeptical we need to set the PATH. If we do, it would be good to know which Flutter command cares about this.

skybrian · 2017-01-20T02:47:24Z

Today's issue on gitter was not this bug, it's #646.

devoncarew · 2017-01-20T12:42:20Z

I haven't reproduced this yet. I have a shell open with PATH='/usr/bin:/bin' and Flutter commands work fine so far, so I'm skeptical we need to set the PATH. If we do, it would be good to know which Flutter command cares about this.

Perhaps chatting by VC? I have seen this is previous tools. I haven't encountered it myself w/ the flutter plugin, but haven't specifically tried to repro, and the issues symptoms people have reported sounds a lot like this issue.

skybrian · 2017-01-20T22:08:19Z

I think what would help most is seeing more user reports, if you remember where they were.

skybrian · 2017-01-26T21:49:55Z

Flutter finds Android home here:
https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/android/android_sdk.dart

I think I wasn't able to reproduce because I had ANDROID_HOME set in ~/.bash_profile, and that's sufficient.

devoncarew · 2017-01-30T21:03:52Z

@skybrian, ping - is this in progress? We're hoping to release 0.1.9 tomorrow

skybrian · 2017-01-30T21:08:35Z

No progress yet but I'm working on it this afternoon/evening.

skybrian · 2017-01-31T02:41:51Z

Update: I don't think this is going to make the release.

skybrian · 2017-02-01T02:49:25Z

On second thought, I'm not sure there's anything to do.

I was able to verify that ANDROID_HOME is not set within the IDEA Java process when run from the dock, but it is set by the time the 'flutter' command starts when I run "flutter doctor" from within IDEA. How does this happen? It appears that IDEA already implements a similar workaround for Macs.

In the GeneralCommandLine class, there is a myParentEnvironmentType field which is set to ParentEnvironmentType.CONSOLE by default. When this is set, EnvironmentUtil.getEnvironmentMap() gets run. The ShellEnvReader class calls a helper script, which is at /Applications/IntelliJ\ IDEA\ CE.app/Contents/bin/printenv.py on my Mac.

As long as we use GeneralCommandLine everywhere, and don't change the default, I think the environment should be set up correctly. If not, I need to figure out what our workaround code should do that IDEA's workaround code doesn't already do.

devoncarew · 2017-02-01T03:04:18Z

Thanks for investigating! Do you know if they have similar workaround code for linux? (but from looking here: https://github.com/JetBrains/intellij-community/blob/master/platform/util/src/com/intellij/util/EnvironmentUtil.java#L164, it seems like they do)

I wonder what the user reports were running into? Perhaps they started IntelliJ, then configured their PATH? If I recall correctly, I think two separate github or gitter issues were resolved by closing IntelliJ and restarting it from the cli.

skybrian · 2017-02-01T03:20:17Z

I think just restarting should do it. The environment is only read once and cached. (There is a static block at the top of the file.)

devoncarew · 2017-02-01T03:34:42Z

OK, thanks again for looking into this. I think we should add a FAQ or troubleshooting item about this on our docs; that the PATH is read once at startup, and if devices show up when running flutter devices on the cli, but not in IntelliJ, a useful troubleshooting step is just to restart IntelliJ.

/cc @mit-mit

skybrian · 2017-02-01T19:32:02Z

Looks like there is a known issue for Linux:
https://youtrack.jetbrains.com/issue/IDEA-146037

skybrian · 2017-02-01T21:55:09Z

I filed a bug for the issue in EnvironmentUtil:
https://youtrack.jetbrains.com/issue/IDEA-167521

devoncarew · 2017-02-01T21:57:29Z

👍

devoncarew · 2017-04-03T15:51:48Z

Closing - the fix for this will happen upstream.

ohir · 2018-03-05T07:49:27Z

I was hit by this (and #634) with AS 3.0 and just installed official beta. There is NO mention of this glitch within documentation. For a quick workaround I set flutter config --android-studio-dir and --android-sdk manually as hinted by above discussion but it costed me finding #634 and this, both closed, issues.

As there is bold advise on the install doc to run doctor twice the quick fix is possible:
flutter doctor SHOULD update config using user's environment.

--
[✓] Flutter (Channel beta, v0.1.5, on Linux, locale pl_PL.utf8)
[✓] Android toolchain - develop for Android devices (Android SDK 26.0.2)
[✓] Android Studio (version 3.0)
[✓] Connected devices (1 available)

devoncarew modified the milestones: On Deck, 0.1.9 Jan 9, 2017

skybrian self-assigned this Jan 13, 2017

devoncarew mentioned this issue Jan 17, 2017

Intellij fails to see connected devices #634

Closed

mit-mit added the topic-getting-started label Jan 18, 2017

devoncarew modified the milestones: On Deck, 0.1.9 - getting started Jan 31, 2017

devoncarew unassigned skybrian Feb 1, 2017

mit-mit added the needs-known-issue-docs label Feb 1, 2017

devoncarew closed this as completed Apr 3, 2017

fehnomenal mentioned this issue Dec 2, 2020

Does not work on IntelliJ IDEA 2020.2.3 (Ultimate Edition) fehnomenal/intellij-direnv#3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

we should detect the user's PATH env var and use that when we shell out to the flutter tool #601

we should detect the user's PATH env var and use that when we shell out to the flutter tool #601

devoncarew commented Jan 9, 2017

skybrian commented Jan 13, 2017

devoncarew commented Jan 13, 2017 •

edited

Loading

skybrian commented Jan 14, 2017

skybrian commented Jan 14, 2017

devoncarew commented Jan 14, 2017

ScottPierce commented Jan 17, 2017 •

edited

Loading

devoncarew commented Jan 19, 2017

skybrian commented Jan 19, 2017

devoncarew commented Jan 19, 2017

skybrian commented Jan 20, 2017

skybrian commented Jan 20, 2017

devoncarew commented Jan 20, 2017

skybrian commented Jan 20, 2017

skybrian commented Jan 26, 2017

devoncarew commented Jan 30, 2017

skybrian commented Jan 30, 2017

skybrian commented Jan 31, 2017

skybrian commented Feb 1, 2017 •

edited

Loading

devoncarew commented Feb 1, 2017

skybrian commented Feb 1, 2017

devoncarew commented Feb 1, 2017

skybrian commented Feb 1, 2017

skybrian commented Feb 1, 2017

devoncarew commented Feb 1, 2017

devoncarew commented Apr 3, 2017

ohir commented Mar 5, 2018

we should detect the user's PATH env var and use that when we shell out to the flutter tool #601

we should detect the user's PATH env var and use that when we shell out to the flutter tool #601

Comments

devoncarew commented Jan 9, 2017

skybrian commented Jan 13, 2017

devoncarew commented Jan 13, 2017 • edited Loading

skybrian commented Jan 14, 2017

skybrian commented Jan 14, 2017

devoncarew commented Jan 14, 2017

ScottPierce commented Jan 17, 2017 • edited Loading

devoncarew commented Jan 19, 2017

skybrian commented Jan 19, 2017

devoncarew commented Jan 19, 2017

skybrian commented Jan 20, 2017

skybrian commented Jan 20, 2017

devoncarew commented Jan 20, 2017

skybrian commented Jan 20, 2017

skybrian commented Jan 26, 2017

devoncarew commented Jan 30, 2017

skybrian commented Jan 30, 2017

skybrian commented Jan 31, 2017

skybrian commented Feb 1, 2017 • edited Loading

devoncarew commented Feb 1, 2017

skybrian commented Feb 1, 2017

devoncarew commented Feb 1, 2017

skybrian commented Feb 1, 2017

skybrian commented Feb 1, 2017

devoncarew commented Feb 1, 2017

devoncarew commented Apr 3, 2017

ohir commented Mar 5, 2018

devoncarew commented Jan 13, 2017 •

edited

Loading

ScottPierce commented Jan 17, 2017 •

edited

Loading

skybrian commented Feb 1, 2017 •

edited

Loading