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

posix_spawn and fork-exec do not treat non-string environment variables the same #128

Closed
shepmaster opened this issue Oct 21, 2017 · 1 comment

Comments

@shepmaster
Copy link

I was setting the environment of a process to this:

{
      TEST_GITHUB_DISABLED: true,
      DATABASE_URL: "A string",
      API_HTTP_PORT: 8080,
}

This was working well, but then I set ChildProcess.posix_spawn = true as suggested by the README. I then started getting exceptions because val no longer implemented include?.

I don't have much of an opinion which way is correct, but it would be nice for them to be consistent.

sds added a commit that referenced this issue Jan 12, 2019
Issue #128 pointed out that there was inconsistency between how
environment variables were treated on *nix systems by ChildProcess'
fork/exec implementation versus POSIX spawn.

Since the behavior of the more common case of fork/exec is to convert
the values to strings, we'll do the same for POSIX spawn. However, a
better long term solution would likely be to raise an error on
non-string values.
@sds
Copy link
Collaborator

sds commented Jan 12, 2019

It's not clear to me whether ChildProcess should convert values like true or numerical values to an equivalent string. Is true the string true? Or the string 1? Something else?

Given the potential ambiguity, it's best for applications to set environment variables as strings explicitly. Unfortunately, the existing implementation of this gem seems to convert values to string equivalents for the non-posix_spawn case.

Given that the common case is the fork/exec implementation, we'll go with that behavior, and have implemented it in 5c57733. However, long-term we should migrate to raising an error on non-string values.

Thanks for the report!

@sds sds closed this as completed Jan 12, 2019
@sds sds mentioned this issue Jan 25, 2019
netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Mar 24, 2020
Update ruby-childprocessto 3.0.0.


### Version 3.0.0 / 2019-09-20

* [#156](enkessler/childprocess#156 unused `rubyforge_project` from gemspec
* [#160](enkessler/childprocess#160): Remove extension to conditionally install `ffi` gem on Windows platforms
* [#160](enkessler/childprocess#160): Remove runtime dependency on `rake` gem

### Version 2.0.0 / 2019-07-11

* [#148](enkessler/childprocess#148): Drop support for Ruby 2.0, 2.1, and 2.2
* [#149](enkessler/childprocess#149): Fix Unix fork reopen to be compatible with Ruby 2.6
* [#152](enkessler/childprocess#152): Fix hangs and permission errors introduced in Ruby 2.6 for leader processes of process groups

### Version 1.0.1 / 2019-02-03

* [#143](enkessler/childprocess#144): Fix installs by adding `rake` gem as runtime dependency
* [#147](enkessler/childprocess#147): Relax `rake` gem constraint from `< 12` to `< 13`

### Version 1.0.0 / 2019-01-28

* [#134](enkessler/childprocess#134): Add support for non-ASCII characters on Windows
* [#132](enkessler/childprocess#132): Install `ffi` gem requirement on Windows only
* [#128](enkessler/childprocess#128): Convert environment variable values to strings when `posix_spawn` enabled
* [#141](enkessler/childprocess#141): Support JRuby on Java >= 9
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

No branches or pull requests

2 participants