8000 [master] Commands with --env=test throw exceptions · Issue #4626 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[master] Commands with --env=test throw exceptions #4626

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
Closed
gnugat opened this issue Jun 21, 2012 · 15 comments
Closed

[master] Commands with --env=test throw exceptions #4626

gnugat opened this issue Jun 21, 2012 · 15 comments
Labels
Console

Comments

@gnugat
Copy link
Contributor
gnugat commented Jun 21, 2012

After updating my dependencies this morning with composer ("symfony/symfony": "2.1.*"), I figured out that every commands using the option "--env=test" would throw the following exception:

[ErrorException]                                                                                                                                                               
  Notice: unserialize(): Error at offset 65 of 67 bytes in [...]/vendor/symfony/symfony/src/Symfony/Component/Config/Resource/FileResource.php line 115

After putting a nice var_dump($serialized); in FileResource::unserialized(), and running different commands we can see that the content of $serialize causing this exception is the following one:

s:59:"[...]/app/cache/test/kernel.tmp";}i:1

This "}i:1" looks particulary suspicious to me.
Anyone having the same trouble?
@stof
Copy link
Member
stof commented Jun 21, 2012

when updating symfony, you need to remove your cache entirely. The serialization of the resources is not BC as it implements the Serializable interface.

@gnugat
Copy link
Contributor Author
gnugat commented Jun 21, 2012

After having this issue, I removed the repo and got a fresh copy (so with an empty cache), but I still had the issue.
I had to do a rm -rf ./app/cache/* (cache:clear --env=test not working) to get rid of it.

Thanks for the tip and sorry for the inconvenience.

@fabpot fabpot closed this as completed Jun 21, 2012
@gnugat
Copy link
Contributor Author
gnugat commented Jun 21, 2012

The issue is still there and is real.
So I tested a bit around with it, and here's what I saw:
Do a first clear:cache, then every time you use a command in the environment of your cc will have this Exception.

Example:

php app/console clear:cache # Good
php app/console --env=dev # Exception!
rm -rf ./app/cache/* # Solve
php app/console clear:cache --env=test # Good
php app/console --env=dev # Good
php app/console --env=test # Exception!

@tmartin tmartin mentioned this issue Jun 21, 2012
Closed
@fabpot
Copy link
Member
fabpot commented Jun 21, 2012

The problematic code has been reverted for now.

@sstok
Copy link
Contributor
sstok commented Jun 22, 2012

I'm not being able to reproduce the exception with an clean install.
And yes I have the patch re-applied (git revert HEAD)

PHP 5.3.8 (32bit TS) on Windows 7.
Will update my PHP version and check again.

No, still no exception thrown.

@gnugat
Copy link
Contributor Author
gnugat commented Jun 22, 2012

The complete series of command I executed:

# Worked fine
php composer.phar self-update
php composer.phar update --dev
php app/console assetic:dump --no-debug --env=test

# Exception
php app/console cache:clear --env=test

# skipped
php app/console doctrine:database:drop --force --env=test
php app/console doctrine:database:create --env=test
php app/console doctrine:schema:update --force --env=test
php app/console widop:fixtures:load --env=test

But as said above, I could clearly reproduce the bug by running the command php app/console clear:cache (which would work fine) and then trying any command in the same environment as the cc command (in this example every --env=dev command would throw this exception after the cc command).

System: PHP 5.4.4, Ubuntu 12.04.

@sstok
Copy link
Contributor
sstok commented Jun 22, 2012

No exception on Windows (tried both 5.3 and 5.4).
Confirmed clean install on Ubuntu 12.04 throws an exception.

The problem does not seem to come from the serialize() in FileResource as this serialize() result is just fine.
So output is changed somewhere in the process.

I think I got it.

The file [...]/app/cache/test_new/kernel.tmp is serialized and valid.
But in CacheClearCommand#warmup() the _new part is removed thus making the serialization invalid.

The ;}i5 is part of another serialization (as its nested), this is causing a chain reaction and making everything fail.

@sstok
Copy link
Contributor
sstok commented Jun 22, 2012

GOT GOT GOT IT!!

Changing this line in FileResource

$this->resource = file_exists($resource) ? realpath($resource) : $resource;

To the old version

$this->resource = realpath($resource);

Fixes the exception.

@fabpot fabpot reopened this Jun 25, 2012
@fabpot
Copy link
Member
fabpot commented Jun 25, 2012

To be taken care of when we reapply #4619. Thanks for the investigation @sstok

@everzet everzet mentioned this issue Feb 17, 2013
Closed
@lsmith77
Copy link
Contributor

ping

@jakzal
Copy link
Contributor
jakzal commented Dec 28, 2013

@sstok I know this is an old issue, but do you still remember how to reproduce it and verify if we need to do anything about it?

@sstok
Copy link
Contributor
sstok commented Dec 29, 2013

From what I remember, when the cache is warmed (with an existing one in place) the directory gets appended with _new. But the CacheClearCommand#warmup() removed the _new from the serialized result, creating a difference in the length, then causing the serialized result to be invalid.

Why always using the realpath() function prevents this is a bit of mystery.
It either ensures the exact length or the new part was not in place when it was fed.

It wasn't really at the place you'd except the failure so it was a challenge to find it 💃

@jakzal
Copy link
Contributor
jakzal commented Dec 29, 2013

Ok, this should be fixed by #7360 which changed the renaming strategy. We don't append _new anymore, but replace the last character with ___ instead. This way the length is the same.

@gnugat
Copy link
Contributor Author
gnugat commented Dec 30, 2013

I haven't encountered this bug for a while and assumed it was fixed, but I forgot about this issue. Should I close it?

@fabpot fabpot closed this as completed Dec 30, 2013
@jakzal
Copy link
Contributor
jakzal commented Dec 30, 2013

Yeah, should be fixed by #7360. Feel free to reopen it if you encounter this issue again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Console
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@fabpot @jakzal @lsmith77 @stof @gnugat @sstok
0