Skip to content
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

PR #9564 breaks manifest update on Windows #9736

Closed
Hexcles opened this issue Mar 1, 2018 · 0 comments · Fixed by #9737
Closed

PR #9564 breaks manifest update on Windows #9736

Hexcles opened this issue Mar 1, 2018 · 0 comments · Fixed by #9737

Comments

@Hexcles
Copy link
Member

Hexcles commented Mar 1, 2018

PR #9564 (which is a fix for #7142) breaks manifest update on Windows because the change violates Windows' restrictions on parallel file access.

There are two issues in this line https://github.com/w3c/web-platform-tests/pull/9564/files#diff-2a239eec150d25cf7486394513fb1e63R242:

  1. The temp file is moved before it's closed, which can be easily fixed.
  2. The much harder problem is the manifest itself. manifest is a file-like object, which means it's already opened somewhere else, so overwriting the file (with a new file handler) will fail on Windows. And we can't simply close manifest here because it'd invalidate the handler, causing other failures.

My proposal is to revert #9564 (which means reopen #7142 ), because currently all wpt commands are unusable on Windows while #7142 is just an edge case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant