From ebee0b70728bea40c42956f9cf07f6f5865f0e9e Mon Sep 17 00:00:00 2001 From: erjia Date: Thu, 31 Mar 2022 15:21:05 +0000 Subject: [PATCH] Remove debugging lines and enable tests --- .github/workflows/ci.yml | 28 ++----------- .github/workflows/domain_ci.yml | 12 +++--- setup.py | 2 +- tools/setup_helpers/extension.py | 2 +- torchdata/_extension.py | 69 +++----------------------------- torchdata/csrc/CMakeLists.txt | 6 +-- 6 files changed, 17 insertions(+), 102 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 021a426ab..18b68c850 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,8 +22,8 @@ jobs: fail-fast: false matrix: os: - # - macos-latest - # - ubuntu-latest + - macos-latest + - ubuntu-latest - windows-latest python-version: - 3.7 @@ -31,7 +31,7 @@ jobs: - 3.9 with-s3: - 1 - # - 0 + - 0 steps: - name: Setup additional system libraries if: startsWith( matrix.os, 'ubuntu' ) @@ -72,20 +72,7 @@ jobs: echo "::set-output name=awssdk::$AWSSDK_PATH" echo "::set-output name=pybind11::$PYBIND11_PATH" id: export_path - # - name: Get Date - # if: matrix.with-s3 == 1 - # id: get_date - # shell: bash - # run: echo "::set-output name=date::$(date "+%Y%m%d")" - # - name: Uses cache for AWS-SDK-CPP - # if: matrix.with-s3 == 1 - # id: aws_sdk_cache - # uses: actions/cache@v3 - # with: - # path: ${{ steps.export_path.outputs.awssdk }} - # key: ${{ matrix.os }}-${{ matrix.python-version }}-${{ steps.get_date.outputs.date }} - name: Install AWS-SDK-CPP on Windows for S3 IO datapipes - # if: matrix.with-s3 == 1 && matrix.os == 'windows-latest' && steps.aws_sdk_cache.outputs.cache-hit != 'true' if: matrix.with-s3 == 1 && matrix.os == 'windows-latest' run: | git clone --recurse-submodules https://github.com/aws/aws-sdk-cpp @@ -95,7 +82,6 @@ jobs: cmake --build build --config Release cmake --install build --config Release - name: Install AWS-SDK-CPP on Non-Windows for S3 IO datapipes - # if: matrix.with-s3 == 1 && matrix.os != 'windows-latest' && steps.aws_sdk_cache.outputs.cache-hit != 'true' if: matrix.with-s3 == 1 && matrix.os != 'windows-latest' run: | git clone --recurse-submodules https://github.com/aws/aws-sdk-cpp @@ -112,14 +98,6 @@ jobs: BUILD_S3: ${{ matrix.with-s3 }} pybind11_DIR: ${{ steps.export_path.outputs.pybind11 }} AWSSDK_DIR: ${{ steps.export_path.outputs.awssdk }} - - name: Find extension - if: always() && matrix.with-s3 == 1 - shell: bash - run: | - ls -al torchdata - if [[ ${{ matrix.os }} == 'windows-latest' ]]; then - find . -name *.dll - fi - name: Install test requirements run: pip3 install expecttest fsspec iopath==0.1.9 numpy pytest rarfile - name: Run DataPipes tests with pytest diff --git a/.github/workflows/domain_ci.yml b/.github/workflows/domain_ci.yml index 5e10ce596..7729848d9 100644 --- a/.github/workflows/domain_ci.yml +++ b/.github/workflows/domain_ci.yml @@ -4,12 +4,12 @@ on: branches: - main - release/* - # pull_request: - # branches: - # - main - # # For PR created by ghstack - # - gh/*/*/base - # - release/* + pull_request: + branches: + - main + # For PR created by ghstack + - gh/*/*/base + - release/* jobs: torchvision: diff --git a/setup.py b/setup.py index a522a1e88..1f10ac571 100644 --- a/setup.py +++ b/setup.py @@ -110,7 +110,7 @@ def remove_extension(pattern): "Topic :: Scientific/Engineering :: Artificial Intelligence", ], # Package Info - packages=find_packages(exclude=["test*", "examples*", "torchdata.csrc*", "tools*"]), + packages=find_packages(exclude=["test*", "examples*", "torchdata.csrc*", "build*", "tools*"]), zip_safe=False, # C++ Extension Modules ext_modules=setup_helpers.get_ext_modules(), diff --git a/tools/setup_helpers/extension.py b/tools/setup_helpers/extension.py index 8de753696..7d63fead6 100644 --- a/tools/setup_helpers/extension.py +++ b/tools/setup_helpers/extension.py @@ -84,7 +84,7 @@ def build_extension(self, ext): build_args = ["--config", cfg] - if _AWSSDK_DIR: + if _BUILD_S3 and _AWSSDK_DIR: cmake_args += [ f"-DAWSSDK_DIR={_AWSSDK_DIR}", ] diff --git a/torchdata/_extension.py b/torchdata/_extension.py index 46e58cd8f..a862c356e 100644 --- a/torchdata/_extension.py +++ b/torchdata/_extension.py @@ -1,7 +1,5 @@ -import ctypes import importlib import os -import sys from pathlib import Path @@ -9,69 +7,12 @@ def _init_extension(): - if sys.platform == "win32": - lib_dir = os.path.dirname(__file__) - py_dll_path = os.path.join(sys.exec_prefix, "Library", "bin") - aws_dll_path = os.path.join(_LIB_DIR.parent.resolve(), "aws-sdk-cpp", "sdk-lib", "lib") - # When users create a virtualenv that inherits the base environment, - # we will need to add the corresponding library directory into - # DLL search directories. Otherwise, it will rely on `PATH` which - # is dependent on user settings. - if sys.exec_prefix != sys.base_exec_prefix: - base_py_dll_path = os.path.join(sys.base_exec_prefix, "Library", "bin") - else: - base_py_dll_path = "" + lib_dir = os.path.dirname(__file__) - dll_paths = list(filter(os.path.exists, [lib_dir, aws_dll_path, py_dll_path, base_py_dll_path])) - - kernel32 = ctypes.WinDLL("kernel32.dll", use_last_error=True) - with_load_library_flags = hasattr(kernel32, "AddDllDirectory") - prev_error_mode = kernel32.SetErrorMode(0x0001) - - kernel32.LoadLibraryW.restype = ctypes.c_void_p - if with_load_library_flags: - kernel32.AddDllDirectory.restype = ctypes.c_void_p - kernel32.LoadLibraryExW.restype = ctypes.c_void_p - - for dll_path in dll_paths: - if sys.version_info >= (3, 8): - print("Add all dll directory", dll_path) - os.add_dll_directory(dll_path) - elif with_load_library_flags: - print("Kernel 32 add dll directory", dll_path) - res = kernel32.AddDllDirectory(dll_path) - if res is None: - err = ctypes.WinError(ctypes.get_last_error()) - err.strerror += f' Error adding "{lib_dir}" to the DLL directories.' - raise err - - import glob - - dlls = glob.glob(os.path.join(aws_dll_path, "*.dll")) - path_patched = False - for dll in dlls: - print("DLL", dll) - is_loaded = False - if with_load_library_flags: - res = kernel32.LoadLibraryExW(dll, None, 0x00001100) - last_error = ctypes.get_last_error() - if res is None and last_error != 126: - err = ctypes.WinError(last_error) - err.strerror += f' Error loading "{dll}" or one of its dependencies.' - raise err - elif res is not None: - is_loaded = True - if not is_loaded: - if not path_patched: - os.environ["PATH"] = ";".join(dll_paths + [os.environ["PATH"]]) - path_patched = True - res = kernel32.LoadLibraryW(dll) - if res is None: - err = ctypes.WinError(ctypes.get_last_error()) - err.strerror += f' Error loading "{dll}" or one of its dependencies.' - raise err - - kernel32.SetErrorMode(prev_error_mode) + # TODO: If any extension had dependency of shared library, + # in order to support load these shred libraries dynamically, + # we need to add logic to load dll path on Windows + # See: https://github.com/pytorch/pytorch/blob/master/torch/__init__.py#L56-L140 loader_details = (importlib.machinery.ExtensionFileLoader, importlib.machinery.EXTENSION_SUFFIXES) diff --git a/torchdata/csrc/CMakeLists.txt b/torchdata/csrc/CMakeLists.txt index d8b1b2490..88740fa24 100644 --- a/torchdata/csrc/CMakeLists.txt +++ b/torchdata/csrc/CMakeLists.txt @@ -30,14 +30,10 @@ if(BUILD_S3) target_include_directories(_torchdata PRIVATE ${PROJECT_SOURCE_DIR} ${Python_INCLUDE_DIR} {pybind11_INCLUDE_DIRS}) message(STATUS "AWSSDK linked libs: ${AWSSDK_LINK_LIBRARIES}") - target_link_libraries( - _torchdata - PUBLIC - pybind11::module - ) target_link_libraries( _torchdata PRIVATE + pybind11::module ${AWSSDK_LINK_LIBRARIES} ${Python_LIBRARIES} ${ADDITIONAL_ITEMS}