Skip to content

Updated build.py to support relative CMake/CTest paths. #205

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.

Sign up for GitHub

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

Merged
edgchen1 merged 1 commit into from
Dec 18, 2018
Merged

Updated build.py to support relative CMake/CTest paths. #205

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
65 changes: 37 additions & 28 deletions tools/ci_build/build.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,11 @@
test_data_url = 'https://onnxruntimetestdata.blob.core.windows.net/models/20181210.zip'
test_data_checksum = 'a966def7447f4ff04f5665bca235b3f3'

class BuildError(Exception):
"""Error from running build steps."""
def __init__(self, *messages):
super().__init__("\n".join(messages))

def parse_arguments():
parser = argparse.ArgumentParser(description="ONNXRuntime CI build driver.",
usage='''
Expand Down Expand Up @@ -84,7 +89,7 @@ def parse_arguments():
parser.add_argument("--ctest_path", default="ctest", help="Path to the CTest program.")
parser.add_argument("--skip_submodule_sync", action='store_true', help="Don't do a 'git submodule update'. Makes the Update phase faster.")

parser.add_argument("--use_jemalloc", action='store_true', help="use jemalloc.")
parser.add_argument("--use_jemalloc", action='store_true', help="Use jemalloc.")
parser.add_argument("--use_openblas", action='store_true', help="Build with OpenBLAS.")
parser.add_argument("--use_mkldnn", action='store_true', help="Build with MKLDNN.")
parser.add_argument("--use_mklml", action='store_true', help="Build with MKLML.")
Expand All @@ -95,14 +100,21 @@ def parse_arguments():
parser.add_argument("--use_llvm", action="store_true", help="Build tvm with llvm")
parser.add_argument("--enable_msinternal", action="store_true", help="Enable for Microsoft internal builds only.")
parser.add_argument("--llvm_path", help="Path to llvm dir")
parser.add_argument("--azure_sas_key", help="azure storage sas key, starts with '?'")
parser.add_argument("--azure_sas_key", help="Azure storage sas key, starts with '?'")
parser.add_argument("--use_brainslice", action="store_true", help="Build with brain slice")
parser.add_argument("--brain_slice_package_path", help="Path to brain slice pacakges")
parser.add_argument("--brain_slice_package_name", help="Name of brain slice pakcages")
parser.add_argument("--brain_slice_package_path", help="Path to brain slice packages")
parser.add_argument("--brain_slice_package_name", help="Name of brain slice packages")
parser.add_argument("--brain_slice_client_package_name", help="Name of brainslice client package")
parser.add_argument("--use_nuphar", action='store_true', help="Build with nuphar")
return parser.parse_args()

def resolve_executable_path(command_or_path):
"""Returns the absolute path of an executable."""
executable_path = shutil.which(command_or_path)
Copy link
Contributor

@snnn snnn Dec 18, 2018

Choose a reason for hiding this comment

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

Why adding this function? What if the input is already an absolute path?

Copy link
Contributor Author

@edgchen1 edgchen1 Dec 18, 2018

Choose a reason for hiding this comment

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

If it's already an absolute path it should be fine. If not, it may not work since the current directory gets changed for some commands. I installed a later cmake version on Linux and gave a relative path from the repo and that didn't work.

Copy link
Contributor

@snnn snnn Dec 18, 2018

Choose a reason for hiding this comment

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

Then why don't you give an absolute path to this script? People often have multiple cmake installed, calling which cmake at here may get unexpected result?

Copy link
Contributor Author

@edgchen1 edgchen1 Dec 18, 2018

Choose a reason for hiding this comment

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

Giving an absolute path works (which is what I had to do). It seemed like a bug that a relative path does not also work.

For --cmake_path, the default is "cmake". So for the default, I assumed running a cmake command starting with the result of shutil.which("cmake") would be equivalent.

if executable_path is None:
raise BuildError("Failed to resolve executable path for '{}'.".format(command_or_path))
return os.path.realpath(executable_path)

def is_windows():
return sys.platform.startswith("win")

Expand Down Expand Up @@ -151,11 +163,10 @@ def install_apt_package(package):
if is_sudo():
run_subprocess(['apt-get', 'install', '-y', package])
else:
log.error(package + " APT package missing. Please re-run this script using sudo to install.")
sys.exit(-1)
raise BuildError(package + " APT package missing. Please re-run this script using sudo to install.")

def install_ubuntu_deps(args):
'Check if the necessary Ubuntu dependencies are installed. Not required on docker. Provider help output if missing.'
'Check if the necessary Ubuntu dependencies are installed. Not required on docker. Provide help output if missing.'

# check we need the packages first
if not (args.enable_pybind or args.use_openblas):
Expand All @@ -171,8 +182,7 @@ def install_ubuntu_deps(args):
install_apt_package("libopenblas-dev")

except Exception as e:
log.error("Error setting up required APT packages. {}".format(str(e)))
sys.exit(-1)
raise BuildError("Error setting up required APT packages. {}".format(str(e)))

def install_python_deps():
dep_packages = ['setuptools', 'wheel', 'numpy']
Expand Down Expand Up @@ -355,17 +365,15 @@ def setup_cuda_vars(args):
cudnn_home_valid = (cudnn_home != None and os.path.exists(cudnn_home))

if (not cuda_home_valid or not cudnn_home_valid):
log.error("cuda_home and cudnn_home paths must be specified and valid.")
log.error("cuda_home='{}' valid={}. cudnn_home='{}' valid={}"
.format(cuda_home, cuda_home_valid, cudnn_home, cudnn_home_valid))
sys.exit(-1)
raise BuildError("cuda_home and cudnn_home paths must be specified and valid.",
"cuda_home='{}' valid={}. cudnn_home='{}' valid={}"
.format(cuda_home, cuda_home_valid, cudnn_home, cudnn_home_valid))

if (is_windows()):
# Validate that the cudnn_home is pointing at the right level
if (not os.path.exists(os.path.join(cudnn_home, "bin"))):
log.error("cudnn_home path should include the 'cuda' folder, and must contain the CUDNN 'bin' directory.")
log.error("cudnn_home='{}'".format(cudnn_home))
sys.exit(-1)
raise BuildError("cudnn_home path should include the 'cuda' folder, and must contain the CUDNN 'bin' directory.",
"cudnn_home='{}'".format(cudnn_home))

os.environ["CUDA_PATH"] = cuda_home
os.environ["CUDA_TOOLKIT_ROOT_DIR"] = cuda_home
Expand All @@ -377,8 +385,7 @@ def setup_cuda_vars(args):
# Add version specific CUDA_PATH_Vx_y value as the Visual Studio build files require that
version_file = os.path.join(cuda_home, 'version.txt')
if not os.path.exists(version_file):
log.error("No version file found in CUDA install directory. Looked for " + version_file)
sys.exit(-1)
raise BuildError("No version file found in CUDA install directory. Looked for " + version_file)

cuda_major_version = "unknown"

Expand All @@ -387,8 +394,7 @@ def setup_cuda_vars(args):
first_line = f.readline()
m = re.match("CUDA Version (\d+).(\d+)", first_line)
if not m:
log.error("Couldn't read version from first line of " + version_file)
sys.exit(-1)
raise BuildError("Couldn't read version from first line of " + version_file)

cuda_major_version = m.group(1)
minor = m.group(2)
Expand All @@ -403,11 +409,10 @@ def setup_cuda_vars(args):
log.warning("See build.md in the root ONNXRuntime directory for instructions on installing the Visual C++ 2017 14.11 toolset if needed.")

elif cuda_major_version == "9" and vc_ver[0] == "14" and int(vc_ver[1]) > 11:
log.error("Visual C++ Tools version not supported by CUDA v9. You must setup the environment to use the 14.11 toolset.")
log.info("Current version is {}. CUDA 9.2 requires version 14.11.*".format(vc_ver_str))
log.info("If necessary manually install the 14.11 toolset using the Visual Studio 2017 updater.")
log.info("See 'Windows CUDA Build' in build.md in the root directory of this repository.")
sys.exit(-1)
raise BuildError("Visual C++ Tools version not supported by CUDA v9. You must setup the environment to use the 14.11 toolset.",
"Current version is {}. CUDA 9.2 requires version 14.11.*".format(vc_ver_str),
"If necessary manually install the 14.11 toolset using the Visual Studio 2017 updater.",
"See 'Windows CUDA Build' in build.md in the root directory of this repository.")

return cuda_home, cudnn_home

Expand Down Expand Up @@ -480,7 +485,6 @@ def build_python_wheel(source_dir, build_dir, configs, use_cuda):
def main():
args = parse_arguments()

cmake_path = args.cmake_path
cmake_extra_defines = args.cmake_extra_defines if args.cmake_extra_defines else []

# if there was no explicit argument saying what to do, default to update, build and test.
Expand All @@ -496,7 +500,8 @@ def main():
configs = set(args.config)

# setup paths and directories
ctest_path = args.ctest_path
cmake_path = resolve_executable_path(args.cmake_path)
ctest_path = resolve_executable_path(args.ctest_path)
build_dir = args.build_dir
script_dir = os.path.realpath(os.path.dirname(__file__))
source_dir = os.path.normpath(os.path.join(script_dir, "..", ".."))
Expand Down Expand Up @@ -563,4 +568,8 @@ def main():
log.info("Build complete")

if __name__ == "__main__":
sys.exit(main())
try:
sys.exit(main())
except BuildError as e:
log.error(str(e))
sys.exit(1)