Skip to content

Conversation

@christopherfujino
Copy link
Contributor

@christopherfujino christopherfujino commented Feb 11, 2022

If a user provides the FLUTTER_STORAGE_BASE_URL env variable, this will be used as the maven host URL. However, the flutter http host doctor validator would still try to validate the upstream maven URL.

This PR updates the validator to only check the upstream maven host if FLUTTER_STORAGE_BASE_URL is NOT provided.

Fixes #98170

@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Feb 11, 2022
@christopherfujino christopherfujino changed the title make maven a conditional validator for http hosts check in doctor [flutter_tools] make maven a conditional validator for http hosts check in doctor Feb 11, 2022
@christopherfujino christopherfujino changed the title [flutter_tools] make maven a conditional validator for http hosts check in doctor [flutter_tools] do not validate maven upstream if cloud storage override provided Feb 14, 2022
@christopherfujino christopherfujino marked this pull request as ready for review February 14, 2022 20:04
];

/// MacOS specific required HTTP hosts.
const List<String> macOSRequiredHttpHosts = <String>[
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the macOSRequiredHttpHosts changed to a method as well to keep it consistent with the android?

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 don't think so, since it would be a function that takes no parameters and always returns the same value. In the end, I guess it's a style choice.

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 fluttergithubbot merged commit 4a11cc4 into flutter:master Feb 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 15, 2022
@christopherfujino christopherfujino deleted the update-http-host-validator-to-make-maven-conditional branch February 15, 2022 20:10
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 15, 2022
engine-flutter-autoroll added a commit to engine-flutter-autoroll/plugins that referenced this pull request Feb 15, 2022
clocksmith pushed a commit to clocksmith/flutter that referenced this pull request Mar 8, 2022
List<String> androidRequiredHttpHosts(Platform platform) {
return <String>[
// If kEnvCloudUrl is set, it will be used as the maven host
if (!platform.environment.containsKey(kEnvCloudUrl))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we check for kEnvCloudUrl in this case?

Copy link
Contributor

Choose a reason for hiding this comment

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

oh i see we always add that one

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.

[flutter_tools] the tool does not provide an override the maven.google.com web host

4 participants