Skip to content
This repository has been archived by the owner on Sep 20, 2024. It is now read-only.

General: Custom function for find executable #2822

Merged
merged 7 commits into from
Mar 8, 2022

Conversation

iLLiCiTiT
Copy link
Member

Brief description

We're using find_executable from distutils but it seems that they'll get deprecated soon and may not work as expected also some older hosts have issues with using distutils too.

Description

Added custom find_executable which should return full path to an executable file.

Changes

  • implemented find_executable in openpype/lib
  • use the function in places where it is needed
  • maketx in extract look does not add ".exe"

Testing notes:

  • try run applications
  • oiio and ffmpeg functions returning path should work and return full path to file

@iLLiCiTiT iLLiCiTiT self-assigned this Mar 1, 2022
@iLLiCiTiT iLLiCiTiT requested review from BigRoy, antirotor, 64qam and kalisp and removed request for 64qam March 1, 2022 11:20
Comment on lines 10 to 64
def find_executable(executable):
"""Find full path to executable.

Also tries additional extensions if passed executable does not contain one.

Paths where it is looked for executable is defined by 'PATH' environment
variable, 'os.confstr("CS_PATH")' or 'os.defpath'.

Args:
executable(str): Name of executable with or without extension. Can be
path to file.

Returns:
str: Full path to executable with extension (is file).
None: When the executable was not found.
"""
if os.path.isfile(executable):
return executable

low_platform = platform.system().lower()
_, ext = os.path.splitext(executable)
variants = [executable]
if not ext:
if low_platform == "windows":
exts = [".exe", ".ps1", ".bat"]
for ext in os.getenv("PATHEXT", "").split(os.pathsep):
ext = ext.lower()
if ext and ext not in exts:
exts.append(ext)
else:
exts = [".sh"]

for ext in exts:
variant = executable + ext
if os.path.isfile(variant):
return variant
variants.append(variant)

path_str = os.environ.get("PATH", None)
if path_str is None:
if hasattr(os, "confstr"):
path_str = os.confstr("CS_PATH")
elif hasattr(os, "defpath"):
path_str = os.defpath

if not path_str:
return None

paths = path_str.split(os.pathsep)
for path in paths:
for variant in variants:
filepath = os.path.abspath(os.path.join(path, executable))
if os.path.isfile(filepath):
return filepath
return None
Copy link
Collaborator

Choose a reason for hiding this comment

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

Out of curiosity. What is this logic based on? It looks like distutils.find_executable but does not look like recent code?

Copy link
Member Author

Choose a reason for hiding this comment

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

Source was distutils, but slightly modified. In distutils find_executable always expect ".exe" on windows, this adds other options from PATHEXT and .sh for linux and mac.

Copy link
Collaborator

Choose a reason for hiding this comment

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

For reference sake this is what which does in Avalon core which looks somewhat similar

Copy link
Member Author

Choose a reason for hiding this comment

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

Handle less cases. When passed executable is existing path it should just return it. If the passed executable is path but don't have extension then this function will just add extension and return it. Then will start to look into PATH for possible variants.
Adding the os.access(filepath, os.X_OK) seems like a good idea.

@antirotor
Copy link
Member

antirotor commented Mar 4, 2022

Just warning about os.access(filepath, os.X_OK) as it might fail on some storage configurations - we've seen this already.

And there is unfortunately one common scenario (and I am not saying we should handle it - just be aware of it) - with mixed windows/linux environments sometimes they have share permission set so +x is set on all files as executable permission is essentially missing on windows.

Copy link
Member

@kalisp kalisp left a comment

Choose a reason for hiding this comment

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

Tested in Windows on AE and Nuke, locally and on Deadline, works fine.

Should be probably tested on different OSes.

@iLLiCiTiT iLLiCiTiT requested a review from 64qam March 4, 2022 14:32
@iLLiCiTiT
Copy link
Member Author

Should be probably tested on different OSes.

@64qam could you find a time to test this on linux?

@mkolar mkolar added the type: enhancement Enhancements to existing functionality label Mar 4, 2022
@iLLiCiTiT iLLiCiTiT merged commit 5bfbe4e into develop Mar 8, 2022
@iLLiCiTiT iLLiCiTiT deleted the enhancement/custom_find_executable branch March 8, 2022 09:44
@mkolar mkolar added this to the 3.9.0 milestone Mar 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: enhancement Enhancements to existing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants