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

Shared folders with the same name in different paths create one shared symlink #356

Closed
jamieburchell opened this Issue Sep 6, 2017 · 8 comments

Comments

3 participants
@jamieburchell

jamieburchell commented Sep 6, 2017

Before submitting your issue, please make sure that you've checked all of the checkboxes below.

  • You're running the latest release version of Deployer.
  • Ensure that you're running at least PHP 5.6.4, you can check this by running php -v
  • You've ran composer install --no-dev from the root of your installation.
  • You've ran npm install --production from the root of your installation.
  • You've ran rm -rf bootstrap/cache/* from the root of your installation.
  • You have restarted the queue listener and node socket server.

To help us better understand your issue, please answer the following.

Expected behaviour

Configuring two or more shared folders in different paths with the same name should be handled correctly.

Actual behaviour

Creating two or more shared folders with the same name in different paths causes a single folder to be created with all symlinks pointing to it.

Consider the following special directories:

application/cache
httpdocs/cache

These are not segregated, instead they are merged together as a single symlink:

shared/cache
@jamieburchell

This comment has been minimized.

Show comment
Hide comment
@jamieburchell

jamieburchell Sep 7, 2017

Possible idea for a fix for this, use/recreate the full path within the shared folder:

application/cache > shared/application/cache
httpdocs/cache > shared/httpdocs/cache
application/foo/.bar > shared/application/foo/.bar

jamieburchell commented Sep 7, 2017

Possible idea for a fix for this, use/recreate the full path within the shared folder:

application/cache > shared/application/cache
httpdocs/cache > shared/httpdocs/cache
application/foo/.bar > shared/application/foo/.bar
@REBELinBLUE

This comment has been minimized.

Show comment
Hide comment
@REBELinBLUE

REBELinBLUE Sep 7, 2017

Owner

Can't believe I missed that, you are right. I will do exactly that. Unfortunately I won't have access to a computer until 19th/20th now as I am away. I will fix it as soon as I get back

Owner

REBELinBLUE commented Sep 7, 2017

Can't believe I missed that, you are right. I will do exactly that. Unfortunately I won't have access to a computer until 19th/20th now as I am away. I will fix it as soon as I get back

@iflamed

This comment has been minimized.

Show comment
Hide comment
@iflamed

iflamed Sep 7, 2017

Contributor
Contributor

iflamed commented Sep 7, 2017

@jamieburchell

This comment has been minimized.

Show comment
Hide comment
@jamieburchell

jamieburchell Sep 7, 2017

Hi @iflamed, I'm not sure what you mean. Can you demonstrate a scenario where this approach wouldn't work?

I don't feel confident that I could create a PR. I'm very new to Laravel and this project. I can see that probably the mkdir will need to have a flag to create parent folders if missing. And probably the file cp would need something similar. Of course, the full path would also need to be passed round, possibility using dirname from pathinfo, which could also be ".".

jamieburchell commented Sep 7, 2017

Hi @iflamed, I'm not sure what you mean. Can you demonstrate a scenario where this approach wouldn't work?

I don't feel confident that I could create a PR. I'm very new to Laravel and this project. I can see that probably the mkdir will need to have a flag to create parent folders if missing. And probably the file cp would need something similar. Of course, the full path would also need to be passed round, possibility using dirname from pathinfo, which could also be ".".

@jamieburchell

This comment has been minimized.

Show comment
Hide comment
@jamieburchell

jamieburchell Sep 7, 2017

For info - having played with Deployer this is how they do it

jamieburchell commented Sep 7, 2017

For info - having played with Deployer this is how they do it

@REBELinBLUE

This comment has been minimized.

Show comment
Hide comment
@REBELinBLUE

REBELinBLUE Sep 7, 2017

Owner

Yeah I'm really not sure what you mean @iflamed

@jamieburchell It shouldn't be a difficult change, my bigger concern is how to transition the existing shared folders people will have, I shall have to think about it

Owner

REBELinBLUE commented Sep 7, 2017

Yeah I'm really not sure what you mean @iflamed

@jamieburchell It shouldn't be a difficult change, my bigger concern is how to transition the existing shared folders people will have, I shall have to think about it

@iflamed

This comment has been minimized.

Show comment
Hide comment
@iflamed

iflamed Sep 8, 2017

Contributor
Contributor

iflamed commented Sep 8, 2017

REBELinBLUE added a commit that referenced this issue Sep 29, 2017

Fixes #356 - Use the full path (relative to the project root) for the…
… path to shared files/folder rather than just the basename
@REBELinBLUE

This comment has been minimized.

Show comment
Hide comment
@REBELinBLUE

REBELinBLUE Sep 29, 2017

Owner

I've fixed this in fafc0b8

But I need to write a migration for existing projects before merging it

Owner

REBELinBLUE commented Sep 29, 2017

I've fixed this in fafc0b8

But I need to write a migration for existing projects before merging it

REBELinBLUE added a commit that referenced this issue Sep 30, 2017

Fixes #356 - Use the full path (relative to the project root) for the…
… path to shared files/folder rather than just the basename

REBELinBLUE added a commit that referenced this issue Sep 30, 2017

Fixes #356 - Use the full path (relative to the project root) for the…
… path to shared files/folder rather than just the basename

REBELinBLUE added a commit that referenced this issue Sep 30, 2017

Fixes #356 - Use the full path (relative to the project root) for the…
… path to shared files/folder rather than just the basename

REBELinBLUE added a commit that referenced this issue Oct 1, 2017

REBELinBLUE added a commit that referenced this issue Oct 2, 2017

Merge branch 'bug/356'
* bug/356:
  Fix #356 - Use the full path (relative to the project root) for the path to shared files/folder rather than just the basename
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment