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

Don't try and create migrations folder #6220

Closed
timkelty opened this issue Jun 16, 2020 · 6 comments
Closed

Don't try and create migrations folder #6220

timkelty opened this issue Jun 16, 2020 · 6 comments

Comments

@timkelty
Copy link
Contributor

timkelty commented Jun 16, 2020

craft\console\controllers\MigrateController tries to create a migrations folder if it doesn't find one.

There is no mention of this in the installation/file permissions docs: https://docs.craftcms.com/v3/installation.html#step-2-set-the-file-permissions

In order to prevent deploy failures as a result of this, I need to create/commit that folder (eg with a .gitinclude file in it), even if I don't have any content migrations.

At the very least, the docs should state that this permission is necessary. However, I'm not sure what value creating an empty migrations folder has, so maybe it could just not even try?

Here's the error you'll get if write permissions don't exist, which is likely in a CI deployment environment:

│ 2020-06-16T04:55:32.746829386Z #1 /app/vendor/composer/craftcms/cms/src/console/controllers/MigrateController.php(166): craft\helpers\FileHelper::createDirectory('/app/migrations')  │
│ 2020-06-16T04:55:32.746832976Z #2 /app/vendor/composer/yiisoft/yii2/base/Controller.php(155): craft\console\controllers\MigrateController->beforeAction(Object(yii\base\InlineAction) │
│ 2020-06-16T04:55:32.746837474Z #3 /app/vendor/composer/yiisoft/yii2/console/Controller.php(164): yii\base\Controller->runAction('all', Array)                                         │
│ 2020-06-16T04:55:32.746840608Z #4 /app/vendor/composer/yiisoft/yii2/base/Module.php(528): yii\console\Controller->runAction('all', Array)                                             │
│ 2020-06-16T04:55:32.746844047Z #5 /app/vendor/composer/yiisoft/yii2/console/Application.php(180): yii\base\Module->runAction('migrate/all', Array)                                    │
│ 2020-06-16T04:55:32.746847675Z #6 /app/vendor/composer/craftcms/cms/src/console/Application.php(87): yii\console\Application->runAction('migrate/all', Array)                         │
│ 2020-06-16T04:55:32.746851363Z #7 /app/vendor/composer/yiisoft/yii2/console/Application.php(147): craft\console\Application->runAction('migrate/all', Array)                          │
│ 2020-06-16T04:55:32.746854983Z #8 /app/vendor/composer/yiisoft/yii2/base/Application.php(386): yii\console\Application->handleRequest(Object(craft\console\Request))                  │
│ 2020-06-16T04:55:32.746858654Z #9 /app/bin/craft(7): yii\base\Application->run()                                                                                                      │
│ 2020-06-16T04:55:32.746861763Z #10 {main}                                                                                                                                             │
@brandonkelly
Copy link
Member

The only time that folder will be automatically created is if you are running a content migrations-spceific command (e.g. migrate/up without --type=app or --plugin=<plugin-handle>). We create the directory in this case to prevent an exception from being thrown:

https://github.com/yiisoft/yii2/blob/60a39654dd2d425a24c64d6c1d641a0754b015bc/framework/console/controllers/BaseMigrateController.php#L134

So doesn’t seem like something that really needs to be documented; it wouldn’t happen if you were running migrate/all or migrate/up --type=app, etc.

In order to prevent deploy failures as a result of this, I need to create/commit that folder (eg with a .gitinclude file in it), even if I don't have any content migrations.

Alternatively, your script could wrap the migrate/up command with a check to make sure the directory exists, which would make it a bit more versatile.

@timkelty
Copy link
Contributor Author

it wouldn’t happen if you were running migrate/all or migrate/up --type=app, etc

Hmm…it seems to be happening for me. My deploy pipeline runs:

craft migrate/all
craft project-config/sync

I seem to get the error when I deploy a Craft upgrade.

@brandonkelly
Copy link
Member

What version of Craft are you running?

@timkelty
Copy link
Contributor Author

Looks like I last saw it on 3.4.23

@brandonkelly
Copy link
Member

Whoops, I was testing on 3.5, and forgot we had refactored things a bit there. Just fixed this for the next 3.4 release.

@brandonkelly
Copy link
Member

Craft 3.4.25 is out now with the fix.

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

No branches or pull requests

2 participants