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

Behavior change for undef params in 20180523.0 #124

Open
oschwald opened this issue Jun 7, 2018 · 5 comments
Open

Behavior change for undef params in 20180523.0 #124

oschwald opened this issue Jun 7, 2018 · 5 comments

Comments

@oschwald
Copy link

oschwald commented Jun 7, 2018

Before 20180523.0, something like run([$^X], \undef, \undef, \undef, undef) or even run([$^X], \undef, undef) would run without an exception. As of the change introduced by #118, this fails with Modification of a read-only value attempted at /home/greg/MaxMind/IPC-Run/lib/IPC/Run.pm line 1686.. Changing:

@args = map { !defined $_ ? bless(\$_, 'IPC::Run::Undef') : $_ } @_;

to:

@args = map { my $v = $_; !defined $v ? bless(\$v, 'IPC::Run::Undef') : $v } @_;

fixes that particular error, but it later fails with other errors. This appears to be because the later code uses ref and length in ways incompatible with undef being replaced by IPC::Run::Undef.

@toddr
Copy link
Member

toddr commented Sep 24, 2018

@oschwald is there a reason you can't change that to:

run([$^X])

@oschwald
Copy link
Author

@toddr, we've worked around the particular issue, but it was a backwards incompatible behavior change not explicitly mentioned in the docs.

If I recall correctly, the reason why the code was like that is that we were conditionally passing in those params. Now we have to check whether we've passed them in.

@toddr
Copy link
Member

toddr commented Sep 24, 2018

The whole interface is a big mess. I don't think I would make any of the existing decisions if I had to re-write it.

@mohawk2
Copy link
Collaborator

mohawk2 commented Sep 25, 2018

Should we either make the docs reflect the current code, or adjust the code to match the docs?

@ppisar
Copy link

ppisar commented Feb 5, 2019

run([$^X]) does not close the descriptors. Using \undef to close a descriptor is the most easiest way. But due to this bug, one cannot do that. This really should be documented or fixed.

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

4 participants