Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[AssetMapper] fix tests on Windows#58152

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
xabbuh merged 1 commit intosymfony:7.2fromxabbuh:pr-57890
Sep 3, 2024
Merged

Conversation

@xabbuh
Copy link
Member

QA
Branch?7.2
Bug fix?no
New feature?no
Deprecations?no
Issues
LicenseMIT

@xabbuh
Copy link
MemberAuthor

There is a temp file created that also needs to be removable when cleaning up in thetearDown() method:https://ci.appveyor.com/project/fabpot/symfony/builds/50521173#L1064

@xabbuhxabbuh merged commit1958d21 intosymfony:7.2Sep 3, 2024
@xabbuhxabbuh deleted the pr-57890 branchSeptember 3, 2024 07:52
@smnandre
Copy link
Member

Thank you@xabbuh this test really broke my mind (#58141 (review))

I'm not sure exacty "when" this temp file is created... my guts say there is something wrong with the /.! format that mess with the Finder list of files, and we end up trying to remove a directory before its content, but until now my guts have been wrong on this so ... 😅

Notes i made trying to debug this:

1) Symfony\Component\AssetMapper\Tests\ImportMap\RemotePackageStorageTest::testSaveThrowsWhenFailingSymfony\Component\Filesystem\Exception\IOException: Failed to remove file "C:\Users\appveyor\AppData\Local\Temp\1/.!!8wd\assets\acme\vendor\module_specifier\modF8A2.tmp": unlink(C:\Users\appveyor\AppData\Local\Temp\1/.!!8wd\assets\acme\vendor\module_specifier\modF8A2.tmp): Permission deniedC:\projects\symfony\src\Symfony\Component\Filesystem\Filesystem.php:197C:\projects\symfony\src\Symfony\Component\Filesystem\Filesystem.php:185C:\projects\symfony\src\Symfony\Component\Filesystem\Filesystem.php:185C:\projects\symfony\src\Symfony\Component\Filesystem\Filesystem.php:185C:\projects\symfony\src\Symfony\Component\Filesystem\Filesystem.php:185C:\projects\symfony\src\Symfony\Component\Filesystem\Filesystem.php:185C:\projects\symfony\src\Symfony\Component\Filesystem\Filesystem.php:153C:\projects\symfony\src\Symfony\Component\AssetMapper\Tests\ImportMap\RemotePackageStorageTest.php:35--

So the exception is thrown during the test teardown (RemotePackageStorageTest.php)

protectedfunctiontearDown():void
{
$this->filesystem->remove(self::$writableRoot);
}

This method calls the Filesystem remove() method (with a directory path)

The remove then call the doRemove with the directory and recursive "false"

And after some iterations of

$filesystemIterator =new \FilesystemIterator($file, \FilesystemIterator::CURRENT_AS_PATHNAME | \FilesystemIterator::SKIP_DOTS);
self::doRemove(iterator_to_array($filesystemIterator,true),true);

We arrive at the final exception

}elseif (!self::box('unlink',$file) && ((self::$lastError &&str_contains(self::$lastError,'Permission denied')) ||file_exists($file))) {
thrownewIOException(\sprintf('Failed to remove file "%s":',$file).self::$lastError);
}

So if you have any way to know precisely when it is created i'd be very interested to know

@xabbuh
Copy link
MemberAuthor

xabbuh commentedSep 5, 2024•
edited
Loading

I just had another look and found the root issue to be located in the Filesystem component. In thedumpFile() method a temporary file is used which reuses the permissions of the target file leading to it not being removed on Windows filesystems afterwards if it has no write permissions:#58185

smnandre reacted with thumbs up emoji

nicolas-grekas added a commit that referenced this pull requestSep 16, 2024
…dows (xabbuh)This PR was merged into the 5.4 branch.Discussion----------[Filesystem] make sure temp files can be cleaned up on Windows| Q             | A| ------------- | ---| Branch?       | 5.4| Bug fix?      | yes| New feature?  | no| Deprecations? | no| Issues        || License       | MITbased on the findings while debugging the AppVeyor failures that we worked around in#58152Commits-------a832b67 make sure temp files can be cleaned up on Windows
@xabbuh
Copy link
MemberAuthor

so we can revert this now:#58302

nicolas-grekas added a commit that referenced this pull requestSep 18, 2024
This PR was merged into the 7.2 branch.Discussion----------[Filesystem] revert test changes| Q             | A| ------------- | ---| Branch?       | 7.2| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        || License       | MITwe can revert#58152 now that the underlying bug was fixed with#58185Commits-------5ea44be revert test changes
fabpot added a commit that referenced this pull requestSep 27, 2024
…apper` command (smnandre)This PR was squashed before being merged into the 7.2 branch.Discussion----------[AssetMapper] Search & filter assets in `debug:asset-mapper` command| Q             | A| ------------- | ---| Branch?       | 7.2| Bug fix?      | no| New feature?  | yes| Deprecations? | no| Issues        | Fix #...| License       | MIT* Add `name` optional argument to filter paths & assets* Add `--ext` option to filter assets by extension (e.g "css")* Add `--vendor` option to only show assets from vendor packages (and `--no-vendor` to hide them)* Update command helpUpdate:~~* skip flappy test using chmod on Windows~~ (done by `@xabbuh` in#58152)Commits-------8806e24 [AssetMapper] Search & filter assets in `debug:asset-mapper` command
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@alexandre-dauboisalexandre-dauboisalexandre-daubois approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

7.2

Development

Successfully merging this pull request may close these issues.

4 participants

@xabbuh@smnandre@alexandre-daubois@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp