Skip to content
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.

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

Fix windows download path issues #9

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Fix windows download path issues #9

wants to merge 5 commits into from

Conversation

jnoller
Copy link
Collaborator

@jnoller jnoller commented Jan 28, 2025

This fixes the following crash below when doing sisyphus download using a windows host. This is mostly pathing fixes with some defensive code for the windows logic to confirm the tar command worked.

Confirmed on Linux and Windows hosts using OSX as the client.

(sisyphus)  ✘   ~/Code/sisyphus   main±  sisyphus download -H 34.239.150.243 -P llama.cpp -d builds
INFO Connected (version 2.0, client OpenSSH_for_Windows_8.0)
INFO Authentication (publickey) failed.
INFO Connected (version 2.0, client OpenSSH_for_Windows_8.0)
INFO Authentication (publickey) successful!
INFO '34.239.150.243' is a Windows host
INFO Build complete
INFO gguf-0.15.4575-py310haa95532_0.tar.bz2 already transmuted
INFO gguf-0.15.4575-py311haa95532_0.tar.bz2 already transmuted
INFO gguf-0.15.4575-py312haa95532_0.tar.bz2 already transmuted
INFO gguf-0.15.4575-py39haa95532_0.tar.bz2 already transmuted
INFO llama.cpp-0.0.4575-cpu_v1_mkl_h4262f35_0.tar.bz2 already transmuted
INFO llama.cpp-0.0.4575-cpu_v2_mkl_h0bdc4df_40.tar.bz2 already transmuted
INFO llama.cpp-0.0.4575-cpu_v3_mkl_h393b973_50.tar.bz2 already transmuted
INFO llama.cpp-0.0.4575-cuda124_v1_h6cbc5b3_200.tar.bz2 already transmuted
INFO llama.cpp-0.0.4575-cuda124_v2_h16922bf_240.tar.bz2 already transmuted
INFO llama.cpp-0.0.4575-cuda124_v3_hf3fb316_250.tar.bz2 already transmuted
INFO llama.cpp-tools-0.0.4575-py310haa95532_0.tar.bz2 already transmuted
INFO llama.cpp-tools-0.0.4575-py311haa95532_0.tar.bz2 already transmuted
INFO llama.cpp-tools-0.0.4575-py312haa95532_0.tar.bz2 already transmuted
INFO llama.cpp-tools-0.0.4575-py39haa95532_0.tar.bz2 already transmuted
INFO Downloading package tarballs in '\sisyphus\llama.cpp\build\win-64'
INFO [chan 21] Opened sftp connection (server version 3)
Traceback (most recent call last):
  File "/opt/homebrew/anaconda3/envs/sisyphus/bin/sisyphus", line 8, in <module>
    sys.exit(cli())
             ~~~^^
  File "/opt/homebrew/anaconda3/envs/sisyphus/lib/python3.13/site-packages/click/core.py", line 1161, in __call__
    return self.main(*args, **kwargs)
           ~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/anaconda3/envs/sisyphus/lib/python3.13/site-packages/click/core.py", line 1082, in main
    rv = self.invoke(ctx)
  File "/opt/homebrew/anaconda3/envs/sisyphus/lib/python3.13/site-packages/click/core.py", line 1697, in invoke
    return _process_result(sub_ctx.command.invoke(sub_ctx))
                           ~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^
  File "/opt/homebrew/anaconda3/envs/sisyphus/lib/python3.13/site-packages/click/core.py", line 1443, in invoke
    return ctx.invoke(self.callback, **ctx.params)
           ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/anaconda3/envs/sisyphus/lib/python3.13/site-packages/click/core.py", line 788, in invoke
    return __callback(*args, **kwargs)
  File "/Users/jesse/Code/sisyphus/sisyphus/main.py", line 171, in download
    h.download(package, destination, all)
    ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/jesse/Code/sisyphus/sisyphus/host.py", line 448, in download
    self.connection.get(tf.replace("\\", "/"))
    ~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/anaconda3/envs/sisyphus/lib/python3.13/site-packages/fabric/connection.py", line 897, in get
    return Transfer(self).get(*args, **kwargs)
           ~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/anaconda3/envs/sisyphus/lib/python3.13/site-packages/fabric/transfer.py", line 170, in get
    self.sftp.get(remotepath=remote, localpath=local)
    ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/anaconda3/envs/sisyphus/lib/python3.13/site-packages/paramiko/sftp_client.py", line 840, in get
    size = self.getfo(
        remotepath,
    ...<3 lines>...
        max_concurrent_prefetch_requests,
    )
  File "/opt/homebrew/anaconda3/envs/sisyphus/lib/python3.13/site-packages/paramiko/sftp_client.py", line 795, in getfo
    file_size = self.stat(remotepath).st_size
                ~~~~~~~~~^^^^^^^^^^^^
  File "/opt/homebrew/anaconda3/envs/sisyphus/lib/python3.13/site-packages/paramiko/sftp_client.py", line 493, in stat
    t, msg = self._request(CMD_STAT, path)
             ~~~~~~~~~~~~~^^^^^^^^^^^^^^^^
  File "/opt/homebrew/anaconda3/envs/sisyphus/lib/python3.13/site-packages/paramiko/sftp_client.py", line 857, in _request
    return self._read_response(num)
           ~~~~~~~~~~~~~~~~~~~^^^^^
  File "/opt/homebrew/anaconda3/envs/sisyphus/lib/python3.13/site-packages/paramiko/sftp_client.py", line 909, in _read_response
    self._convert_status(msg)
    ~~~~~~~~~~~~~~~~~~~~^^^^^
  File "/opt/homebrew/anaconda3/envs/sisyphus/lib/python3.13/site-packages/paramiko/sftp_client.py", line 942, in _convert_status
    raise IOError(text)
OSError: Failure

sisyphus/host.py Outdated
Comment on lines 420 to 425
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"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I clearly remember testing this in both situations and it worked, so I wonder what's going on here.

I'll look at it again because that's weird.

sisyphus/host.py Outdated Show resolved Hide resolved
sisyphus/host.py Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants