Skip to content

Conversation

grivanov
Copy link

Currently there is an ARG TARGETARCH that defaults to 'arm64', so the build fails on 'amd64'.

The documentation in Ollama-instruction.md doesn't mention this in Step 4.
It only provides this: docker build -f Dockerfile-ollama-local -t deepwiki:ollama-local . without mentioning the actually required --build-arg TARGETARCH=<arch>:

This PR aims to fix this and actually detect the build arch.

Copy link
Contributor

Summary of Changes

Hello @grivanov, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the Dockerfile-ollama-local by introducing automatic architecture detection. Previously, the Dockerfile defaulted to arm64, causing build issues on amd64 systems and requiring a non-obvious build argument. The update streamlines the build process, allowing users to build the Ollama image without manually specifying the target architecture, thereby improving usability and preventing common build errors.

Highlights

  • Automatic Architecture Detection: The Dockerfile-ollama-local now automatically detects the system's architecture (ARM64 or AMD64) during the build process.
  • Removed Manual TARGETARCH: The explicit ARG TARGETARCH=arm64 and the previous requirement for a --build-arg are eliminated, simplifying the build command.
  • Improved Build Robustness: This change prevents build failures on amd64 systems that previously occurred due to the hardcoded arm64 default, making the build process more reliable.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly automates the architecture detection in Dockerfile-ollama-local, removing the need for a manual build argument. This is a great improvement for usability. I've added one suggestion to refactor the shell script logic using a case statement, which is more idiomatic and readable for this scenario.

Comment on lines +27 to 35
RUN OLLAMA_ARCH=$(arch | sed s/aarch64/arm64/ | sed s/x86_64/amd64/) && \
if [ "$OLLAMA_ARCH" = "arm64" ]; then \
echo "Building for ARM64 architecture."; \
elif [ "$OLLAMA_ARCH" = "amd64" ]; then \
echo "Building for AMD64 architecture."; \
else \
echo "Error: Unsupported architecture '$TARGETARCH'. Supported architectures are 'arm64' and 'amd64'." >&2 && \
echo "Error: Unsupported architecture '$OLLAMA_ARCH'. Supported architectures are 'arm64' and 'amd64'." >&2 && \
exit 1; \
fi && \
Copy link
Contributor

Choose a reason for hiding this comment

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

medium

While the if/elif/else structure works, using a case statement is more idiomatic and readable for shell scripts when checking a variable against multiple possible values. This also provides an opportunity to use the more portable uname -m command instead of arch, slightly optimize the sed command by combining expressions, and remove unnecessary semicolons.

RUN OLLAMA_ARCH=$(uname -m | sed -e 's/aarch64/arm64/' -e 's/x86_64/amd64/') && \
    case "$OLLAMA_ARCH" in \
        arm64) \
            echo "Building for ARM64 architecture." \
            ;; \
        amd64) \
            echo "Building for AMD64 architecture." \
            ;; \
        *) \
            echo "Error: Unsupported architecture '$OLLAMA_ARCH'. Supported architectures are 'arm64' and 'amd64'." >&2 && \
            exit 1 \
            ;; \
    esac && \

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.

1 participant