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

Unified Builds of jsapi and jsglue #553

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Redfire75369
Copy link
Contributor

@Redfire75369 Redfire75369 commented Jan 30, 2025

Unified flags, as well as the usage of cc and bindgen for jsapi and jsglue.
Reorganised the build.rs primarily for the unification, but there are some changes in ordering and module organisation.
I'm not 100% sure on the ordering, so if you would like, that can be changed as well.

Resolves #435 through the removal of -x c++ and addition of --driver-mode=cl on windows from the invocations of bindgen. Replaces #437.

Note the failures in aarch64-pc-windows-msvc are due to an error in llvm 18 llvm/llvm-project#93235 (see rust-lang/rust-bindgen#2842 also). LLVM 18.1.8 is installed on github actions runners for windows-2022. I have tested that LLVM 19 works correctly on spiderfire.

@sagudev
Copy link
Member

sagudev commented Jan 30, 2025

Note the failures in aarch64-pc-windows-msvc are due an error in llvm 18 llvm/llvm-project#93235 (see rust-lang/rust-bindgen#2842 also). LLVM 18.1.8 is installed on github actions runners for windows-2022. I have tested that LLVM 19 works correctly on spiderfire.

We should pin LLVM to 19 in CI then (but only for aarch64), this is not used in servo so it should not be a problem there.

mozjs-sys/build.rs Outdated Show resolved Hide resolved
mozjs-sys/build.rs Outdated Show resolved Hide resolved
@sagudev
Copy link
Member

sagudev commented Jan 30, 2025

This makes everything much clear and consistent!

Before landing this we should make companion PR to servo to be sure we don't brake anything there.

Unified cc and bindgen Flags
Reorganized Buildscript

Signed-off-by: Redfire <[email protected]>
@Redfire75369 Redfire75369 force-pushed the build/unified branch 3 times, most recently from 021bda7 to 84e15af Compare January 30, 2025 10:43
@Redfire75369 Redfire75369 marked this pull request as ready for review January 30, 2025 10:46
@sagudev
Copy link
Member

sagudev commented Jan 30, 2025

I think this will need mozjs-sys bump.

@Redfire75369
Copy link
Contributor Author

Should it be a minor or patch bump?

@sagudev
Copy link
Member

sagudev commented Jan 30, 2025

Should it be a minor or patch bump?

just the -X part needs to be bumped (I think this is called metadata/build part in semver).

EDIT: 0.128.6-1 -> 0.128.6-2

Signed-off-by: Redfire <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Windows Builds Fail when clang-cl Flags are added to CXXFLAGS
2 participants