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

Hooked stream_metadata(..) with chown(..). #4027

Closed
wants to merge 1 commit into from

Conversation

kevinxucs
Copy link
Contributor

Part of #3687. So far, only chown(..) in user-stream-wrapper has been hooked with stream_metadata(..) function. Similar to commit 1f55a08.

Also passes vfsStream's chownChangesUser test case.

Part of facebook#3687. So far, only chown(..) in user-stream-wrapper has
been hooked with stream_metadata(..) function. Similar to commit
1f55a08.
@facebook-github-bot
Copy link
Contributor

This pull request has been imported into Phabricator, and discussion and review of the diff will take place at https://reviews.facebook.net/D25407

@fredemmott
Copy link
Contributor

Sorry, closing in favor of #4031

@fredemmott fredemmott closed this Oct 25, 2014
hhvm-bot pushed a commit that referenced this pull request Oct 28, 2014
…and lchgrp

Summary: PHP added stream metadata in PHP 5.4 for chmod, chown, chgrp, lchmod and lchgrp.
http://php.net/manual/en/streamwrapper.stream-metadata.php

Closes #3687. Closes #4027.
Closes #4031

Reviewed By: @ptarjan

Differential Revision: D1629484

Signature: t1:1629484:1414509316:a2f3ceafa61520133dc25f6d58689e156d1fdad3
@@ -629,7 +629,7 @@ org\bovigo\vfs\vfsStreamWrapperTestCase::chmod
org\bovigo\vfs\vfsStreamWrapperTestCase::chmodModifiesPermissions
F
org\bovigo\vfs\vfsStreamWrapperTestCase::chownChangesUser
F
.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kevinxucs: I think I missed this in f87415e, do you want to send PR? Also chgrp (and maybe some other as well?) may be fixed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Majkl578 Almost forgot. Thanks for reminding me of that!

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 this pull request may close these issues.

4 participants