-
Notifications
You must be signed in to change notification settings - Fork 51
Fix build on macOS #5044
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 build on macOS #5044
Conversation
scripts/build.sh
Outdated
STREAMS_FILES=$OUTDIR/lib/streams/* | ||
# patch isUint8Array until https://github.com/openpgpjs/web-stream-tools/pull/23 is resolved | ||
ISUINT8ARRAY_REGEX="s/^(\s*)return\x20Uint8Array\.prototype\.isPrototypeOf\(input\);/\1return\x20Uint8Array\.prototype\.isPrototypeOf\(input\)\x20\|\|\x20globalThis\.Uint8Array\.prototype\.isPrototypeOf\(input\);/mg" | ||
ISUINT8ARRAY_REGEX="s/^(\s*)return\x20Uint8Array\.prototype\.isPrototypeOf\(input\);/\1return\x20Uint8Array\.prototype\.isPrototypeOf\(input\)\x20\|\|\x20globalThis\.Uint8Array\.prototype\.isPrototypeOf\(input\);/g" |
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.
@rrrooommmaaa build on macOS failed because of m
flag. I removed it and seems everything works well even without it
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 patch only matters for Firefox, and Firefox is not tested automatically, so we can't tell whether it works well.
Does this statement actually patched the file -- does the new file differ?
The function isUint8Array
should look like this:
function isUint8Array(input) {
return Uint8Array.prototype.isPrototypeOf(input) || globalThis.Uint8Array.prototype.isPrototypeOf(input);
}
The m
flag is supposed to give ^
meaning "beginning of any line" instead of "beginning of file"
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.
I re-checked - files were not patched without m
flag.
But I updated ISUINT8ARRAY_REGEX
, so it's not using ^
, and now files are patched.
What do you think about updated regex, is it ok?
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.
I think it's ok to not use ^
,
It'd be nicer to leave "one or more whitespace" before return
with (\s+)
and \1
in this case.
What is wrong with sed
on MacOS? Is it some old version? Can't it be updated?
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.
I updated regex to use (\s*)
(as it didn't work after changing to (\s+)
)
What is wrong with sed on MacOS? Is it some old version? Can't it be updated?
Yes, macOS includes an older version of sed
which doesn't support some functionality.
It can be updated by installing another version and then linking it to the default sed
command.
I thought about adding this info to our README
, but as this ISUINT8ARRAY_REGEX
patch is temporary (before openpgpjs/web-stream-tools#23 is fixed) I think updating regex should be ok for now.
This PR fixes build on macOS
close #5043
Tests (delete all except exactly one):
To be filled by reviewers
I have reviewed that this PR... (tick whichever items you personally focused on during this review):