Skip to content

Switch to amphp/amp v3#24

Open
przepompownia wants to merge 4 commits into
phpactor:masterfrom
przepompownia:amp3
Open

Switch to amphp/amp v3#24
przepompownia wants to merge 4 commits into
phpactor:masterfrom
przepompownia:amp3

Conversation

@przepompownia

Copy link
Copy Markdown

No description provided.

@przepompownia przepompownia changed the title Amp3 (WIP): switch to AMP3 Dec 29, 2025
@przepompownia

przepompownia commented Dec 29, 2025

Copy link
Copy Markdown
Author

Please keep in mind that I'm still learning cooperative multitasking/AMP{2,3}/Revolt/Fibers and still finding basic examples to dispel numerous doubts (especially related to transition from v2 to v3).

I started rehearsing with the phpactor/language-server project, then stashed the changes and switched here since it's probably the simplest dependency placed between Phpactor and AMP.

Not sure if it's a good approach, but (instead of tests) I started from the demo executable (./bin/watcher) and InotifyWatcher, then after (perhaps apparent) success updated FindWatcher.

Expect elementary mistakes. Here I still need a hard proof (a test) that all previously non-blocking operations remain so. Early feedback is also welcome, so I've released this change as a draft.

@przepompownia przepompownia marked this pull request as ready for review December 29, 2025 23:21
@przepompownia przepompownia changed the title (WIP): switch to AMP3 Switch to amphp/amp v3 Dec 29, 2025
@przepompownia przepompownia marked this pull request as draft December 29, 2025 23:50
@przepompownia

przepompownia commented Dec 30, 2025

Copy link
Copy Markdown
Author

Still unsure if (on watchers) we need to wrap wait body with async and await its Future somewhere outside - unfinished loop on Phpactor\AmpFsWatcher\Tests\Watcher\Find\FindWatcherTest::testSingleFileChange (and other cases) causes exceeding timeout 5s when running on my local machine.

An alternative is an external wrap - for example at the test (each case) level

    public function testSingleFileChange(): void
    {
        $process = $this->startProcess();
        $this->delay();
        $this->workspace()->put('foobar', '');
        $this->delay();

        async(function () use ($process): void {
            self::assertEquals(
                new ModifiedFile(
                    $this->workspace()->path('foobar'),
                    ModifiedFile::TYPE_FILE
                ),
                $process->wait()
            );
        });

        $process->stop();
    }

and (maybe accidentally) allows to satisfy the assertion and stop the process.

@przepompownia

Copy link
Copy Markdown
Author

Both in the bin/watch demo and test cases I turned wait() into async outside its body.

For some reason that is still unclear, the test stopped displaying the unfinished loop message after increasing the delay between operations.

@dantleech what do you think about the current state?

@przepompownia przepompownia marked this pull request as ready for review January 29, 2026 01:57
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.

1 participant