Support fileMode fileOwnership in create migration #18676

rhertogh · 2021-05-23T17:34:50Z

Q	A
Is bugfix?	❌
New feature?	✔️
Breaks BC?	❌
Tests pass?	✔️
Fixed issues	yiisoft/yii2-docker/issues/15

update

…ller and BaseFileHelper

…` of the MigrateController

…_file_mode_and_owner_in_create_migration

…FileOwnership to reflect the scope of the settings

samdark · 2021-05-23T18:20:45Z

Are you sure you've specified correct issue number?

rhertogh · 2021-05-23T18:26:49Z

Are you sure you've specified correct issue number?

You mean the 'Fixed issues'? Yes, I had the same problem when the migration was created inside a Docker container and the file is edited on the host (mounted via WSL2 on Ubuntu) changes can't be written to the file.

framework/helpers/BaseFileHelper.php

framework/console/controllers/BaseMigrateController.php

framework/helpers/BaseFileHelper.php

…ontroller and BaseFileHelper::changeOwnership()

framework/CHANGELOG.md

framework/console/controllers/BaseMigrateController.php

Co-authored-by: Bizley <[email protected]>

…hip` in BaseMigrateController

rhertogh · 2021-05-24T12:27:46Z

@samdark @bizley We could add a changeModeAndOwnership($file, $mode = null, $ownership = null) function to the BaseFileHelper. This would make the code in other places more clean and reusable. What do you think?

framework/console/controllers/BaseMigrateController.php

bizley · 2021-05-24T14:59:33Z

Changing mode is pretty straight forward, but maybe this could be added indeed as a bonus to this new method (so more like a changeOwnershipAndMode).

…_file_mode_and_owner_in_create_migration

…n changing ownership or mode fails

rhertogh · 2021-05-24T17:32:05Z

@bizley

Changing mode is pretty straight forward, but maybe this could be added indeed as a bonus to this new method (so more like a changeOwnershipAndMode).

I've added the $mode parameter to changeOwnership() but left the name unchanged since $mode is optional.
The @s are also gone from the ch*() functions since changeOwnership() always raises an error on failure. This also gives a better error message. For example var_dump(chmod('test.txt','test')); outputs PHP Warning: chown(): Unable to find uid for test.
Now changeOwnership() no longer returns a value but always (except on Windows) ensures the change of ownership and/or permissions as discussed here: #18676 (comment).

rhertogh · 2021-05-24T18:08:48Z

Working on unit test

…_file_mode_and_owner_in_create_migration

…stChangeOwnershipInvalidArguments() in FileHelperTest since setup() hasn't run yet when data provider is called.

This reverts commit a574908.

…ship() when not run as root()

This reverts commit 2e1ea0f.

…nsion is not loaded

…nsion is not loaded (this time actually when it's NOT loaded)

rhertogh · 2021-05-28T18:14:13Z

Unit tests are added. Some require root user to complete.

bizley

Just some code style, rest LGTM

framework/helpers/BaseFileHelper.php

@bizley

Fixed code style and spelling based on feedback from @bizley Co-authored-by: Bizley <[email protected]>

bizley

While porting this change to my own migration extension I've noticed that getmyuid() and getmygid() returning not the ids of the current user and user's group but rather the ids of the running script owner. So this needs to be changed (proposed here). Also noticed few more exception messages adjustments.

tests/framework/helpers/FileHelperTest.php

framework/helpers/BaseFileHelper.php

@bizley

Getting correct uid and gid from user thanks to @bizley Co-authored-by: Bizley <[email protected]>

…ation

samdark · 2021-05-30T16:26:20Z

👍

…nership()` and properties `newFileMode`/`newFileOwnership` in `yii\console\controllers\BaseMigrateController` Co-authored-by: Bizley <[email protected]>

rhertogh and others added 7 commits July 30, 2015 10:39

Merge pull request #1 from yiisoft/master

83fda4b

update

Merge branch 'master' of https://github.com/yiisoft/yii2

5231894

Merge branch 'master' of https://github.com/yiisoft/yii2

e388dfc

Merge branch 'master' of https://github.com/yiisoft/yii2

6b56e2c

Merge branch 'master' of https://github.com/yiisoft/yii2 into master

1676da7

Merge branch 'master' of https://github.com/yiisoft/yii2 into master

accf01e

Added support for fileMode and fileOwnership in BaseMigrateContro…

191ad7d

…ller and BaseFileHelper

rhertogh changed the title ~~Support file mode and owner in create migration~~ Support fileMode fileOwnership in create migration May 23, 2021

rhertogh added 3 commits May 23, 2021 19:56

Updated changelog and documentation for fileMode and `fileOwnership…

0782abc

…` of the MigrateController

Merge branch 'master' of https://github.com/yiisoft/yii2 into support…

a700921

…_file_mode_and_owner_in_create_migration

Fixed format of changelog after merge

6aac35f

rhertogh marked this pull request as ready for review May 23, 2021 18:02

rhertogh added 2 commits May 23, 2021 20:12

Changed names of to fileMode and fileOwnership to newFileMode and new…

6bb0b48

…FileOwnership to reflect the scope of the settings

Removed incorrectly added newlines in db-migrations.md

3610f8a

samdark added this to the 2.0.43 milestone May 23, 2021

bizley reviewed May 24, 2021

View reviewed changes

framework/helpers/BaseFileHelper.php Outdated Show resolved Hide resolved

Fixed function return type and don't change group if user change failed

57dfd2e

samdark reviewed May 24, 2021

View reviewed changes

framework/console/controllers/BaseMigrateController.php Show resolved Hide resolved

framework/console/controllers/BaseMigrateController.php Show resolved Hide resolved

framework/helpers/BaseFileHelper.php Show resolved Hide resolved

Fixed phpdoc bloks for fileMode and fileOwnership in BaseMigrateC…

5967d5d

…ontroller and BaseFileHelper::changeOwnership()

samdark approved these changes May 24, 2021

View reviewed changes

samdark requested review from bizley and a team May 24, 2021 10:06

bizley requested changes May 24, 2021

View reviewed changes

framework/CHANGELOG.md Outdated Show resolved Hide resolved

framework/console/controllers/BaseMigrateController.php Outdated Show resolved Hide resolved

rhertogh and others added 2 commits May 24, 2021 12:24

Update framework/CHANGELOG.md

ec73d07

Co-authored-by: Bizley <[email protected]>

Raising exception when failing to set newFileMode or `newFileOwners…

0d7ceed

…hip` in BaseMigrateController

bizley reviewed May 24, 2021

View reviewed changes

framework/console/controllers/BaseMigrateController.php Outdated Show resolved Hide resolved

rhertogh added 2 commits May 24, 2021 19:14

Merge branch 'master' of https://github.com/yiisoft/yii2 into support…

e17986c

…_file_mode_and_owner_in_create_migration

Added $mode to BaseFileHelper::changeOwnership() and raise errors whe…

cae6863

…n changing ownership or mode fails

rhertogh marked this pull request as draft May 24, 2021 18:08

rhertogh added 10 commits May 24, 2021 23:44

Added unit test for FileHelper::changeOwnership()

6ffeb57

Merge branch 'master' of https://github.com/yiisoft/yii2 into support…

9c52020

…_file_mode_and_owner_in_create_migration

Moved setup code from changeOwnershipInvalidArgumentsProvider() to te…

122c0d2

…stChangeOwnershipInvalidArguments() in FileHelperTest since setup() hasn't run yet when data provider is called.

Skipping chown() tests in FileHelperTest::testChangeOwnership()

a574908

Revert "Skipping chown() tests in FileHelperTest::testChangeOwnership()"

0fc0745

This reverts commit a574908.

Skipping chown() and chgrp() tests in FileHelperTest::testChangeOwner…

da294d7

…ship() when not run as root()

Require-dev posix extension in composer

2e1ea0f

Revert "Require-dev posix extension in composer"

0a7ff6f

This reverts commit 2e1ea0f.

Marking FileHelperTest::testChangeOwnership() skipped when posix exte…

df5083c

…nsion is not loaded

Marking FileHelperTest::testChangeOwnership() skipped when posix exte…

37311c7

…nsion is not loaded (this time actually when it's NOT loaded)

rhertogh marked this pull request as ready for review May 28, 2021 18:13

samdark approved these changes May 28, 2021

View reviewed changes

samdark requested a review from bizley May 28, 2021 20:04

samdark added the type:enhancement label May 28, 2021

bizley requested changes May 29, 2021

View reviewed changes

Apply suggestions from code review

4e4ddf6

Fixed code style and spelling based on feedback from @bizley Co-authored-by: Bizley <[email protected]>

bizley approved these changes May 29, 2021

View reviewed changes

bizley requested changes May 29, 2021

View reviewed changes

rhertogh and others added 2 commits May 29, 2021 16:23

Apply suggestions from code review

bff926e

Getting correct uid and gid from user thanks to @bizley Co-authored-by: Bizley <[email protected]>

Merge branch 'master' into support_file_mode_and_owner_in_create_migr…

407b4a7

…ation

bizley approved these changes May 29, 2021

View reviewed changes

samdark merged commit 31ca0fc into yiisoft:master May 30, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support fileMode fileOwnership in create migration #18676

Support fileMode fileOwnership in create migration #18676

rhertogh commented May 23, 2021

samdark commented May 23, 2021 •

edited

Loading

rhertogh commented May 23, 2021

rhertogh commented May 24, 2021

bizley commented May 24, 2021

rhertogh commented May 24, 2021

rhertogh commented May 24, 2021

rhertogh commented May 28, 2021

bizley left a comment

bizley left a comment

samdark commented May 30, 2021

Support fileMode fileOwnership in create migration #18676

Support fileMode fileOwnership in create migration #18676

Conversation

rhertogh commented May 23, 2021

samdark commented May 23, 2021 • edited Loading

rhertogh commented May 23, 2021

rhertogh commented May 24, 2021

bizley commented May 24, 2021

rhertogh commented May 24, 2021

rhertogh commented May 24, 2021

rhertogh commented May 28, 2021

bizley left a comment

Choose a reason for hiding this comment

bizley left a comment

Choose a reason for hiding this comment

samdark commented May 30, 2021

samdark commented May 23, 2021 •

edited

Loading