Skip to content

Add ability to set worker file and cwd (Phar support) #32

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.

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

Closed
wants to merge 3 commits into from

Conversation

azurre
Copy link

@azurre azurre commented Jul 29, 2020

Fix make able use reactphp-sqlite with phar applications + fix filename regexp (windows path support)

@azurre azurre changed the title Add ability to set worker file and cwd (for phar) Add ability to set worker file and cwd (Phar support) Jul 29, 2020
@azurre
Copy link
Author

azurre commented Jul 29, 2020

The problem:

  • Make phar application with simple insert (from readme)
  • Run php test.phar
  • Get an error: PHP Fatal error: Uncaught RuntimeException: Unable to launch a new process: proc_open(): CreateProcess failed, error code - 267

Achieve wanted:

  • Make able run phar application

Exapmle:

<?php
use Clue\React\SQLite\Factory;
(function() {
    // --- additional part for phar application ---
    $options = getopt('', ['sqlite::']);
    if (isset($options['sqlite'])) { // Run worker
        if (isset($_SERVER['argv'][2])) { // Handle Windows
            $_SERVER['argv'][1] = isset($_SERVER['argv'][2]) ? $_SERVER['argv'][2] : $_SERVER['argv'][1];
        } else {
            unset($_SERVER['argv'][1]); // Handle *nix
        }
        return include __DIR__ . '/vendor/clue/reactphp-sqlite/res/sqlite-worker.php';
    }
    $worker = $cwd = null;
    if (strpos(__DIR__, 'phar://') === 0) {
        $app = Phar::running(false);
        $worker = "{$app} --sqlite=1";
        $cwd = dirname($app);
    }
    // -------------------------------------------
    require_once __DIR__ . '/vendor/autoload.php';
    $loop = React\EventLoop\Factory::create();
    $factory = new Factory($loop, $worker, $cwd);
    $db = $factory->openLazy('users.db');
    $db->exec('CREATE TABLE IF NOT EXISTS foo (id INTEGER PRIMARY KEY AUTOINCREMENT, bar STRING)');
    $name = 'Alice';
    $db->query('INSERT INTO foo (bar) VALUES (?)', [$name])->then(
        function(Clue\React\SQLite\Result $result) use ($name) {
            echo 'New ID for ' . $name . ': ' . $result->insertId . PHP_EOL;
        }
    );
    $db->quit();
    $loop->run();
})();

As a result
$ php test.phar

New ID for Alice: 8

and

$ php test.php

New ID for Alice: 9

works as expected on windows & *nix

Copy link
Owner

@clue clue left a comment

Choose a reason for hiding this comment

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

@azurre Thanks for filing this PR! 👍

Unfortunately, the PR does not currently contain any documentation, examples or tests, so it's a bit unclear how this can be used.

May I ask you provide a simple gist of what you're trying to achieve and how this looks from a consumer perspective? Thanks!

@azurre
Copy link
Author

azurre commented Aug 2, 2020

Hi @clue

I've updated my previous comment

@clue clue added the new feature New feature or request label Aug 2, 2020
Copy link
Owner

@clue clue left a comment

Choose a reason for hiding this comment

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

@azurre Thanks for looking into this and filing a PR to address #5!

This definitely looks like an interesting starting point and I'm happy to see you've got this working! I think this PR boils down to having to include the worker script from within a PHP file itself rather than passing its path to the PHP executable.

I would still like to look into the option discussed in #5 to see if we can integrate this without requiring consumers of this library to take care of this path handling. It's my understanding we should be able to launch the worker script with PHP in a way that does not require modification of the consumer application.

As much as I'd like to commit to specific date, I can only say that we will get to this feature as soon as time permits. In the meantime, I think your changes LGTM and you can always keep using this approach from your private fork. 👍

@clue
Copy link
Owner

clue commented Nov 12, 2021

I'm closing this for now as it hasn't received any input in a while and I believe this is best addressed with #5 in a future version. If you feel this is still relevant, please come back with more details and we can always reopen this 👍

Thank you for your effort, keep it up! 👍

@clue clue closed this Nov 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants