Skip to content

Conversation

saksmt
Copy link
Contributor

@saksmt saksmt commented Jun 2, 2015

[DoctrineBridge] Fixed compatibility with entities packed in Phar

Q A
Bug fix? yes
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets #14840
License MIT
Doc PR none

Fixed bug with Phars, `realpath` and mapping driver config setter in `Symfony\Bridge\Doctrine\DependencyInjection\AbstractDoctrineExtension`
throw new \InvalidArgumentException(sprintf('Invalid Doctrine mapping path given. Cannot load Doctrine mapping/bundle named "%s".', $mappingName));
}

$this->drivers[$mappingConfig['type']][$mappingConfig['prefix']] = realpath($mappingConfig['dir']);
if (strpos($mappingDirectory, 'phar://') === false) {
Copy link
Contributor

Choose a reason for hiding this comment

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

it should only look at the beginning of the string, not anywhere

Somehow optimized the solution.
@CybersecDan
Copy link

I am being sent these random issues via code triage. Is there anything to help with on this one or is it just basically waiting to be merged?

@saksmt
Copy link
Contributor Author

saksmt commented Jul 14, 2015

@danielrose28 It just needs to be merged, bug was really small.

@CybersecDan
Copy link

@saksmt Cool. Just wanted to see if there was anything I could do. Thanks.

@fabpot
Copy link
Member

fabpot commented Sep 14, 2015

Thank you @saksmt.

fabpot added a commit that referenced this pull request Sep 14, 2015
This PR was squashed before being merged into the 2.3 branch (closes #14841).

Discussion
----------

[DoctrineBridge] Fixed #14840

[DoctrineBridge] Fixed compatibility with entities packed in Phar

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #14840
| License       | MIT
| Doc PR        | none

Commits
-------

92ad5df [DoctrineBridge] Fixed #14840
@fabpot fabpot closed this Sep 14, 2015
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.

4 participants