Skip to content

Use https for the gitmodule instead of ssh #42

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

Closed
wants to merge 1 commit into from

Conversation

nsarrazin
Copy link

@nsarrazin nsarrazin commented Apr 7, 2023

Why

The submodule is defined using SSH, which means that you cannot initialize the submodule if you don't have SSH set up.

I was building llama-cpp-python from source inside of a docker container which doesn't have SSH set up and I couldn't clone the submodule.

How

I just replaced the url in .gitmodules, it should have no other side effects!

@abetlen
Copy link
Owner

abetlen commented Apr 8, 2023

I probably won't merge this because I think git ssh URLs are the recommended approach. A few thoughts though

  • if you're building docker images there's a feature to forward ssh agents into the container
  • pip installing the package already builds from source by default
  • if you do get this working as a docker image without this change I'll definitely merge it in and add it to the build process

@nsarrazin
Copy link
Author

nsarrazin commented Apr 8, 2023

I think github doesn't specifically recommend ssh over https, it even used to be the other way 😆

But for my project I don't specifically need to build the bindings from source, I just want to make sure llama.cpp gets compiled on the user's machine and we're not shipping a binary because there are issues with CPU feature sets if you do.

So if I do pip install it will build llama.cpp from source ?

@abetlen
Copy link
Owner

abetlen commented Apr 8, 2023

@nsarrazin yes that's correct, and I plan to keep that the default (no wheels in the PyPI package)

@nsarrazin
Copy link
Author

Alright that's great to hear. This will simplify my dockerfile, thanks haha

I'm closing this PR then!

@nsarrazin nsarrazin closed this Apr 8, 2023
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