Skip to content
This repository was archived by the owner on Apr 8, 2025. It is now read-only.

Sourcery refactored master branch #1

Open
sourcery-ai wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Sourcery refactored master branch #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 63 additions & 86 deletions build_scripts/build_usd.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,8 @@ def GetXcodeDeveloperDirectory():
if not MacOS():
return None

try:
with contextlib.suppress(subprocess.CalledProcessError):
return subprocess.check_output("xcode-select -p").strip()
except subprocess.CalledProcessError:
pass
Comment on lines -79 to -82
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function GetXcodeDeveloperDirectory refactored with the following changes:

return None

def GetVisualStudioCompilerAndVersion():
Expand All @@ -89,12 +87,11 @@ def GetVisualStudioCompilerAndVersion():
if not Windows():
return None

msvcCompiler = find_executable('cl')
if msvcCompiler:
match = re.search(
"Compiler Version (\d+).(\d+).(\d+)",
subprocess.check_output("cl", stderr=subprocess.STDOUT))
if match:
if msvcCompiler := find_executable('cl'):
if match := re.search(
"Compiler Version (\d+).(\d+).(\d+)",
subprocess.check_output("cl", stderr=subprocess.STDOUT),
):
Comment on lines -92 to +94
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function GetVisualStudioCompilerAndVersion refactored with the following changes:

return (msvcCompiler, tuple(int(v) for v in match.groups()))
return None

Expand Down Expand Up @@ -192,8 +189,7 @@ def RunCMake(context, force, extraArgs = None):
# On Windows, we need to explicitly specify the generator to ensure we're
# building a 64-bit project. (Surely there is a better way to do this?)
if generator is None and Windows():
msvcCompilerAndVersion = GetVisualStudioCompilerAndVersion()
if msvcCompilerAndVersion:
if msvcCompilerAndVersion := GetVisualStudioCompilerAndVersion():
Comment on lines -195 to +192
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function RunCMake refactored with the following changes:

This removes the following comments ( why? ):

# On MacOS, enable the use of @rpath for relocatable builds.

_, version = msvcCompilerAndVersion
if version >= MSVC_2017_COMPILER_VERSION:
generator = "Visual Studio 15 2017 Win64"
Expand All @@ -202,12 +198,8 @@ def RunCMake(context, force, extraArgs = None):

if generator is not None:
generator = '-G "{gen}"'.format(gen=generator)

# On MacOS, enable the use of @rpath for relocatable builds.
osx_rpath = None
if MacOS():
osx_rpath = "-DCMAKE_MACOSX_RPATH=ON"

osx_rpath = "-DCMAKE_MACOSX_RPATH=ON" if MacOS() else None
with CurrentWorkingDirectory(buildDir):
Run('cmake '
'-DCMAKE_INSTALL_PREFIX="{instDir}" '
Expand All @@ -234,9 +226,12 @@ def PatchFile(filename, patches):
for (oldLine, newLine) in patches:
newLines = [s.replace(oldLine, newLine) for s in newLines]
if newLines != oldLines:
PrintInfo("Patching file {filename} (original in {oldFilename})..."
.format(filename=filename, oldFilename=filename + ".old"))
shutil.copy(filename, filename + ".old")
PrintInfo(
"Patching file {filename} (original in {oldFilename})...".format(
filename=filename, oldFilename=f"{filename}.old"
)
)
shutil.copy(filename, f"{filename}.old")
Comment on lines -237 to +234
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function PatchFile refactored with the following changes:

open(filename, 'w').writelines(newLines)

def DownloadURL(url, context, force, dontExtract = None):
Expand All @@ -250,7 +245,7 @@ def DownloadURL(url, context, force, dontExtract = None):
been extracted."""
with CurrentWorkingDirectory(context.srcDir):
# Extract filename from URL and see if file already exists.
filename = url.split("/")[-1]
filename = url.split("/")[-1]
Comment on lines -253 to +248
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function DownloadURL refactored with the following changes:

if force and os.path.exists(filename):
os.remove(filename)

Expand All @@ -270,7 +265,7 @@ def DownloadURL(url, context, force, dontExtract = None):
# Download to a temporary file and rename it to the expected
# filename when complete. This ensures that incomplete downloads
# will be retried if the script is run again.
tmpFilename = filename + ".tmp"
tmpFilename = f"{filename}.tmp"
if os.path.exists(tmpFilename):
os.remove(tmpFilename)

Expand Down Expand Up @@ -364,7 +359,7 @@ def DownloadURL(url, context, force, dontExtract = None):
# If extraction failed for whatever reason, assume the
# archive file was bad and move it aside so that re-running
# the script will try downloading and extracting again.
shutil.move(filename, filename + ".bad")
shutil.move(filename, f"{filename}.bad")
raise RuntimeError("Failed to extract archive {filename}: {err}"
.format(filename=filename, err=e))

Expand All @@ -378,8 +373,10 @@ def __init__(self, name, installer, *files):
self.filesToCheck = files

def Exists(self, context):
return all([os.path.isfile(os.path.join(context.instDir, f))
for f in self.filesToCheck])
return all(
os.path.isfile(os.path.join(context.instDir, f))
for f in self.filesToCheck
)
Comment on lines -381 to +379
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function Dependency.Exists refactored with the following changes:


class PythonDependency(object):
def __init__(self, name, getInstructions, moduleNames):
Expand All @@ -390,19 +387,17 @@ def __init__(self, name, getInstructions, moduleNames):
def Exists(self, context):
# If one of the modules in our list exists we are good
for moduleName in self.moduleNames:
try:
with contextlib.suppress(subprocess.CalledProcessError):
# Eat all output; we just care if the import succeeded or not.
subprocess.check_output(shlex.split(
'python -c "import {module}"'.format(module=moduleName)),
stderr=subprocess.STDOUT)
return True
except subprocess.CalledProcessError:
pass
Comment on lines -393 to -400
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function PythonDependency.Exists refactored with the following changes:

return False


def AnyPythonDependencies(deps):
return any([type(d) is PythonDependency for d in deps])
return any(type(d) is PythonDependency for d in deps)
Comment on lines -405 to +400
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function AnyPythonDependencies refactored with the following changes:


############################################################
# zlib
Expand Down Expand Up @@ -433,7 +428,7 @@ def InstallBoost(context, force):
dontExtract = ["*/doc/*", "*/libs/*/doc/*"]

with CurrentWorkingDirectory(DownloadURL(BOOST_URL, context, force,
dontExtract)):
dontExtract)):
Comment on lines -436 to +431
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function InstallBoost refactored with the following changes:

This removes the following comments ( why? ):

# support. We'll get a lot of messages about an unknown compiler
# Boost 1.61 doesn't support Visual Studio 2017.  If that's what
# version but it will build.
# we're using then patch the project-config.jam file to hack in

bootstrap = "bootstrap.bat" if Windows() else "./bootstrap.sh"
Run('{bootstrap} --prefix="{instDir}"'
.format(bootstrap=bootstrap, instDir=context.instDir))
Expand Down Expand Up @@ -467,13 +462,8 @@ def InstallBoost(context, force):

if Windows():
b2_settings.append("toolset=msvc-14.0")

# Boost 1.61 doesn't support Visual Studio 2017. If that's what
# we're using then patch the project-config.jam file to hack in
# support. We'll get a lot of messages about an unknown compiler
# version but it will build.
msvcCompilerAndVersion = GetVisualStudioCompilerAndVersion()
if msvcCompilerAndVersion:

if msvcCompilerAndVersion := GetVisualStudioCompilerAndVersion():
compiler, version = msvcCompilerAndVersion
if version >= MSVC_2017_COMPILER_VERSION:
PatchFile('project-config.jam',
Expand Down Expand Up @@ -696,20 +686,13 @@ def InstallPtex_LinuxOrMacOS(context, force):

def InstallOpenImageIO(context, force):
with CurrentWorkingDirectory(DownloadURL(OIIO_URL, context, force)):
extraArgs = ['-DOIIO_BUILD_TOOLS=OFF',
'-DOIIO_BUILD_TESTS=OFF',
'-DUSE_PYTHON=OFF',
'-DSTOP_ON_WARNING=OFF']

# OIIO's FindOpenEXR module circumvents CMake's normal library
# search order, which causes versions of OpenEXR installed in
# /usr/local or other hard-coded locations in the module to
# take precedence over the version we've built, which would
# normally be picked up when we specify CMAKE_PREFIX_PATH.
# This may lead to undefined symbol errors at build or runtime.
# So, we explicitly specify the OpenEXR we want to use here.
extraArgs.append('-DOPENEXR_HOME="{instDir}"'
.format(instDir=context.instDir))
extraArgs = [
'-DOIIO_BUILD_TOOLS=OFF',
'-DOIIO_BUILD_TESTS=OFF',
'-DUSE_PYTHON=OFF',
'-DSTOP_ON_WARNING=OFF',
'-DOPENEXR_HOME="{instDir}"'.format(instDir=context.instDir),
]
Comment on lines -699 to +695
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function InstallOpenImageIO refactored with the following changes:

This removes the following comments ( why? ):

# /usr/local or other hard-coded locations in the module to
# normally be picked up when we specify CMAKE_PREFIX_PATH.
# search order, which causes versions of OpenEXR installed in
# This may lead to undefined symbol errors at build or runtime.
# So, we explicitly specify the OpenEXR we want to use here.
# OIIO's FindOpenEXR module circumvents CMake's normal library
# take precedence over the version we've built, which would


# If Ptex support is disabled in USD, disable support in OpenImageIO
# as well. This ensures OIIO doesn't accidentally pick up a Ptex
Expand Down Expand Up @@ -739,13 +722,9 @@ def InstallOpenSubdiv(context, force):
'-DNO_OPENCL=ON',
'-DNO_DX=ON',
'-DNO_TESTS=ON',
'-DGLEW_LOCATION="{instDir}"'.format(instDir=context.instDir),
]

# OpenSubdiv's FindGLEW module won't look in CMAKE_PREFIX_PATH, so
# we need to explicitly specify GLEW_LOCATION here.
extraArgs.append('-DGLEW_LOCATION="{instDir}"'
.format(instDir=context.instDir))
Comment on lines +725 to -747
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function InstallOpenSubdiv refactored with the following changes:

This removes the following comments ( why? ):

# we need to explicitly specify GLEW_LOCATION here.
# OpenSubdiv's FindGLEW module won't look in CMAKE_PREFIX_PATH, so


# If Ptex support is disabled in USD, disable support in OpenSubdiv
# as well. This ensures OSD doesn't accidentally pick up a Ptex
# library outside of our build.
Expand Down Expand Up @@ -862,17 +841,17 @@ def InstallUSD(context):
extraArgs.append('-DBUILD_SHARED_LIBS=ON')
elif context.buildMonolithic:
extraArgs.append('-DPXR_BUILD_MONOLITHIC=ON')

if context.buildDocs:
extraArgs.append('-DPXR_BUILD_DOCUMENTATION=ON')
else:
extraArgs.append('-DPXR_BUILD_DOCUMENTATION=OFF')

if context.buildTests:
extraArgs.append('-DPXR_BUILD_TESTS=ON')
else:
extraArgs.append('-DPXR_BUILD_TESTS=OFF')

Comment on lines -865 to +854
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function InstallUSD refactored with the following changes:

This removes the following comments ( why? ):

# to find the HDF5 we've built, so provide an extra hint.
# CMAKE_PREFIX_PATH isn't sufficient for the FindHDF5 module

if context.buildImaging:
extraArgs.append('-DPXR_BUILD_IMAGING=ON')
if context.enablePtex:
Expand All @@ -898,12 +877,14 @@ def InstallUSD(context):
if context.buildAlembic:
extraArgs.append('-DPXR_BUILD_ALEMBIC_PLUGIN=ON')
if context.enableHDF5:
extraArgs.append('-DPXR_ENABLE_HDF5_SUPPORT=ON')

# CMAKE_PREFIX_PATH isn't sufficient for the FindHDF5 module
# to find the HDF5 we've built, so provide an extra hint.
extraArgs.append('-DHDF5_ROOT="{instDir}"'
.format(instDir=context.instDir))
extraArgs.extend(
(
'-DPXR_ENABLE_HDF5_SUPPORT=ON',
'-DHDF5_ROOT="{instDir}"'.format(
instDir=context.instDir
),
)
)
else:
extraArgs.append('-DPXR_ENABLE_HDF5_SUPPORT=OFF')
else:
Expand Down Expand Up @@ -1114,7 +1095,7 @@ def __init__(self, args):
# Directory where dependencies will be downloaded and extracted
self.srcDir = (os.path.abspath(args.src) if args.src
else os.path.join(self.usdInstDir, "src"))

Comment on lines -1117 to +1098
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function InstallContext.__init__ refactored with the following changes:

# Directory where USD and dependencies will be built
self.buildDir = (os.path.abspath(args.build) if args.build
else os.path.join(self.usdInstDir, "build"))
Expand Down Expand Up @@ -1144,8 +1125,7 @@ def __init__(self, args):
self.buildPython = args.build_python

# - Imaging
self.buildImaging = (args.build_imaging == IMAGING or
args.build_imaging == USD_IMAGING)
self.buildImaging = args.build_imaging in [IMAGING, USD_IMAGING]
self.enablePtex = self.buildImaging and args.enable_ptex

# - USD Imaging
Expand Down Expand Up @@ -1194,9 +1174,12 @@ def ForceBuildDependency(self, dep):
extraPaths = []
extraPythonPaths = []
if Windows():
extraPaths.append(os.path.join(context.instDir, "lib"))
extraPaths.append(os.path.join(context.instDir, "bin"))

extraPaths.extend(
(
os.path.join(context.instDir, "lib"),
os.path.join(context.instDir, "bin"),
)
)
Comment on lines -1197 to +1182
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Lines 1197-1443 refactored with the following changes:

This removes the following comments ( why? ):

# Scan for any dependencies that the user is required to install themselves
# and print those instructions first.

if extraPaths:
paths = os.environ.get('PATH', '').split(os.pathsep) + extraPaths
os.environ['PATH'] = os.pathsep.join(paths)
Expand All @@ -1220,7 +1203,7 @@ def ForceBuildDependency(self, dep):

requiredDependencies += [JPEG, TIFF, PNG, OPENEXR, GLEW,
OPENIMAGEIO, OPENSUBDIV]

if context.buildUsdImaging and context.buildPython:
requiredDependencies += [PYOPENGL, PYSIDE]

Expand Down Expand Up @@ -1286,7 +1269,7 @@ def ForceBuildDependency(self, dep):
if not find_executable("doxygen"):
PrintError("doxygen not found -- please install it and adjust your PATH")
sys.exit(1)

if not find_executable("dot"):
PrintError("dot not found -- please install graphviz and adjust your "
"PATH")
Expand All @@ -1297,9 +1280,9 @@ def ForceBuildDependency(self, dep):
# not found, so check for it here to avoid confusing users. This list of
# PySide executable names comes from cmake/modules/FindPySide.cmake
pysideUic = ["pyside-uic", "python2-pyside-uic", "pyside-uic-2.7"]
found_pysideUic = any([find_executable(p) for p in pysideUic])
found_pysideUic = any(find_executable(p) for p in pysideUic)
pyside2Uic = ["pyside2-uic", "python2-pyside2-uic", "pyside2-uic-2.7"]
found_pyside2Uic = any([find_executable(p) for p in pyside2Uic])
found_pyside2Uic = any(find_executable(p) for p in pyside2Uic)
if not found_pysideUic and not found_pyside2Uic:
PrintError("pyside-uic not found -- please install PySide and adjust "
"your PATH. (Note that this program may be named {0} "
Expand Down Expand Up @@ -1367,11 +1350,9 @@ def ForceBuildDependency(self, dep):
if args.dry_run:
sys.exit(0)

# Scan for any dependencies that the user is required to install themselves
# and print those instructions first.
pythonDependencies = \
[dep for dep in dependenciesToBuild if type(dep) is PythonDependency]
if pythonDependencies:
if pythonDependencies := [
dep for dep in dependenciesToBuild if type(dep) is PythonDependency
]:
for dep in pythonDependencies:
Print(dep.getInstructions())
sys.exit(1)
Expand Down Expand Up @@ -1406,14 +1387,10 @@ def ForceBuildDependency(self, dep):
sys.exit(1)

# Done. Print out a final status message.
requiredInPythonPath = set([
os.path.join(context.usdInstDir, "lib", "python")
])
requiredInPythonPath = {os.path.join(context.usdInstDir, "lib", "python")}
requiredInPythonPath.update(extraPythonPaths)

requiredInPath = set([
os.path.join(context.usdInstDir, "bin")
])
requiredInPath = {os.path.join(context.usdInstDir, "bin")}
requiredInPath.update(extraPaths)

if Windows():
Expand All @@ -1440,7 +1417,7 @@ def ForceBuildDependency(self, dep):
if context.buildMaya:
Print("See documentation at http://openusd.org/docs/Maya-USD-Plugins.html "
"for setting up the Maya plugin.\n")

if context.buildKatana:
Print("See documentation at http://openusd.org/docs/Katana-USD-Plugins.html "
"for setting up the Katana plugin.\n")
Expand Down
6 changes: 3 additions & 3 deletions cmake/macros/generateDocs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,11 +121,11 @@ def _checkPath(path, ident, perm):
elif not os.access(path, perm):
permString = '-permission'
if perm == os.W_OK:
permString = 'write'+permString
permString = f'write{permString}'
elif perm == os.R_OK:
permString = 'read'+permString
permString = f'read{permString}'
elif perm == os.X_OK:
permString = 'execute'+permString
permString = f'execute{permString}'
Comment on lines -124 to +128
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function _checkPath refactored with the following changes:

sys.exit('Error: insufficient permission for path %s, '
'%s required.' % (path, permString))

Expand Down
4 changes: 1 addition & 3 deletions cmake/macros/testWrapper.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,9 +176,7 @@ def _splitCmd(cmd):
# want the exit code 134 as that is what the script would return when run
# from the shell. This is well defined to be 128 + (signal number).
def _convertRetCode(retcode):
if retcode < 0:
return 128 + abs(retcode)
return retcode
return 128 + abs(retcode) if retcode < 0 else retcode
Copy link
Copy Markdown
Author

@sourcery-ai sourcery-ai bot Dec 4, 2023

Choose a reason for hiding this comment

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

Function _convertRetCode refactored with the following changes:


def _getRedirects(out_redir, err_redir):
return (open(out_redir, 'w') if out_redir else None,
Expand Down
Loading