msbuild icon indicating copy to clipboard operation
msbuild copied to clipboard

Accept `TaskParameterMessageKind.TaskOutput` when filtering events for EmbedInBinlog

Open MeikTranel opened this issue 1 year ago • 3 comments

Before we only interpreted plain AddItem messages as directives for file embedding.

Resolves #7665

Changes Made

Added TaskParameterEvents with kind TaskOutput to the event filter for embedding files.

Testing

I started out writing tests to catch a dedicated FileImported, because i thought the file was embedded as a payload inside a BuildEvent somehow, but if i get this correctly the archive is just a secondary payload to the binlog - meant for readers to use as a dictionary to look for files without any guarantees that files are actually there. I'm not actually digging inside the .binlog file, but i found reading the external *.ProjectImports.zip to be an acceptable alternative to get at least some coverage regarding EmbedInBinlog resulting in embedded files.

MeikTranel avatar Aug 08 '22 22:08 MeikTranel

I can just drop the explicit path - that fixes building on both windows and linux. More importantly now that the feature itself works - i just tested via WSL and it fails on the zip assertion because the Name is the full path instead of just the file name + extension. So i'm asking myself here - is this normal behavior with the internal ZipArchive on linux/debian/wsl ? image image

MeikTranel avatar Aug 09 '22 19:08 MeikTranel

I looked at documentation for ZipArchive.

It explicitly states that it looks after the last path separator character (\). I'm guessing it hasn't been made unix-friendly and doesn't realize that there are other slashes 😄

Forgind avatar Aug 09 '22 21:08 Forgind

Oh my lord - 😂😂 - well imma have to work around that now 🥳🥳 i guess .EndsWith works.

@KirillOsenkov i assume this doesn't show up on your end because you are working with the fully qualified path anyways?

MeikTranel avatar Aug 09 '22 21:08 MeikTranel

Thanks for the contribution!

benvillalobos avatar Aug 16 '22 17:08 benvillalobos