5 ms·

Dude, this is NEVER ok. What in the world??? A third party LIBRARY running sudo commands? That’s just insane. You just fail and print a nice error message tell

• pxc 1y ago
  How unusual is this for the ecosystem?
  • pshirshov 1y ago
    Unfortunately, Python ecosystem is the Wild West.
• danielhanchen 1y ago
  Yes I had that at the start, but people kept complaining they don't know how to actually run terminal commands, hence the shortcut :( I was thinking if I can do it during the pip install or via setup.py which will do the apt-get instead. As a fallback, I'll probably for now remove shell executions and just warn the user
  • devin 1y ago
    Don't optimize for these people.
    • danielhanchen 1y ago
      Yep agreed - I primarily thought it was a reasonable "hack", but it's pretty bad security wise, so apologies again. The current solution hopefully is in between - ie sudo is gone, apt-get will run only after the user agrees by pressing enter, and if it fails, it'll tell the user to read docs on installing llama.cpp
      • woile 1y ago
        Don't apologize, you are doing amazing work. I appreciate the effort you put. Usually you don't make assumptions on the host OS, just try to find the things you need and if not, fail, ideally with good feedback. If you want to provide the "hack", you can still do it, but ideally behind a flag, `allow_installation` or something like that. This is, if you want your code to reach broader audiences.
        • danielhanchen 1y ago
          Thank you! :)
        • shaan7 1y ago
          Yep there's no need to apologize, you've been very courteous and took all that feedback constructively. Good stuff :)
          • danielhanchen 1y ago
            Thank you!
      • deleted 1y ago
        [deleted]
  • rfoo 1y ago
    IMO the correct thing to do to make these people happy, while being sane, is - do not build llama.cpp on their system. Instead, bundle a portable llama.cpp binary along with unsloth, so that when they install unsloth with `pip` (or `uv`) they get it. Some people may prefer using whatever llama.cpp in $PATH, it's okay to support that, though I'd say doing so may lead to more confused noob users spam - they may just have an outdated version lurking in $PATH. Doing so makes unsloth wheel platform-dependent, if this is too much of a burden, then maybe you can just package llama.cpp binary and have it on PyPI, like how scipy guys maintain a https://pypi.org/project/cmake/ https://pypi.org/project/cmake/ on PyPI (yes, you can `pip install cmake`), and then depends on it (maybe in an optional group, I see you already have a lot due to cuda shit).
    • danielhanchen 1y ago
      Oh yes I was working on providing binaries together with pip - currently we're relying on pyproject.toml, but once we utilize setup.py (I think), using binaries gets much simpler I'm still working on it, but sadly I'm not a packaging person so progress has been nearly zero :(
      • ffsm8 1y ago
        I think you misunderstood rfoos suggestion slightly. From how I interpreted it, he meant you could create a new python package, this would effectively be the binary you need. In your current package, you could depend on the new one, and through that - pull in the binary. This would let you easily decouple your package from the binary,too - so it'd be easy to update the binary to latest even without pushing a new version of your original package I've maintained release pipelines before and handled packaging in a previous job, but I'm not particularly into the python ecosystem, so take this with a grain of salt: an approach would be Pip Packages : * Unsloth: current package, prefers using unsloth-llama, and uses path llama-cpp as fallback (with error msg as final fallback if neither exist, promoting install for unsloth-llama) * Unsloth-llama: new package which only bundles the llama cpp binary
        • danielhanchen 1y ago
• danielhanchen 1y ago
  As an update, I pushed https://github.com/unslothai/unsloth-zoo/commit/ae675a0a2d208a20f6dbdccccec0c8ce995fe6ec https://github.com/unslothai/unsloth-zoo/commit/ae675a0a2d20... (1) Removed and disabled sudo (2) Installing via apt-get will ask user's input() for permission (3) Added an error if failed llama.cpp and provides instructions to manual compile llama.cpp Again apologies on my dumbness and thanks for pointing it out!
• lyu07282 1y ago
  on a meta level its kind of worrying for the ecosystem that there is nothing in PyPI that blocks & bans developers who try to run sudo on setup. I get they don't have the resources to do manual checks, but literally no checks against malicious packages?
  • danielhanchen 1y ago
    Sadly not - you can run anything within a python shell since there's os system, subprocess popen and exec - it's actually very common for setup.py files where installers execute commands But I do agree maybe for better security pypi should check for commands and warn