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

Fix the make configuration for macOS 11.0.1 #2865

Merged
merged 1 commit into from
Nov 17, 2020

Conversation

idyll
Copy link
Contributor

@idyll idyll commented Nov 13, 2020

This change is needed to ensure that we're able to configure make on macOS 11.0.1.

A smilar fix was applied for 11.0 in #2700

Main changes:

  • Fix the integer macOS version correctly for the build version check

Verified via ./otp_build tests

@CLAassistant
Copy link

CLAassistant commented Nov 13, 2020

CLA assistant check
All committers have signed the CLA.

@idyll
Copy link
Contributor Author

idyll commented Nov 13, 2020

Let me know if I need to open this against maint to make sure that it is applied to the next 23.1.x release

@bjorng
Copy link
Contributor

bjorng commented Nov 13, 2020

Let me know if I need to open this against maint to make sure that it is applied to the next 23.1.x release

Yes, please do. (You can edit this pull request to base it on maint. If your commit is currently is based on master, you will also need to rebase it and force-push.)

@idyll idyll changed the base branch from master to maint November 13, 2020 06:11
@idyll
Copy link
Contributor Author

idyll commented Nov 13, 2020

Yes, please do. (You can edit this pull request to base it on maint. If your commit is currently is based on master, you will also need to rebase it and force-push.)

Done. :)

@aselder
Copy link

aselder commented Nov 13, 2020

yay!!! Can we get this merged to master and maint ASAP?

@theoks
Copy link

theoks commented Nov 14, 2020

Thank you for the contribution, I hope this appears on maint soon. Will this fix be included in a 22.* version as well?

@IngelaAndin IngelaAndin added the team:VM Assigned to OTP team VM label Nov 16, 2020
@bjorng bjorng added the testing currently being tested, tag is used by OTP internal CI label Nov 16, 2020
@verrchu
Copy link

verrchu commented Nov 16, 2020

I have successfully built maint branch with this patch on macOS 11.0.1
please merge it

@bjorng bjorng merged commit f413bdc into erlang:maint Nov 17, 2020
@idyll idyll deleted the mm-macos-11-0-1 branch November 17, 2020 05:59
@ryandesign
Copy link

Both this fix and the previous one for macOS 11.0 in b93b9f9 are wrong and break things for Mac OS X 10.x. See https://bugs.erlang.org/browse/ERL-1407.

@bjorng
Copy link
Contributor

bjorng commented Nov 17, 2020

How does it break 10.x? The branch builds fine on my Mac with macOS 10.15.7 as well on a Mac with 10.5.8 in our lab. On which version would it be a problem?

I have not yet tested the branch myself on macOS 11.0.1, but @verrchu state above that it works.

I am open to suggestions for a better fix of the problem. Perhaps it would be better to remove the version check. I don't remember why we added it in the first place.

@ryandesign
Copy link

It would be a problem on OS X 10.9 and earlier. See https://bugs.erlang.org/browse/ERL-1407.

@bjorng
Copy link
Contributor

bjorng commented Nov 17, 2020

It would be a problem on OS X 10.9 and earlier.

Not on all versions before 10.9. As I said, it built fine on 10.5.8.

I'll try to take closer look on this problem before the release of 23.2 to avoid breaking certain version of macOS. For now, this pull request does seem to fix the immediate problem of building on macOS 11.0.1.

@ryandesign
Copy link

It may build fine, but the test is no longer testing what you think it's testing.

@bjorng
Copy link
Contributor

bjorng commented Nov 17, 2020

I see no reason to keep the test, so I'm removing it in #2871.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants