getavalon / core

The safe post-production pipeline - https://getavalon.github.io/2.0
MIT License
218 stars 49 forks source link

creationFlags are not supported on linux/osx #459

Closed antirotor closed 4 years ago

antirotor commented 4 years ago

Avalon is using creationFlags to launch subprocess via its launch() function. CreationFlags are supported only on Windows in Python. It is therefor failing on linux and macos.

What's changed? I've just added condition to use it only on Windows.

BigRoy commented 4 years ago

Looking good. Does it behave as expected in that condition for Linux/Mac? For example, as CREATE_NEW_CONSOLE is defined does it launch in a separate console/process (e.g. detaching itself).

antirotor commented 4 years ago

well no, there is no easy way to do it. If you are running from terminal, there is really no concept of detaching. Running under desktop environment, there you would need to specify what terminal emulator you are using, what shell you are using and then spawn it and run subprocess. This is by my opinion too much hustle to do. Now it doesn't detach iself but I don't think it affect usability very much - I've ran into it when trying to launch Python action from Launcher. Now the Python interpreter will start in the same terminal we used to start avalon. There is one almost universal way - using xterm, but using it is like time travel to Middle Ages.

BigRoy commented 4 years ago

Ah ok, the only caveat then is that the CREATE_NEW_CONSOLE environment variable might be misleading on Mac OS / Linux as it wouldn't actually do so. Not sure if a problem, we just need to make sure anyone using it is aware of it.

Should we maybe mention it in a comment in the code too?

antirotor commented 4 years ago

True, I've added comment there explaining behaviour. We could probably later add such functionality for linux, but it would require lots of code I don't think is worth to invest there. Those flags were until now show-stopper on linux so this little change will stop crashes there albeit without full functionality as on Windows.

mottosso commented 4 years ago

Wait, why would you be setting CREATE_NEW_CONSOLE on Linux/Mac? That's also Windows-specific, so I don't see how this changes anything?

antirotor commented 4 years ago

This is not problem with CREATE_NEW_CONSOLE but with setting creationgflags at all, they are not recognized at all on Linux and lead to crash and traceback stating so.

antirotor commented 4 years ago

To be more precise I found out Launcher action starting Python is setting CREATE_NEW_CONSOLE regardless of platform. This is obviously not good, but its better to fix it directly in core to make it more robust - not to allow set creationflags on other platforms then Windows.

mottosso commented 4 years ago

Looks good, thanks!

davidlatwe commented 4 years ago

Add another note for future referencing :) https://stackoverflow.com/a/25124003/4145300