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

Fix puma_bind unix socket path #70

Merged
merged 1 commit into from
Oct 5, 2014
Merged

Fix puma_bind unix socket path #70

merged 1 commit into from
Oct 5, 2014

Conversation

hnatt
Copy link
Contributor

@hnatt hnatt commented Oct 5, 2014

I don't know what was the decision behind setting all paths with File.join instead of an interpolated string, and if it is worth rolling back (so I didn't), but File.join seems to mess up the unix socket URL:

> File.join('unix://', '/home/deploy/app', 'tmp', 'sockets', 'puma.socket') # => "unix:/home/deploy/app/tmp/sockets/puma.socket"
> File.join('unix://home/deploy/app', 'tmp', 'sockets', 'puma.socket') # => "unix://home/deploy/app/tmp/sockets/puma.socket"

@seuros
Copy link
Owner

seuros commented Oct 5, 2014

Nice catch. Thanks you.

seuros added a commit that referenced this pull request Oct 5, 2014
Fix puma_bind unix socket path
@seuros seuros merged commit 50490bc into seuros:master Oct 5, 2014
@hnatt
Copy link
Contributor Author

hnatt commented Oct 5, 2014

No problem! Should all path assignments stay lambdas with File.join? I think it's nicer and simpler when they are just strings, like in "Configurable options" in README. I doubt somebody will ever deploy Puma to a Windows or OS/2 host, where path separators are different. If they do, it'll be their least problem.

@hnatt hnatt deleted the fix-unix-sockets branch October 5, 2014 09:54
@seuros
Copy link
Owner

seuros commented Oct 5, 2014

No, we can change this. The is no reason to use join.

@hnatt
Copy link
Contributor Author

hnatt commented Oct 5, 2014

But why was File.join used in the first place? Won't it break someones workflow if we throw away the lambdas?

@seuros
Copy link
Owner

seuros commented Oct 5, 2014

No specific reason, probably habit.
shared_path is undefined without the lambda. It used to delay it resolution.

@hnatt
Copy link
Contributor Author

hnatt commented Oct 5, 2014

You are right about lamdas.

Probably I'll leave it as is. It seems to work and I feel uneasy refactoring something not knowing if it breaks.

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.

2 participants