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

No relative links into symlinked dirs #99

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

possi
Copy link

@possi possi commented Jul 14, 2015

Example struct:

  • testproject/
    • .modman/
      • a/

        • example/
        • modman
        example example
        
      • b/

        • example/
          • file
        • modman
        example/file example/file
        

Without the patch the project will contain these two symlinks:

  • example => .modman/a/example
  • example/file => ../.modman/b/example/file (broken symlink!)

The patch will change 2nd symlink to an absolute symlink.

Before you judge: Yes that's not a good project structure. I agree. But this also applies to symlinks not created by modman and pointing out of the project root. You may still not like it, but it shouldn't break any existing setups, only support more bad cases.
Lets pray for Magento 2.0 to end that mess ;)

@colinmollenhour
Copy link
Owner

Thanks for the PR. I will have to review this carefully later, but unless I find a regression will be happy to merge it.

@colinmollenhour
Copy link
Owner

On my test project some symlinks that were previously relative symlinks and were not broken were replaced with absolute symlinks, so this is definitely a regression. This module uses the --basedir option so that may be a good place to start looking.

@possi
Copy link
Author

possi commented Jul 17, 2015

Okay. Didn't used the setting yet. I'll look into it.
Also it happen to not work as expected in some of my projects to. So yeah, this is definitive blocked for now.

I'll try to reproduce your issue later. But if you like, you could give me a sample project structure to test with.

@possi
Copy link
Author

possi commented Jul 21, 2015

Modified the PR to fullfill some more cases. basedir should work fine now.
Created a Test-Project to test different cases: https://github.com/possi/modman-test/tree/master/default-project

Seems to work as expected on my live project too.

@colinmollenhour
Copy link
Owner

Just ran a quick test and it is still creating absolute links in my projects where it should be using relative links. I will try to update your test repo sometime with a case that exhibits the issue.

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.

2 participants