Skip to content

Commit

Permalink
feat: better archives relative paths (#3656)
Browse files Browse the repository at this point in the history
with this patch, a config like:

```yaml
archives:
  - format: tar.gz
    # this name template makes the OS and Arch compatible with the results of uname.
    name_template: >-
      {{ .ProjectName }}_
      {{- title .Os }}_
      {{- if eq .Arch "amd64" }}x86_64
      {{- else if eq .Arch "386" }}i386
      {{- else }}{{ .Arch }}{{ end }}
      {{- if .Arm }}v{{ .Arm }}{{ end }}
    rlcp: true
    files:
      - src: "build/**/*"
        dst: .

nfpms:
  - package_name: foo
    contents:
      - src: "build/**/*"
        dst: usr/share/foo
    formats:
      - apk

```

will eval this:

<img width="1384" alt="CleanShot 2022-12-21 at 22 21 00@2x"
src="https://user-images.githubusercontent.com/245435/209034244-7c31b5f7-cfcd-4825-bb2f-7dd463c5286a.png">

as much as I would like to make this the default, it would be a breaking
change, so we really can't do it.

If `dst` is empty, it'll have the same behavior as before (no rlcp), and
if `strip_parent` is set, it will also still have the same behavior.
Finally, if the format is binary, `rlcp` is ignored too (as it doesn't
make sense).

So, this only changes if:
- your format is not binary; and
- you have files with `src` and `dst` set

Then, goreleaser will warn you to set `rlcp: true`.

## todo

- [x] docs
- [x] more tests probably
- [x] any ideas for a better name for the new config option?

fixes #3655

Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
  • Loading branch information
caarlos0 committed Dec 27, 2022
1 parent 46fdb55 commit a209757
Show file tree
Hide file tree
Showing 10 changed files with 253 additions and 20 deletions.
1 change: 1 addition & 0 deletions .goreleaser.yaml
Expand Up @@ -142,6 +142,7 @@ archives:
builds_info:
group: root
owner: root
rlcp: true
files:
- README.md
- LICENSE.md
Expand Down
63 changes: 58 additions & 5 deletions internal/archivefiles/archivefiles.go
Expand Up @@ -2,7 +2,10 @@
package archivefiles

import (
"errors"
"fmt"
"io/fs"
"os"
"path/filepath"
"sort"
"time"
Expand All @@ -14,7 +17,7 @@ import (
)

// Eval evaluates the given list of files to their final form.
func Eval(template *tmpl.Template, files []config.File) ([]config.File, error) {
func Eval(template *tmpl.Template, rlcp bool, files []config.File) ([]config.File, error) {
var result []config.File
for _, f := range files {
replaced, err := template.Apply(f.Source)
Expand Down Expand Up @@ -46,10 +49,20 @@ func Eval(template *tmpl.Template, files []config.File) ([]config.File, error) {
}
}

// the prefix may not be a complete path or may use glob patterns, in that case use the parent directory
prefix := replaced
if _, err := os.Stat(prefix); errors.Is(err, fs.ErrNotExist) || fileglob.ContainsMatchers(prefix) {
prefix = filepath.Dir(longestCommonPrefix(files))
}

for _, file := range files {
dst, err := destinationFor(f, prefix, file, rlcp)
if err != nil {
return nil, err
}
result = append(result, config.File{
Source: file,
Destination: destinationFor(f, file),
Destination: dst,
Info: f.Info,
})
}
Expand Down Expand Up @@ -83,9 +96,49 @@ func unique(in []config.File) []config.File {
return result
}

func destinationFor(f config.File, path string) string {
func destinationFor(f config.File, prefix, path string, rlcp bool) (string, error) {
if f.StripParent {
return filepath.Join(f.Destination, filepath.Base(path))
return filepath.Join(f.Destination, filepath.Base(path)), nil
}

if rlcp && f.Destination != "" {
relpath, err := filepath.Rel(prefix, path)
if err != nil {
// since prefix is a prefix of src a relative path should always be found
return "", err
}
return filepath.ToSlash(filepath.Join(f.Destination, relpath)), nil
}

return filepath.Join(f.Destination, path), nil
}

// longestCommonPrefix returns the longest prefix of all strings the argument
// slice. If the slice is empty the empty string is returned.
// copied from nfpm
func longestCommonPrefix(strs []string) string {
if len(strs) == 0 {
return ""
}
lcp := strs[0]
for _, str := range strs {
lcp = strlcp(lcp, str)
}
return lcp
}

// copied from nfpm
func strlcp(a, b string) string {
var min int
if len(a) > len(b) {
min = len(b)
} else {
min = len(a)
}
for i := 0; i < min; i++ {
if a[i] != b[i] {
return a[0:i]
}
}
return filepath.Join(f.Destination, path)
return a[0:min]
}
108 changes: 96 additions & 12 deletions internal/archivefiles/archivefiles_test.go
Expand Up @@ -14,13 +14,49 @@ import (
func TestEval(t *testing.T) {
now := time.Now().Truncate(time.Second)
ctx := context.New(config.Project{
Env: []string{"OWNER=carlos"},
Env: []string{"OWNER=carlos", "FOLDER=d"},
})
ctx.Git.CommitDate = now
tmpl := tmpl.New(ctx)

t.Run("invalid glob", func(t *testing.T) {
_, err := Eval(tmpl, false, []config.File{
{
Source: "../testdata/**/nope.txt",
Destination: "var/foobar/d.txt",
},
})
require.Error(t, err)
})

t.Run("templated src", func(t *testing.T) {
result, err := Eval(tmpl, false, []config.File{
{
Source: "./testdata/**/{{ .Env.FOLDER }}.txt",
Destination: "var/foobar/d.txt",
},
})
require.NoError(t, err)
require.Equal(t, []config.File{
{
Source: "testdata/a/b/c/d.txt",
Destination: "var/foobar/d.txt/testdata/a/b/c/d.txt",
},
}, result)
})

t.Run("templated src error", func(t *testing.T) {
_, err := Eval(tmpl, false, []config.File{
{
Source: "./testdata/**/{{ .Env.NOPE }}.txt",
Destination: "var/foobar/d.txt",
},
})
testlib.RequireTemplateError(t, err)
})

t.Run("templated info", func(t *testing.T) {
result, err := Eval(tmpl, []config.File{
result, err := Eval(tmpl, false, []config.File{
{
Source: "./testdata/**/d.txt",
Destination: "var/foobar/d.txt",
Expand Down Expand Up @@ -49,7 +85,7 @@ func TestEval(t *testing.T) {

t.Run("template info errors", func(t *testing.T) {
t.Run("owner", func(t *testing.T) {
_, err := Eval(tmpl, []config.File{{
_, err := Eval(tmpl, false, []config.File{{
Source: "./testdata/**/d.txt",
Destination: "var/foobar/d.txt",
Info: config.FileInfo{
Expand All @@ -59,7 +95,7 @@ func TestEval(t *testing.T) {
testlib.RequireTemplateError(t, err)
})
t.Run("group", func(t *testing.T) {
_, err := Eval(tmpl, []config.File{{
_, err := Eval(tmpl, false, []config.File{{
Source: "./testdata/**/d.txt",
Destination: "var/foobar/d.txt",
Info: config.FileInfo{
Expand All @@ -69,7 +105,7 @@ func TestEval(t *testing.T) {
testlib.RequireTemplateError(t, err)
})
t.Run("mtime", func(t *testing.T) {
_, err := Eval(tmpl, []config.File{{
_, err := Eval(tmpl, false, []config.File{{
Source: "./testdata/**/d.txt",
Destination: "var/foobar/d.txt",
Info: config.FileInfo{
Expand All @@ -79,7 +115,7 @@ func TestEval(t *testing.T) {
testlib.RequireTemplateError(t, err)
})
t.Run("mtime format", func(t *testing.T) {
_, err := Eval(tmpl, []config.File{{
_, err := Eval(tmpl, false, []config.File{{
Source: "./testdata/**/d.txt",
Destination: "var/foobar/d.txt",
Info: config.FileInfo{
Expand All @@ -91,7 +127,7 @@ func TestEval(t *testing.T) {
})

t.Run("single file", func(t *testing.T) {
result, err := Eval(tmpl, []config.File{
result, err := Eval(tmpl, false, []config.File{
{
Source: "./testdata/**/d.txt",
Destination: "var/foobar/d.txt",
Expand All @@ -107,8 +143,43 @@ func TestEval(t *testing.T) {
}, result)
})

t.Run("rlcp", func(t *testing.T) {
result, err := Eval(tmpl, true, []config.File{{
Source: "./testdata/a/**/*",
Destination: "foo/bar",
}})

require.NoError(t, err)
require.Equal(t, []config.File{
{Source: "testdata/a/b/a.txt", Destination: "foo/bar/a.txt"},
{Source: "testdata/a/b/c/d.txt", Destination: "foo/bar/c/d.txt"},
}, result)
})

t.Run("rlcp empty destination", func(t *testing.T) {
result, err := Eval(tmpl, true, []config.File{{
Source: "./testdata/a/**/*",
}})

require.NoError(t, err)
require.Equal(t, []config.File{
{Source: "testdata/a/b/a.txt", Destination: "testdata/a/b/a.txt"},
{Source: "testdata/a/b/c/d.txt", Destination: "testdata/a/b/c/d.txt"},
}, result)
})

t.Run("rlcp no results", func(t *testing.T) {
result, err := Eval(tmpl, true, []config.File{{
Source: "./testdata/abc/**/*",
Destination: "foo/bar",
}})

require.NoError(t, err)
require.Empty(t, result)
})

t.Run("strip parent plays nicely with destination omitted", func(t *testing.T) {
result, err := Eval(tmpl, []config.File{{Source: "./testdata/a/b", StripParent: true}})
result, err := Eval(tmpl, false, []config.File{{Source: "./testdata/a/b", StripParent: true}})

require.NoError(t, err)
require.Equal(t, []config.File{
Expand All @@ -118,7 +189,7 @@ func TestEval(t *testing.T) {
})

t.Run("strip parent plays nicely with destination as an empty string", func(t *testing.T) {
result, err := Eval(tmpl, []config.File{{Source: "./testdata/a/b", Destination: "", StripParent: true}})
result, err := Eval(tmpl, false, []config.File{{Source: "./testdata/a/b", Destination: "", StripParent: true}})

require.NoError(t, err)
require.Equal(t, []config.File{
Expand All @@ -128,7 +199,7 @@ func TestEval(t *testing.T) {
})

t.Run("match multiple files within tree without destination", func(t *testing.T) {
result, err := Eval(tmpl, []config.File{{Source: "./testdata/a"}})
result, err := Eval(tmpl, false, []config.File{{Source: "./testdata/a"}})

require.NoError(t, err)
require.Equal(t, []config.File{
Expand All @@ -139,7 +210,7 @@ func TestEval(t *testing.T) {
})

t.Run("match multiple files within tree specific destination", func(t *testing.T) {
result, err := Eval(tmpl, []config.File{
result, err := Eval(tmpl, false, []config.File{
{
Source: "./testdata/a",
Destination: "usr/local/test",
Expand Down Expand Up @@ -188,7 +259,7 @@ func TestEval(t *testing.T) {
})

t.Run("match multiple files within tree specific destination stripping parents", func(t *testing.T) {
result, err := Eval(tmpl, []config.File{
result, err := Eval(tmpl, false, []config.File{
{
Source: "./testdata/a",
Destination: "usr/local/test",
Expand Down Expand Up @@ -227,3 +298,16 @@ func TestEval(t *testing.T) {
}, result)
})
}

func TestStrlcp(t *testing.T) {
for k, v := range map[string][2]string{
"/var/": {"/var/lib/foo", "/var/share/aaa"},
"/var/lib/": {"/var/lib/foo", "/var/lib/share/aaa"},
"/usr/share/": {"/usr/share/lib", "/usr/share/bin"},
"/usr/": {"/usr/share/lib", "/usr/bin"},
} {
t.Run(k, func(t *testing.T) {
require.Equal(t, k, strlcp(v[0], v[1]))
})
}
}
5 changes: 4 additions & 1 deletion internal/pipe/archive/archive.go
Expand Up @@ -59,6 +59,9 @@ func (Pipe) Default(ctx *context.Context) error {
if archive.ID == "" {
archive.ID = "default"
}
if !archive.RLCP && archive.Format != "binary" && len(archive.Files) > 0 {
deprecate.NoticeCustom(ctx, "archives.rlcp", "`{{ .Property }}` will be the default soon, check {{ .URL }} for more info")
}
if len(archive.Files) == 0 {
archive.Files = []config.File{
{Source: "license*"},
Expand Down Expand Up @@ -185,7 +188,7 @@ func doCreate(ctx *context.Context, arch config.Archive, binaries []*artifact.Ar
a = NewEnhancedArchive(a, wrap)
defer a.Close()

files, err := archivefiles.Eval(template, arch.Files)
files, err := archivefiles.Eval(template, arch.RLCP, arch.Files)
if err != nil {
return fmt.Errorf("failed to find files to archive: %w", err)
}
Expand Down
18 changes: 18 additions & 0 deletions internal/pipe/archive/archive_test.go
Expand Up @@ -771,6 +771,7 @@ func TestDefault(t *testing.T) {
require.NotEmpty(t, ctx.Config.Archives[0].NameTemplate)
require.Equal(t, "tar.gz", ctx.Config.Archives[0].Format)
require.NotEmpty(t, ctx.Config.Archives[0].Files)
require.False(t, ctx.Config.Archives[0].RLCP)
}

func TestDefaultSet(t *testing.T) {
Expand All @@ -791,9 +792,25 @@ func TestDefaultSet(t *testing.T) {
require.NoError(t, Pipe{}.Default(ctx))
require.Equal(t, "foo", ctx.Config.Archives[0].NameTemplate)
require.Equal(t, "zip", ctx.Config.Archives[0].Format)
require.False(t, ctx.Config.Archives[0].RLCP)
require.Equal(t, config.File{Source: "foo"}, ctx.Config.Archives[0].Files[0])
}

func TestDefaultNoFiles(t *testing.T) {
ctx := &context.Context{
Config: config.Project{
Archives: []config.Archive{
{
Format: "tar.gz",
},
},
},
}
require.NoError(t, Pipe{}.Default(ctx))
require.Equal(t, defaultNameTemplate, ctx.Config.Archives[0].NameTemplate)
require.False(t, ctx.Config.Archives[0].RLCP)
}

func TestDefaultFormatBinary(t *testing.T) {
ctx := &context.Context{
Config: config.Project{
Expand All @@ -806,6 +823,7 @@ func TestDefaultFormatBinary(t *testing.T) {
}
require.NoError(t, Pipe{}.Default(ctx))
require.Equal(t, defaultBinaryNameTemplate, ctx.Config.Archives[0].NameTemplate)
require.False(t, ctx.Config.Archives[0].RLCP)
}

func TestFormatFor(t *testing.T) {
Expand Down
11 changes: 10 additions & 1 deletion internal/pipe/sourcearchive/source.go
Expand Up @@ -10,6 +10,7 @@ import (
"github.com/caarlos0/log"
"github.com/goreleaser/goreleaser/internal/archivefiles"
"github.com/goreleaser/goreleaser/internal/artifact"
"github.com/goreleaser/goreleaser/internal/deprecate"
"github.com/goreleaser/goreleaser/internal/git"
"github.com/goreleaser/goreleaser/internal/tmpl"
"github.com/goreleaser/goreleaser/pkg/archive"
Expand Down Expand Up @@ -68,7 +69,11 @@ func (Pipe) Run(ctx *context.Context) (err error) {
Source: f,
})
}
files, err := archivefiles.Eval(tmpl.New(ctx), append(ff, ctx.Config.Source.Files...))
files, err := archivefiles.Eval(
tmpl.New(ctx),
ctx.Config.Source.RLCP,
append(ff, ctx.Config.Source.Files...),
)
if err != nil {
return err
}
Expand Down Expand Up @@ -107,5 +112,9 @@ func (Pipe) Default(ctx *context.Context) error {
if archive.NameTemplate == "" {
archive.NameTemplate = "{{ .ProjectName }}-{{ .Version }}"
}

if archive.Enabled && !archive.RLCP {
deprecate.NoticeCustom(ctx, "source.rlcp", "`{{ .Property }}` will be the default soon, check {{ .URL }} for more info")
}
return nil
}

0 comments on commit a209757

Please sign in to comment.