Skip to content

stdlib: check for ARM/ARM64/AArch64 more thoroughly #20709

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

Merged
merged 1 commit into from
Feb 20, 2019
Merged

stdlib: check for ARM/ARM64/AArch64 more thoroughly #20709

merged 1 commit into from
Feb 20, 2019

Conversation

futurejones
Copy link
Contributor

Changes needed for successful builds of 4.2 for AArch64 on the Swift Community CI.
(cherry picked from #19462)

Resolves SR-NNNN.

@compnerd
Copy link
Member

@swift-ci please test

@compnerd
Copy link
Member

@airspeedswift - seems like @futurejones would like to nominate this for the 4.2-branch. This impacts the swift standard library, but should only impact the non-Apple ARM/AArch64 build targets, and is simply enabling the same paths under different names, so I consider it a low risk change.

@bob-wilson
Copy link
Contributor

We generally don't add enhancements like this on old release branches. We should focus new work on the master branch and the active release branches. Besides, why do we need CI for a branch that isn't changing anymore?

@futurejones
Copy link
Contributor Author

futurejones commented Nov 24, 2018

@bob-wilson

  1. As far as I am aware the swift-4.2-branch is the "current" release branch and defiantly not an "old" release branch. It is also "active" as we can see from the recent 4.2.1 release.
  2. This is not an enhancement, it is a fix for a breaking change that was introduced in 4.2 that is preventing successful builds on AArch64 systems.
  3. The CI you are referring to is generously setup and maintained by members of the Swift open-source community.

In Swift 4.1 we saw 4.1, 4.1.1, 4.1.2 and 4.1.3 releases. There absolutely no reason why we can not see a similar pattern of releases for Swift 4.2.

Also these changes have already been added to the master - #19462

@bob-wilson
Copy link
Contributor

We're not planning any more releases from the 4.2 branch. The current release branch is for Swift 5.0, and I think we should focus our energies on that branch.

@futurejones
Copy link
Contributor Author

@bob-wilson as stated before - these changes have already been added in PR 19462
There is no reason not to allow this to be added to swift-4.2-branch as well.

@hpux735
Copy link
Contributor

hpux735 commented Nov 27, 2018

This puts the Swift Arm community in a position again to maintain parallel repos. We've tried to work with the core team about this over the years, and they've expressed interest in helping us stay on mainline. However, not being willing to work with us pushes us away again.

Swift 5 has not been released, and it's not usable for any user in production settings, let alone on Arm64. Saying that we should accept the fact that Swift 4.2 doesn't work on Arm64, and never will, because we need to wait until some indeterminate point in the future (WWDC 2019?), when Swift 5 is released is pretty painful.

If you look at the changes proposed, they are incredibly minor. Furthermore, master has diverged from this enough that this patch would not be able to be cherry-picked from master into this branch.

@futurejones
Copy link
Contributor Author

@weissi @kevints is it possible to get this into the 4.2.3 Release?
Thanks.

@weissi
Copy link
Contributor

weissi commented Feb 20, 2019

@tkremenek / @rjmccall what are your thoughts on this one? It looks okay to me

Copy link
Contributor

@rjmccall rjmccall left a comment

Choose a reason for hiding this comment

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

LGTM.

@weissi
Copy link
Contributor

weissi commented Feb 20, 2019

thanks @rjmccall , will take this one then

@weissi weissi merged commit b0ebc8c into swiftlang:swift-4.2-branch Feb 20, 2019
@futurejones
Copy link
Contributor Author

Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants