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

[tool] Skip pathified analysis on resolver errors #4647

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .ci/scripts/analyze_pathified.sh
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,13 @@ set -e
# This uses --run-on-dirty-packages rather than --packages-for-branch
# since only the packages changed by 'make-deps-path-based' need to be
# re-checked.
# --skip-if-resolving-fails is used to avoid failing if there's a resolution
# conflict when using path-based dependencies, because that indicates that
# the failing packages won't pick up the new versions of the changed packages
# when they are published anyway, so publishing won't cause an out-of-band
# failure regardless.
dart ./script/tool/bin/flutter_plugin_tools.dart analyze --run-on-dirty-packages \
--skip-if-resolving-fails \
--log-timing --custom-analysis=script/configs/custom_analysis.yaml
# Restore the tree to a clean state, to avoid accidental issues if
# other script steps are added to the enclosing task.
Expand Down
22 changes: 22 additions & 0 deletions script/tool/lib/src/analyze_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

import 'package:file/file.dart';
import 'package:yaml/yaml.dart';

Expand Down Expand Up @@ -34,12 +36,18 @@ class AnalyzeCommand extends PackageLoopingCommand {
argParser.addFlag(_libOnlyFlag,
help: 'Only analyze the lib/ directory of the main package, not the '
'entire package.');
argParser.addFlag(_skipIfResolvingFailsFlag,
help: 'If resolution fails, skip the package. This is only '
'intended to be used with pathified analysis, where a resolver '
'failure indicates that no out-of-band failure can result anyway.',
hide: true);
}

static const String _customAnalysisFlag = 'custom-analysis';
static const String _downgradeFlag = 'downgrade';
static const String _libOnlyFlag = 'lib-only';
static const String _analysisSdk = 'analysis-sdk';
static const String _skipIfResolvingFailsFlag = 'skip-if-resolving-fails';

late String _dartBinaryPath;

Expand Down Expand Up @@ -134,6 +142,20 @@ class AnalyzeCommand extends PackageLoopingCommand {
.pubspecFile
.existsSync()) {
if (!await _runPubCommand(packageToGet, 'get')) {
if (getBoolArg(_skipIfResolvingFailsFlag)) {
// Re-run, capturing output, to see if the failure was a resolver
// failure. (This is slightly inefficient, but this should be a
// very rare case.)
const String resolverFailureMessage = 'version solving failed';
final io.ProcessResult result = await processRunner.run(
flutterCommand, <String>['pub', 'get'],
workingDir: packageToGet.directory);
if ((result.stderr as String).contains(resolverFailureMessage) ||
(result.stdout as String).contains(resolverFailureMessage)) {
logWarning('Skipping package due to pub resolution failure.');
return PackageResult.skip('Resolution failed.');
}
}
return PackageResult.fail(<String>['Unable to get dependencies']);
}
}
Expand Down
33 changes: 33 additions & 0 deletions script/tool/test/analyze_command_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -315,6 +315,39 @@ void main() {
});
});

test('skips if requested if "pub get" fails in the resolver', () async {
final RepositoryPackage plugin = createFakePlugin('foo', packagesDir);

final FakeProcessInfo failingPubGet = FakeProcessInfo(
MockProcess(
exitCode: 1,
stderr: 'So, because foo depends on both thing_one ^1.0.0 and '
'thing_two from path, version solving failed.'),
<String>['pub', 'get']);
processRunner.mockProcessesForExecutable['flutter'] = <FakeProcessInfo>[
failingPubGet,
// The command re-runs failures when --skip-if-resolver-fails is passed
// to check the output, so provide the same failing outcome.
failingPubGet,
];

final List<String> output = await runCapturingPrint(
runner, <String>['analyze', '--skip-if-resolving-fails']);

expect(
output,
containsAllInOrder(<Matcher>[
contains('Skipping package due to pub resolution failure.'),
]),
);
expect(
processRunner.recordedCalls,
orderedEquals(<ProcessCall>[
ProcessCall('flutter', const <String>['pub', 'get'], plugin.path),
ProcessCall('flutter', const <String>['pub', 'get'], plugin.path),
]));
});

test('fails if "pub get" fails', () async {
createFakePlugin('foo', packagesDir);

Expand Down