-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Embed project.assets.json in binlog #16840
Embed project.assets.json in binlog #16840
Conversation
We're thinking of adding a way to embed arbitrary files in the binlog. See dotnet/msbuild#6339 project.assets.json is commonly requested to be embedded. This would fix dotnet/msbuild#3529 Binlog size does increase from 3.5 MB -> 5 MB, so we'll need to think perhaps about making it off by default.
I couldn't figure out the best area label to add to this PR. If you have write-permissions please help me learn by adding exactly one area label. |
Given that it's super useful to have the project.asset.jsons in the binlog, and given that you've optimized and reduced the binlog size way beyond the extra size added by the jsons, I think it's worth having it on by default. |
@KirillOsenkov is there anything in the project.assetts.json file that is more risk from a privacy perspective than that's already included in the binlogs? Do we need tests in this area as we don't have existing tests for binlogs in the sdk? |
Nothing in project.assets.json that could be PII as far as I can think. It does include full paths to .csproj files, but those are already in the binlog. Don't think we need tests for this case, as it's literally an item group and it's non-essential. There's bigger areas in MSBuild that are in more need of tests so I'd say let's spend efforts there instead. |
This change just helped me work around a race condition that's been plaguing me for days. Great change, thanks! |
We're thinking of adding a way to embed arbitrary files in the binlog.
See dotnet/msbuild#6339
project.assets.json is commonly requested to be embedded. This would fix dotnet/msbuild#3529
Binlog size does increase from 3.5 MB -> 5 MB, so we'll need to think perhaps about making it off by default.