-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Block all ProjectImports.zip files #5050
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
Conversation
|
Hello @asklar! Because this pull request has the Do note that I've been instructed to only help merge pull requests of this repository that have been opened for at least 10 hours, a condition that will be fulfilled in about 9 hours 43 minutes. No worries though, I will be back when the time is right! 😉 p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
dannyvv
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
|
Was also thinking to update the logging (since the text logs go to %TEMP%, move those to a well defined relative folder i.e. |
|
Do you also need to update the metro.config.js in packages/playground? #Resolved |
@kmelmon |
|
thanks! In reply to: 636170652 [](ancestors = 636170652) |
kmelmon
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
![]()
Fixes #5033 - at least the ProjectImports.zip part :)
Grab yourselves a chair because this is a doozy.
We've seen intermittent reports of Metro bundler hitting file-in-use errors on
msbuild.ProjectImports.zipand similar. However there are no such files... WTH is going on and who is creating these files?!We invoke MSBuild with the
/blflag to produce binary logs that we can inspect should something go wrong while building. These binary logs include all the commands that the build ran as well as all of the project files, props, targets, etc. that were imported. You can control whether the imported files are included in the.binlogfile by choosing whether you want toEmbedthe imports (this is the default), create a separate zip file (ZipFile) or not include the imports at all (None) by specifying something like/bl:ProjectImports=None, etc.We don't specify any
ProjectImportsflag so we are getting the default (Embed). So why is Metro complaining? Well, after looking through the MSBuild code, it turns out that the way this is implemented in MSBuild is that the zip file is created, msbuild writes to it as the build progresses, and then when it's about to finish it closes the file, reads all its contents and writes the contents into the binlog, then deletes the file. This approach will create file contention obviously.Relevant MSBuild code
So then the question is but why does Metro care about this file? Why is it even monitoring it?
It turns out Metro has a set of asset and source extensions that are configurable (see configuration). The default Metro set of asset extensions however, includes zip files: Default metro configuration
This change updates our
metro.config.jsfiles to ignore all*.ProjectImports.zipfiles, not justmsbuild.ProjectImports.zip. Note that the prefix is given by the name of the binary log file, which is why we started seeing errors aboutDeploy.ProjectImports.ziptoo.I've also filed a doc bug and a product bug against MSBuild
Microsoft Reviewers: Open in CodeFlow