[WIP] Fix ClangCL ARM64 cross-compilation#53003
Closed
StefanStojanovic wants to merge 3 commits intonodejs:mainfrom
Closed
[WIP] Fix ClangCL ARM64 cross-compilation#53003StefanStojanovic wants to merge 3 commits intonodejs:mainfrom
StefanStojanovic wants to merge 3 commits intonodejs:mainfrom
Conversation
nodejs-github-bot
added
dependencies
srl295
requested changes
May 15, 2024
Member
There was a problem hiding this comment.
After this is reviewed and discussed, I plan to open a PR in the ICU and try to land it there.
hi! Please file a ticket (or see ICU-21633 and linked tickets) and PR to ICU first. I'm -1 on innovating here, it's just going to get dropped the next time through. This PR modifies a generated file, which is ignored under some contexts.
Depending on the affected upstream projects and their release cycles and updates, we may need to use the patches on our side for some time
I sent you a note on Slack on that topic. If there does need to be a patch, I'd rather it was a backport of a PR actually landed upstream.
7 tasks
7 tasks
Contributor
Author
|
Closing this one, since all of the changes from this PR already landed in the intended way. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a WIP for fixing the ClangCL cross-compilation to ARM64. It consists of 2 dependency updates that should be done upstream and 1 change in Node.js itself to make everything work:
A fix in ICU a6932ad - To increase the chances of this landing upstream, the change is done by adding an option for this use case so it doesn't break any previously experienced behavior. After this is reviewed and discussed, I plan to open a PR in the ICU and try to land it there.
A fix in FP16 f8b52b5 - This already landed in the base repo. Since that project is a dependency of the V8, a CL to update it in V8 will be opened.
Node.js changes 176f1c6 - Once everything else is done, the change in the main repo is a simple one.
NOTE: Depending on the affected upstream projects and their release cycles and updates, we may need to use the patches on our side for some time.
cc @targos @lemire @nodejs/i18n-api
Refs: #52809