Add support for symfony/lock

[digilist]

This PR adds the ability to use a symfony/lock storage to prevent command overlapping, as discussed in #125.

I did not remove the file based locking, as this might be BC break (because of the lockFile() method etc., which is not private). However, this could be removed with the next major release.

This should also solve the request of @IkeLutra, because he can now use the FlockStorage and define the path.

Additionally, I added a unit test that verifies that the task lockig is working properly.

[PabloKowalczyk]

Hello @digilist, thank you very much! I appreciate your work, but, sorry, you should target branch 1.x, as this is active development branch. Could you please backport your PR to PHP 5.6? If you want you can close this PR and open new.
Thanks again, good work.

[digilist]

No problem, I was able to rebase the commits and change the target branch of this PR.

[PabloKowalczyk]

Ah yes, very nice. There are some problems with AppVeyor CI, i will monitor it, but fail(s) can be unrelated to your changes.

[PabloKowalczyk]

Could you also add example, of locking to different path (as mentiond in #125), to README?

[digilist]

I added some examples to the README.

Regarding AppVeyor I do not see how the failing tests could be related to my changes and do not think it is 😕

[PabloKowalczyk]

Looks like the issue with AppVeyor is related to this PR (last build https://ci.appveyor.com/project/lavary/crunz/builds/19558199/job/5egmyi1aw64uhw08), CI killed the build after 1h, something blocked the end of PHPUnit. On the other hand my PR without symfony/lock was successful (#155).

I have some idea. On Lock component's docs (https://symfony.com/doc/3.4/components/lock.html#flockstore) we can read:

The FlockStore uses the file system on the local computer to create the locks. It does not support expiration, but the lock is automatically released when the PHP process is terminated

Also some warning:

Beware that some file systems (such as some types of NFS) do not support locking. In those cases, it's better to use a directory on a local disk drive or a remote store based on Redis or Memcached.

This case reminds me also bug PHPCsFixer PHP-CS-Fixer/PHP-CS-Fixer#3460 which was related to file locks on Vagrant's NFS.

So, to the conclusion, i think that weird behavior of AppVeoyr is related to FlockStore. For some reason PHP can't release Lock (maybe AppVeoyor uses NFS, IDK) and process hangs for 1h.
IMO this issue can be easy fixed by swapping lock store to SemaphoreStore.
WDYT?

[digilist]

Interesting idea, thanks. It looks like the file locking in general is the problem. Even the "old" code does not work, it's not only symfony lock. And as there was no test case for the locking previously, it is also no longer surprising for me, that it is failing with my changes now 😜

I only noticed just now, that AppVeyor is based on Windows, so the problem is (probably) related to Windows.

I changed the condition of the loop from while($event->isLocked()) to while($event->getProcess()->isRunning()) and the build is no longer timing out. Now, the last assertion fails, because the event is still locked, but I do not yet know why.

I will continue my testing to find a solution. I will clean up my test commits later - and I am sorry for that many pushes (if you get an email from github for every push ;)). I currently see no other way to debug it, I have no Windows system at hand.

[PabloKowalczyk]

If you want some WIndows for tests i think that you can use one from https://developer.microsoft.com/en-us/microsoft-edge/tools/vms/ and run it in VirtualBox, hope it helps :)

[digilist]

Thanks! I found the issue. It was related to my overriden manageStartedEvents() function, where I forgot to call the afterCallbacks of the event (after the process finished). So the lock file on Windows was not deleted and the process was locked forever.

I noticed some issue on Windows here: If the lock file is not deleted, because the EventRunner dies (for whatever reason), the event will never start again. This is because the event is considered locked if the lock file exists (https://github.com/lavary/crunz/blob/1.x/src/Event.php#L1028).

I found this solution to check if a process is actually still running: https://stackoverflow.com/questions/13648304/detecting-if-a-windows-process-and-application-is-running. Probably, we can fix it this way, but I would do it in another PR.

[PabloKowalczyk]

Very nice, good job! All checks green, Is it ready to merge now?

[digilist]

I would say so, yes.

[PabloKowalczyk]

Thanks @digilist!