Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Fix System.Globalization.Native build on Big Sur #28181

Merged
merged 3 commits into from
Jun 15, 2021
Merged

Fix System.Globalization.Native build on Big Sur #28181

merged 3 commits into from
Jun 15, 2021

Conversation

safern
Copy link
Member

@safern safern commented Jun 15, 2021

Port of: dotnet/runtime#39900 to be able to build globalization shim on big sur.

cc: @danmoseley @tarekgh

@safern safern mentioned this pull request Jun 15, 2021
Merged
@tarekgh
Copy link
Member

tarekgh commented Jun 15, 2021

@safern looks this change causing some compile errors:

[  1%] Linking C shared library System.Globalization.Native.dylib
ld: unknown option: --build-id=sha1
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [src/corefx/System.Globalization.Native/System.Globalization.Native.dylib] Error 1
make[1]: *** [src/corefx/System.Globalization.Native/CMakeFiles/System.Globalization.Native.dir/all] Error 2

@safern
Copy link
Member Author

safern commented Jun 15, 2021

Yes now I'm looking at that error, thanks 😊

@jeffschwMSFT
Copy link
Member

@safern the ci finished with errors.
There appear to be a few issues:
MacOS build failed: https://dev.azure.com/dnceng/public/_build/results?buildId=1188408&view=logs&j=5d252662-176a-5e67-1101-74d1a75ad6cb&t=220080c0-1a38-5406-f449-63dd4d7c3755&l=389
System.Tex.RegularExpressions test was killed: https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-coreclr-refs-pull-28181-merge-53f25af7c12b43648d/System.Text.RegularExpressions.Tests/console.14252b93.log?sv=2019-07-07&se=2021-07-05T18%3A50%3A39Z&sr=c&sp=rl&sig=C8mTAXbM6SrOhCFJhw%2FTk6ppx3qCUtvaPTi%2FsPTfaow%3D
System.Net failures due to cert: https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-coreclr-refs-pull-28181-merge-53f25af7c12b43648d/System.Net.Primitives.UnitTests.Tests/console.d00fe4f0.log?sv=2019-07-07&se=2021-07-05T18%3A50%3A39Z&sr=c&sp=rl&sig=C8mTAXbM6SrOhCFJhw%2FTk6ppx3qCUtvaPTi%2FsPTfaow%3D

cc @wtgodbe

@safern
Copy link
Member Author

safern commented Jun 15, 2021

The build failure I'm hitting on OSX was introduced on: #28143 --build-id flag should only be set on Linux and FreeBSD. cc: @ivdiazsa

@jeffschwMSFT
Copy link
Member

@mangod9

@safern
Copy link
Member Author

safern commented Jun 15, 2021

It seems like this line should not be added:

coreclr/configurecompiler.cmake

Line 344 in 3d4e15b

set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--build-id=sha1 -Wl,-z,relro,-z,now")

Those flags are already set for FreeBSD and Linux down on that file:

coreclr/configurecompiler.cmake

Line 362 in 3d4e15b

set(CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS} -Wl,--build-id=sha1")

The original commit that the PR in question was porting didn't include that line:
https://github.com/dotnet/coreclr/pull/26323/files#diff-bcae6f2abcd5726994b49517ade5a2f939f873deac34644642e8e9f710cef757R344

So it seems like that line was added accidentally.

@safern
Copy link
Member Author

safern commented Jun 15, 2021

I pushed a commit to make configurecompiler.cmake match the change on release/5.0 that was ported.

@safern safern requested review from janvorli, mangod9 and ivdiazsa June 15, 2021 22:06
janvorli
janvorli approved these changes Jun 15, 2021
View reviewed changes
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

@safern
Copy link
Member Author

safern commented Jun 15, 2021

/azp run coreclr-ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

jeffschwMSFT
jeffschwMSFT approved these changes Jun 15, 2021
View reviewed changes
Copy link
Member

@jeffschwMSFT jeffschwMSFT left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

@jeffschwMSFT
Copy link
Member

Merging to unblock the build

@jeffschwMSFT jeffschwMSFT merged commit 53cff17 into release/3.1 Jun 15, 2021
@danmoseley danmoseley deleted the GlobalizationBigSur branch June 15, 2021 23:12
hoyosjs added a commit that referenced this pull request Jul 14, 2021
@hoyosjs @ivdiazsa
@wtgodbe @safern
Create 3.1.17 crossdac merge (#28185)
cae34e4 
* Update branding to 3.1.17 (#28173)

* Port from 5.0: Fix Position Independent Code in CMake files (#28143)

* CoreCLR PR26323 Port: Fix PIE options

* Added missing PIE and RELRO compilation flags.

* Merged PR 15832: Port from 5.0: Fix Position Independent Code in CMake files (#28143)

Port from 5.0: Fix Position Independent Code in CMake files (#28143)

* CoreCLR PR26323 Port: Fix PIE options

* Added missing PIE and RELRO compilation flags.

* Fix System.Globalization.Native build on Big Sur (#28181)

* Fix System.Globalization.Native build on Big Sur

* Fix build

* Add flags for Linux

* [release/3.1] Handle Counter Polling Interval of 0 (#28180)

* Backport  dotnet/runtime#53836

* Fix test build

* update test

Co-authored-by: Jan Jahoda <[email protected]>
Co-authored-by: Ivan Diaz Sanchez <[email protected]>
Co-authored-by: Will Godbe <[email protected]>
Co-authored-by: Santiago Fernandez Madero <[email protected]>
Co-authored-by: John Salem <[email protected]>
hoyosjs added a commit that referenced this pull request Aug 11, 2021
@hoyosjs @ivdiazsa
@wtgodbe @safern @Anipik @dotnet-bot @dotnet-maestro
Crossdac v3.1.18 (#28195)
e51df22 
* Update branding to 3.1.17 (#28173)

* Port from 5.0: Fix Position Independent Code in CMake files (#28143)

* CoreCLR PR26323 Port: Fix PIE options

* Added missing PIE and RELRO compilation flags.

* Merged PR 15832: Port from 5.0: Fix Position Independent Code in CMake files (#28143)

Port from 5.0: Fix Position Independent Code in CMake files (#28143)

* CoreCLR PR26323 Port: Fix PIE options

* Added missing PIE and RELRO compilation flags.

* Fix System.Globalization.Native build on Big Sur (#28181)

* Fix System.Globalization.Native build on Big Sur

* Fix build

* Add flags for Linux

* [release/3.1] Handle Counter Polling Interval of 0 (#28180)

* Backport  dotnet/runtime#53836

* Fix test build

* update test

* update branding to 3.1.18 (#28182)

* Update dependencies from https://github.com/dotnet/core-setup build 20210609.1 (#28178)

Microsoft.NETCore.App
 From Version 3.1.9-servicing.20459.3 -> To Version 3.1.17-servicing.21309.1

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>

* Merged PR 15716: [release/3.1] Use user read+write access for coredump file descriptor open

* [release/3.1] #28183 Fix OSX native dependency installation

* Avoid upgrading packages that are explicitly installed.
* Use a brewfile
* Change shebang to use bash and directly execute.

* [release/3.1] Update dependencies from dotnet/corefx (#28179)

[release/3.1] Update dependencies from dotnet/corefx


 - Add 3.1 AzDO feeds to the test build to ensure that the test build can correctly restore the CoreFX build.

It seems that the new CoreFX build isn't on the old blob feeds, so without this fix, we end up restoring the first 5.0.0-alpha.1 build instead of the build we want.

 - Exclude tests that depend on OOB assemblies.

 - Turn off CoreFX test legs in CI.

Now that the branch is on servicing, and the churn is low, exclude these as they
break far more often than they detect issues. These already run in CoreFX CI on release bits.
In case we want to bring them back for checked testing, we need to fix CoreFX.depproj.
It has a package - Microsoft.Private.CoreFx.OOB - that's supposed to bring in all deps
that are out of box. These are currently not getting restored and this ends up causing
File not found issues in the binder when compiling tests, making test exclusions impossible.
Please enter the commit message for your changes. Lines starting
with '#' will be ignored, and an empty message aborts the commit.

Co-authored-by: Jan Jahoda <[email protected]>
Co-authored-by: Ivan Diaz Sanchez <[email protected]>
Co-authored-by: Will Godbe <[email protected]>
Co-authored-by: Santiago Fernandez Madero <[email protected]>
Co-authored-by: John Salem <[email protected]>
Co-authored-by: Anirudh Agnihotry <[email protected]>
Co-authored-by: dotnet-bot <[email protected]>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
@hoyosjs hoyosjs mentioned this pull request Sep 15, 2021
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mangod9 mangod9 mangod9 approved these changes

@tarekgh tarekgh tarekgh approved these changes

@janvorli janvorli janvorli approved these changes

@jeffschwMSFT jeffschwMSFT jeffschwMSFT approved these changes

@ivdiazsa ivdiazsa Awaiting requested review from ivdiazsa

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@safern @tarekgh @jeffschwMSFT @janvorli @mangod9