What is this being used for?

See #132985 (comment). In short, it's to help disambiguate in scenarios where multiple declarations affect the same asset:

assets:
  - path: assets/paid/
    flavor: paid
  - path: assets/paid/pip.mp3 # For pip.mp3, this declaration overrides the previous one.
    flavor: free

This could probably just be a ToolExit instead though.

Can you ellaborate a bit on what this error state is? Its an error in your implementation to have a flavor specific asset with more than one flavor by listing it twice?

I've tried to capture the intended behavior in the tests. In short, the answer is, yes, this implementation doesn't support multiple flavors for one asset. However, we could add support for that by, for example, changing the manifest syntax to accept a flavors list rather than a flavor string.

The error logic you pointed out is certainly complicated. To better understand where it came from, start by considering this case:

assets:
  - assets/
  - path: assets/free.mp3
    flavor: free

To me, it is intuitive to bundle free.mp3 if and only if --flavor is set to free. Following that logic, my first intuition was that any asset covered by an entry with a flavor is globally limited to that flavor. Therefore, two declarations that list the same asset but with different flavors results in an ambiguous manifest¹:

assets:
  - path: assets/pip.mp3
    flavor: free
  - path: assets/pip.mp3
    flavor: paid

# asset/pip.mp3 can't be limited *only* to "free" and also *only* to "paid"

To finally circle back to the weird error logic you highlighted, consider this kind of case (which is hopefully extremely rare):

assets:
  - path: assets/paid/
    flavor: paid
  - path: assets/paid/pip.mp3
    flavor: free

Here, for pip.mp3, the second entry takes precedence over the first since it is more specific. However, in hindsight, I think it's okay to make the code simpler by having this also result in a ToolExit, because I don't expect many—if any—users would specify something like this.

One alternative to all of this is reconsidering one of my old ideas where flavor-specific assets aren't listed in the asset manifest but rather within separate flavor manifests:

assets:
  - assets/
# ...
flavors:
  - name: free
    additional-assets: # Assets can only be added, not removed
     - assets/free/

Still, if we add other conditional logic to assets in the future (e.g. platform-specificity), we'll still run into the same kind of problem. What if I have an asset that is specific to a combination of platform and flavor?

Interesting! As a side note, these sort of comments are absolute 🥇 so consider adding these as documentation in the code.

I don't remember, but I suspect what happens today is that when you list the same asset multiple times, or list a directory and then a specific asset, that it gets ignored? And I suppose this probably works fine since we only need a signal as to whether or not the asset is included - there is no additional metadata.

But now with flavor specific assets, we do have additional data. I think for the first version I like this approach - lets keep it simple and define this as an error case, we can always weaken it later. The alternative is having rules for resolving confilicts which sounds like it would only be more complicated.

Consider pulling this code into its own method with your comment as a docstring.

I don't remember, but I suspect what happens today is that when you list the same asset multiple times, or list a directory and then a specific asset, that it gets ignored?

IIRC, redundant entries will get processed, but (as you alluded to) it's essentially a no-op (we just overwrite entries in the output manifest with identical data).

But now with flavor specific assets, we do have additional data. I think for the first version I like this approach - lets keep it simple and define this as an error case, we can always weaken it later.

I assume you are referring to this case (and implicitly the one just before it):

assets:
  - path: assets/paid/
    flavor: paid
  - path: assets/paid/pip.mp3
    flavor: free

If so, yeah, I think it makes sense to just have this be an error case for now. Initially, I thought this case might be more common if/when we support glob/regex declarations, but even then I don't think any significant number of users would need conflict-resolving logic.

Consider pulling this code into its own method with your comment as a docstring.

Sure, I'll work on that. I'm glad you enjoyed my writing :)

Everything here SGTM

I did some more pondering on this (cheers to @jonahwilliams for chatting with me on this). To recap, the central question is "what should we do when multiple asset entries without identical¹ conditions affect the same file?" Should conditions from separate entries be considered disjunctively (with "or" logic) or conjunctively (with "and" logic)? Should it depend on the specificity of those entries (i.e. file vs directory)? I don't believe there are obvious answers to these questions, but we don't need answers right now—if at all. We can instead throw a ToolExit when such ambiguities arise, and if users later overwhelmingly agree that such conditions should be combined in a specific way, we can change the behavior later without breaking anyone. In the meantime, the users can avoid these ToolExits by writing more explicit assets manifests and/or organizing assets into individual directories².

To be clear, all the example assets manifests I provided previously in this thread (including the first one) would result in a ToolExit being thrown. Here's a specific example to focus on:

assets:
  - assets/
  - path: assets/free.mp3
    flavor: free

Assuming the file assets/free.mp3 exists, this would result in a ToolExit. The message would explain that the tool could not determine whether or not to bundle assets/free.mp3 and where the ambiguity the came from, along with a vague—but still useful—hint as to how to remove the ambiguity.

Is it correct that this ignores flavor?

nit: use a StringBuffer

I checked with #hackers-dart, and I learned that the Dart compiler does not automatically optimize this in. I've replaced errorMessage with a StringBuffer in c494656.

(IIRC OpenJDK and maybe Rosyln have such optimizations, but that's years old memory and I could be very wrong)

interesting discussion on this: dart-lang/sdk#50304 (comment)

If you return you don't need to do an else:

if (yaml is String) {
  ...
  return ..;
}

if (yaml is Map) {
  ...
}

Addressed in nits

Addressed in nits

Consider putting flavor on debugging options instead.

This reminded me that the flavor is already available in debuggingOptions.buildInfo, so we can just reference that. Addressed in remove redundant flavor arg from HotRunner