Skip to content
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

[🐛 BUG]: handle_serve_command function call error when having spaces on the server.command path #1667

Closed
1 task done
nunomaduro opened this issue Aug 3, 2023 · 31 comments · Fixed by roadrunner-server/server#51
Assignees
Labels
B-bug Bug: bug, exception
Milestone

Comments

@nunomaduro
Copy link

No duplicates 🥲.

  • I have searched for a similar issue in our bug tracker and didn't find any solutions.

What happened?

Seems to be impossible to provide a server.command with spaces on the command's path. Here is an example:

./rr '-c' '/Users/nunomaduro/Work/Code/laravel/.rr.yaml' '-o' 'version=3' '-o' 'server.command="/Users/nunomaduro/Library/Application Support/Herd/bin/php82"

I've tried multiple aways to escape the "Application Support", however the result is aways the same:

handle_serve_command: Function call error:
        got initial serve error from the Vertex *http.Plugin, stopping execution, error: static_pool_allocate_workers: WorkerAllocate: fork/exec /Users/nunomaduro/Library/Application: no such file or directory

Any ideas for what to solve this issue?

Version (rr --version)

rr version 2023.2.1 (build time: 2023-07-27T17:12:33+0000, go1.20.6), OS: darwin, arch: arm64

How to reproduce the issue?

rr version 2023.2.1

Relevant log output

handle_serve_command: Function call error:
        got initial serve error from the Vertex *http.Plugin, stopping execution, error: static_pool_allocate_workers: WorkerAllocate: fork/exec /Users/nunomaduro/Library/Application: no such file or directory
@rustatian
Copy link
Member

rustatian commented Aug 3, 2023

Hey @nunomaduro 👋🏻
In general, spaces are not allowed not only in the RR command field, but in most other Unix commands, such as ls, cd, etc. Words after spaces are treated as command delimiters or additional arguments. You can try to use backslash or quotation marks, such as:

  1. -o server.command=/Users/nunomaduro/Library/Application\ Support/Herd/bin/php82
  2. -o server.command=/Users/nunomaduro/Library/'Application Support'/Herd/bin/php82

@rustatian
Copy link
Member

Also, I'm not sure this is the right place to install binaries on MacOS. In general, this folder is used for the configuration files, program working data, etc., but not for the binaries.

The usual place for the binaries is /usr/local/bin or ~/Applications.

@rustatian
Copy link
Member

rustatian commented Aug 3, 2023

Yes, I double-checked the docs, this is not an RR bug, but rather a Herd inconsistent with OS recommendations. Here is the link. Have a look at the table 1-2, 1-3:

  • ~/Library/Application Support/...:
Use this directory to store all app data files except those associated with the user’s documents. For example, you might use this directory to store app-created data files, configuration files, templates, or other fixed or modifiable resources that are managed by the app. An app might use this directory to store a modifiable copy of resources contained initially in the app’s bundle. A game might use this directory to store new levels purchased by the user and downloaded from a server.

@nunomaduro
Copy link
Author

You can try to use backslash or quotation marks...

None of this options is working for me.

@marcelpociot Any ideas here buddy?

@rustatian
Copy link
Member

@nunomaduro As a third option, you can create a symlink to the /usr/local folder.

@mpociot
Copy link

mpociot commented Aug 3, 2023

Well the Application Support folder is a folder that holds all files that an app needs in order to operate (files supporting the application).
I agree that it is mostly common for files to live in /usr/local/bin, but this sounds like a bug with spaces in the path rather than an issue with Herd 🤷‍♂️

@rustatian
Copy link
Member

rustatian commented Aug 3, 2023

Hey @mpociot 👋🏻 Nice to meet you.

Unfortunately, I can't agree with your idea that support files (or files supporting the application) can contain other (downloaded) binaries. And Apple docs say about this - "App-created data files, configuration files, templates, or other fixed or modifiable resources managed by the application". App created data files are not the same as binary files (executables). It is a good idea to put php.ini in application support, but not the binary.

The second example is Homebrew. Homebrew installs all downloaded binaries into /usr/local/Cellar or /usr/local/bin. But the configuration files for all these executables are stored in the Application Support folder.

@rustatian
Copy link
Member

rustatian commented Aug 3, 2023

By can contain other (downloaded) binaries I meant files that are downloaded and placed in the Application Support folder.
If you want to download a binary and treat it like a data-file supporting the Herd application, then Herd should use it exclusively, not other tools.

But still, as a workaround, the PHP binary from Application Support could be symlinked to the correct folder or added to the PATH system-wide to be used by the other tools.

@nunomaduro
Copy link
Author

Well, the server.command option doesn't seem to support paths with "spaces". This seems unrelated to whether Herd is using Application Support or not. That been said, do you mind of allowing "spaces" on that option?

@rustatian
Copy link
Member

spaces are only one part of the problem. I'm thinking of quotes, slashes, etc. Which may also be a problem for the command in the future.

So, instead of:

command: "php psr-worker.php"

Use something like this, w/o any limitation on any symbols in the paths:

command: 
    - "/Users/nunomaduro/Library/'Application Support'/Herd/bin/php82" # Command
    - "arg1"
    - "arg2"
    - "etc"

Or:

command: ["/Users/nunomaduro/Library/'Application Support'/Herd/bin/php82", "arg1", "arg2" ,"etc" ]

So, what do you think?

@rustatian
Copy link
Member

So the future version will be backward compatible with the previous configurations with the string as command and arguments. But it will also support the new syntax.

@nunomaduro
Copy link
Author

nunomaduro commented Aug 4, 2023

Can you give me a bash command that works as "inline" option? In other words, can you adjust the command bellow to something that works with "spaces"?

./rr '-c' '/Users/nunomaduro/Work/Code/laravel/.rr.yaml' '-o' 'version=3' '-o' 'http.address=127.0.0.1:8000' '-o' 'server.command=/Users/nunomaduro/Library/Application Support/Herd/bin/php82 /Users/nunomaduro/Work/Code/laravel/vendor/bin/roadrunner-worker' '-o' 'http.pool.num_workers=0' '-o' 'http.pool.max_jobs=500' '-o' 'rpc.listen=tcp://127.0.0.1:6001' '-o' 'http.pool.supervisor.exec_ttl=30s' '-o' 'http.static.dir=/Users/nunomaduro/Work/Code/laravel/public' '-o' 'http.middleware=static' '-o' 'logs.mode=production' '-o' 'logs.level=debug' '-o' 'logs.output=stdout' '-o' 'logs.encoding=json' 'serve'

@rustatian
Copy link
Member

rustatian commented Aug 4, 2023

I think that in your case, it might be an option to do the following (after we implement this syntax for the command):

... '-o' 'server.command=["/Users/nunomaduro/Library/Application Support/Herd/bin/php82", "/Users/nunomaduro/Work/Code/laravel/vendor/bin/roadrunner-worker"]' ... -o

EDIT: The first argument is a path to the executable, all other elements in the array are arguments to be passed to the executable (as usual). The same as we use in the grpc.proto, http.middleware, etc.

@nunomaduro
Copy link
Author

Thank you - however seems to be failing. So, my understanding is that you will add support for this new syntax correct?

➜ ./rr '-c' '/Users/nunomaduro/Work/Code/laravel/.rr.yaml' '-o' 'version=3' '-o' 'http.address=127.0.0.1:8000' '-o' 'server.command=["/Users/nunomaduro/Library/Application Support/Herd/bin/php82", "/Users/nunomaduro/Work/Code/laravel/vendor/bin/roadrunner-worker"]' '-o' 'http.pool.num_workers=0' '-o' 'http.pool.max_jobs=500' '-o' 'rpc.listen=tcp://127.0.0.1:6001' '-o' 'http.pool.supervisor.exec_ttl=30s' '-o' 'http.static.dir=/Users/nunomaduro/Work/Code/laravel/public' '-o' 'http.middleware=static' '-o' 'logs.mode=production' '-o' 'logs.level=debug' '-o' 'logs.output=stdout' '-o' 'logs.encoding=json' 'serve'

handle_serve_command: Function call error:
        got initial serve error from the Vertex *http.Plugin, stopping execution, error: static_pool_allocate_workers: WorkerAllocate: fork/exec ["/Users/nunomaduro/Library/Application: no such file or directory

@rustatian
Copy link
Member

rustatian commented Aug 4, 2023

Yes, I'm just suggesting a new syntax before implementation 😃 If it works for you, I'll add this issue to the 2023.3.0 milestone and add support for all commands in our configuration.

@nunomaduro
Copy link
Author

Thanks - just ping me here once is ready! And thanks again!

@rustatian
Copy link
Member

@nunomaduro 👋🏻

Fixed, 2023.3 is scheduled for October 6th. I'll be releasing beta versions in early September. If you want to play with the new features now, feel free to use Velox to build your own RR binary. Or you can download nightly builds from RR's master branch directly from GitHub Actions: https://github.com/roadrunner-server/roadrunner/actions/runs/5779656841 (like in this one for example).
I'll push updates to the RR's master early next week.

@nunomaduro
Copy link
Author

nunomaduro commented Aug 8, 2023

Thank you. However, @rustatian, we've downloaded the binary from that action, and we are still facing the same issue:

Running:

./rr '-c' '/Users/nunomaduro/Work/Code/folio-test/.rr.yaml' '-o' 'version=3' '-o' 'http.address=127.0.0.1:8000' '-o' 'server.command=["/Users/nunomaduro/Library/Application Support/Herd/bin/php82", "/Users/nunomaduro/Work/Code/folio-test/vendor/bin/roadrunner-worker"]' '-o' 'http.pool.num_workers=0' '-o' 'http.pool.max_jobs=500' '-o' 'rpc.listen=tcp://127.0.0.1:6001' '-o' 'http.pool.supervisor.exec_ttl=30s' '-o' 'http.static.dir=/Users/nunomaduro/Work/Code/folio-test/public' '-o' 'http.middleware=static' '-o' 'logs.mode=production' '-o' 'logs.level=debug' '-o' 'logs.output=stdout' '-o' 'logs.encoding=json' 'serve'

Output:

handle_serve_command: Function call error:
        serve error from the plugin *http.Plugin stopping execution, error: static_pool_allocate_workers: WorkerAllocate: fork/exec ["/Users/nunomaduro/Library/Application: no such file or directory

@rustatian
Copy link
Member

@nunomaduro

I'll push updates to the RR's master early next week.

^^^^^^

@rustatian
Copy link
Member

@nunomaduro After I push an updated plugins to the master (early next week), only after that you can check the latest GitHub action (for the previous one I said -> for example like this one to show you the way how you can download it) to download the binaries.

@rustatian
Copy link
Member

@rustatian
Copy link
Member

@nunomaduro Have you tried the build? Any problems with it?

@nunomaduro
Copy link
Author

nunomaduro commented Aug 15, 2023

Still not working, here is the output:

./rr '-c' '/Users/nunomaduro/Work/Code/folio-test/.rr.yaml' '-o' 'version=3' '-o' 'http.address=127.0.0.1:8000' '-o' 'server.command=["/Users/nunomaduro/Library/Application Support/Herd/bin/php82", "/Users/nunomaduro/Work/Code/folio-test/vendor/bin/roadrunner-worker"]' '-o' 'http.pool.num_workers=0' '-o' 'http.pool.max_jobs=500' '-o' 'rpc.listen=tcp://127.0.0.1:6001' '-o' 'http.pool.supervisor.exec_ttl=30s' '-o' 'http.static.dir=/Users/nunomaduro/Work/Code/folio-test/public' '-o' 'http.middleware=static' '-o' 'logs.mode=production' '-o' 'logs.level=debug' '-o' 'logs.output=stdout' '-o' 'logs.encoding=json' 'serve'
   ERROR  	serve error from the plugin *http.Plugin stopping execution, error: static_pool_allocate_workers: WorkerAllocate: fork/exec ["/Users/james/Library/Application Support/Herd/bin/php82": no such file or directory

-- With escaped space:

./rr '-c' '/Users/nunomaduro/Work/Code/folio-test/.rr.yaml' '-o' 'version=3' '-o' 'http.address=127.0.0.1:8000' '-o' 'server.command=["/Users/nunomaduro/Library/Application\ Support/Herd/bin/php82", "/Users/nunomaduro/Work/Code/folio-test/vendor/bin/roadrunner-worker"]' '-o' 'http.pool.num_workers=0' '-o' 'http.pool.max_jobs=500' '-o' 'rpc.listen=tcp://127.0.0.1:6001' '-o' 'http.pool.supervisor.exec_ttl=30s' '-o' 'http.static.dir=/Users/nunomaduro/Work/Code/folio-test/public' '-o' 'http.middleware=static' '-o' 'logs.mode=production' '-o' 'logs.level=debug' '-o' 'logs.output=stdout' '-o' 'logs.encoding=json' 'serve'
   ERROR  	serve error from the plugin *http.Plugin stopping execution, error: static_pool_allocate_workers: WorkerAllocate: fork/exec ["/Users/james/Library/Application\ Support/Herd/bin/php82": no such file or directory

@rustatian
Copy link
Member

rustatian commented Aug 15, 2023

@nunomaduro This is yaml parsing limitation unfortinately.

ATM this feature is fully supported via configuration (I double checked).
If you use the -o flag without configuration, it is hard to determine the actual type of the value (like slice with ints/strings or just value or smt else).
As a workaround - you can omit the space here: ...php82", <space here> "/Users/... and try it again (this works for me).
As this is not a bug but a feature request, you can file a feature request ticket and I would be happy to help anyone who wants to implement this functionality in the configuration plugin for the -o flag. (PR's are always welcome 👍🏻 )

@jbrooksuk
Copy link

@rustatian I've tried removing the space from the path from the inline option:

exec '/Users/james/Code/nunomaduro/demo-nuno/rr' '-c' '/Users/james/Code/nunomaduro/demo-nuno/.rr.yaml' '-o' 'version=3' '-o' 'http.address=127.0.0.1:8000' '-o' 'server.command=["/Users/james/Library/Application Support/Herd/bin/php82","/Users/james/Code/nunomaduro/demo-nuno/vendor/bin/roadrunner-worker"]' '-o' 'http.pool.num_workers=0' '-o' 'http.pool.max_jobs=500' '-o' 'rpc.listen=tcp://127.0.0.1:6001' '-o' 'http.pool.supervisor.exec_ttl=30s' '-o' 'http.static.dir=/Users/james/Code/nunomaduro/demo-nuno/public' '-o' 'http.middleware=static' '-o' 'logs.mode=production' '-o' 'logs.level=debug' '-o' 'logs.output=stdout' '-o' 'logs.encoding=json' 'serve'

But still getting an error:

   ERROR  handle_serve_command: Function call error:

   ERROR  	serve error from the plugin *http.Plugin stopping execution, error: static_pool_allocate_workers: WorkerAllocate: fork/exec ["/Users/james/Library/Application Support/Herd/bin/php82": no such file or directory

@rustatian
Copy link
Member

rustatian commented Aug 17, 2023

@jbrooksuk Hey 👋🏻

Sorry to hear that. This feature is supported in configuration (.rr.yaml). If someone creates a feature request to support it via -o flag, I would be happy to help with advice.

At the moment, I don't plan to support it, since folders with spaces is a non-standard behavior. And while this bug is about support in general, the problem here (as I posted above, apple dev docs) is incorrectly placing the binary (herd php binary) in the folder with configuration files (Application Support).

@nunomaduro
Copy link
Author

@rustatian .rr.yaml is not the ideal solution for us. As you can imagine, not everyone in the same team has the PHP binary in the same place. Anyways, @mpociot you think you can place the binary elsewhere without the "space" issue?

@rustatian
Copy link
Member

rustatian commented Aug 17, 2023

@nunomaduro

is not the ideal solution for us

I'm always open to PRs 😃

As you can imagine, not everyone in the same team has the PHP binary in the same place

Most have the binaries in the same place (MacOS, Linux) - /usr/bin - system/user, /usr/local - user. Debian/Ubuntu even have update-alternatives to switch between the same binary versions. As an ArchLinux user, I can say that any PHP binary that is not the latest has its place in the default folder (e.g.: https://aur.archlinux.org/cgit/aur.git/tree/PKGBUILD?h=php74).
If not, there is a special place for them in /usr/local/<any_folder_to_symlink>.

If you want to put binaries in a different folder, like /home/<user>/.config/, you can do that for yourself or your team (minority) and there is nothing bad about that, but not for the majority.
Here is a Filesystem Hierarchy Standard: link. Link to the apple dev docs you may find above.

I'm just trying to explain to you my reasoning, not just - this is not the ideal solution for us.

@rustatian
Copy link
Member

I suggested you a standard solution - do not add herd binaries located in the Application Support to the $PATH. Switch between different versions using symlinks to the /usr/bin/php. In this case, this is fine to use any non-standard folder to store binaries but at the same time, follow the Unix/Linux standards.
You may read about Ubuntu update-alternatives which I guess similar to what's doing Herd: https://devicetests.com/understanding-update-alternatives-ubuntu

@bnzo
Copy link

bnzo commented Aug 29, 2023

It seems to be working using the -o flag when removing the array brackets and quotes from the server.command string.
I have used this rr build: https://github.com/roadrunner-server/roadrunner/actions/runs/5990264085

@jbrooksuk could you give it a try? Your modified command below:

exec '/Users/james/Code/nunomaduro/demo-nuno/rr' '-c' '/Users/james/Code/nunomaduro/demo-nuno/.rr.yaml' '-o' 'version=3' '-o' 'http.address=127.0.0.1:8000' '-o' 'server.command=/Users/james/Library/Application Support/Herd/bin/php82,/Users/james/Code/nunomaduro/demo-nuno/vendor/bin/roadrunner-worker' '-o' 'http.pool.num_workers=0' '-o' 'http.pool.max_jobs=500' '-o' 'rpc.listen=tcp://127.0.0.1:6001' '-o' 'http.pool.supervisor.exec_ttl=30s' '-o' 'http.static.dir=/Users/james/Code/nunomaduro/demo-nuno/public' '-o' 'http.middleware=static' '-o' 'logs.mode=production' '-o' 'logs.level=debug' '-o' 'logs.output=stdout' '-o' 'logs.encoding=json' 'serve'

@nunomaduro
Copy link
Author

@bnzo That works. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
B-bug Bug: bug, exception
Projects
No open projects
Status: Unreleased
Development

Successfully merging a pull request may close this issue.

5 participants