-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Whitelist packages with names matching those specified before generating rules #2733
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? 8000 Sign in to your account
Conversation
…ing rules Addresses composer#2690 doesn't do any performance optimisations yet which we could do now
@@ -46,7 +46,7 @@ public function remove($packageName, LinkConstraintInterface $constraint = null) | |||
protected function addJob($packageName, $cmd, LinkConstraintInterface $constraint = null) | |||
{ | |||
$packageName = strtolower($packageName); | |||
$packages = $this->pool->whatProvides($packageName, $constraint); | |||
$packages = $this->pool->whatProvides($packageName, $constraint, true); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This may have to be a false, but it should be evaluated after the whitelist is generated I think. As it is, if you request the installation of symfony/process, it wouldn't be resolvable by installing symfony/symfony anymore - do we not have a test for that case?
Working on another test, this doesn't seem to work quite right yet. |
Added tests for the changes in this PR and it seems to be working now. |
Whitelist packages with names matching those specified before generating rules
{ | ||
if (isset($this->providerCache[$name][(string) $constraint])) { | ||
return $this->providerCache[$name][(string) $constraint]; | ||
$key = ((string) (int) $mustMatchName).((string) $constraint); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no need to cast to string explicitly. the concatenation operator already does it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in eb5c785
includes security fix composer/composer#2733
includes security fix composer/composer#2733
Addresses #2690 but doesn't do any performance optimizations yet which we
could do now