Skip to content

Support running from PHAR #5

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
clue opened this issue Jan 22, 2019 · 5 comments · Fixed by #55
Closed

Support running from PHAR #5

clue opened this issue Jan 22, 2019 · 5 comments · Fixed by #55
Labels
help wanted Extra attention is needed new feature New feature or request
Milestone

Comments

@clue
Copy link
Owner

clue commented Jan 22, 2019

No description provided.

@clue clue added new feature New feature or request help wanted Extra attention is needed labels Jan 22, 2019
@seankndy
Copy link

seankndy commented May 6, 2020

I noticed when running from a Phar I receive a 'Database closed' Exception from open()'s Promise. Digging a little deeper on the Exception and found that the sqlite-worker.php file couldn't ever be launched.

Is the issue that the sqlite-worker.php is bundled into the phar and there is no way to launch a new php process on that file within the archive? Perhaps it would be possible to make a version of the sqlite-worker.php that is itself a phar and allow users to simply copy that to some location on their machine and then just supply an option to be passed in to open() that specified where the sqlite-worker phar is located? It's not a very elegant solution, but it would give a quick option to those wanting to use phars. I'm interested to hear what the challenges actually are and what could be done to overcome it.

@seankndy
Copy link

seankndy commented May 6, 2020

I forked this to create essentially what I described above. I needed this functionality asap so I can package up my app in a phar.

I placed the sqlite-worker.php code into a function (runWorker()) so that users of the package can make their own worker that is built into the phar. I then updated Factory and related to support passing in the custom worker command.

This is what I have in my main entry file to my project:

$options = getopt(null, [
    'sqlite-worker',
]);
if (isset($options['sqlite-worker'])) {
    Clue\React\SQLite\runWorker($_SERVER['argv'][2] ?? null);
    exit;
}

Then during instantiation of the sqlite database, I check for Phar and alter the worker command:

$options = ['worker_command' => \Phar::running(false) ? \Phar::running(false) . ' --sqlite-worker' : null];
return $factory->openLazy(
    $c->get('config')['database.file'],
    null,
    $options
);

https://github.com/seankndy/reactphp-sqlite/tree/modular-worker-for-phar-support

Unit tests are still passing. I can submit a pull if you want this merged (sorry for excessive commits).

@clue
Copy link
Owner Author

clue commented May 15, 2020

[…] found that the sqlite-worker.php file couldn't ever be launched.

Is the issue that the sqlite-worker.php is bundled into the phar and there is no way to launch a new php process on that file within the archive?

Yep, that's the main problem here. We could extract (parts of) the phar to a temporary location or use something like this to instruct PHP to directly spawn a file inside a phar:

$ php -r 'require("phar://sqlite.phar/res/sqlite-worker.php");'

I placed the sqlite-worker.php code into a function (runWorker()) so that users of the package can make their own worker that is built into the phar. I then updated Factory and related to support passing in the custom worker command.

It's definitely an interesting work-around that is probably good enough to get this to work for your specific use case. Before including this in this library, I would rather work out a more generic approach that doesn't require explicitly modifying paths and the main entry script.

There are currently no immediate plans to build this from my end (no demand at the moment and more important outstanding issues currently), but I would be really happy to accept PRs 👍

(If you need this for a commercial project and if you want to help sponsor this feature, feel free to reach out and/or send me an email)

@clue
Copy link
Owner Author

clue commented Aug 2, 2020

We could extract (parts of) the phar to a temporary location or use something like this to instruct PHP to directly spawn a file inside a phar:

$ php -r 'require("phar://sqlite.phar/res/sqlite-worker.php");'

I've started working on this and have a working prototype ready 🎉

So far, I've checked this with a number of phars to see this works as expected. I have yet to add some automated tests to ensure we do not introduce any regressions in the future (this requires some non-trivial amount of work). I've used some patches locally, but a more permanent solution may require clue/phar-composer#29 or an even more sophisticated test suite.

Other than that, this is ready to be shipped :shipit:

@clue
Copy link
Owner Author

clue commented Feb 15, 2022

This should now be solved via #55 and #61. :shipit:

Together with the referenced tickets and the upstream releases required to get this in, you're looking at several days worth of work to ensure this works across all supported platforms. The updated test suite confirms this works across all supported platforms, including Windows (which requires some special care via #60).

I hope this helps! 👍 If so, consider supporting this project, for example by becoming a sponsor ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed new feature New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants