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

Globbing Replacement #232

Closed
rtpt-erikgeiser opened this issue Oct 12, 2020 · 10 comments · Fixed by #244
Closed

Globbing Replacement #232

rtpt-erikgeiser opened this issue Oct 12, 2020 · 10 comments · Fixed by #244
Labels
bug Something isn't working

Comments

@rtpt-erikgeiser
Copy link
Contributor

rtpt-erikgeiser commented Oct 12, 2020

As far as I am aware, it is currently impossible to package files or directories that are enclosed in curly braces (such as {foo}). An example would be Firefox extensions which use {UUID} directories. The reason for this is that https://github.com/mattn/go-zglob does not seem to support wildcard escaping and because also because of a bug (mattn/go-zglob#32).

A possible alternative to replace https://github.com/mattn/go-zglob with is https://github.com/gobwas/glob which supports the same features, escaping and more extensive tests. However, this library only does string matching and the directory walking will have to be performed in nfpm.

@rtpt-erikgeiser
Copy link
Contributor Author

The maintainer of https://github.com/mattn/go-zglob has just added escaping support and the issue should be fixed if the library is upgraded. However, I'm not a fan of zglob's approach of building a regular expression. Maybe we can discuss moving to https://github.com/gobwas/glob at some point @caarlos0, wdyt?

@caarlos0
Copy link
Member

hmm, interesting... we can test the other lib too, yes... can you open a PR?

@caarlos0 caarlos0 added the bug Something isn't working label Oct 13, 2020
@rtpt-erikgeiser
Copy link
Contributor Author

I just tried to upgrade zglob but it is still buggy, I created another issue over there. If the maintainer fixes it we should just upgrade first and at some point I will also create a PR with https://github.com/gobwas/glob which we can then try out and compare before merging it.

@caarlos0
Copy link
Member

awesome, thanks!

@erikgeiser
Copy link
Member

It's fixed now, at least for my use case. However, I think asterisks still cannot be escaped but I'm not sure.

For a long term solution I still think we should switch to https://github.com/gobwas/glob which uses a proper lexer, parser and compiler instead of building a regular expression. The only issue is that it only does matching, so we'll have to implement file walking ourselves. Another thing I would like to add is a an option to disable globbing altogether (we can just use glob.QuoteMeta for this). This option would make nfpm far more predictable when using it as part of a bigger build system that auto-populates the files field.

What do you think @caarlos0?

@caarlos0
Copy link
Member

caarlos0 commented Nov 3, 2020

sounds like a plan @erikgeiser

@erikgeiser erikgeiser changed the title Globbing is broken at least for files with curly braces Globbing Replacement Nov 3, 2020
@nhooyr
Copy link

nhooyr commented Nov 16, 2020

This seems to have broken recursive globs like the following:

https://github.com/cdr/code-server/blob/1826399cd230acc1f62556a5af7bba9c25a03a25/ci/build/nfpm.yaml#L19

./release-standalone/**/*: "/usr/lib/code-server/"

Only files two levels below the top level are copied now whereas before all files were.

Caused a regression in code-server as nfpm wouldn't include the files in ./release-standalone/* but only ./release-standalone/*/**/*. i.e only files in subdirectories of the top level.

See coder/code-server#2310

@nhooyr
Copy link

nhooyr commented Nov 16, 2020

For now we've rolled back nfpm to v1.9.0

@caarlos0
Copy link
Member

addressed @nhooyr's comments in #256

@github-actions
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants