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

feat: better archives relative paths #3656

Merged
merged 15 commits into from Dec 27, 2022
Merged

feat: better archives relative paths #3656

merged 15 commits into from Dec 27, 2022

Conversation

caarlos0
Copy link
Member

@caarlos0 caarlos0 commented Dec 22, 2022

with this patch, a config like:

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

  • docs
  • more tests probably
  • any ideas for a better name for the new config option?

fixes #3655

Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Dec 22, 2022
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Dec 22, 2022
@caarlos0 caarlos0 self-assigned this Dec 22, 2022
@caarlos0 caarlos0 added bug Something isn't working enhancement New feature or request labels Dec 22, 2022
@vercel vercel bot temporarily deployed to Preview December 22, 2022 01:30 Inactive
@caarlos0
Copy link
Member Author

caarlos0 commented Dec 22, 2022

One idea:

  • make it warns that relative: true will be default soon (maybe specify a version, e.g. v1.21.0)
  • wait 6mo (or until the specified version)
  • make it the default
  • deprecation warning for 6 months so people can remove the option from their configs
  • remove it completely

would take 1 year to phase it all, but IMHO is the safest way to do so, if we decide to go that way.

Thoughts?

@codecov
Copy link

codecov bot commented Dec 22, 2022

Codecov Report

Merging #3656 (51192d2) into main (46fdb55) will increase coverage by 0.00%.
The diff coverage is 87.03%.

❗ Current head 51192d2 differs from pull request most recent head 0bb05f4. Consider uploading reports for the commit 0bb05f4 to get more accurate results

@@           Coverage Diff           @@
##             main    #3656   +/-   ##
=======================================
  Coverage   83.59%   83.59%           
=======================================
  Files         118      118           
  Lines        9875     9919   +44     
=======================================
+ Hits         8255     8292   +37     
- Misses       1306     1311    +5     
- Partials      314      316    +2     
Impacted Files Coverage Δ
pkg/config/config.go 95.21% <ø> (ø)
internal/archivefiles/archivefiles.go 92.55% <83.33%> (-7.45%) ⬇️
internal/pipe/archive/archive.go 92.88% <100.00%> (+0.09%) ⬆️
internal/pipe/sourcearchive/source.go 80.76% <100.00%> (+1.89%) ⬆️
internal/artifact/artifact.go 85.37% <0.00%> (-0.05%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview December 22, 2022 12:46 Inactive
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview December 22, 2022 12:50 Inactive
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview December 22, 2022 12:53 Inactive
@ioga
Copy link

ioga commented Dec 22, 2022

One idea:

* make it warns that `relative: true` will be default soon (maybe specify a version, e.g. v1.21.0)

* wait 6mo (or until the specified version)

* make it the default

* deprecation warning for 6 months so people can remove the option from their configs

* remove it completely

would take 1 year to phase it all, but IMHO is the safest way to do so, if we decide to go that way.

Thoughts?

since relative: true will be a new default global behavior, I'd suggest making it an option on archives (or at the top level), instead of having it to be an option on each entry under archives -> files, because you probably don't want to let people mix old and new behavior in their configurations, as it'll make the migration harder.

@caarlos0
Copy link
Member Author

yes, that makes sense @ioga.

Will do it! Thank you!

@vercel vercel bot temporarily deployed to Preview December 23, 2022 12:23 Inactive
@vercel vercel bot temporarily deployed to Preview December 23, 2022 12:44 Inactive
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview December 23, 2022 13:17 Inactive
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
@vercel
Copy link

vercel bot commented Dec 23, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
goreleaser 🔄 Building (Inspect) Dec 27, 2022 at 8:31PM (UTC)

@vercel vercel bot temporarily deployed to Preview December 23, 2022 13:32 Inactive
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview December 23, 2022 13:35 Inactive
Signed-off-by: Carlos A Becker <caarlos0@users.noreply.github.com>
@vercel vercel bot temporarily deployed to Preview December 27, 2022 20:32 Inactive
@caarlos0
Copy link
Member Author

fwiw, renamed to rlcp (relative longest common path) so its less confusing, and purposefully require a little bit of reading.

@caarlos0 caarlos0 merged commit a209757 into main Dec 27, 2022
@caarlos0 caarlos0 deleted the archive branch December 27, 2022 20:42
@github-actions github-actions bot added this to the v1.14.0 milestone Dec 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

different behavior for extra file trees included in archives and nfpms.
2 participants