Skip to content

build: Make lint-locale-dependence.sh work with BSD grep (avoid empty subexpressions) #13720

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.

Sign up for GitHub

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

Jump to bottom
Closed
practicalswift wants to merge 2 commits into from
Closed

build: Make lint-locale-dependence.sh work with BSD grep (avoid empty subexpressions) #13720

practicalswift wants to merge 2 commits into from

Conversation

@practicalswift
Copy link
Contributor

@practicalswift practicalswift commented Jul 19, 2018

Make lint-locale-dependence.sh work with BSD grep by avoiding empty subexpressions.

macOS High Sierra:

$ grep --version
grep (BSD grep) 2.5.1-FreeBSD
$ echo foo | grep -E '^foo(_r|_s)?$'
foo
$ echo foo | grep -E '^foo(|_r|_s)$'
grep: empty (sub)expression

Ubuntu 18.04 LTS:

$ grep --version
grep (GNU grep) 3.1
$ echo foo | grep -E '^foo(_r|_s)?$'
foo
$ echo foo | grep -E '^foo(|_r|_s)$'
foo

Reported by @masonicboom in #13708 (comment). Thanks!

@practicalswift practicalswift mentioned this pull request Jul 19, 2018
Closed
@Empact
Copy link
Contributor

Empact commented Jul 19, 2018

utACK 5a3eefc4 how much work would it be to make the scripts fully compatible with BSD grep? Would be nice to remove the gnu grep requirement.

@Empact
Copy link
Contributor

Empact commented Jul 19, 2018

There's another empty subexpression in lint-shell-locale.sh, line 18

@fanquake fanquake added the Tests label Jul 19, 2018
@practicalswift practicalswift force-pushed the bsd-grep-compatibility branch from 5a3eefc to 3b04ac2 Compare July 20, 2018 09:40
@practicalswift
Copy link
Contributor Author

practicalswift commented Jul 20, 2018

@Empact Fixed the GNU:Ism in lint-shell-locale.sh. Do you know of any other incompatibility with BSD grep? I've now added a linter that will help us avoid introducing GNU:isms in our shell scripts :-)

Please re-review!

@practicalswift practicalswift force-pushed the bsd-grep-compatibility branch from 3b04ac2 to 0780029 Compare July 20, 2018 09:46
@laanwj
Copy link
Member

laanwj commented Jul 20, 2018

Thanks for doing this but...
sigh more shell-lowest-common-denominator-porting
I wish we just bit the bullet ported this stuff to Python so that it's, by definition, not dependent on external tools.

@Empact Empact mentioned this pull request Jul 20, 2018
Merged
@Empact
Copy link
Contributor

Empact commented Jul 20, 2018 •
edited
Loading

As an alternative to the linter here, I set up running the linter checks on Mac via Travis, #13728.

@DrahtBot
Copy link
Contributor

DrahtBot commented Jul 22, 2018

Note to reviewers: This pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

@practicalswift
Copy link
Contributor Author

practicalswift commented Aug 6, 2018 •
edited
Loading

I prefer @Empact:s solution in #13728 which runs the linter chers on Mac via Travis

Closing this PR since it won't be relevant when #13728 has been merged.

@practicalswift practicalswift deleted the bsd-grep-compatibility branch April 10, 2021 19:35
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Aug 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

Tests

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@practicalswift @Empact @laanwj @DrahtBot @fanquake