feat: replace zglob with fileglob #1889
Conversation
Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/goreleaser/goreleaser/96j3jwue0 |
pull-request-size
bot
added
the
size/M
caarlos0
added
automerge
enhancement
caarlos0
changed the title
pull-request-size
bot
added
the
size/M
Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>
|
LGTM, but quick question on why not use https://golang.org/pkg/path/filepath/#Glob |
|
Previously zglob was used which allows for zsh-style globs. If users use these additional features their packages won't build with filepath/glob. |
|
I can review this PR later today. |
There was a problem hiding this comment.
Looks like @djgilcrease already asked the question I would also asked and @erikgeiser answered it, so I don't have much to add here.
As for the breaking change - I would certainly call it out in the changelog and will defer to your decision about versioning or any other impact.
FWIW I would usually judge the severity (who would be impacted) based on how reasonable the previous behaviour/expectation was. As you mentioned the ./**/*.txt behaviour currently just seems wrong anyway, so not a huge deal in my opinion. 🤷 In fact I would even go as far as documenting it as a bug fix (alongside the "breaking change" entry in the changelog).
Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>
|
depends on goreleaser/fileglob#2 |
| @@ -157,8 +157,6 @@ func TestRunPipe(t *testing.T) { | |||
| t, | |||
| []string{ | |||
| fmt.Sprintf("README.%s.md", os), | |||
| "foo/bar", | |||
There was a problem hiding this comment.
not sure if this was wrong before or if it is wrong now...
There was a problem hiding this comment.
as far as I tested, archives still work ok even if missing the "dirs"...
| @@ -163,8 +163,6 @@ func TestRunPipeWithIDsThenFilters(t *testing.T) { | |||
| require.True(t, client.UploadedFile) | |||
| require.Contains(t, client.UploadedFileNames, "bin.deb") | |||
| require.Contains(t, client.UploadedFileNames, "bin.tar.gz") | |||
| require.Contains(t, client.UploadedFileNames, "release1.golden") | |||
There was a problem hiding this comment.
this was wrong before...
| @@ -426,7 +424,7 @@ func TestRunPipeInvalidGlob(t *testing.T) { | |||
| "ID": "default", | |||
| }, | |||
| }) | |||
| require.EqualError(t, Pipe{}.Run(ctx), `failed to find files to archive: globbing failed for pattern [x-]: file does not exist`) | |||
| require.EqualError(t, Pipe{}.Run(ctx), `failed to find files to archive: globbing failed for pattern [x-]: unexpected end of input`) | |||
Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>
pull-request-size
bot
removed
the
size/M
pull-request-size
bot
added
the
size/L
Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #1889 +/- ##
==========================================
- Coverage 81.75% 81.67% -0.08%
==========================================
Files 73 73
Lines 3803 3803
==========================================
- Hits 3109 3106 -3
- Misses 571 573 +2
- Partials 123 124 +1
Continue to review full report at Codecov.
|
Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>
Signed-off-by: Carlos Alexandro Becker <caarlos0@gmail.com>
caarlos0
changed the title
|
Just a random note here that this change broke one of our builds silently in a way that caused an outage on production. I guess that's on us for not pinning our goreleaser version in our Github action, but this was a surprise. |
|
Yeah same here, we had a low frequency deployment fail today.
So apparently with the new fileglob it is now searching folders zglob previously did not. In our case it was a restricted folder so it was stopped and that failed the deployment. Seems like a workaround is to set the license to something other than default so it does not search for this pattern which is the default setting. |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
BREAKING CHANGE: may have unintended side-effects.
so we can be consistent and have a single glob dependency, as per goreleaser/nfpm#244
one case that came out of this is, imagine a tree like:
and a glob
./**/*.txt.On the previous glob library it would return both txt files, now it will only return
bar.txt, which I think is correct, since the./**/implies that it should be in a subfolder.