ffi: port semi-colon fix for riscv64 (and others)#63794
Open
sxa wants to merge 1 commit into
Open
Conversation
Signed-off-by: Stewart X Addison <sxa@ibm.com>
Collaborator
|
Review requested:
|
10 tasks
Contributor
|
@sxa I'm not sure about this. I guess we should wait for a official libffi fix and then run the update, or am I wrong? |
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 pulls across the fix which has been accepted upstream at libffi/libffi#968 and will therefore be in any future versions of libffi that we take.
@ShogunPanda I'm not sure if we have implemented any specific rules/restrictions on taking fixes like this so hopefully this is ok to go in as a patch since I had it merged upstream before raising this.
Fixes nodejs/unofficial-builds#235 which has the explanation of when and why it occurs.
While this includes all platforms which would have the same issue I'll note that loongarch does not show the problem in the unofficial builds on that platform but that is likely because it is using
gccand nodeclang. Also noting that we haveloongarch64in this PR as opposed to the upstreamloongarchdirectory - that is because of this upstream change where the directory was renamed.FYI @nodejs/platform-loong64 @nodejs/platform-riscv64