Skip to content

Use IO.popen to start a process - #295

Open
ekohl wants to merge 1 commit into
OpenVoxProject:mainfrom
ekohl:avoid-open-to-exec
Open

Use IO.popen to start a process#295
ekohl wants to merge 1 commit into
OpenVoxProject:mainfrom
ekohl:avoid-open-to-exec

Conversation

@ekohl

@ekohl ekohl commented Jan 6, 2026

Copy link
Copy Markdown
Contributor

RuboCop rightfully complains about using open() to spawn a process. This uses the way simpler IO.popen() method.

I have no idea if this is going to break or not and whether it's an API breakage.

@ekohl ekohl mentioned this pull request Jan 6, 2026
Comment thread lib/puppet/util/execution.rb Outdated
@ekohl
ekohl force-pushed the avoid-open-to-exec branch 3 times, most recently from 6f1ca15 to 7d9a6e0 Compare January 6, 2026 22:12
@ekohl

ekohl commented Jan 6, 2026

Copy link
Copy Markdown
Contributor Author

I know this is incomplete and some tests still fail, but it's a start.

@ekohl
ekohl force-pushed the avoid-open-to-exec branch from 7d9a6e0 to fd54e75 Compare January 6, 2026 22:37
@ekohl
ekohl marked this pull request as ready for review January 6, 2026 22:41
@bastelfreak

Copy link
Copy Markdown
Contributor

I pushed your branch to the OpenVoxProject repo, so I can kick off the build action: https://github.com/OpenVoxProject/openvox/actions/runs/21248923654

raise Puppet::ExecutionFailure, output if failonfail
end

if failonfail && exitstatus != 0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Note to self: I don't know if exitstatus is properly set here. https://rubyapi.org/3.4/o/io#method-c-popen doesn't mention setting $CHILD_STATUS, only $?.

@bastelfreak bastelfreak added the enhancement New feature or request label Feb 10, 2026
RuboCop rightfully complains about using open() to spawn a process. This
uses the way simpler IO.popen() method.
@ekohl
ekohl force-pushed the avoid-open-to-exec branch from fd54e75 to f16e28f Compare August 1, 2026 15:44
@ekohl

ekohl commented Aug 1, 2026

Copy link
Copy Markdown
Contributor Author

Rebased to resolve conflicts. With OpenVox 9 coming, perhaps a good time to merge this?

@bastelfreak bastelfreak added backwards-incompatible This change will lead to a major version bump for the next release and removed enhancement New feature or request labels Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backwards-incompatible This change will lead to a major version bump for the next release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants