From a209757ad2b47cb4a6f7a1c6b7dbe5dcf4fece15 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Tue, 27 Dec 2022 17:42:55 -0300 Subject: [PATCH] feat: better archives relative paths (#3656) 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: CleanShot 2022-12-21 at 22 21 00@2x 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 --- .goreleaser.yaml | 1 + internal/archivefiles/archivefiles.go | 63 +++++++++++- internal/archivefiles/archivefiles_test.go | 108 ++++++++++++++++++--- internal/pipe/archive/archive.go | 5 +- internal/pipe/archive/archive_test.go | 18 ++++ internal/pipe/sourcearchive/source.go | 11 ++- pkg/config/config.go | 2 + www/docs/customization/archive.md | 12 +++ www/docs/customization/source.md | 12 ++- www/docs/deprecations.md | 41 ++++++++ 10 files changed, 253 insertions(+), 20 deletions(-) diff --git a/.goreleaser.yaml b/.goreleaser.yaml index c6921bbc00c..46901cb95ad 100644 --- a/.goreleaser.yaml +++ b/.goreleaser.yaml @@ -142,6 +142,7 @@ archives: builds_info: group: root owner: root + rlcp: true files: - README.md - LICENSE.md diff --git a/internal/archivefiles/archivefiles.go b/internal/archivefiles/archivefiles.go index fd8b96cefdd..11217e6df7e 100644 --- a/internal/archivefiles/archivefiles.go +++ b/internal/archivefiles/archivefiles.go @@ -2,7 +2,10 @@ package archivefiles import ( + "errors" "fmt" + "io/fs" + "os" "path/filepath" "sort" "time" @@ -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) @@ -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, }) } @@ -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] } diff --git a/internal/archivefiles/archivefiles_test.go b/internal/archivefiles/archivefiles_test.go index 48717bdb24c..0bf098d4cad 100644 --- a/internal/archivefiles/archivefiles_test.go +++ b/internal/archivefiles/archivefiles_test.go @@ -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", @@ -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{ @@ -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{ @@ -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{ @@ -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{ @@ -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", @@ -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{ @@ -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{ @@ -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{ @@ -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", @@ -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", @@ -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])) + }) + } +} diff --git a/internal/pipe/archive/archive.go b/internal/pipe/archive/archive.go index 32c2dd5d0ea..3d10f43bd5f 100644 --- a/internal/pipe/archive/archive.go +++ b/internal/pipe/archive/archive.go @@ -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*"}, @@ -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) } diff --git a/internal/pipe/archive/archive_test.go b/internal/pipe/archive/archive_test.go index 27c7b4dc953..c3dd25cc8f2 100644 --- a/internal/pipe/archive/archive_test.go +++ b/internal/pipe/archive/archive_test.go @@ -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) { @@ -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{ @@ -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) { diff --git a/internal/pipe/sourcearchive/source.go b/internal/pipe/sourcearchive/source.go index 102713bb1d3..44d118a9b3c 100644 --- a/internal/pipe/sourcearchive/source.go +++ b/internal/pipe/sourcearchive/source.go @@ -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" @@ -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 } @@ -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 } diff --git a/pkg/config/config.go b/pkg/config/config.go index e23528cc1d4..6b546637cca 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -488,6 +488,7 @@ type Archive struct { FormatOverrides []FormatOverride `yaml:"format_overrides,omitempty" json:"format_overrides,omitempty"` WrapInDirectory string `yaml:"wrap_in_directory,omitempty" json:"wrap_in_directory,omitempty" jsonschema:"oneof_type=string;boolean"` StripParentBinaryFolder bool `yaml:"strip_parent_binary_folder,omitempty" json:"strip_parent_binary_folder,omitempty"` + RLCP bool `yaml:"rlcp,omitempty" json:"rlcp,omitempty"` Files []File `yaml:"files,omitempty" json:"files,omitempty"` Meta bool `yaml:"meta,omitempty" json:"meta,omitempty"` AllowDifferentBinaryCount bool `yaml:"allow_different_binary_count,omitempty" json:"allow_different_binary_count,omitempty"` @@ -903,6 +904,7 @@ type Source struct { Enabled bool `yaml:"enabled,omitempty" json:"enabled,omitempty"` PrefixTemplate string `yaml:"prefix_template,omitempty" json:"prefix_template,omitempty"` Files []File `yaml:"files,omitempty" json:"files,omitempty"` + RLCP bool `yaml:"rlcp,omitempty" json:"rlcp,omitempty"` } // Project includes all project configuration. diff --git a/www/docs/customization/archive.md b/www/docs/customization/archive.md index 74f258c2903..e56da67cbba 100644 --- a/www/docs/customization/archive.md +++ b/www/docs/customization/archive.md @@ -69,6 +69,16 @@ archives: # Since: v1.11. strip_parent_binary_folder: true + + # This will make the destination paths be relative to the longest common + # path prefix between all the files matched and the source glob. + # Enabling this essentially mimic the behavior of nfpm's contents section. + # It will be the default by June 2023. + # + # Default: false + # Since: v1.14. + rlcp: true + # Can be used to change the archive formats for specific GOOSs. # Most common use case is to archive as zip on Windows. # Default is empty. @@ -89,9 +99,11 @@ archives: # a more complete example, check the globbing deep dive below - src: '*.md' dst: docs + # Strip parent folders when adding files to the archive. # Default: false strip_parent: true + # File info. # Not all fields are supported by all formats available formats. # Defaults to the file info of the actual file if not provided. diff --git a/www/docs/customization/source.md b/www/docs/customization/source.md index 31fa2018f99..2a393d56c14 100644 --- a/www/docs/customization/source.md +++ b/www/docs/customization/source.md @@ -24,6 +24,15 @@ source: # Defaults to empty prefix_template: '{{ .ProjectName }}-{{ .Version }}/' + # This will make the destination paths be relative to the longest common + # path prefix between all the files matched and the source glob. + # Enabling this essentially mimic the behavior of nfpm's contents section. + # It will be the default by June 2023. + # + # Default: false + # Since: v1.14. + rlcp: true + # Additional files/template/globs you want to add to the source archive. # # Default: empty. @@ -38,9 +47,11 @@ source: # a more complete example, check the globbing deep dive below - src: '*.md' dst: docs + # Strip parent folders when adding files to the archive. # Default: false strip_parent: true + # File info. # Not all fields are supported by all formats available formats. # Defaults to the file info of the actual file if not provided. @@ -50,7 +61,6 @@ source: mode: 0644 # format is `time.RFC3339Nano` mtime: 2008-01-02T15:04:05Z - ``` !!! tip diff --git a/www/docs/deprecations.md b/www/docs/deprecations.md index 6ddd12a64b7..22d392c312e 100644 --- a/www/docs/deprecations.md +++ b/www/docs/deprecations.md @@ -36,6 +36,47 @@ Description. --> + +### archives.rlcp + +> since 2022-12-23 (v1.14.0) + +This is not so much a deprecation property (yet), as it is a default behavior +change. + +The usage of relative longest common path (`rlcp`) on the destination side of +archive files will be enabled by default by June 2023. Then, this option will be +deprecated, and you will have another 6 months (until December 2023) to remove +it. + +For now, if you want to keep the old behavior, no action is required, but it +would be nice to have your opinion [here][rlcp-discuss]. + +[rlcp-discuss]: https://github.com/goreleaser/goreleaser/discussions/3659 + +If you want to make sure your releases will keep working properly, you can +enable this option and test it out with +`goreleaser release --snapshot --rm-dist`. + +=== "After" + ``` yaml + archives: + - + rlcp: true + ``` + +### source.rlcp + +> since 2022-12-23 (v1.14.0) + +Same as [`archives.rlcp`](#archivesrlcp). + +=== "After" + ``` yaml + source: + rlcp: true + ``` + ### archives.replacements > since 2022-11-24 (v1.14.0)