Parse engine src path from an absolute --local-engine#78496
Parse engine src path from an absolute --local-engine#78496jmagman merged 2 commits intoflutter:masterfrom
Conversation
jmagman
added
c: contributor-productivity
| return null; | ||
| } | ||
|
|
||
| Future<String> _findEngineSourceBySkyEngine(String packagePath) async { |
There was a problem hiding this comment.
This method was extracted from findEnginePath to make that method more readable, but has not been changed.
| return null; | ||
| } | ||
|
|
||
| String _findEngineSourceByLocalEngine(String localEngine) { |
| _logger.printTrace('Could not find engine source for $localEngine, skipping'); | ||
| } | ||
| if (localEngine != null) { | ||
| throwToolExit( |
There was a problem hiding this comment.
Now throw instead of silently fail if --local-engine is set but the engineSourcePath can't be found.
yjbanov
left a comment
yjbanov
left a comment
There was a problem hiding this comment.
Thank you! Just a couple of nitpicks. LGTM.
| final Directory localEngineDirectory = _fileSystem.directory(localEngine); | ||
| final Directory outDirectory = localEngineDirectory?.parent; | ||
| final Directory srcDirectory = outDirectory?.parent; | ||
| if (localEngineDirectory.existsSync() && outDirectory.basename == 'out' && srcDirectory.basename == 'src') { |
There was a problem hiding this comment.
On line 98 localEngineDirectory is guarded against null, which means on this line it may still be null. Should we guard against null here? Or remove the guard above?
| return null; | ||
| } | ||
|
|
||
| Future<String> _findEngineSourceBySkyEngine(String packagePath) async { |
There was a problem hiding this comment.
nit: would _findEngineSourceByDotPackagesFile be a better name?
There was a problem hiding this comment.
How about _findEngineSourceByPackageConfig?
|
Google testing failure unrelated. |
2 participants
This PR parses the
srcdirectory from--local-engineif it is an absolute path when--local-engine-src-pathis not specified. This will support LUCI recipes with local engine builds that are not a sibling to the flutter root (like--local-engine=/b/s/w/ir/cache/builder/src/out/host_debug_unopt).The tool will then continue with the previous resolution steps: parse the
srcdirectory from.packagessky_enginepath, then checks if theengineis a sibling directory to thefluttercheckout.It also exits the tool with a failure if
--local-engineis set but thesrcdirectory cannot be found, instead of silently failing and using thebin/cacheengine.Fixes #78072