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

Path repository inside its source #6085

Closed
marcosh opened this issue Jan 19, 2017 · 14 comments
Closed

Path repository inside its source #6085

marcosh opened this issue Jan 19, 2017 · 14 comments
Labels

Comments

@marcosh
Copy link

marcosh commented Jan 19, 2017

I am working on a project with the following structure

apps
    admin
        composer.json
    public
        composer.json
src
composer.json

where in src I have a package that needs to be used by the two applications in apps/admin and apps/public.

My composer.json of apps/admin, for example, looks like:

{
    "repositories": [
        {
            "type": "path",
            "url": "../..",
            "options": {
                "symlink": true
            }
        }
    ]
}

This produces a error of the type Package %s cannot install to "%s" inside its source at "%s" in the class Composer\Downloader\PathDownloader.

I agree this definitely needs to happen when using STRATEGY_MIRROR, but I was wandering if what I am doing should actually work when using STRATEGY_SYMLINK. In this case you would get an infinite recursive directory structure, since you are symlinking a parent path, but this should actually not create any problem.

Would it make sende to move the above mentioned exception only for the STRATEGY_MIRROR case?

@Seldaek
Copy link
Member

Seldaek commented Jan 22, 2017

This use case is only supported if you have the root src and composer.json within another directory, like:

apps
    admin
        composer.json
    public
        composer.json
lib
    src
    composer.json

Then you can require "../../lib" as the path and that'll work. We can't support recursive structures reliably and cross-platform as it causes issues with updates/removals.

@Seldaek Seldaek closed this as completed Jan 22, 2017
@quazardous
Copy link

but it was working before ...

typical use was a demo subfolder using ../ as repo....

@weotch
Copy link

weotch commented Mar 20, 2017

I also wanted to have a demo or example subfolder and use the parent as the repo.

@curry684
Copy link
Contributor

but it was working before ...

Except that it wasn't. The check was added because the scenario caused real world issues. The scenario isn't unavoidable, and therefore it was deemed better to disallow it than to risk accidentally deleting days or weeks of work.

@weotch
Copy link

weotch commented Mar 20, 2017

My workaround, which appears to be working, is to add this to the composer.json:

{
    "scripts": {
        "post-install-cmd": [
            "rm -rf vendor/my/package && ln -s ../../../ vendor/my/package"
        ],
        "post-update-cmd": [
            "rm -rf vendor/my/package && ln -s ../../../ vendor/my/package"
        ]
    }
}

@mvasin
Copy link

mvasin commented Mar 5, 2018

I have a tests/ directory where I need to execute some integration tests before I push the package to the repository:

- composer.json
- tests
  \_ IntegrationTests.php
  \_ composer.json

The integration test's composer.json fetches the package-under-test from the directory above

    "repositories": [
        {
            "type": "path",
            "url": "../"
        }

and fails for the reason stated in this issue.

Integration tests are a must, and they should be included in the package (or at least be able to be included), so directory structure like this

- package
  \_ composer.json
- tests
  \_ IntegrationTests.php
  \_ composer.php

is less then ideal.

@weotch Your workaround does not seem to be cross-platform, did you work around the workaround to make it cross-platform since then?

This relates to vinkla/wordplate#196.

@curry684
Copy link
Contributor

curry684 commented Mar 5, 2018

I'm not really sure I understand your scenario well. You mention "integration tests", but that is what dev dependencies are for - ensuring optional packages are present during development and testing. If it's not about optional dependencies it would imply that you're running integration tests for an entire system within a single component of the system, which is architecturally wrong to begin with.

@mvasin
Copy link

mvasin commented Mar 5, 2018

@curry684 Could you please expand on what's wrong with testing a component from within the environment you are designing the component for?

Imagine I develop a WordPress plugin. Here's my directory structure:

- somefile.php
- composer.json

I can require-dev the WordPress core, a WordPress theme and a bunch of other plugins I plan to use my plugin with, and thanks to composer/installers those will land in appropriate subdirectories. But my plugin stays at the root directory, and it won't be picked up by WordPress there. It must be in an appropriate subfolder, alongside other plugins.

I have to bootstrap a brand new WordPress site using tests/composer.json, require everything including my plugin in its current state and make sure it works well with the WordPress core and other plugins/themes I care about.

@mvasin
Copy link

mvasin commented Mar 7, 2018

Here's my current workaround. This is tests/composer.json:

{
    "repositories": [
        {
            "type": "artifact",
            "url": "package-under-test"
        }
    ],
    "require-dev": {
      "my/package": "@dev"

    },
    "scripts": {
        "archive-package": [
          "mkdir -p package-under-test",
          "@composer archive --working-dir=../ --ignore-filters --dir=tests/package-under-test --file=my-package"
        ],
        "pre-install-cmd": "@archive-package",
        "pre-update-cmd": "@archive-package"
    }
}

If you don't specify the package version in composer.json it's important to add --ignore-filters so .git folder is archived as well, and composer can infer the package version from git branches/tags.

@mricherzhagen
Copy link

mricherzhagen commented Apr 12, 2018

I am running into the same error message, but with the mirror strategy i can fix it by rm -rf vendor/ (Dont do that when using symlinks - might delete your sources!). I also think this should be a valid use case and at the moment i don't see how it can cause any unavoidable problems.

Could someone care to elaborate, why this isn't supported?

EDIT: There is a comment in the source mentioning discussion/attempts in #5974 and #6174

@curry684
Copy link
Contributor

You can also do that with symlinks as rm -rf is symlink safe.

In the code however we're bound to using other mechanisms that are not guaranteed to be symlink safe, therefore we took the futureproof approach that it's better to disable a mechanism that might one day trash days of work for thousands of developers given that the situation is simply not unavoidable.

@mricherzhagen
Copy link

@curry684 I think i found a different bug then, for me the exception is thrown under very weird circumstances, that are hard to explain. I will open a separate issue for this, because my directory setup is also the other way around. See #7255

Thinking about it now i think the symlinks are unsafe no matter where they are pointing, though. its probably never desired behaivior, that any source files are deleted when you clean your vendor directory.

d13r added a commit to d13r/laravel-breadcrumbs that referenced this issue Jul 26, 2018
Reverts 6bf02cf and several other commits

I thought using Docker would make it easier to run multiple versions of PHP.
In reality it just made development slower and harder.
Having to write a script for every app was tedious.
Having to wait while every dependency installed was also.
So it's back to Plan A.

For my current development environment see:
https://github.com/davejamesmiller/dev-ansible
(Basically Ubuntu 18.04 + Apache 2.4 + PHP 7.2 & 7.1 FPM)

Unfortunately that also means putting the test app in a separate repo, because
Composer doesn't like it when the test project is in a directory below the package.
composer/composer#6085
(In Docker I worked around this with a second mount point.)
@DavidBruchmann
Copy link

Had an issue with an own installer, after updating composer itself and clearing the cache everything worked like expected.

opensource521 added a commit to opensource521/laravel.breadcrumb that referenced this issue Apr 12, 2020
Reverts 6bf02cf681db48003701f317cc52d73e2c4bc47a and several other commits

I thought using Docker would make it easier to run multiple versions of PHP.
In reality it just made development slower and harder.
Having to write a script for every app was tedious.
Having to wait while every dependency installed was also.
So it's back to Plan A.

For my current development environment see:
https://github.com/davejamesmiller/dev-ansible
(Basically Ubuntu 18.04 + Apache 2.4 + PHP 7.2 & 7.1 FPM)

Unfortunately that also means putting the test app in a separate repo, because
Composer doesn't like it when the test project is in a directory below the package.
composer/composer#6085
(In Docker I worked around this with a second mount point.)
@GromNaN
Copy link
Contributor

GromNaN commented Sep 30, 2020

I hit the same use-case. Since path repos are allowed to point to their own source dir as install target (#8254), I created the symlink in the vendor dir that goes to the parent directory.

For the initial example, it is:

apps/
    admin/
        composer.json
        vendor/
            my-vendor
                my-package -> ../../../../
src/
composer.json

And apps/admin/composer.json contains the following repositories section:

{
    "repositories": [
        {
            "type": "path",
            "url": "vendor/my-vendor/my-package",
            "options": {
                "symlink": true
            }
        }
    ],
    "scripts": {
        "symlink-package": [
            "mkdir -p vendor/my-vendor && rm -f vendor/my-vendor/my-package && ln -s -f ../../../../ vendor/my-vendor/my-package"
        ],
        "pre-install-cmd": "@symlink-package",
        "pre-update-cmd": "@symlink-package"
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants