From 9693c8ce9fc16d5ce47f101f32c88ec2e44ce4e3 Mon Sep 17 00:00:00 2001 From: Jesse Noller Date: Tue, 28 Jan 2025 14:00:10 -0700 Subject: [PATCH 1/5] Fix windows download path issues --- sisyphus/host.py | 69 +++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 60 insertions(+), 9 deletions(-) diff --git a/sisyphus/host.py b/sisyphus/host.py index ac84434..14a52f2 100644 --- a/sisyphus/host.py +++ b/sisyphus/host.py @@ -417,17 +417,51 @@ def download(self, package, destination, all=False): self.transmute(package) builddir = self.path(package, "build") - tf = self.topdir + self.separator + "sisyphus.tar" + # Modify how we construct the temporary tar file path + if self.type == WINDOWS_TYPE: + # Use the build directory instead of C: root + tf = self.path_join(builddir, "sisyphus.tar") + else: + tf = self.topdir + self.separator + "sisyphus.tar" + logging.info("Downloading package tarballs in '%s%s%s'", builddir, self.separator, self.pkgdir) # Create a tarball containing either just packages or the whole build directory - if all: - self.run(f"cd {self.sisyphus_dir} && cd .. && tar -cf {tf} sisyphus") - else: - if self.type == LINUX_TYPE: - self.run(f"cd {builddir} && tar -cf {tf} {self.pkgdir}/*.conda {self.pkgdir}/*.tar.bz2 2>/dev/null || true") - elif self.type == WINDOWS_TYPE: - self.run(f'cd {builddir} && tar -cf {tf} $(dir /s {self.pkgdir}\\*.conda {self.pkgdir}\\*.tar.bz2 2>nul )', quiet=True) + try: + if all: + result = self.run(f"cd {self.sisyphus_dir} && cd .. && tar -cf {tf} sisyphus") + else: + if self.type == LINUX_TYPE: + result = self.run(f"cd {builddir} && tar -cf {tf} {self.pkgdir}/*.conda {self.pkgdir}/*.tar.bz2 2>/dev/null || true") + elif self.type == WINDOWS_TYPE: + # First get the list of files - make sure we're in builddir first + files = self.run(f'cd {builddir} && dir /b /s {self.pkgdir}\\*.conda {self.pkgdir}\\*.tar.bz2 2>nul', quiet=True) + + if not files: + logging.warning("No package files found to tar") + result = self.run(f'cd {builddir} && tar -cf "{tf}" --files-from NUL', quiet=True) + else: + # Create file list - strip builddir prefix since we'll cd there + file_list = " ".join(f'"{f.replace(builddir + "\\", "")}"' for f in files.splitlines()) + result = self.run(f'cd {builddir} && tar -cf "{tf}" {file_list}', quiet=True) + + # Verify the tar file was created + try: + if self.type == WINDOWS_TYPE: + size_check = self.run(f'cd {builddir} && dir "{tf}"') + else: + # Linux uses -c format, macOS uses -f format + size_check = self.run(f'stat -c%s "{tf}"') + + if not size_check: + raise Exception(f"Tar file is missing") + + except Exception as e: + raise Exception(f"Failed to verify tar file: {str(e)}") + + except Exception as e: + logging.error(f"Failed to create tar file: {str(e)}") + raise # Download and untar it dest = os.path.join(destination, package) @@ -445,7 +479,24 @@ def download(self, package, destination, all=False): except: pass os.chdir(dest) - self.connection.get(tf.replace("\\", "/")) + + # Ensure proper path format for SFTP + remote_path = tf.replace("\\", "/") + if remote_path.startswith("//"): + remote_path = remote_path[1:] # Remove one leading slash if there are two + + logging.debug(f"Attempting to download from remote path: {remote_path}") + try: + # Check if file exists by trying to stat it + try: + self.connection.sftp().stat(remote_path) + except FileNotFoundError: + raise FileNotFoundError(f"Remote file not found: {remote_path}") + + self.connection.get(remote_path) + except Exception as e: + logging.error(f"Download failed for {remote_path}: {str(e)}") + raise with tarfile.open("sisyphus.tar", "r") as tar: tar.extractall() From 31bfa83af23349e018e227f6979215aba1785a37 Mon Sep 17 00:00:00 2001 From: Jesse Noller Date: Tue, 28 Jan 2025 15:07:28 -0700 Subject: [PATCH 2/5] remove hanging debug var --- sisyphus/host.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sisyphus/host.py b/sisyphus/host.py index 14a52f2..36493eb 100644 --- a/sisyphus/host.py +++ b/sisyphus/host.py @@ -429,21 +429,21 @@ def download(self, package, destination, all=False): # Create a tarball containing either just packages or the whole build directory try: if all: - result = self.run(f"cd {self.sisyphus_dir} && cd .. && tar -cf {tf} sisyphus") + self.run(f"cd {self.sisyphus_dir} && cd .. && tar -cf {tf} sisyphus") else: if self.type == LINUX_TYPE: - result = self.run(f"cd {builddir} && tar -cf {tf} {self.pkgdir}/*.conda {self.pkgdir}/*.tar.bz2 2>/dev/null || true") + self.run(f"cd {builddir} && tar -cf {tf} {self.pkgdir}/*.conda {self.pkgdir}/*.tar.bz2 2>/dev/null || true") elif self.type == WINDOWS_TYPE: # First get the list of files - make sure we're in builddir first files = self.run(f'cd {builddir} && dir /b /s {self.pkgdir}\\*.conda {self.pkgdir}\\*.tar.bz2 2>nul', quiet=True) if not files: logging.warning("No package files found to tar") - result = self.run(f'cd {builddir} && tar -cf "{tf}" --files-from NUL', quiet=True) + self.run(f'cd {builddir} && tar -cf "{tf}" --files-from NUL', quiet=True) else: # Create file list - strip builddir prefix since we'll cd there file_list = " ".join(f'"{f.replace(builddir + "\\", "")}"' for f in files.splitlines()) - result = self.run(f'cd {builddir} && tar -cf "{tf}" {file_list}', quiet=True) + self.run(f'cd {builddir} && tar -cf "{tf}" {file_list}', quiet=True) # Verify the tar file was created try: From d98273e7f4cf173cfcde2ecd6257a2d259275574 Mon Sep 17 00:00:00 2001 From: Jesse Noller Date: Tue, 28 Jan 2025 15:13:54 -0700 Subject: [PATCH 3/5] Use exists instead of artisinal stat --- sisyphus/host.py | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/sisyphus/host.py b/sisyphus/host.py index 36493eb..07c64d6 100644 --- a/sisyphus/host.py +++ b/sisyphus/host.py @@ -447,15 +447,8 @@ def download(self, package, destination, all=False): # Verify the tar file was created try: - if self.type == WINDOWS_TYPE: - size_check = self.run(f'cd {builddir} && dir "{tf}"') - else: - # Linux uses -c format, macOS uses -f format - size_check = self.run(f'stat -c%s "{tf}"') - - if not size_check: + if not self.exists(tf): raise Exception(f"Tar file is missing") - except Exception as e: raise Exception(f"Failed to verify tar file: {str(e)}") From e1bcdfc4ccfd44b7fc9e5d4ccaf882d17e983cf0 Mon Sep 17 00:00:00 2001 From: Denis Dupeyron Date: Tue, 28 Jan 2025 16:40:46 -0700 Subject: [PATCH 4/5] Remove dead space --- sisyphus/host.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/sisyphus/host.py b/sisyphus/host.py index 07c64d6..6f8d69b 100644 --- a/sisyphus/host.py +++ b/sisyphus/host.py @@ -423,7 +423,7 @@ def download(self, package, destination, all=False): tf = self.path_join(builddir, "sisyphus.tar") else: tf = self.topdir + self.separator + "sisyphus.tar" - + logging.info("Downloading package tarballs in '%s%s%s'", builddir, self.separator, self.pkgdir) # Create a tarball containing either just packages or the whole build directory @@ -436,7 +436,7 @@ def download(self, package, destination, all=False): elif self.type == WINDOWS_TYPE: # First get the list of files - make sure we're in builddir first files = self.run(f'cd {builddir} && dir /b /s {self.pkgdir}\\*.conda {self.pkgdir}\\*.tar.bz2 2>nul', quiet=True) - + if not files: logging.warning("No package files found to tar") self.run(f'cd {builddir} && tar -cf "{tf}" --files-from NUL', quiet=True) @@ -444,14 +444,14 @@ def download(self, package, destination, all=False): # Create file list - strip builddir prefix since we'll cd there file_list = " ".join(f'"{f.replace(builddir + "\\", "")}"' for f in files.splitlines()) self.run(f'cd {builddir} && tar -cf "{tf}" {file_list}', quiet=True) - + # Verify the tar file was created try: if not self.exists(tf): raise Exception(f"Tar file is missing") except Exception as e: raise Exception(f"Failed to verify tar file: {str(e)}") - + except Exception as e: logging.error(f"Failed to create tar file: {str(e)}") raise @@ -472,12 +472,12 @@ def download(self, package, destination, all=False): except: pass os.chdir(dest) - + # Ensure proper path format for SFTP remote_path = tf.replace("\\", "/") if remote_path.startswith("//"): remote_path = remote_path[1:] # Remove one leading slash if there are two - + logging.debug(f"Attempting to download from remote path: {remote_path}") try: # Check if file exists by trying to stat it @@ -485,7 +485,7 @@ def download(self, package, destination, all=False): self.connection.sftp().stat(remote_path) except FileNotFoundError: raise FileNotFoundError(f"Remote file not found: {remote_path}") - + self.connection.get(remote_path) except Exception as e: logging.error(f"Download failed for {remote_path}: {str(e)}") @@ -520,4 +520,4 @@ def transmute(self, package): logging.info("%s already exists", bz2_pkg) else: logging.info("Transmuting %s to .tar.bz2", conda_pkg) - self.run(f"{ACTIVATE} cd {pkgdir} && cph t {conda_pkg} .tar.bz2") \ No newline at end of file + self.run(f"{ACTIVATE} cd {pkgdir} && cph t {conda_pkg} .tar.bz2") From 8cf861238a3677d7f2553e087ac54cf049ee6999 Mon Sep 17 00:00:00 2001 From: Denis Dupeyron Date: Wed, 29 Jan 2025 16:18:25 -0700 Subject: [PATCH 5/5] Fix the fix --- sisyphus/host.py | 74 ++++++++++++++++++------------------------------ 1 file changed, 28 insertions(+), 46 deletions(-) diff --git a/sisyphus/host.py b/sisyphus/host.py index 6f8d69b..7e84972 100644 --- a/sisyphus/host.py +++ b/sisyphus/host.py @@ -11,8 +11,8 @@ WINDOWS_TYPE = "windows" LINUX_USER = "ec2-user" WINDOWS_USER = "dev-admin" -LINUX_TOPDIR = "/tmp" -WINDOWS_TOPDIR = "\\" +LINUX_TOPDIR = "/tmp" # These two have to be a directory, +WINDOWS_TOPDIR = "\\tmp" # not the root of a device like / or \\ CONDA_PACKAGES = "conda-build distro-tooling::anaconda-linter git anaconda-client conda-package-handling" BUILD_OPTIONS = "--error-overlinking -c ai-staging" ACTIVATE = "conda activate sisyphus &&" @@ -416,45 +416,38 @@ def download(self, package, destination, all=False): # Transmute packages if needed self.transmute(package) + # Check whether there are packaes to download, if not bail out builddir = self.path(package, "build") - # Modify how we construct the temporary tar file path - if self.type == WINDOWS_TYPE: - # Use the build directory instead of C: root - tf = self.path_join(builddir, "sisyphus.tar") - else: - tf = self.topdir + self.separator + "sisyphus.tar" + pkgdir = self.path_join(builddir, self.pkgdir) + files = [self.path_join(self.pkgdir, f) for f in self.ls(pkgdir) if f.endswith('.tar.bz2') or f.endswith('.conda')] + if not files: + logging.warning("No packages to download") + return + + tf_name = f"sisyphus_{package}_{self.type}.tar" + tf = self.path_join(self.topdir, tf_name) - logging.info("Downloading package tarballs in '%s%s%s'", builddir, self.separator, self.pkgdir) # Create a tarball containing either just packages or the whole build directory try: if all: - self.run(f"cd {self.sisyphus_dir} && cd .. && tar -cf {tf} sisyphus") + logging.info("Downloading complete Sisyphus data at '%s'", self.sisyphus_dir) + self.run(f"cd {self.topdir} && tar -cf {tf} sisyphus") else: + logging.info("Downloading %d package tarballs in '%s'", len(files), pkgdir) if self.type == LINUX_TYPE: - self.run(f"cd {builddir} && tar -cf {tf} {self.pkgdir}/*.conda {self.pkgdir}/*.tar.bz2 2>/dev/null || true") + self.run(f"cd {builddir} && tar -cf {tf} {" ".join(files)} 2>/dev/null || true") elif self.type == WINDOWS_TYPE: - # First get the list of files - make sure we're in builddir first - files = self.run(f'cd {builddir} && dir /b /s {self.pkgdir}\\*.conda {self.pkgdir}\\*.tar.bz2 2>nul', quiet=True) - - if not files: - logging.warning("No package files found to tar") - self.run(f'cd {builddir} && tar -cf "{tf}" --files-from NUL', quiet=True) - else: - # Create file list - strip builddir prefix since we'll cd there - file_list = " ".join(f'"{f.replace(builddir + "\\", "")}"' for f in files.splitlines()) - self.run(f'cd {builddir} && tar -cf "{tf}" {file_list}', quiet=True) - - # Verify the tar file was created - try: - if not self.exists(tf): - raise Exception(f"Tar file is missing") - except Exception as e: - raise Exception(f"Failed to verify tar file: {str(e)}") + self.run(f'cd {builddir} && tar -cf {tf} {" ".join(files)} 2>nul )', quiet=True) except Exception as e: logging.error(f"Failed to create tar file: {str(e)}") - raise + raise SystemExit(1) + + # Verify the tar file was created + if not self.exists(tf): + logging.error("Tar file '%s' is missing", tf) + raise SystemExit(1) # Download and untar it dest = os.path.join(destination, package) @@ -473,28 +466,17 @@ def download(self, package, destination, all=False): pass os.chdir(dest) - # Ensure proper path format for SFTP - remote_path = tf.replace("\\", "/") - if remote_path.startswith("//"): - remote_path = remote_path[1:] # Remove one leading slash if there are two - - logging.debug(f"Attempting to download from remote path: {remote_path}") + logging.debug(f"Attempting to download from remote path: {tf}") try: - # Check if file exists by trying to stat it - try: - self.connection.sftp().stat(remote_path) - except FileNotFoundError: - raise FileNotFoundError(f"Remote file not found: {remote_path}") - - self.connection.get(remote_path) + self.connection.get(tf.replace("\\", "/")) except Exception as e: - logging.error(f"Download failed for {remote_path}: {str(e)}") - raise - with tarfile.open("sisyphus.tar", "r") as tar: + logging.error(f"Download failed for {tf}: {str(e)}") + raise SystemExit(1) + with tarfile.open(tf_name, "r") as tar: tar.extractall() # Cleanup - os.remove("sisyphus.tar") + os.remove(tf_name) self.rm(tf) logging.info("Done")