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

Try to fix MSVC Buck Build #1278

Closed
wants to merge 1 commit into from

Conversation

NickGerleman
Copy link
Contributor

@NickGerleman NickGerleman commented May 4, 2023

#1269 broke building Yoga Buck targets with MSVC. The build logs suggest this is because headers may be double included from exported_headers and headers, causing redefinition. We are not saved on the MSVC build by #pragma once since headers are imported from different paths (but the Clang build seems okay with this).

This diff attempts to isolate headers so that nothing may be double exported. This is done in a little bit of a tricky way, since Starlark doesn't seem to support globs in the form of {a,b}, and Yoga's directory structure does not separate public and private headers cleanly (we should fix this, but it needs to be changed in multiple build systems at once which I want to do not in this change).

We also get a side effect that every file which isn't a public header needs to use the namespaced form to refer to a public header, even if in the same directory. Though this is going to be needed anyway if we end up wanting to move the public headers to a new directory.

Differential Revision: D45552325

@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45552325

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request May 4, 2023
Summary:
Pull Request resolved: facebook#37242

X-link: facebook/yoga#1278

Reviewed By: yungsters

Differential Revision: D45552325

fbshipit-source-id: 550734cfab44e41f30baf183c9ce24594fe477fd
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request May 4, 2023
Summary:
X-link: facebook/react-native#37242

Pull Request resolved: facebook#1278

Reviewed By: yungsters

Differential Revision: D45552325

fbshipit-source-id: 549a9c01a45bc06b344ee4319528aadd3fc8326e
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45552325

NickGerleman added a commit to NickGerleman/yoga that referenced this pull request May 4, 2023
Summary:
X-link: facebook/react-native#37242

Pull Request resolved: facebook#1278

Reviewed By: yungsters

Differential Revision: D45552325

fbshipit-source-id: 881e83b72166900fc475bcf77ec2849759766894
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45552325

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request May 4, 2023
Summary:
Pull Request resolved: facebook#37242

X-link: facebook/yoga#1278

Reviewed By: yungsters

Differential Revision: D45552325

fbshipit-source-id: 4393303641b7e42d1c24e0c4479870552666caba
NickGerleman added a commit to NickGerleman/litho that referenced this pull request May 4, 2023
Summary:
X-link: facebook/react-native#37242

X-link: facebook/yoga#1278

facebook/yoga#1269 broke building Yoga Buck targets with MSVC. The build logs suggest this is because headers may be double included from `exported_headers` and `headers`, causing redefinition. We are not saved on the MSVC build by `#pragma once` since headers are imported from different paths (but the Clang build seems okay with this).

This diff attempts to isolate headers so that nothing may be double exported. This is done in a little bit of a tricky way, since Starlark doesn't seem to support globs in the form of `{a,b}`, and Yoga's directory structure does not separate public and private headers cleanly (we should fix this, but it needs to be changed in multiple build systems at once which I want to do not in this change).

We also get a side effect that every file which isn't a public header needs to use the namespaced form to refer to a public header, even if in the same directory. Though this is going to be needed anyway if we end up wanting to move the public headers to a new directory.

Reviewed By: yungsters

Differential Revision: D45552325

fbshipit-source-id: 407fda6b062692c1016e157b066cac286c29ec49
NickGerleman added a commit to NickGerleman/yoga that referenced this pull request May 4, 2023
Summary:
X-link: facebook/react-native#37242

Pull Request resolved: facebook#1278

facebook#1269 broke building Yoga Buck targets with MSVC. The build logs suggest this is because headers may be double included from `exported_headers` and `headers`, causing redefinition. We are not saved on the MSVC build by `#pragma once` since headers are imported from different paths (but the Clang build seems okay with this).

This diff attempts to isolate headers so that nothing may be double exported. This is done in a little bit of a tricky way, since Starlark doesn't seem to support globs in the form of `{a,b}`, and Yoga's directory structure does not separate public and private headers cleanly (we should fix this, but it needs to be changed in multiple build systems at once which I want to do not in this change).

We also get a side effect that every file which isn't a public header needs to use the namespaced form to refer to a public header, even if in the same directory. Though this is going to be needed anyway if we end up wanting to move the public headers to a new directory.

Reviewed By: yungsters

Differential Revision: D45552325

fbshipit-source-id: f11852242c600044e4fb6da3aa7b522da248ca4b
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45552325

NickGerleman added a commit to NickGerleman/react-native that referenced this pull request May 4, 2023
Summary:
Pull Request resolved: facebook#37242

X-link: facebook/yoga#1278

facebook/yoga#1269 broke building Yoga Buck targets with MSVC. The build logs suggest this is because headers may be double included from `exported_headers` and `headers`, causing redefinition. We are not saved on the MSVC build by `#pragma once` since headers are imported from different paths (but the Clang build seems okay with this).

This diff attempts to isolate headers so that nothing may be double exported. This is done in a little bit of a tricky way, since Starlark doesn't seem to support globs in the form of `{a,b}`, and Yoga's directory structure does not separate public and private headers cleanly (we should fix this, but it needs to be changed in multiple build systems at once which I want to do not in this change).

We also get a side effect that every file which isn't a public header needs to use the namespaced form to refer to a public header, even if in the same directory. Though this is going to be needed anyway if we end up wanting to move the public headers to a new directory.

Reviewed By: yungsters

Differential Revision: D45552325

fbshipit-source-id: e35afac1ff001057715e2039473be89c7ecb0bc8
Summary:
X-link: facebook/react-native#37242

Pull Request resolved: facebook#1278

Reviewed By: yungsters

Differential Revision: D45552325

fbshipit-source-id: 9d22fa0ffa24a5994c789ada98e07070ede4b9e2
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D45552325

NickGerleman added a commit to NickGerleman/litho that referenced this pull request May 4, 2023
Summary:
X-link: facebook/react-native#37242

X-link: facebook/yoga#1278

Reviewed By: yungsters

Differential Revision: D45552325

fbshipit-source-id: 3df09db5aeec30566e2d8a60105cdb59ebc7409c
NickGerleman added a commit to NickGerleman/react-native that referenced this pull request May 4, 2023
Summary:
Pull Request resolved: facebook#37242

X-link: facebook/yoga#1278

Reviewed By: yungsters

Differential Revision: D45552325

fbshipit-source-id: 723cacbebcf952776135181f9068d113da2e925d
facebook-github-bot pushed a commit to facebook/litho that referenced this pull request May 4, 2023
Summary:
X-link: facebook/react-native#37242

X-link: facebook/yoga#1278

Reviewed By: yungsters

Differential Revision: D45552325

fbshipit-source-id: 5687e8ec27a7a70df66e2f89e800210e3ce21ba3
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 70153cc.

facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request May 4, 2023
Summary:
Pull Request resolved: #37242

X-link: facebook/yoga#1278

Reviewed By: yungsters

Differential Revision: D45552325

fbshipit-source-id: 5687e8ec27a7a70df66e2f89e800210e3ce21ba3
jeongshin pushed a commit to jeongshin/react-native that referenced this pull request May 7, 2023
Summary:
Pull Request resolved: facebook#37242

X-link: facebook/yoga#1278

Reviewed By: yungsters

Differential Revision: D45552325

fbshipit-source-id: 5687e8ec27a7a70df66e2f89e800210e3ce21ba3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants