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
Fix for #18 - support running with --no-scripts flag #33
Changes from all commits
361a532
ecf8760
20783c9
c70fd71
9599960
0a1a91e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
<?php | ||
|
||
namespace PackageVersions; | ||
|
||
/** | ||
* @internal | ||
* | ||
* This is a fallback for {@see \PackageVersions\Versions::getVersion()} | ||
* Do not use this class directly: it is intended to be only used when | ||
* {@see \PackageVersions\Versions} fails to be generated, which typically | ||
* happens when running composer with `--no-scripts` flag) | ||
*/ | ||
final class FallbackVersions | ||
{ | ||
private function __construct() | ||
{ | ||
} | ||
|
||
/** | ||
* @param string $packageName | ||
* | ||
* @return string | ||
* | ||
* @throws \OutOfBoundsException if a version cannot be located | ||
* @throws \UnexpectedValueException if the composer.lock file could not be located | ||
*/ | ||
public static function getVersion(string $packageName) : string | ||
{ | ||
$versions = iterator_to_array(self::getVersions(self::getComposerLockPath())); | ||
|
||
if (! array_key_exists($packageName, $versions)) { | ||
throw new \OutOfBoundsException( | ||
'Required package "' . $packageName . '" is not installed: cannot detect its version' | ||
); | ||
} | ||
|
||
return $versions[$packageName]; | ||
} | ||
|
||
/** | ||
* @return string | ||
* | ||
* @throws \UnexpectedValueException | ||
*/ | ||
private static function getComposerLockPath() : string | ||
{ | ||
// bold assumption, but there's not here to fix everyone's problems. | ||
$checkedPaths = [__DIR__ . '/../../../../../composer.lock', __DIR__ . '/../../composer.lock']; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @Ocramius Perhaps some There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @nikolaposa I don't trust intermediate levels |
||
|
||
foreach ($checkedPaths as $path) { | ||
if (file_exists($path)) { | ||
return $path; | ||
} | ||
} | ||
|
||
throw new \UnexpectedValueException(sprintf( | ||
'PackageVersions could not locate your `composer.lock` location. This is assumed to be in %s. ' | ||
. 'If you customized your composer vendor directory and ran composer installation with --no-scripts, ' | ||
. 'then you are on your own, and we can\'t really help you. Fix your shit and cut the tooling some slack.', | ||
json_encode($checkedPaths) | ||
)); | ||
} | ||
|
||
private static function getVersions(string $composerLockFile) : \Generator | ||
{ | ||
$lockData = json_decode(file_get_contents($composerLockFile), true); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you should actually check installed packages, not locked ones. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is there an array key with the installed packages? I currently merge There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. installed packages are not in the lock file. they are in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, but what interests us is just the version of a package. For example, On 22 Jul 2016 17:17, "Christophe Coevoet" notifications@github.com wrote:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, so it is OK if the package is not there anymore ? This should be clearly documented then There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, if the package is not installed (because of non-dev install) then the On 22 Jul 2016 18:12, "Christophe Coevoet" notifications@github.com wrote:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The readme should explain it then |
||
|
||
$lockData['packages-dev'] = $lockData['packages-dev'] ?? []; | ||
|
||
foreach (array_merge($lockData['packages'], $lockData['packages-dev']) as $package) { | ||
yield $package['name'] => $package['version'] . '@' . ( | ||
$package['source']['reference']?? $package['dist']['reference'] ?? '' | ||
); | ||
} | ||
|
||
yield 'unknown/root-package@UNKNOWN'; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
<?php | ||
|
||
namespace PackageVersions; | ||
|
||
/** | ||
* This is a stub class: it is in place only for scenarios where PackageVersions | ||
* is installed with a `--no-scripts` flag, in which scenarios the Versions class | ||
* is not being replaced. | ||
* | ||
* If you are reading this docBlock inside your `vendor/` dir, then this means | ||
* that PackageVersions didn't correctly install, and is in "fallback" mode. | ||
*/ | ||
final class Versions | ||
{ | ||
const VERSIONS = []; | ||
|
||
private function __construct() | ||
{ | ||
} | ||
|
||
/** | ||
* @throws \OutOfBoundsException if a version cannot be located | ||
* @throws \UnexpectedValueException if the composer.lock file could not be located | ||
*/ | ||
public static function getVersion(string $packageName) : string | ||
{ | ||
return FallbackVersions::getVersion($packageName); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
<?php | ||
|
||
namespace PackageVersionsTest; | ||
|
||
use PackageVersions\FallbackVersions; | ||
use PackageVersions\Versions; | ||
use PHPUnit_Framework_TestCase; | ||
|
||
/** | ||
* @covers \PackageVersions\FallbackVersions | ||
*/ | ||
final class FallbackVersionsTest extends PHPUnit_Framework_TestCase | ||
{ | ||
public function testWillFailWithoutValidComposerLockLocation() | ||
{ | ||
rename(__DIR__ . '/../../composer.lock', __DIR__ . '/../../composer.lock.backup'); | ||
|
||
try { | ||
FallbackVersions::getVersion('phpunit/phpunit'); | ||
|
||
self::fail('An exception was supposed to be thrown'); | ||
} catch (\UnexpectedValueException $lockFileNotFound) { | ||
$srcDir = realpath(__DIR__ . '/../../src/PackageVersions'); | ||
|
||
self::assertSame( | ||
'PackageVersions could not locate your `composer.lock` location. ' | ||
. 'This is assumed to be in ' | ||
. json_encode([$srcDir . '/../../../../../composer.lock', $srcDir . '/../../composer.lock']) | ||
. '. If you customized your composer vendor directory and ran composer installation with --no-scripts, ' | ||
. 'then you are on your own, and we can\'t really help you. ' | ||
. 'Fix your shit and cut the tooling some slack.', | ||
$lockFileNotFound->getMessage() | ||
); | ||
} | ||
|
||
rename(__DIR__ . '/../../composer.lock.backup', __DIR__ . '/../../composer.lock'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this will not rename things properly when there is a test failure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be done in a try/finally There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
} | ||
|
||
public function testValidVersions() | ||
{ | ||
$lockData = json_decode(file_get_contents(__DIR__ . '/../../composer.lock'), true); | ||
|
||
$packages = array_merge($lockData['packages'], $lockData['packages-dev']); | ||
|
||
self::assertNotEmpty($packages); | ||
|
||
foreach ($packages as $package) { | ||
self::assertSame( | ||
$package['version'] . '@' . $package['source']['reference'], | ||
Versions::getVersion($package['name']) | ||
); | ||
} | ||
} | ||
|
||
public function testInvalidVersionsAreRejected() | ||
{ | ||
$this->expectException(\OutOfBoundsException::class); | ||
|
||
Versions::getVersion(uniqid('', true) . '/' . uniqid('', 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.
why building an array of all versions when you could use a loop and break when the right package name is found, taking advantage of your generator ?
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.
copy-paste + laziness, mainly. Code can be improved, ofc, just didn't want to introduce bugs just by rewriting it