-
-
Notifications
You must be signed in to change notification settings - Fork 251
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
Making grep -q -e -e usable on solaris #3788
Conversation
replacing -q by /dev/null repalcing double -e by two greps
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Putting a block on this purely because it was a problem I reported and I want to verify that it works before anyone merges it ;-) It also may require egrep
instead of grep
but the linter will likely whinge about that.
Will test tomorrow - thanks for the quick turnaround @judovana
Sure!
…--
Mgr. Jiri Vanek
***@***.***
---------- Původní e-mail ----------
Od: Stewart X Addison ***@***.***>
Komu: adoptium/temurin-build ***@***.***>
Kopie: judovana ***@***.***>, Mention ***@***.***>
Datum: 2. 5. 2024 20:46:19
Předmět: Re: [adoptium/temurin-build] Making grep -q -e -e usable on solaris
(PR #3788)
"
@sxa requested changes on this pull request.
Putting a block on this purely because it was a problem I reported and I
want to verify that it works before anyone merges it ;-)
Will test tomorrow - thanks for the quick turnaround @judovana
(https://github.com/judovana)
—
Reply to this email directly, view it on GitHub
(#3788 (review))
, or unsubscribe
(https://github.com/notifications/unsubscribe-auth/AAWFCS2DRW2YGTUNLBEY5XDZAKCXLAVCNFSM6AAAAABHEHJG66VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDAMZWGYYDKMRUGM)
.
You are receiving this because you were mentioned. Message ID: <adoptium/
***@***.***>
"
|
ERROR the following files don't have a valid license header: Is unrelated |
Thanx @karianna I was wondering.... |
@sxa I had make it much nicer to move that or to regex. The only disadvantage is that it is now unreadable:( and -E is not reliable over distros. But with that explanation in echo just on another line it should be ok |
I'm getting:
from this change on Solaris - that's from A bit if diagnoses shows that EDIT: The first version with the two separate checks looks ok though - |
Hm echo jdk8u | grep '^\(jdk\|jdk[0-9]\{1,3\}[u]\{0,1\}\)$' jdk8u I do nothave soalris around to try it up. maybe -E and removal of slashes can help, Maybe try it in the previous 669a1a5 revision. Generally I'mm running out of ideas. |
Yeah I think I'd vote for using the previous revision at this point - it's a bit messy using two variables but it's reasonably clear and should work everywhere. |
The 669a1a5 really worked for you??? and others had not? /me amazed. WIll remove the second commit then |
This reverts commit 8695cbc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 669a1a5 really worked for you??? and others had not? /me amazed. Will remove the second commit then
Purely on the basis of running this on solaris: echo jdk8u | grep "^jdk[0-9]\\{1,3\\}[u]\\{0,1\\}$"
is true ($?=0) and the same using echo jdk8x
at the start gives $?=1
so I think that's good enough if you're happy with it :-)
Failing github check seems to be covered by #3789 and not relevant to this PR |
Ok. added comment on this topic to the code. Ok by me, plesae review/merge as necessary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggested wording change
Co-authored-by: Martijn Verburg <[email protected]>
Thank you. I keep learning. |
replacing -q by /dev/null
repalcing double -e by or in regex itself