Skip to content

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.

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

Merged
merged 3 commits into from
Mar 29, 2023
+1 −1
Merged

Fix build on macOS #5044

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3e9f9c6
remove ‘m’ flag
sosnovsky Mar 28, 2023
d8d8ce5
update regex
sosnovsky Mar 28, 2023
cf7729b
update regex
sosnovsky Mar 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion scripts/build.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ cp node_modules/@openpgp/web-stream-tools/lib/*.js $OUTDIR/lib/streams
STREAMS_REGEX="s/'\.\/(streams|util|writer|reader|node-conversions)'/'\.\/\1\.js'/g"
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"
Copy link
Collaborator Author

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

Copy link
Contributor

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"

Copy link
Collaborator Author

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?

Copy link
Contributor

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?

Copy link
Collaborator Author

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.

OPENPGP_FILE=$OUTDIR/lib/openpgp.js
if [[ "$OSTYPE" =~ ^darwin ]]; then # macOS needs additional parameter for backup files
sed -i '' -E $STREAMS_REGEX $STREAMS_FILES
Expand Down