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

[Backport Request] Fix ReactCommon Break for Windows #33086

Merged
merged 1 commit into from Feb 19, 2022

Conversation

chiaramooney
Copy link
Contributor

@chiaramooney chiaramooney commented Feb 10, 2022

Summary

Changes to MapBuffer code in aaff15c...d287598 broke build for Windows. Errors included incompatible type conversions, the use of __attribute__(__packed__) which is only supported by GCC and Clang, and the usage of designated initializers which are only supported on C++20.

Changes here restore build on Windows.

Original PR: #33047

Changelog

[General] [Fixed] - Fix build break on Windows with ReactCommon

Test Plan

React Native project built on Windows and passes react-native-windows repository pipeline. These edits are currently merged into the main branch of react-native-windows.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 10, 2022
@pull-bot
Copy link

Fails
🚫

node` failed.

Log

Error:  RequestError [HttpError]: Must have admin rights to Repository.
    at /root/react-native/bots/node_modules/@octokit/request/dist-node/index.js:86:21
    at processTicksAndRejections (internal/process/task_queues.js:95:5) {
  status: 403,
  response: {
    url: 'https://api.github.com/repos/facebook/react-native/issues/33086/labels',
    status: 403,
    headers: {
      'access-control-allow-origin': '*',
      'access-control-expose-headers': 'ETag, Link, Location, Retry-After, X-GitHub-OTP, X-RateLimit-Limit, X-RateLimit-Remaining, X-RateLimit-Used, X-RateLimit-Resource, X-RateLimit-Reset, X-OAuth-Scopes, X-Accepted-OAuth-Scopes, X-Poll-Interval, X-GitHub-Media-Type, X-GitHub-SSO, X-GitHub-Request-Id, Deprecation, Sunset',
      connection: 'close',
      'content-encoding': 'gzip',
      'content-security-policy': "default-src 'none'",
      'content-type': 'application/json; charset=utf-8',
      date: 'Thu, 10 Feb 2022 22:51:22 GMT',
      'referrer-policy': 'origin-when-cross-origin, strict-origin-when-cross-origin',
      server: 'GitHub.com',
      'strict-transport-security': 'max-age=31536000; includeSubdomains; preload',
      'transfer-encoding': 'chunked',
      vary: 'Accept-Encoding, Accept, X-Requested-With',
      'x-accepted-oauth-scopes': '',
      'x-content-type-options': 'nosniff',
      'x-frame-options': 'deny',
      'x-github-media-type': 'github.v3; format=json',
      'x-github-request-id': '9466:93BD:102CBB3:254C192:620596EA',
      'x-oauth-scopes': 'public_repo',
      'x-ratelimit-limit': '5000',
      'x-ratelimit-remaining': '4992',
      'x-ratelimit-reset': '1644537081',
      'x-ratelimit-resource': 'core',
      'x-ratelimit-used': '8',
      'x-xss-protection': '0'
    },
    data: {
      message: 'Must have admin rights to Repository.',
      documentation_url: 'https://docs.github.com/rest/reference/issues#add-labels-to-an-issue'
    }
  },
  request: {
    method: 'POST',
    url: 'https://api.github.com/repos/facebook/react-native/issues/33086/labels',
    headers: {
      accept: 'application/vnd.github.v3+json',
      'user-agent': 'octokit.js/16.43.2 Node.js/14.18.1 (Linux 5.11; x64)',
      authorization: 'token [REDACTED]',
      'content-type': 'application/json; charset=utf-8'
    },
    body: '{"labels":["Pick Request"]}',
    request: { hook: [Function: bound bound register] }
  }
}
danger-results://tmp/danger-results.json

Generated by 🚫 dangerJS against 78452b5

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 9bc6c0f
Branch: main

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,125,225 -62,802
android hermes armeabi-v7a 7,726,129 -62,329
android hermes x86 8,495,399 -62,712
android hermes x86_64 8,447,534 -62,973
android jsc arm64-v8a 9,790,660 -66,418
android jsc armeabi-v7a 8,776,400 -66,002
android jsc x86 9,757,431 -66,343
android jsc x86_64 10,353,597 -66,642

Base commit: 9bc6c0f
Branch: main

@lunaleaps
Copy link
Contributor

Interesting, looks like the DangerJS bot needs to be using an account that has write permissions to add the label (unrelated to this PR -- sorry most folks haven't been creating pick request PRs so we're still fixing up the process -- so thank you for doing so!)

K going to land this (heads up @ShikaSD)

@lunaleaps lunaleaps merged commit 1b2b219 into facebook:0.68-stable Feb 19, 2022
@ShikaSD
Copy link
Contributor

ShikaSD commented Feb 19, 2022

Thanks, @lunaleaps, I was planning to pick this manually for the next rc anyways ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants