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

fire raises "preexec_fn is not supported on Windows platforms" when using setuptools in windows #236

Closed
acerv opened this issue Mar 25, 2020 · 8 comments

Comments

@acerv
Copy link

acerv commented Mar 25, 2020

Hello,

I'm using fire 0.3.0 and I had an issue running fire script in Window 10. Code is the following:
Inside auttools/ethrelay.py module I defined:

    .... ....

def main():
    fire.Fire(EthRelayTool)

Inside setup.py:

entry_points={
    'console_scripts': 'ethrelay=auttools.ethrelay:main',
},

Error:

Traceback (most recent call last):
  File "C:\userdata\workspaces\app_automation_tools\tools\.venv\Scripts\ethrelay-script.py", line 11, in <module>
	load_entry_point('auttools', 'console_scripts', 'ethrelay')()
  File "c:\userdata\workspaces\app_automation_tools\tools\auttools\ethrelay.py", line 80, in main
	fire.Fire(EthRelayTool)
  File "c:\userdata\workspaces\app_automation_tools\tools\.venv\lib\site-packages\fire\core.py", line 160, in Fire
	Display(output, out=sys.stderr)
  File "c:\userdata\workspaces\app_automation_tools\tools\.venv\lib\site-packages\fire\core.py", line 171, in Display
	console_io.More(text, out=out)
  File "c:\userdata\workspaces\app_automation_tools\tools\.venv\lib\site-packages\fire\console\console_io.py", line 111, in More
	pager, stdin=subprocess.PIPE, shell=True, preexec_fn=PreexecFunc)
  File "c:\python\3.7.5\Lib\subprocess.py", line 706, in __init__
	raise ValueError("preexec_fn is not supported on Windows "
ValueError: preexec_fn is not supported on Windows platforms

Best regards,
Andrea

@dbieber
Copy link
Member

dbieber commented Mar 25, 2020

Thanks for reporting the bug.
I don't have a windows machine available to test this unfortunately.
Any help resolving would be appreciated.

@qckzr
Copy link

qckzr commented Mar 26, 2020

Hey there!,

So subprocess.py has the following statement:

if _mswindows:
  if preexec_fn is not None:
    raise ValueError("preexec_fn is not supported on Windows  platforms")

And console_io.py is calling the function PreexecFunc

def PreexecFunc():
  signal.signal(signal.SIGINT, signal.SIG_IGN)

Maybe adding return None when working in windows could help?, I'm not an expert in subprocess rutines ;-)

Also checking documentation https://docs.python.org/3/library/signal.html#signal.signal:

On Windows, signal() can only be called with SIGABRT, SIGFPE, SIGILL, SIGINT, SIGSEGV, SIGTERM, or SIGBREAK. A ValueError will be raised in any other case. Note that not all systems define the same set of signal names; an AttributeError will be raised if a signal name is not defined as SIG* module level constant.

@dbieber
Copy link
Member

dbieber commented Mar 26, 2020

Sounds like this is the change that introduced the issue: 074f4b2
It was introduced in order to resolve #205.

@acerv
Copy link
Author

acerv commented Mar 27, 2020

One way to handle preexec_fn in Windows is by doing:

preexec_fn = None

if platform.system() == "Linux":
    preexec_fn = os.setsid

p = subprocess.Popen(
    pager, stdin=subprocess.PIPE, shell=True, preexec_fn=PreexecFunc)

But If software is not tested in Windows I'm aware there might be some other issues.

@dbieber
Copy link
Member

dbieber commented Apr 3, 2020

I got someone to try a couple things on a Windows machine, but they weren't able to reproduce the error. What version of windows? What version of Python? (Edit: I see it's 3.7.5) And can you confirm that python example.py -- --help reproduces the error for you for even a simple CLI?

@dbieber
Copy link
Member

dbieber commented Apr 3, 2020

Turns out we don't need preexec_fn, even on non-Windows systems. The fix is in af9d848 and we'll do a minor release early next week.

Summary of the issue, now that we understand it:

  1. preexec_fn doesn't work on Windows. Then why did so few people hit this bug? Our code only tries to use preexec_fn if you have less installed on your machine, and iiuc most Windows machines use more, but not less.
  2. preexec_fn isn't needed anywhere. We only need to stop the handling of SIGINT in the parent processs; the child process (less or another pager) can deal with SIGINT however it pleases. To avoid the bug Control-C on command instructions loses cursor. #205 we simply need to stop the parent from dying while the child is running.

@dbieber dbieber closed this as completed Apr 3, 2020
@acerv
Copy link
Author

acerv commented Apr 3, 2020

Thanks
Andrea

@dbieber
Copy link
Member

dbieber commented Apr 7, 2020

The release v0.3.1 is live now. pip install -U fire to upgrade.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants