flint / Lurker

Resource Tracking
http://lurker.rtfd.org
MIT License
179 stars 24 forks source link

Unable to track a non-existent resource - but resource exists #15

Closed burzum closed 9 years ago

burzum commented 9 years ago

The problem

I have no idea what has changed, but since a few days our task runners (Robo) watcher tasks were not working any more. I've started debugging the issue and figured out that the cause is the is_dir() call when the object is passed.

class DirectoryResource extends BaseDirectoryResource implements ResourceInterface
{
    public function exists()
    {
        return is_dir($this); // Causes the error
                // vs
        return is_dir($this->getResource()); // Works fine
    }
}

I have no idea why this suddenly happens, it happens on php 5.6 which we were using before and it started to happen on Windows and OSX as well.

When I run the tests there is only one failing for me but it seems to be not related to my problem.

There was 1 failure:

1) Lurker\Tests\Tracker\RecursiveIteratorTrackerTest::testTrackSimpleFileChanges
Can not find "C:\Users\Florian\AppData\Local\Temp\sf2_resource_watcher_tests/foo" change event

C:\webstack\htdocs\wa-crm\vendor\henrikbjorn\lurker\tests\Lurker\Tests\Tracker\TrackerTest.php:276
C:\webstack\htdocs\wa-crm\vendor\henrikbjorn\lurker\tests\Lurker\Tests\Tracker\TrackerTest.php:85

←[37;41m←[2KFAILURES!
←[0m←[37;41m←[2KTests: 66, Assertions: 30, Failures: 1, Errors: 18, Skipped: 34.
←[0m←[2K
C:\webstack\htdocs\wa-crm\vendor\henrikbjorn\lurker>

I think it would be better to change the code to use is_dir($this->getResource()) instead of passing the object $this to is_dir(). The documentation states it will accept a string, not an object. I guess the object implements __toString() but for some reason it's not working here.

The (possible) cause

After further investigation I think the cause is probably a change in Symfony\Component\Config\Resource\DirectoryResource:

    public function __toString()
    {
        return md5(serialize(array($this->resource, $this->pattern)));
    }

This won't return the actual path but a hash. Doing print_r((string) $this); shows indeed a hash.

henrikbjorn commented 9 years ago

@burzum thanks for reporting the issue and the initial change. I tried the unit tests locally and found a place where it assumed the __toString() still returned the resource.

burzum commented 9 years ago

@henrikbjorn thanks for fixing it.

There are still failing tests? https://travis-ci.org/henrikbjorn/Lurker

I think they're related to inotify and should be made optional, run them only when it is installed?

Also are you going to tag a new version? 1.0.1?

henrikbjorn commented 9 years ago

They are only being run when inotify is present, the problem is the number of events we expect, and the number of events that inotify gives us is different.

Swader commented 8 years ago

Running into this issue as well now.

Swader commented 8 years ago

Ah, just realized it hasn't been tagged in a release yet. Pulling in dev-master fixed things. @henrikbjorn could you tag this so Packagist is aware of the change and pulls in newest?