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

[WIP] Fixed autoloader generator #1223

Merged
merged 6 commits into from Oct 17, 2012
Merged

Conversation

hason
Copy link
Contributor

@hason hason commented Oct 16, 2012

Fixes #956, #1051, #1100

}

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.

@Seldaek Seldaek merged commit 9582a8a into composer:master Oct 17, 2012
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

Successfully merging this pull request may close these issues.

Autoload files - load vendors first
2 participants