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.

Sign up for GitHub

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

Jump to bottom

[WIP] Fixed autoloader generator #1223

Merged
merged 6 commits into from
Oct 17, 2012
Merged

[WIP] Fixed autoloader generator #1223

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b683aeb
Fix expected values in tests for autoloader generator
hason Oct 16, 2012
b7f96d5
Fixed autoloader generator
hason Oct 16, 2012
e75abcb
Fixed order of packages in the classmap autoloader
hason Oct 16, 2012
c63f0d9
Preserved original position of package if required packages are befor…
hason Oct 17, 2012
1ff1b8a
Sorted generated classmap
hason Oct 17, 2012
9582a8a
Added tests for consistency of data in AutoloadGenerator::sortPackageMap
hason Oct 17, 2012
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
129 changes: 87 additions & 42 deletions src/Composer/Autoload/AutoloadGenerator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,6 @@ public static function autoload(\$class)

// flatten array
$classMap = array();
$autoloads['classmap'] = new \RecursiveIteratorIterator(new \RecursiveArrayIterator($autoloads['classmap']));

if ($scanPsr0Packages) {
foreach ($autoloads['psr-0'] as $namespace => $paths) {
foreach ($paths as $dir) {
Expand All @@ -139,12 +137,16 @@ public static function autoload(\$class)
}
}
}

$autoloads['classmap'] = new \RecursiveIteratorIterator(new \RecursiveArrayIterator($autoloads['classmap']));
foreach ($autoloads['classmap'] as $dir) {
foreach (ClassMapGenerator::createMap($dir) as $class => $path) {
$path = '/'.$filesystem->findShortestPath(getcwd(), $path, true);
$classMap[$class] = '$baseDir . '.var_export($path, true).",\n";
}
}

ksort($classMap);
foreach ($classMap as $class => $code) {
$classmapFile .= ' '.var_export($class, true).' => '.$code;
}
Expand All @@ -170,28 +172,13 @@ public function buildPackageMap(InstallationManager $installationManager, Packag
{
// build package => install path map
$packageMap = array();
$packages[] = $mainPackage;

// sort packages by dependencies
usort($packages, function (PackageInterface $a, PackageInterface $b) {
foreach (array_merge($a->getRequires(), $a->getDevRequires()) as $link) {
if (in_array($link->getTarget(), $b->getNames())) {
return 1;
}
}
foreach (array_merge($b->getRequires(), $b->getDevRequires()) as $link) {
if (in_array($link->getTarget(), $a->getNames())) {
return -1;
}
}

return strcmp($a->getName(), $b->getName());
});
array_unshift($packages, $mainPackage);

foreach ($packages as $package) {
if ($package instanceof AliasPackage) {
continue;
}

if ($package === $mainPackage) {
$packageMap[] = array($mainPackage, '');
continue;
Expand All @@ -213,32 +200,15 @@ public function buildPackageMap(InstallationManager $installationManager, Packag
*/
public function parseAutoloads(array $packageMap)
{
$autoloads = array('classmap' => array(), 'psr-0' => array(), 'files' => array());
foreach ($packageMap as $item) {
list($package, $installPath) = $item;
$sortedPackageMap = $this->sortPackageMap($packageMap);

if (null !== $package->getTargetDir()) {
$installPath = substr($installPath, 0, -strlen('/'.$package->getTargetDir()));
}
$psr0 = $this->parseAutoloadsType($packageMap, 'psr-0');
$classmap = $this->parseAutoloadsType($sortedPackageMap, 'classmap');
$files = $this->parseAutoloadsType($sortedPackageMap, 'files');

foreach ($package->getAutoload() as $type => $mapping) {
// skip misconfigured packages
if (!is_array($mapping)) {
continue;
}

foreach ($mapping as $namespace => $paths) {
foreach ((array) $paths as $path) {
$autoloads[$type][$namespace][] = empty($installPath) ? $path : $installPath.'/'.$path;
}
}
}
}

krsort($autoloads['classmap']);
krsort($autoloads['psr-0']);
krsort($psr0);

return $autoloads;
return array('psr-0' => $psr0, 'classmap' => $classmap, 'files' => $files);
}

/**
Expand Down Expand Up @@ -445,4 +415,79 @@ public static function getLoader()

}

protected function parseAutoloadsType(array $packageMap, $type)
{
$autoloads = array();
foreach ($packageMap as $item) {
list($package, $installPath) = $item;

$autoload = $package->getAutoload();
// skip misconfigured packages
if (!isset($autoload[$type]) || !is_array($autoload[$type])) {
continue;
}

if (null !== $package->getTargetDir()) {
$installPath = substr($installPath, 0, -strlen('/'.$package->getTargetDir()));
}

foreach ($autoload[$type] as $namespace => $paths) {
foreach ((array) $paths as $path) {
$autoloads[$namespace][] = empty($installPath) ? $path : $installPath.'/'.$path;
}
}
}

return $autoloads;
}

protected function sortPackageMap(array $packageMap)
{
$groups = array();
$names = array();
foreach ($packageMap as $key => $item) {
$groups[$key] = array($item);
$mainName = $item[0]->getName();
foreach ($item[0]->getNames() as $name) {
if (!isset($names[$name])) {
$names[$name] = $name == $mainName ? $key : $mainName;
}
}
}

foreach ($packageMap as $item) {
foreach (array_merge($item[0]->getRequires(), $item[0]->getDevRequires()) as $link) {
$target = $link->getTarget();
if (!isset($names[$target])) {
continue;
}

$targetKey = $names[$target];
if (is_string($targetKey)) {
if (!isset($names[$targetKey])) {
continue;
}
$targetKey = $names[$targetKey];
}

$packageKey = $names[$item[0]->getName()];
if ($targetKey <= $packageKey || !isset($groups[$packageKey])) {
continue;
}

foreach ($groups[$packageKey] as $originalItem) {
$groups[$targetKey][] = $originalItem;
$names[$originalItem[0]->getName()] = $targetKey;
}
unset($groups[$packageKey]);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I honestly have no clue what's going on in there, but it explodes when generating autoloads for packagist: Undefined offset: 27 when reading $groups[$packageKey] at line 462. If you can either fix it or add comments so that it's understandable I might be able to merge.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll look into it. The problem is that in the PHP doesn't exist sorting function which preserves original order of items.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you mean, the point of sorting is to sort and not preserve the order? Copying the array allows you to keep the old order as well though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Seldaek Fixed, Can you test it, please? We should to find better solution. Each type of class loader require different order of packages:

Available packages:
    a (requires: c), b, d, c (requires: d)

Main package requires:
    [a, b]

Packages in repository:
    [a, b, d, c] or anything else

PSR0 loader requires ordered packages:
    [a, b, c, d]

Classmap and files loader requires reverse ordered packages (same as in installed.json?):
    [b, d, c, a] or [d, c, a, b]?

This PR solves reported issues but doesn't provide complex solution.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PSR0 just needs the namespaces to be reserve ordered, so that more specific ones get hit first, but within a namespace if two packages provide the same namespace it shold follow the same as classmap (i.e. first package wins, and root package should always beat the others). files autoload needs packages ordered by requirements, which shouldn't affect the other sortings (as long as the root is kept on top no matter if it's required by some of its dependencies)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyway it seems fine for now so I'll merge, but if you think you can improve according to what I just wrote, feel free to send a new PR.

}

$sortedPackageMap = array();
foreach ($groups as $group) {
$sortedPackageMap = array_merge($sortedPackageMap, $group);
}

return $sortedPackageMap;
}
}
41 changes: 30 additions & 11 deletions tests/Composer/Test/Autoload/AutoloadGeneratorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -238,9 +238,9 @@ public function testVendorsClassMapAutoloading()
$this->assertTrue(file_exists($this->vendorDir.'/composer/autoload_classmap.php'), "ClassMap file needs to be generated.");
$this->assertEquals(
array(
'ClassMapFoo' => $this->workingDir.'/composer-test-autoload/a/a/src/a.php',
'ClassMapBar' => $this->workingDir.'/composer-test-autoload/b/b/src/b.php',
'ClassMapBaz' => $this->workingDir.'/composer-test-autoload/b/b/lib/c.php',
'ClassMapFoo' => $this->workingDir.'/composer-test-autoload/a/a/src/a.php',
),
include ($this->vendorDir.'/composer/autoload_classmap.php')
);
Expand Down Expand Up @@ -275,9 +275,9 @@ public function testClassMapAutoloadingEmptyDirAndExactFile()
$this->assertTrue(file_exists($this->vendorDir.'/composer/autoload_classmap.php'), "ClassMap file needs to be generated.");
$this->assertEquals(
array(
'ClassMapFoo' => $this->workingDir.'/composer-test-autoload/a/a/src/a.php',
'ClassMapBar' => $this->workingDir.'/composer-test-autoload/b/b/test.php',
'ClassMapBaz' => $this->workingDir.'/composer-test-autoload/c/c/foo/test.php',
'ClassMapFoo' => $this->workingDir.'/composer-test-autoload/a/a/src/a.php',
),
include ($this->vendorDir.'/composer/autoload_classmap.php')
);
Expand Down Expand Up @@ -322,31 +322,42 @@ public function testFilesAutoloadOrderByDependencies()
{
$package = new Package('a', '1.0', '1.0');
$package->setAutoload(array('files' => array('root.php')));
$package->setRequires(array(new Link('a', 'a/foo')));
$package->setRequires(array(new Link('a', 'z/foo')));

$packages = array();
$packages[] = $a = new Package('a/foo', '1.0', '1.0');
$packages[] = $z = new Package('z/foo', '1.0', '1.0');
$packages[] = $b = new Package('b/bar', '1.0', '1.0');
$packages[] = $c = new Package('c/lorem', '1.0', '1.0');
$packages[] = $d = new Package('d/d', '1.0', '1.0');
$packages[] = $e = new Package('e/e', '1.0', '1.0');

$a->setAutoload(array('files' => array('testA.php')));
$a->setRequires(array(new Link('a/foo', 'c/lorem')));
$z->setAutoload(array('files' => array('testA.php')));
$z->setRequires(array(new Link('z/foo', 'c/lorem')));

$b->setAutoload(array('files' => array('testB.php')));
$b->setRequires(array(new Link('b/bar', 'c/lorem')));

$c->setAutoload(array('files' => array('testC.php')));

$d->setAutoload(array('files' => array('testD.php')));

$e->setAutoload(array('files' => array('testE.php')));
$e->setRequires(array(new Link('e/e', 'c/lorem')));

$this->repository->expects($this->once())
->method('getPackages')
->will($this->returnValue($packages));

$this->fs->ensureDirectoryExists($this->vendorDir . '/a/foo');
$this->fs->ensureDirectoryExists($this->vendorDir . '/z/foo');
$this->fs->ensureDirectoryExists($this->vendorDir . '/b/bar');
$this->fs->ensureDirectoryExists($this->vendorDir . '/c/lorem');
file_put_contents($this->vendorDir . '/a/foo/testA.php', '<?php function testFilesAutoloadOrderByDependency1() {}');
$this->fs->ensureDirectoryExists($this->vendorDir . '/d/d');
$this->fs->ensureDirectoryExists($this->vendorDir . '/e/e');
file_put_contents($this->vendorDir . '/z/foo/testA.php', '<?php function testFilesAutoloadOrderByDependency1() {}');
file_put_contents($this->vendorDir . '/b/bar/testB.php', '<?php function testFilesAutoloadOrderByDependency2() {}');
file_put_contents($this->vendorDir . '/c/lorem/testC.php', '<?php function testFilesAutoloadOrderByDependency3() {}');
file_put_contents($this->vendorDir . '/d/d/testD.php', '<?php function testFilesAutoloadOrderByDependency4() {}');
file_put_contents($this->vendorDir . '/e/e/testE.php', '<?php function testFilesAutoloadOrderByDependency5() {}');
file_put_contents($this->workingDir . '/root.php', '<?php function testFilesAutoloadOrderByDependencyRoot() {}');

$this->generator->dump($this->config, $this->repository, $package, $this->im, 'composer', false, 'FilesAutoloadOrder');
Expand All @@ -361,31 +372,38 @@ public function testFilesAutoloadOrderByDependencies()
$this->assertTrue(function_exists('testFilesAutoloadOrderByDependency1'));
$this->assertTrue(function_exists('testFilesAutoloadOrderByDependency2'));
$this->assertTrue(function_exists('testFilesAutoloadOrderByDependency3'));
$this->assertTrue(function_exists('testFilesAutoloadOrderByDependency4'));
$this->assertTrue(function_exists('testFilesAutoloadOrderByDependency5'));
$this->assertTrue(function_exists('testFilesAutoloadOrderByDependencyRoot'));
}

public function testOverrideVendorsAutoloading()
{
$package = new Package('a', '1.0', '1.0');
$package->setAutoload(array('psr-0' => array('A\\B' => $this->workingDir.'/lib')));
$package = new Package('z', '1.0', '1.0');
$package->setAutoload(array('psr-0' => array('A\\B' => $this->workingDir.'/lib'), 'classmap' => array($this->workingDir.'/src')));
$package->setRequires(array(new Link('z', 'a/a')));

$packages = array();
$packages[] = $a = new Package('a/a', '1.0', '1.0');
$packages[] = $b = new Package('b/b', '1.0', '1.0');
$a->setAutoload(array('psr-0' => array('A' => 'src/', 'A\\B' => 'lib/')));
$a->setAutoload(array('psr-0' => array('A' => 'src/', 'A\\B' => 'lib/'), 'classmap' => array('classmap')));
$b->setAutoload(array('psr-0' => array('B\\Sub\\Name' => 'src/')));

$this->repository->expects($this->once())
->method('getPackages')
->will($this->returnValue($packages));

$this->fs->ensureDirectoryExists($this->workingDir.'/lib/A/B');
$this->fs->ensureDirectoryExists($this->workingDir.'/src/');
$this->fs->ensureDirectoryExists($this->vendorDir.'/composer');
$this->fs->ensureDirectoryExists($this->vendorDir.'/a/a/classmap');
$this->fs->ensureDirectoryExists($this->vendorDir.'/a/a/src');
$this->fs->ensureDirectoryExists($this->vendorDir.'/a/a/lib/A/B');
$this->fs->ensureDirectoryExists($this->vendorDir.'/b/b/src');
file_put_contents($this->workingDir.'/lib/A/B/C.php', '<?php namespace A\\B; class C {}');
file_put_contents($this->workingDir.'/src/classes.php', '<?php namespace Foo; class Bar {}');
file_put_contents($this->vendorDir.'/a/a/lib/A/B/C.php', '<?php namespace A\\B; class C {}');
file_put_contents($this->vendorDir.'/a/a/classmap/classes.php', '<?php namespace Foo; class Bar {}');

$workDir = strtr($this->workingDir, '\\', '/');
$expectedNamespace = <<<EOF
Expand Down Expand Up @@ -414,6 +432,7 @@ public function testOverrideVendorsAutoloading()

return array(
'A\\\\B\\\\C' => \$baseDir . '/lib/A/B/C.php',
'Foo\\\\Bar' => \$baseDir . '/src/classes.php',
);

EOF;
Expand Down
2 changes: 1 addition & 1 deletion tests/Composer/Test/Autoload/Fixtures/autoload_classmap3.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,6 @@
$baseDir = $vendorDir;

return array(
'Main\\Foo' => $baseDir . '/src/Main/Foo.php',
'ClassMapFoo' => $baseDir . '/composersrc/foo.php',
'Main\\Foo' => $baseDir . '/src/Main/Foo.php',
);
2 changes: 1 addition & 1 deletion tests/Composer/Test/Autoload/Fixtures/autoload_classmap4.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
$baseDir = dirname($vendorDir);

return array(
'ClassMapBar' => $baseDir . '/composer-test-autoload/b/b/src/b.php',
'ClassMapBaz' => $baseDir . '/composer-test-autoload/b/b/lib/c.php',
'ClassMapFoo' => $baseDir . '/composer-test-autoload/a/a/src/a.php',
'ClassMapBar' => $baseDir . '/composer-test-autoload/b/b/src/b.php',
);
2 changes: 1 addition & 1 deletion tests/Composer/Test/Autoload/Fixtures/autoload_classmap5.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
$baseDir = dirname($vendorDir);

return array(
'ClassMapFoo' => $baseDir . '/composer-test-autoload/a/a/src/a.php',
'ClassMapBar' => $baseDir . '/composer-test-autoload/b/b/test.php',
'ClassMapBaz' => $baseDir . '/composer-test-autoload/c/c/foo/test.php',
'ClassMapFoo' => $baseDir . '/composer-test-autoload/a/a/src/a.php',
);
4 changes: 3 additions & 1 deletion tests/Composer/Test/Autoload/Fixtures/autoload_real_files_by_dependency.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ public static function getLoader()
$loader->register();

require $vendorDir . '/c/lorem/testC.php';
require $vendorDir . '/a/foo/testA.php';
require $vendorDir . '/z/foo/testA.php';
require $baseDir . '/root.php';
require $vendorDir . '/b/bar/testB.php';
require $vendorDir . '/d/d/testD.php';
require $vendorDir . '/e/e/testE.php';

return $loader;
}
Expand Down