[native_assets_cli] Consider dynamically injecting verifiers that run after `build()` / `link()` methods called user-provided closure

[mkustermann]

With the recent refactoring to make the CLI extensible, we removed the verification happening inside the hook, now only the invoker of the hook does the verification.

We could restore this regression by dynamically injecting verifier methods in the extension methods, e.g. output.codeAssets.add() could install a validateCodeAssetBuildOutput verifier into a Set<> that the build implementation will then run.

[dcharkes]

Ah you came up with an idea to achieve this!

I was also thinking that instead of validateCodeAssetBuildOutput in the PR, we could have CodeAsset.validateBuildOutput (static method). Doing that for all asset types makes the validators easy to find.

[mkustermann]

CodeAsset.validateBuildOutput (static method). Doing that for all asset types makes the validators easy to find.

I didn't want to do that, because a normal hook/build.dart writer shouldn't have to know about those validation methods, they shouldn't need to call them. So they shouldn't be in scope / available for code completion / ... (without explicitly importing them)

[mkustermann]

I still believe that the hook/{build,link}.dart should not do the validation. Conceptually this is the job of the bundling tool, it has to do validation.

Here's a few reasons why I'm convinced of this:

• Users are not expected to execute these hooks themselves, they should be invoked by a bundling tool which already will do validation and reports it.
• If users want to write tests for their hooks, then those tests should use a native assets builder infrastructure which supports verification.
• The bundling tool will do more validation than what a hook/{build,link}.dart hook even can do: per asset type but also across all assets from all packages - the hook itself cannot do that
• There's a version skew: The bundling tool (Flutter SDK / Dart SDK / ...) may use a different version of the verification logic than the hook writer. That means the bundling tool may do slightly different validation than the self-validation the hook would do => That can lead to really weird situations where a hook may throw an exception because it's validation fails but the bundling tool would actually accept the hook output. Or the hook may run validation without errors and the bundling tool has extra validation that fails.
• We create a situation where validation errors can occur in two places and manifest in two different ways: => A hook may throw an exception with a validation error (and possibly print the error to stdout/stderr) => The bundling tool may report validation errors in a different way (logging it in to a Logging instance) => This isn't a clean user experience because validation errors can be reported in --verbose or log files in two very different ways

So despite my idea of how we could do it, I still think we shouldn't do it: Follow the single-responsibility design principle: Validation is the responsibility of one component, which is the bundling tool.

[dcharkes]

• Users are not expected to execute these hooks themselves

I believe when the output is erroneous, the easiest way for users to triage the errors is to step through their hook execution with a debugger.

I believe it's better to report errors early instead of letting them propagate.

But let's disagree and commit...

#notPlanned

[dcharkes]

We cannot have validation for temporary asset types (that only live between build and link) in embedders, because embedders don't know about those. So if we want to support validation for those, we should consider implementing this.

[mkustermann]

We cannot have validation for temporary asset types (that only live between build and link) in embedders, because embedders don't know about those. So if we want to support validation for those, we should consider implementing this.

If we allow arbitrary asset types between build & link then even the current system cannot do any validation between those (i.e. it's unrelated to https://github.com/dart-lang/native/pull/1623)

I'd say that's something the asset type support and the linker have to agree / deal with - as the bundling tool really doesn't care about the data that flows from build hooks to link hooks, it only cares about the final assets it has to deal with (and embed in the application bundle).