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

Fix escaping for Makefile under Windows #10577

Closed
wants to merge 5 commits into from

Conversation

guillep2k
Copy link
Member

Uses different find \( and \) escaping for Windows and Linux.

@guillep2k guillep2k added type/bug topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. labels Mar 2, 2020
@guillep2k guillep2k added this to the 1.12.0 milestone Mar 2, 2020
@jolheiser jolheiser mentioned this pull request Mar 2, 2020
Copy link
Member

@jolheiser jolheiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I felt a need to make this joke a second time.

Gitea is a self-hosted Git service written in Makefile

@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 2, 2020
@silverwind
Copy link
Member

silverwind commented Mar 2, 2020

This does break it on Cygwin for me

/bin/sh: -c: line 0: syntax error near unexpected token `('
/bin/sh: -c: line 0: `find . -type d \\( -path ./node_modules -o -path ./docs -o -path ./public -o -path ./options -o -path ./contrib -o -path ./data \\) -prune -o -type f -name '*.go' -print'

Copy link
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need something better to not break Cygwin.

@silverwind
Copy link
Member

@jolheiser what exact environment are you running in?

@guillep2k
Copy link
Member Author

@jolheiser @silverwind Could you both try with this, please?:

GO_SOURCES ?= $(shell find . -type d '(' -path ./node_modules -o -path ./docs -o -path ./public -o -path ./options -o -path ./contrib -o -path ./data ')' -prune -o -type f -name
 '*.go' -print)

@silverwind
Copy link
Member

silverwind commented Mar 2, 2020

@jolheiser @silverwind Could you both try with this, please?:

Yes, it appears '(' works on Cygwin.

@jolheiser
Copy link
Member

jolheiser commented Mar 2, 2020

@silverwind I run on Windows 10, Git Bash using MINGW64.

And '(' doesn't work in it, either.

@silverwind
Copy link
Member

I'll try this in Git Bash.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Mar 2, 2020
Copy link
Member

@jolheiser jolheiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙇

@silverwind
Copy link
Member

Looks like macOS find does not know -regextype, checking...

@silverwind
Copy link
Member

@guillep2k we need one more change for Darwin and BSD which do not know the -regextype option, but do know -E:

     -E      Interpret regular expressions followed by -regex and -iregex
             primaries as extended (modern) regular expressions rather than
             basic regular expressions (BRE's).  The re_format(7) manual page
             fully describes both formats.

So for Darwin/BSD (-E before path is required):

find -E . -regex '\./(node_modules|docs|public|options|contrib|data)' -prune -o -name "*.go" -type f -print

And all others:

find . -regextype posix-egrep -regex '\./(node_modules|docs|public|options|contrib|data)' -prune -o -name "*.go" -type f -print

I guess we can do something similar to the SED_INPLACE, like FIND_REGEX.

@guillep2k guillep2k closed this Mar 2, 2020
silverwind pushed a commit to silverwind/gitea that referenced this pull request Mar 2, 2020
Continuation of go-gitea#10577

This version also includes an additional fix for Darwin and FreeBSD
which do not accept the `-regextype` option, but only `-E` and the
argument order is specifically required like this for `-E` to work.
zeripath pushed a commit that referenced this pull request Mar 2, 2020
Continuation of #10577

This version also includes an additional fix for Darwin and FreeBSD
which do not accept the `-regextype` option, but only `-E` and the
argument order is specifically required like this for `-E` to work.

Co-authored-by: guillep2k <18600385+guillep2k@users.noreply.github.com>
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/build PR changes how Gitea is built, i.e. regarding Docker or the Makefile type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants