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

Trips up when presented with non-standard packages #22

Closed
Flickwire opened this issue Jun 7, 2021 · 4 comments
Closed

Trips up when presented with non-standard packages #22

Flickwire opened this issue Jun 7, 2021 · 4 comments

Comments

@Flickwire
Copy link

As metapackages do not follow the standard directory structure, Strauss has trouble dealing with them, as it cannot find their composer.json. An example of this can be seen by requiring league/omnipay. A workaround for this is to require the packages from the metapackage separately, but it would be more elegant if Strauss could handle this gracefully.

This also applies to other non-standard package types. For instance, if requiring composer/installers such that we can declare our package as a wordpress-plugin, rather than a library, strauss is unable to locate the package 'composer-plugin-api' and fails to compose dependencies.

I am unsure the best way to deal with these special cases - Perhaps at least in the case of composer-plugin-api we simply exclude this package by default

@Flickwire
Copy link
Author

Separately, though related, is the case of 'provides' type requirements, such as php-http/client-implementation. Obviously these should be excluded (as presumably a providing package is being required somewhere and will be picked up by Strauss separately), and this can be done manually in the strauss configuration for the root package, but skipping these automagically would be nice, though I am not sure how possible this is.

@BrianHenryIE
Copy link
Owner

Looks like the Packagist API will help address packages like league/omnipay.

Probably by doing ~ if(!file_exists(...)), which would also work with other non-standard package types, but I haven't looked at any yet.

I didn't notice that Strauss handles the provides key of packages. It's certainly not explicitly doing so, but maybe they're being pulled in via Composer\Package\PackageInterface::getRequires(). There are already default exclusions for psr* and ext-* which I'd expected would be expanded.

I'm busy for the next week but will take a look into this afterwards.

@BrianHenryIE
Copy link
Owner

OK, I've a branch with passing tests:
https://github.com/BrianHenryIE/strauss/blob/meta-packages/tests/Issues/StraussIssue22Test.php

It's reading the meta package from composer.lock (which I looked for before but thought it wasn't there!) and is ignoring the virtual package.

There some code duplication that needs to be tidied up:

/**
* 2. Built flat list of packages and dependencies.
*
* 2.1 Initiate getting dependencies for the project composer.json.
*
* @see Compose::flatDependencyTree
*/
protected function buildDependencyList()
{
$requiredPackageNames = $this->config->getPackages();
$virtualPackages = array(
'php-http/client-implementation'
);
// Unset PHP, ext-*, ...
$removePhpExt = function ($element) use ($virtualPackages) {
return !(
0 === strpos($element, 'ext')
|| 'php' === $element
|| in_array($element, $virtualPackages)
);
};
$requiredPackageNames = array_filter($requiredPackageNames, $removePhpExt);
foreach ($requiredPackageNames as $requiredPackageName) {
$packageComposerFile = $this->workingDir . $this->config->getVendorDirectory()
. $requiredPackageName . DIRECTORY_SEPARATOR . 'composer.json';
if (!file_exists($packageComposerFile)) {
$composerLock = json_decode(file_get_contents($this->workingDir . 'composer.lock'));
$requiredPackageComposerJson = null;
foreach ($composerLock->packages as $packageJson) {
if ($requiredPackageName === $packageJson->name) {
$requiredPackageComposerJson = $packageJson;
break;
}
}
$tempComposerFile = sys_get_temp_dir() . DIRECTORY_SEPARATOR . $requiredPackageName . DIRECTORY_SEPARATOR . time();
mkdir($tempComposerFile, 0777, true);
$requiredPackageComposerJsonString = json_encode($requiredPackageComposerJson);
$tempComposerFile = $tempComposerFile . DIRECTORY_SEPARATOR . 'composer.json';
$result = file_put_contents($tempComposerFile, $requiredPackageComposerJsonString);
if (false == $result) {
throw new Exception();
}
$packageComposerFile = $tempComposerFile;
}
$overrideAutoload = isset($this->config->getOverrideAutoload()[$requiredPackageName])
? $this->config->getOverrideAutoload()[$requiredPackageName]
: null;
$requiredComposerPackage = new ComposerPackage($packageComposerFile, $overrideAutoload);
$this->flatDependencyTree[$requiredComposerPackage->getName()] = $requiredComposerPackage;
$this->getAllDependencies($requiredComposerPackage);
}
}
/**
* 2.2 Recursive function to get dependencies.
*
* @param ComposerPackage $requiredDependency
*/
protected function getAllDependencies(ComposerPackage $requiredDependency): void
{
$excludedPackagesNames = $this->config->getExcludePackagesFromPrefixing();
$requiredPackageNames = $requiredDependency->getRequiresNames();
$virtualPackageNames = array(
'php-http/client-implementation'
);
// Unset PHP, ext-*, ...
$removePhpExt = function ($element) use ($excludedPackagesNames, $virtualPackageNames) {
return !(
0 === strpos($element, 'ext')
|| 'php' === $element
|| in_array($element, $excludedPackagesNames)
|| in_array($element, $virtualPackageNames)
);
};
$requiredPackageNames = array_filter($requiredPackageNames, $removePhpExt);
foreach ($requiredPackageNames as $dependencyName) {
$overrideAutoload = isset($this->config->getOverrideAutoload()[$dependencyName])
? $this->config->getOverrideAutoload()[$dependencyName]
: null;
$packageComposerFile = $this->workingDir . $this->config->getVendorDirectory()
. $dependencyName . DIRECTORY_SEPARATOR . 'composer.json';
if (!file_exists($packageComposerFile)) {
$composerLock = json_decode(file_get_contents($this->workingDir . 'composer.lock'));
$requiredPackageComposerJson = null;
foreach ($composerLock->packages as $packageJson) {
if ($dependencyName === $packageJson->name) {
$requiredPackageComposerJson = $packageJson;
break;
}
}
$tempComposerFile = sys_get_temp_dir() . DIRECTORY_SEPARATOR . $dependencyName . DIRECTORY_SEPARATOR . time();
mkdir($tempComposerFile, 0777, true);
$requiredPackageComposerJsonString = json_encode($requiredPackageComposerJson);
$tempComposerFile = $tempComposerFile . DIRECTORY_SEPARATOR . 'composer.json';
$result = file_put_contents($tempComposerFile, $requiredPackageComposerJsonString);
if (false == $result) {
throw new Exception();
}
$packageComposerFile = $tempComposerFile;
}
$dependencyComposerPackage = new ComposerPackage($packageComposerFile, $overrideAutoload);
$this->flatDependencyTree[$dependencyName] = $dependencyComposerPackage;
$this->getAllDependencies($dependencyComposerPackage);
}
}

And it needs a test with a package that requires a virtual package. And needs to be tested on Windows.

@BrianHenryIE
Copy link
Owner

I pushed some more changes to the branch. I don't foresee any more. Once I get a chance to write a few lines in the README I'll merge and push a new release to Packagist.

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

No branches or pull requests

2 participants