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

Fixes the bug 1196 #1232

Merged
merged 14 commits into from
Nov 6, 2023
Merged

Conversation

diego-minguzzi
Copy link
Contributor

Description

This PR fixes the bug 1196. Previously Llama Cpp failed because it did not find gRPC libraries and the protoc compiler.

Now it is possible to build Local AI along with gRPC using the command:

BUILD_GRPC=ON make build

It was modified the Makefile in the root directory. In case the variable BUILD_GRPC is defined, it runs the script at the path:

backend/cpp/grpc/script/build_grpc.sh

Which clones the gRPC repo and installs it at the path backend/cpp/grpc/installed_packages

The command:
make clean
removes gRPC and the installed files.

The command:
make rebuild

does not affects the installed gRPC.
Makefile was modified at the lines 405-424.

Moreover, under Linux Ubuntu, it gRPC requires the packages:
build-essential autoconf libtool pkg-config

diego-minguzzi and others added 2 commits October 31, 2023 00:58
Signed-off-by: Diego <38375572+diego-minguzzi@users.noreply.github.com>
@mudler
Copy link
Owner

mudler commented Nov 1, 2023

Hi @diego-minguzzi thanks for having a look at this!


backend/cpp/llama/grpc-server:
ifdef BUILD_GRPC
backend/cpp/grpc/script/build_grpc.sh ${INSTALLED_PACKAGES}
Copy link
Owner

Choose a reason for hiding this comment

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

I'm wondering - here probably the best approach would be to use cmake directly.

I was reading initially https://github.com/grpc/grpc/tree/master/src/cpp#fetchcontent but leaving this for a follow-up, maybe you'd like to take a look first?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I think that at least the bash-based solution allows people to build without docker, when the PR is approved.
Then, I can try to modify the gRPC installation using the CMake.
Does it sound good?

Copy link
Owner

Choose a reason for hiding this comment

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

Sounds good!

@diego-minguzzi
Copy link
Contributor Author

@lunamidori5 : could you please review the PR #1232 ? Thanks

@lunamidori5
Copy link
Collaborator

@mudler is already checking it out

mudler
mudler previously approved these changes Nov 2, 2023
Copy link
Owner

@mudler mudler left a comment

Choose a reason for hiding this comment

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

As a first shot at it looks good! Thanks @diego-minguzzi

@lunamidori5
Copy link
Collaborator

@mudler is it going to auto merge?

lunamidori5
lunamidori5 previously approved these changes Nov 2, 2023
@mudler mudler enabled auto-merge (squash) November 2, 2023 11:27
@diego-minguzzi
Copy link
Contributor Author

@mudler , @lunamidori5 : I have found that my PR fails to build because of the following error:

38 3370.4 In file included from /build/backend/cpp/llama/llama.cpp/build/examples/grpc-server/backend.grpc.pb.cc:5:
#38 3370.4 /build/backend/cpp/llama/llama.cpp/build/examples/grpc-server/backend.pb.h:19:2: error: #error "This file was generated by an older version of protoc which is"
#38 3370.4    19 | #error "This file was generated by an older version of protoc which is"
#38 3370.4       |  ^~~~~
#38 3370.4 /build/backend/cpp/llama/llama.cpp/build/examples/grpc-server/backend.pb.h:20:2: error: #error "incompatible with your Protocol Buffer headers. Please"
#38 3370.4    20 | #error "incompatible with your Protocol Buffer headers. Please"

I have found that the Dockerfile uses gRPC 1.58.0, as I do.

In the file LocalAI/pkg/grpc/proto/backend_grpc.pb.go there is: Requires gRPC-Go v1.32.0 or later.

Maybe gRPC must be downgraded , e.g. to 1.32?

Moreover, I think it is not the content of my PR that causes the build to fail, since the docker files do not use the build option that runs the gRPC build script, it is something else.
Let me know your thoughts on this.

@lunamidori5
Copy link
Collaborator

@diego-minguzzi so due to something odd with github, every time I add @mudler for review it adds me too (I do not know why) just act as if I am not here <3

@mudler
Copy link
Owner

mudler commented Nov 3, 2023

@mudler , @lunamidori5 : I have found that my PR fails to build because of the following error:

38 3370.4 In file included from /build/backend/cpp/llama/llama.cpp/build/examples/grpc-server/backend.grpc.pb.cc:5:
#38 3370.4 /build/backend/cpp/llama/llama.cpp/build/examples/grpc-server/backend.pb.h:19:2: error: #error "This file was generated by an older version of protoc which is"
#38 3370.4    19 | #error "This file was generated by an older version of protoc which is"
#38 3370.4       |  ^~~~~
#38 3370.4 /build/backend/cpp/llama/llama.cpp/build/examples/grpc-server/backend.pb.h:20:2: error: #error "incompatible with your Protocol Buffer headers. Please"
#38 3370.4    20 | #error "incompatible with your Protocol Buffer headers. Please"

I have found that the Dockerfile uses gRPC 1.58.0, as I do.

In the file LocalAI/pkg/grpc/proto/backend_grpc.pb.go there is: Requires gRPC-Go v1.32.0 or later.

Maybe gRPC must be downgraded , e.g. to 1.32?

Moreover, I think it is not the content of my PR that causes the build to fail, since the docker files do not use the build option that runs the gRPC build script, it is something else. Let me know your thoughts on this.

In the logs I clearly see the script kicking in:

#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/impl/server_callback_handlers.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/impl/server_initializer.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/impl/service_type.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/impl/status.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/impl/sync.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/resource_quota.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/security/audit_logging.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/security/auth_context.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/security/auth_metadata_processor.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/security/authorization_policy_provider.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/security/credentials.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/security/server_credentials.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/security/tls_certificate_provider.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/security/tls_certificate_verifier.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/security/tls_credentials_options.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/server.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/server_builder.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/server_context.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/server_interface.h
#38 3244.7 -- Up-to-date: /build/backend/cpp/grpc/installed_packages/include/grpcpp/server_posix.h

See the logs here: https://github.com/mudler/LocalAI/actions/runs/6731857467/job/18297419421 - it looks like that BUILD_GRPC is always on @diego-minguzzi can you make it look for an explicit ON? better make it disabled by default

auto-merge was automatically disabled November 4, 2023 21:49

Head branch was pushed to by a user without write access

@diego-minguzzi
Copy link
Contributor Author

diego-minguzzi commented Nov 4, 2023

@mudler : yes, the BUILD_GRPC was on because the Dockerfile uses the same variable for the same purpose.
Now I changed the variable that causes the script to run into BUILD_GRPC_FOR_BACKEND_LLAMA.
The command is thus:
BUILD_GRPC_FOR_BACKEND_LLAMA=ON make build

Should work now, I have no way to use it with Docker.
Can I trigger the rerun of the pipelines? I am not very familiar with the github site, I suppose not.

Aisuko
Aisuko previously requested changes Nov 5, 2023
go.mod Outdated Show resolved Hide resolved
@diego-minguzzi
Copy link
Contributor Author

Merged the master and the remote into my branch.
In my branch the go.mod was aligned with what requested by @Aisuko .

Copy link
Owner

@mudler mudler left a comment

Choose a reason for hiding this comment

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

Looking good here, thanks!

@mudler mudler dismissed Aisuko’s stale review November 6, 2023 18:07

changes addressed

@mudler mudler merged commit e7fa2e0 into mudler:master Nov 6, 2023
8 checks passed
@diego-minguzzi diego-minguzzi deleted the fix_bug_1196_compilation_fails branch November 6, 2023 19:17
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.

4 participants