Merge pull request #1552 from jmorganca/mxyng/lint-test

add lint and test on pull_request

.github/workflows/test.yaml

@@ -0,0 +1,78 @@
+name: test
+
+on:
+  pull_request:
+
+jobs:
+  generate:
+    strategy:
+      matrix:
+        os: [ubuntu-latest, macos-latest, windows-latest]
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-go@v4
+        with:
+          go-version: '1.20'
+          cache: true
+      - if: ${{ startsWith(matrix.os, 'windows-') }}
+        shell: pwsh
+        run: |
+          $path = vswhere -latest -products * -requires Microsoft.VisualStudio.Component.VC.Tools.x86.x64 -property installationPath
+          if ($path) {
+              $path = join-path $path 'Common7\Tools\vsdevcmd.bat'
+              if (test-path $path) {
+                  cmd /s /c """$path"" $args && set" | where { $_ -match '(\w+)=(.*)' } | foreach {
+                      echo "$($Matches[1])=$($Matches[2])" | Out-File -FilePath $Env:GITHUB_ENV -Encoding utf8 -Append
+                  }
+              }
+          }
+
+          echo "C:\Program Files\Git\usr\bin" | Out-File -FilePath $Env:GITHUB_PATH -Encoding utf8 -Append
+      - run: go get ./...
+      - run: go generate -x ./...
+      - uses: actions/upload-artifact@v4
+        with:
+          name: ${{ matrix.os }}-libraries
+          path: |
+            llm/llama.cpp/build/**/lib/*
+  lint:
+    needs: generate
+    strategy:
+      matrix:
+        os: [ubuntu-latest, macos-latest, windows-latest]
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          submodules: recursive
+      - uses: actions/setup-go@v4
+        with:
+          go-version: '1.20'
+          cache: false
+      - uses: actions/download-artifact@v4
+        with:
+          name: ${{ matrix.os }}-libraries
+          path: llm/llama.cpp/build
+      - uses: golangci/golangci-lint-action@v3
+  test:
+    needs: generate
+    strategy:
+      matrix:
+        os: [ubuntu-latest, macos-latest, windows-latest]
+    runs-on: ${{ matrix.os }}
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          submodules: recursive
+      - uses: actions/setup-go@v4
+        with:
+          go-version: '1.20'
+          cache: true
+      - run: go get
+      - uses: actions/download-artifact@v4
+        with:
+          name: ${{ matrix.os }}-libraries
+          path: llm/llama.cpp/build
+      - run: go build
+      - run: go test -v ./...

.golangci.yaml

@@ -0,0 +1,27 @@
+run:
+  timeout: 5m
+linters:
+  enable:
+    - asasalint
+    - bidichk
+    - bodyclose
+    - containedctx
+    - contextcheck
+    - exportloopref
+    - gocheckcompilerdirectives
+    # FIXME: for some reason this errors on windows
+    # - gofmt
+    # - goimports
+    - misspell
+    - nilerr
+    - unused
+linters-settings:
+  errcheck:
+    # exclude the following functions since we don't generally
+    # need to be concerned with the returned errors
+    exclude-functions:
+      - encoding/binary.Read
+      - (*os.File).Seek
+      - (*bufio.Writer).WriteString
+      - (*github.com/spf13/pflag.FlagSet).Set
+      - (*github.com/jmorganca/ollama/llm.readSeekOffset).Seek

cmd/interactive.go

@@ -238,10 +238,7 @@ func generateInteractive(cmd *cobra.Command, opts generateOptions) error {
 						usageParameters()
 						continue
 					}
-					var params []string
-					for _, p := range args[3:] {
-						params = append(params, p)
-					}
+					params := args[3:]
 					fp, err := api.FormatParams(map[string][]string{args[2]: params})
 					if err != nil {
 						fmt.Printf("Couldn't set parameter: %q\n\n", err)

llm/ggml.go

@@ -98,9 +98,9 @@ func (c *containerLORA) Name() string {
 	return "ggla"
 }
 
-func (c *containerLORA) Decode(ro *readSeekOffset) (model, error) {
+func (c *containerLORA) Decode(rso *readSeekOffset) (model, error) {
 	var version uint32
-	binary.Read(ro, binary.LittleEndian, &version)
+	binary.Read(rso, binary.LittleEndian, &version)
 
 	switch version {
 	case 1:
@@ -111,7 +111,7 @@ func (c *containerLORA) Decode(ro *readSeekOffset) (model, error) {
 	c.version = version
 
 	// remaining file contents aren't decoded
-	ro.Seek(0, io.SeekEnd)
+	rso.Seek(0, io.SeekEnd)
 
 	return nil, nil
 }

llm/llama.go

@@ -1,17 +1,11 @@
 package llm
 
 import (
-	"bytes"
-	"context"
 	_ "embed"
-	"errors"
 	"fmt"
-	"os"
-	"os/exec"
 	"time"
 
 	"github.com/jmorganca/ollama/api"
-	"github.com/jmorganca/ollama/format"
 )
 
 const jsonGrammar = `
@@ -42,51 +36,12 @@ number ::= ("-"? ([0-9] | [1-9] [0-9]*)) ("." [0-9]+)? ([eE] [-+]? [0-9]+)? ws
 ws ::= ([ \t\n] ws)?
 `
 
-type Running struct {
-	Port          int
-	Cmd           *exec.Cmd
-	Cancel        context.CancelFunc
-	*StatusWriter // captures error messages from the llama runner process
-}
-
 type ImageData struct {
 	Data []byte `json:"data"`
 	ID   int    `json:"id"`
 }
 
-var (
-	errNvidiaSMI     = errors.New("warning: gpu support may not be enabled, check that you have installed GPU drivers: nvidia-smi command failed")
-	errAvailableVRAM = errors.New("not enough VRAM available, falling back to CPU only")
-	payloadMissing   = fmt.Errorf("expected dynamic library payloads not included in this build of ollama")
-)
-
-// StatusWriter is a writer that captures error messages from the llama runner process
-type StatusWriter struct {
-	ErrCh      chan error
-	LastErrMsg string
-}
-
-func NewStatusWriter() *StatusWriter {
-	return &StatusWriter{
-		ErrCh: make(chan error, 1),
-	}
-}
-
-func (w *StatusWriter) Write(b []byte) (int, error) {
-	var errMsg string
-	if _, after, ok := bytes.Cut(b, []byte("error:")); ok {
-		errMsg = string(bytes.TrimSpace(after))
-	} else if _, after, ok := bytes.Cut(b, []byte("CUDA error")); ok {
-		errMsg = string(bytes.TrimSpace(after))
-	}
-
-	if errMsg != "" {
-		w.LastErrMsg = errMsg
-		w.ErrCh <- fmt.Errorf("llama runner: %s", errMsg)
-	}
-
-	return os.Stderr.Write(b)
-}
+var payloadMissing = fmt.Errorf("expected dynamic library payloads not included in this build of ollama")
 
 type prediction struct {
 	Content string `json:"content"`
@@ -102,9 +57,7 @@ type prediction struct {
 	}
 }
 
-const maxBufferSize = 512 * format.KiloByte
 const maxRetries = 3
-const retryDelay = 1 * time.Second
 
 type PredictOpts struct {
 	Prompt  string

llm/llm.go

@@ -47,7 +47,7 @@ func New(workDir, model string, adapters, projectors []string, opts api.Options)
 	kv := 2 * 2 * int64(opts.NumCtx) * int64(ggml.NumLayers()) * int64(ggml.NumEmbed()) * int64(ggml.NumHeadKv()) / int64(ggml.NumHead())
 
 	// this amount is the overhead + tensors in memory
-	// TODO: get this from the llama.cpp's graph calcluations instead of
+	// TODO: get this from the llama.cpp's graph calculations instead of
 	// estimating it's 1/6 * kv_cache_size * num_gqa
 	graph := int64(ggml.NumGQA()) * kv / 6
 

progress/progress.go

@@ -77,7 +77,7 @@ func (p *Progress) Add(key string, state State) {
 	p.states = append(p.states, state)
 }
 
-func (p *Progress) render() error {
+func (p *Progress) render() {
 	p.mu.Lock()
 	defer p.mu.Unlock()
 
@@ -101,8 +101,6 @@ func (p *Progress) render() error {
 	}
 
 	p.pos = len(p.states)
-
-	return nil
 }
 
 func (p *Progress) start() {

readline/history.go

@@ -23,7 +23,7 @@ type History struct {
 func NewHistory() (*History, error) {
 	h := &History{
 		Buf:      arraylist.New(),
-		Limit:    100, //resizeme
+		Limit:    100, // resizeme
 		Autosave: true,
 		Enabled:  true,
 	}
@@ -49,7 +49,7 @@ func (h *History) Init() error {
 
 	h.Filename = path
 
-	f, err := os.OpenFile(path, os.O_CREATE|os.O_RDONLY, 0600)
+	f, err := os.OpenFile(path, os.O_CREATE|os.O_RDONLY, 0o600)
 	if err != nil {
 		if errors.Is(err, os.ErrNotExist) {
 			return nil
@@ -84,7 +84,7 @@ func (h *History) Add(l []rune) {
 	h.Compact()
 	h.Pos = h.Size()
 	if h.Autosave {
-		h.Save()
+		_ = h.Save()
 	}
 }
 
@@ -132,7 +132,7 @@ func (h *History) Save() error {
 
 	tmpFile := h.Filename + ".tmp"
 
-	f, err := os.OpenFile(tmpFile, os.O_CREATE|os.O_WRONLY|os.O_TRUNC|os.O_APPEND, 0666)
+	f, err := os.OpenFile(tmpFile, os.O_CREATE|os.O_WRONLY|os.O_TRUNC|os.O_APPEND, 0o600)
 	if err != nil {
 		return err
 	}

readline/readline.go

@@ -72,6 +72,7 @@ func (i *Instance) Readline() (string, error) {
 	if err != nil {
 		return "", err
 	}
+	// nolint: errcheck
 	defer UnsetRawMode(fd, termios)
 
 	buf, _ := NewBuffer(i.Prompt)

readline/readline_unix.go

@@ -11,7 +11,7 @@ func handleCharCtrlZ(fd int, termios *Termios) (string, error) {
 		return "", err
 	}
 
-	syscall.Kill(0, syscall.SIGSTOP)
+	_ = syscall.Kill(0, syscall.SIGSTOP)
 
 	// on resume...
 	return "", nil

server/download.go

@@ -98,7 +98,7 @@ func (b *blobDownload) Prepare(ctx context.Context, requestURL *url.URL, opts *R
 
 		b.Total, _ = strconv.ParseInt(resp.Header.Get("Content-Length"), 10, 64)
 
-		var size = b.Total / numDownloadParts
+		size := b.Total / numDownloadParts
 		switch {
 		case size < minDownloadPartSize:
 			size = minDownloadPartSize
@@ -132,13 +132,13 @@ func (b *blobDownload) run(ctx context.Context, requestURL *url.URL, opts *Regis
 	defer blobDownloadManager.Delete(b.Digest)
 	ctx, b.CancelFunc = context.WithCancel(ctx)
 
-	file, err := os.OpenFile(b.Name+"-partial", os.O_CREATE|os.O_RDWR, 0644)
+	file, err := os.OpenFile(b.Name+"-partial", os.O_CREATE|os.O_RDWR, 0o644)
 	if err != nil {
 		return err
 	}
 	defer file.Close()
 
-	file.Truncate(b.Total)
+	_ = file.Truncate(b.Total)
 
 	g, inner := errgroup.WithContext(ctx)
 	g.SetLimit(numDownloadParts)
@@ -246,7 +246,7 @@ func (b *blobDownload) readPart(partName string) (*blobDownloadPart, error) {
 }
 
 func (b *blobDownload) writePart(partName string, part *blobDownloadPart) error {
-	partFile, err := os.OpenFile(partName, os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0644)
+	partFile, err := os.OpenFile(partName, os.O_CREATE|os.O_RDWR|os.O_TRUNC, 0o644)
 	if err != nil {
 		return err
 	}
@@ -340,6 +340,7 @@ func downloadBlob(ctx context.Context, opts downloadOpts) error {
 			return err
 		}
 
+		// nolint: contextcheck
 		go download.Run(context.Background(), requestURL, opts.regOpts)
 	}
 

server/images.go

@@ -747,6 +747,7 @@ func deleteUnusedLayers(skipModelPath *ModelPath, deleteMap map[string]struct{},
 		// save (i.e. delete from the deleteMap) any files used in other manifests
 		manifest, _, err := GetManifest(fmp)
 		if err != nil {
+			// nolint: nilerr
 			return nil
 		}
 

server/manifests.go

@@ -26,9 +26,9 @@ func WriteManifest(name string, config *Layer, layers []*Layer) error {
 		return err
 	}
 
-	if err := os.MkdirAll(filepath.Dir(manifestPath), 0755); err != nil {
+	if err := os.MkdirAll(filepath.Dir(manifestPath), 0o755); err != nil {
 		return err
 	}
 
-	return os.WriteFile(manifestPath, b.Bytes(), 0644)
+	return os.WriteFile(manifestPath, b.Bytes(), 0o644)
 }

server/modelpath.go

@@ -46,7 +46,7 @@ func ParseModelPath(name string) ModelPath {
 		name = after
 	}
 
-	parts := strings.Split(name, string(os.PathSeparator))
+	parts := strings.Split(name, "/")
 	switch len(parts) {
 	case 3:
 		mp.Registry = parts[0]

server/routes.go

@@ -198,7 +198,8 @@ func GenerateHandler(c *gin.Context) {
 		c.JSON(http.StatusOK, api.GenerateResponse{
 			CreatedAt: time.Now().UTC(),
 			Model:     req.Model,
-			Done:      true})
+			Done:      true,
+		})
 		return
 	}
 
@@ -710,7 +711,7 @@ func GetModelInfo(req api.ShowRequest) (*api.ShowResponse, error) {
 
 func ListModelsHandler(c *gin.Context) {
 	models := make([]api.ModelResponse, 0)
-	fp, err := GetManifestPath()
+	manifestsPath, err := GetManifestPath()
 	if err != nil {
 		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 		return
@@ -740,13 +741,15 @@ func ListModelsHandler(c *gin.Context) {
 
 	walkFunc := func(path string, info os.FileInfo, _ error) error {
 		if !info.IsDir() {
-			dir, file := filepath.Split(path)
-			dir = strings.Trim(strings.TrimPrefix(dir, fp), string(os.PathSeparator))
-			tag := strings.Join([]string{dir, file}, ":")
+			path, tag := filepath.Split(path)
+			model := strings.Trim(strings.TrimPrefix(path, manifestsPath), string(os.PathSeparator))
+			modelPath := strings.Join([]string{model, tag}, ":")
+			canonicalModelPath := strings.ReplaceAll(modelPath, string(os.PathSeparator), "/")
 
-			resp, err := modelResponse(tag)
+			resp, err := modelResponse(canonicalModelPath)
 			if err != nil {
-				log.Printf("skipping file: %s", fp)
+				log.Printf("skipping file: %s", canonicalModelPath)
+				// nolint: nilerr
 				return nil
 			}
 
@@ -757,7 +760,7 @@ func ListModelsHandler(c *gin.Context) {
 		return nil
 	}
 
-	if err := filepath.Walk(fp, walkFunc); err != nil {
+	if err := filepath.Walk(manifestsPath, walkFunc); err != nil {
 		c.JSON(http.StatusInternalServerError, gin.H{"error": err.Error()})
 		return
 	}

server/routes_test.go

@@ -193,13 +193,12 @@ func Test_Routes(t *testing.T) {
 		}
 
 		resp, err := httpSrv.Client().Do(req)
-		defer resp.Body.Close()
 		assert.Nil(t, err)
+		defer resp.Body.Close()
 
 		if tc.Expected != nil {
 			tc.Expected(t, resp)
 		}
 
 	}
-
 }

server/upload.go

@@ -88,7 +88,7 @@ func (b *blobUpload) Prepare(ctx context.Context, requestURL *url.URL, opts *Reg
 		return nil
 	}
 
-	var size = b.Total / numUploadParts
+	size := b.Total / numUploadParts
 	switch {
 	case size < minUploadPartSize:
 		size = minUploadPartSize
@@ -395,6 +395,7 @@ func uploadBlob(ctx context.Context, mp ModelPath, layer *Layer, opts *RegistryO
 			return err
 		}
 
+		// nolint: contextcheck
 		go upload.Run(context.Background(), opts)
 	}