-
Notifications
You must be signed in to change notification settings - Fork 29k
[flutter_tools] add working directory to ProcessException when pub get fails #91436
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
Conversation
@@ -2,6 +2,8 @@ | |||
// Use of this source code is governed by a BSD-style license that can be | |||
// found in the LICENSE file. | |||
|
|||
import 'dart:io' as io; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a forbidden import, you'll need to add this path to
flutter/packages/flutter_tools/test/integration.shard/forbidden_imports_test.dart
Line 64 in 2d07436
test('no unauthorized imports of dart:io', () { |
Also, let's limit the package leak to just ProcessException
.
import 'dart:io' as io; | |
import 'dart:io' as io show ProcessException; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually, it's already in scope as io
from line 14. I'm surprised this compiles...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm w/ question
throw io.ProcessException( | ||
exception.executable, | ||
exception.arguments, | ||
'${exception.message}\nWorking directory: "$directory"', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there anything else that would be useful to put in here while you're at it? Anything from the environment or maybe the flutterRootOverride
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks like flutterRootOverride
is only ever set in tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(i'm looking for relevant env vars)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added what we're sending to pub as PUB_ENVIRONMENT
Add debugging for #80418