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

use package's install-path instead of guessing #293

Closed
jnkowa-gfk opened this issue Aug 3, 2021 · 9 comments
Closed

use package's install-path instead of guessing #293

jnkowa-gfk opened this issue Aug 3, 2021 · 9 comments

Comments

@jnkowa-gfk
Copy link

current implementation tries to "guess" the path a required package was installed to.
see

$vendorDirs[$vendorName] = $packageDir . '/' . $configVendorDir . '/' . $vendorName;

this might be correct for the most time, but in general this "guessing" seams like a lucky shot.
ISSUE: In some cases its is just wrong -- especially when a installer-plugin was used. (some frameworks like typo3 do that).

When using composer2: the actual package install path of each required package can be found as install_path in file
$vendorDir . '/composer/installed.json' or $vendorDir . '/composer/installed.php'
(just as used in https://github.com/maglnet/ComposerRequireChecker/blob/master/src/ComposerRequireChecker/FileLocator/LocateComposerPackageDirectDependenciesSourceFiles.php#L57)

Example:

click to unfold an examply from my `installed.json`

{
    "packages": [
        {
            "name": "typo3/cms-extbase",
            "version": "v9.5.28",
            "version_normalized": "9.5.28.0",
            "source": {
                "type": "git",
                "url": "https://github.com/TYPO3-CMS/extbase.git",
                "reference": "676336cabfd589c369056c24ab5a2c94ee9f4c1a"
            },
            "dist": {
                "type": "zip",
                "url": "https://api.github.com/repos/TYPO3-CMS/extbase/zipball/676336cabfd589c369056c24ab5a2c94ee9f4c1a",
                "reference": "676336cabfd589c369056c24ab5a2c94ee9f4c1a",
                "shasum": ""
            },
            "require": {
                "typo3/cms-core": "9.5.28"
            },
            "conflict": {
                "typo3/cms": "*"
            },
            "suggest": {
                "typo3/cms-scheduler": "Additional scheduler tasks"
            },
            "time": "2021-07-20T09:35:15+00:00",
            "type": "typo3-cms-framework",
            "extra": {
                "typo3/cms": {
                    "Package": {
                        "protected": true,
                        "partOfFactoryDefault": true,
                        "partOfMinimalUsableSystem": true
                    },
                    "extension-key": "extbase"
                },
                "typo3/class-alias-loader": {
                    "class-alias-maps": [
                        "Migrations/Code/ClassAliasMap.php"
                    ]
                }
            },
            "installation-source": "dist",
            "autoload": {
                "psr-4": {
                    "TYPO3\\CMS\\Extbase\\": "Classes/"
                }
            },
            "notification-url": "https://packagist.org/downloads/",
            "license": [
                "GPL-2.0-or-later"
            ],
            "authors": [
                {
                    "name": "TYPO3 Core Team",
                    "email": "[email protected]",
                    "role": "Developer"
                }
            ],
            "description": "A framework to build extensions for TYPO3 CMS.",
            "homepage": "https://typo3.org",
            "support": {
                "source": "https://github.com/TYPO3-CMS/extbase/tree/v9.5.28"
            },
            "install-path": "../../Web/typo3/sysext/extbase"
        }
    ]
}

@jnkowa-gfk
Copy link
Author

i read the discussion in #182
especially #182 (comment)

i still suggest to try reading those generated files, and prefer the results over a guess. in case of errors, the guess could be used anyways.

@SvenRtbg
Copy link
Contributor

SvenRtbg commented Aug 3, 2021

Also re-reading the previous issue, I think even if there would be an easy way to identify the paths, it still may present a problem to distinguish the two codebases "own code" and "dependency code" if dependencies are allowed to just deploy themself into paths that are originally used by the "own code" part. Unless such a path is completely wiped before anything gets installed there, but I doubt this is the intention of this feature.

Or in other words: If for example a Typo3 module is required to be installed inside the "own code" because it wouldn't work otherwise, there is no clear way for me to identify if a file that is located there is from the "own" or "dependency" part. This tool is just running on an existing directory tree, it has no knowledge of why a file is in a specific location.

Still the way to run "composer install --no-plugins" for the check will work as it installs everything inside "vendor", and hopefully composer would be able to untangle any files if you switch between plugin and no-plugin install.

@jnkowa-gfk
Copy link
Author

i see what you mean, @SvenRtbg .

Still the way to run "composer install --no-plugins" for the check will work as it installs everything inside "vendor", and hopefully composer would be able to untangle any files if you switch between plugin and no-plugin install.
Thats that is correct for ComposerRequireChecker

Unfortunately a setup with --no-plugins will break all toolings that already require the setup with plugins.
like integration tests that require a complete setup.


some background information to the setup of typo3 packages/extensions (in my case):
the setup will be done with a composer.json like this

{
	// ...
	"config": {
		"bin-dir": ".Build/bin",
		"vendor-dir": ".Build/vendor"
	},
	"extra": {
		"typo3/cms": {
			"extension-key": "pp_userpanel",
			"app-dir": ".Build",
			"web-dir": ".Build/Web"
		}
	}
	// ... 
}

and the resulting dir tree after composer install will look like this:
example_tree

@Ocramius
Copy link
Collaborator

Ocramius commented Aug 3, 2021

Ref: #182 (comment)

I'm aware that drupal also does this sort of stuff. The endorsement remains the same: don't do it.

@Ocramius
Copy link
Collaborator

Ocramius commented Aug 3, 2021

I suggest bringing this up with the upstream project that is using such exotic setup.

@jnkowa-gfk
Copy link
Author

jnkowa-gfk commented Aug 3, 2021

I suggest bringing this up with the upstream project that is using such exotic setup.

well, it is part ob the composer's behavior to put packages in other paths than $vendor/$packagename.
its nothing exotic.

it actually is a feature of composer, to implement a custom installer that implements
Composer\Installer\InstallerInterface::getInstallPath()
see https://github.com/composer/composer/blob/338017879877f293b9c78603aa7471b12d92860b/src/Composer/Installer/InstallerInterface.php#L119
just as done by this example:
https://github.com/TYPO3/CmsComposerInstallers/blob/3092660e3cb629782b364266985b5b9b1f51f165/src/Installer/ExtensionInstaller.php#L72

would be great if ComposerRequireChecker would honor this feature of composer.

@Ocramius
Copy link
Collaborator

Ocramius commented Aug 3, 2021

Aware: the general/safe assumption is to look at vendor/$vendorName/$packageName, and pretty much all tools do that.

Alternatively, start work on a library that, given a composer.json, composer.lock and installed.json, can do the path resolution from symbol to real path, and then it can be included here.

@Ocramius
Copy link
Collaborator

Ocramius commented Aug 3, 2021

Inspiration can be taken from here: https://github.com/Roave/BetterReflection/blob/86135105e7f6e9684efd65c688ab2c49cf240143/src/SourceLocator/Type/Composer/Factory/MakeLocatorForComposerJsonAndInstalledJson.php#L32

In practice: that massive piece of (tested) logic also doesn't support install-path, since it's mostly an inconvenience that has been forced into composer for people that still run this stuff on shared hosting.

@SvenRtbg
Copy link
Contributor

SvenRtbg commented Aug 6, 2021

I do want to point once again to the readme that is explaining the workaround: https://github.com/maglnet/ComposerRequireChecker/blob/3.4.x/README.md#dealing-with-custom-installer-plugins

This would be easy to implement in a CI pipeline where you'd be able to just checkout from scratch and do a different install than usual. And it would not fit into your existing workflow that is doing plenty of checks with only one installation of the repo - it is a workaround.

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

No branches or pull requests

3 participants