-
Notifications
You must be signed in to change notification settings - Fork 60
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
Add set-codebase-owner function #320
base: development
Are you sure you want to change the base?
Conversation
@DonRichards Drupal failed to install when I ran make starter:
|
Ran make clean and make starter again and this time Drupal was installed, but I'm still getting errors trying to use composer:
The permissions are the same with this PR as they were before, nginx:nginx on the drupal container, and josh:101 locally |
@joshdentremont I think I was able to pinpoint the build issue. When those errors came up in the past, it was always either a missing codebase directory or a file permission issue. These changes should fire the permissions reset when it's needed. |
Still getting the same permissions and error message. I am running If I manually set settings.php to be owned by root and give the owner write permissions Composer seems to work. What is weird is that this is not necessary on Linux. If I run composer with a read only file on linux it doesn't throw this error
|
Just noticed today that changing settings.php to have write permissions worked, but when I ran |
@joshdentremont Did that error happen before using this function? |
@DonRichards It happened when using this commit, which should have run your code automatically I think. I added some more detail to the original issue that might help clear things up: #306 Essentially, it seems like the issue is a mac permission vs linux permissions thing. My guess is that since the codebase is mounted to the local filesystem it's not allowing root in the docker container to write to a read only file, which is allowed on Linux apparently. Since it's a read only file, changing the owner doesn't seem to make any difference. |
It took me a while to replicate this issue. OK, I think I have found a solution. |
@joshdentremont Would you mind giving that a try? |
I added it's ability to fall back if docker isn't running and if the codebase directory doesn't exist it just skips it. |
This appears to work as intended, but it's different to the issue I was having so it would probably be good to have someone else test it as well. One concern I have is that it can take a bit of time to change these permissions so adding it to |
@joshdentremont The permissions need to be correct to build correctly. This only makes changes if the permissions are wrong. |
@DonRichards, could you please resolve the merge conflicts? |
a669407
to
6d638b3
Compare
Rebased |
During the rebase I added a 2nd "config-export" unintentionally. I just pushed the removal of it. |
This works as described. If there is no codebase folder it skips and moves on, installing via make starter or make starter_dev works as intended, and running |
Improve the approach and handle the vendor directory
This should add a function to set permissions of "codebase/" during the build process.
This also adds a function that you're able to call with a make command at any time.
The command uses
find
with a couple of exclusions for improved performance of running a permissions change./sites/default/files/
to prevent causing issues with File systems like S3.During the build process, the vendor directory may already be there or is set to root, or in some cases, the entire codebase directory is root (this happens in some MACs due to user IDs).
This shouldn't impact any existing installations.
How to test
make clean
(to start with a completely clean build).make set-codebase-owner
a. The codebase should not exist at this point so that it will output the following error message. "No codebase/ folder found, skipping"
make starter-dev
b. This should build as normal. Check that the build is completed as normal.