Skip to content

Commit d7ee75a

Browse files
[tool] Skip pathified analysis on resolver errors (flutter#4647)
If adding a pathified dependency creates a resolver error, then skip it instead of failing when running pathified analysis. The purpose of pathified analysis it to pre-detect failures that would happen on publishing, and if there's a resolver error that means the publishing even won't affect the package anyway. See flutter/packages#4483 (comment) for an example case where we need this. (In theory we could get delayed OOB errors that this will miss�e.g., in the case above if the PR would actually break rfw/example/wasm, then if at some later date `wasm` updated to use a newer `ffi`, eliminating the resolver conflict, then suddenly rfw/example/wasm would pick up the PR and break. That seems *extremely* unlikely, however, so I'm not concerned that this will be a problem in practice. We can revisit if that changes.)
1 parent 992422c commit d7ee75a

File tree

3 files changed

+61
-0
lines changed

3 files changed

+61
-0
lines changed

.ci/scripts/analyze_pathified.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,13 @@ set -e
1010
# This uses --run-on-dirty-packages rather than --packages-for-branch
1111
# since only the packages changed by 'make-deps-path-based' need to be
1212
# re-checked.
13+
# --skip-if-resolving-fails is used to avoid failing if there's a resolution
14+
# conflict when using path-based dependencies, because that indicates that
15+
# the failing packages won't pick up the new versions of the changed packages
16+
# when they are published anyway, so publishing won't cause an out-of-band
17+
# failure regardless.
1318
dart ./script/tool/bin/flutter_plugin_tools.dart analyze --run-on-dirty-packages \
19+
--skip-if-resolving-fails \
1420
--log-timing --custom-analysis=script/configs/custom_analysis.yaml
1521
# Restore the tree to a clean state, to avoid accidental issues if
1622
# other script steps are added to the enclosing task.

script/tool/lib/src/analyze_command.dart

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import 'dart:io' as io;
6+
57
import 'package:file/file.dart';
68
import 'package:yaml/yaml.dart';
79

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

3946
static const String _customAnalysisFlag = 'custom-analysis';
4047
static const String _downgradeFlag = 'downgrade';
4148
static const String _libOnlyFlag = 'lib-only';
4249
static const String _analysisSdk = 'analysis-sdk';
50+
static const String _skipIfResolvingFailsFlag = 'skip-if-resolving-fails';
4351

4452
late String _dartBinaryPath;
4553

@@ -134,6 +142,20 @@ class AnalyzeCommand extends PackageLoopingCommand {
134142
.pubspecFile
135143
.existsSync()) {
136144
if (!await _runPubCommand(packageToGet, 'get')) {
145+
if (getBoolArg(_skipIfResolvingFailsFlag)) {
146+
// Re-run, capturing output, to see if the failure was a resolver
147+
// failure. (This is slightly inefficient, but this should be a
148+
// very rare case.)
149+
const String resolverFailureMessage = 'version solving failed';
150+
final io.ProcessResult result = await processRunner.run(
151+
flutterCommand, <String>['pub', 'get'],
152+
workingDir: packageToGet.directory);
153+
if ((result.stderr as String).contains(resolverFailureMessage) ||
154+
(result.stdout as String).contains(resolverFailureMessage)) {
155+
logWarning('Skipping package due to pub resolution failure.');
156+
return PackageResult.skip('Resolution failed.');
157+
}
158+
}
137159
return PackageResult.fail(<String>['Unable to get dependencies']);
138160
}
139161
}

script/tool/test/analyze_command_test.dart

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -315,6 +315,39 @@ void main() {
315315
});
316316
});
317317

318+
test('skips if requested if "pub get" fails in the resolver', () async {
319+
final RepositoryPackage plugin = createFakePlugin('foo', packagesDir);
320+
321+
final FakeProcessInfo failingPubGet = FakeProcessInfo(
322+
MockProcess(
323+
exitCode: 1,
324+
stderr: 'So, because foo depends on both thing_one ^1.0.0 and '
325+
'thing_two from path, version solving failed.'),
326+
<String>['pub', 'get']);
327+
processRunner.mockProcessesForExecutable['flutter'] = <FakeProcessInfo>[
328+
failingPubGet,
329+
// The command re-runs failures when --skip-if-resolver-fails is passed
330+
// to check the output, so provide the same failing outcome.
331+
failingPubGet,
332+
];
333+
334+
final List<String> output = await runCapturingPrint(
335+
runner, <String>['analyze', '--skip-if-resolving-fails']);
336+
337+
expect(
338+
output,
339+
containsAllInOrder(<Matcher>[
340+
contains('Skipping package due to pub resolution failure.'),
341+
]),
342+
);
343+
expect(
344+
processRunner.recordedCalls,
345+
orderedEquals(<ProcessCall>[
346+
ProcessCall('flutter', const <String>['pub', 'get'], plugin.path),
347+
ProcessCall('flutter', const <String>['pub', 'get'], plugin.path),
348+
]));
349+
});
350+
318351
test('fails if "pub get" fails', () async {
319352
createFakePlugin('foo', packagesDir);
320353

0 commit comments

Comments
 (0)