Fix `CGDisplayCreateUUIDFromDisplayID` linking (again) #2078

madsmtm · 2021-11-29T23:19:37Z

See previous issue: #1626.

The cocoa crate links to AppKit, which made the symbol CGDisplayCreateUUIDFromDisplayID from ApplicationServices/ColorSync (which AppKit uses internally) available to us (on macOS 10.8 to 10.13).

However, this does not work on macOS 10.7 (where AppKit does not link to ColorSync internally). Instead of relying on this, we should just link to ApplicationServices directly.

Also, lately something changed in rustc so that the linker's macOS version is 10.7, see rust-lang/rust#91372, which is why the nightly builds were failing.

madsmtm · 2021-11-29T23:36:19Z

~~Nope, I was wrong, this doesn't fix it~~

madsmtm · 2021-12-01T14:15:10Z

I've investigated this some more, and I'm pretty sure I've found the correct way of doing it (see the updated description)

@Imberflur and @ArturKovacs, sorry for pinging you before, but this is now ready to be tested.

Also pinging @ryanisaacg, don't know what macOS version you have but the more testers of this the merrier 😉

madsmtm · 2021-12-01T14:26:30Z

Just FYI, the nightly android builds will be fixed by rust-lang/rust#91381

ArturKovacs · 2021-12-01T21:58:51Z

Unfortunately I won't have access to a macOS device until January. Feel free to ping me again at that time if this is still open then.

madsmtm · 2021-12-02T00:27:05Z

There is kind of a deadline on this, winit will break for macOS users in Rust 1.58, and I would really like to get this fix rolled out a while before that (and perhaps backported to some of the older, yet still quite widely used winit versions?)

See rust-windowing/winit#2078

This is a work around for rust-windowing/winit#2078

ArturKovacs · 2021-12-06T14:24:29Z

Tagging a few people who are likely affected by this

@azriel91 from amethyst
@hecrj from iced
@icefoxen from ggez

madsmtm · 2021-12-06T21:12:11Z

To get this merged quicker, I took out the WINIT_LINK_COLORSYNC changes, will follow up with those in a separate PR.

madsmtm · 2021-12-06T21:13:48Z

src/platform_impl/macos/ffi.rs

 #[cfg_attr(
    not(use_colorsync_cgdisplaycreateuuidfromdisplayid),
-    link(name = "CoreGraphics", kind = "framework")
+    link(name = "ApplicationServices", kind = "framework")


Note that a bit further down in this file we're linking against CoreGraphics (this link attribute was redundant), so the only change is that we now also link to ApplicationServices.

Tested on MBP running macOS 12.0.1 and it works 👍

Imberflur · 2021-12-07T01:27:21Z

@madsmtm I tested this with the docker setup and it links fine 👍 (although we aren't using that setup anymore currently)

icefoxen · 2021-12-09T18:08:35Z

Deferring to new ggez devs: @PSteinhaus , @nobbele and @IGBC .

Due to the winit bug: rust-windowing/winit#2078

- These are failing on: rust-windowing/winit#2078

hecrj · 2021-12-17T10:24:54Z

@ArturKovacs Thanks for the heads up!

Indeed, there are a bunch of projects I maintain that have failing CI with the beta toolchain. Ideally, a fix should be merged and released before the new stable release.

Is there any way I can help?

madsmtm · 2021-12-17T21:23:33Z

If we could get this tested by someone with a macOS version < 10.13 that would be nice, but this PR very probably works as-is (the previous behaviour utilized undocumented behaviour on older platforms, while linking to ApplicationServices matches what the docs say).

madsmtm · 2021-12-17T21:27:19Z

Relatedly, an update to the people following this issue:

The changes to rustc have been reverted, and the revert will (very likely) be backported to beta. So the rush for getting this rolled out before 1.58 is gone (though we should still fix the issue!)

ArturKovacs · 2022-01-01T10:18:22Z

I have macOS 10.15 so it's not that easy for me to test on macOS < 10.13

However given that my machine is from mid 2012, I think I can reinstall it to the old OS version. That's a bit intrusive but I can do it if it's reeeealy needed.

hkratz · 2022-01-01T10:44:48Z

I have an old Macbook with 10.10 lying around. Not being familiar with winit what would be the best way to test it? Just cargo build / cargo test?

madsmtm · 2022-01-01T14:05:15Z

Just something where the linking is triggered. Try:

cargo clean
MACOSX_DEPLOYMENT_TARGET=10.7 cargo build --example window
cargo clean
cargo build --example window

hkratz · 2022-01-01T16:54:24Z

Looks good:

$ git remote -v
origin	git@github.com:madsmtm/winit.git (fetch)
origin	git@github.com:madsmtm/winit.git (push)
$ git status
On branch fix-colorsync-linking
Your branch is up to date with 'origin/fix-colorsync-linking'.

nothing to commit, working tree clean
$ sw_vers
ProductName:	Mac OS X
ProductVersion:	10.10.5
BuildVersion:	14F2511
$ rustc --version
rustc 1.57.0 (f1edd0429 2021-11-29)
$ cargo clean
$ cargo build --example window --quiet
$ MACOSX_DEPLOYMENT_TARGET=10.7 cargo build --example window --quiet
$ cargo clean
$ cargo build --example window --quiet
$

madsmtm · 2022-01-01T17:09:47Z

Wonderful, thanks a lot @hkratz!

Just out of interest, could I get you to do:

cargo clean
MACOSX_DEPLOYMENT_TARGET=10.7 cargo run --example window

And just verify that a window actually opens? I think it's rare that winit is actually used on older machines, so it would be nice to hear that working

hkratz · 2022-01-03T20:35:07Z

cargo clean
MACOSX_DEPLOYMENT_TARGET=10.7 cargo run --example window

@madsmtm It shows a small window and produces lots of logging output after adding a workaround for borntyping/rust-simple_logger#43

madsmtm · 2022-01-03T20:37:54Z

Wonderful, thanks!

See also rust-windowing#1626. The `cocoa` crate links to AppKit, which made the symbol `CGDisplayCreateUUIDFromDisplayID` from ApplicationServices/ColorSync (which AppKit uses internally) available to us on macOS 10.8 to 10.13. However, this does not work on macOS 10.7 (where AppKit does not link to ColorSync internally). Instead of relying on this, we should just link to ApplicationServices directly.

madsmtm · 2022-01-05T13:38:03Z

Well, I think this has had enough eyes on it

Since rust-windowing#2078 we link to `ApplicationServices`, which contains the `CGDisplayCreateUUIDFromDisplayID` symbol in all versions.

Since #2078 we link to `ApplicationServices`, which contains the `CGDisplayCreateUUIDFromDisplayID` symbol in all versions.

madsmtm mentioned this pull request Nov 29, 2021

Fix warnings #2076

Merged

8 tasks

maroider added the DS - macos label Nov 30, 2021

madsmtm force-pushed the fix-colorsync-linking branch 3 times, most recently from f36f127 to 4a3f5b6 Compare December 1, 2021 13:56

madsmtm mentioned this pull request Dec 1, 2021

Linking to private symbols from frameworks between compilation units rust-lang/rust#91372

Closed

est31 added a commit to est31/glium that referenced this pull request Dec 2, 2021

Disable Mac OS for now for the CI

ee2d490

See rust-windowing/winit#2078

thomcc added a commit to thomcc/imagine that referenced this pull request Dec 5, 2021

Allow demo to link on macOS

c7bda3f

This is a work around for rust-windowing/winit#2078

thomcc mentioned this pull request Dec 5, 2021

Allow demo to link on macOS Lokathor/imagine#4

Merged

Lokathor pushed a commit to Lokathor/imagine that referenced this pull request Dec 5, 2021

Allow demo to link on macOS (#4)

756bcd6

This is a work around for rust-windowing/winit#2078

madsmtm force-pushed the fix-colorsync-linking branch from 4a3f5b6 to 106d77c Compare December 6, 2021 21:10

madsmtm commented Dec 6, 2021

View reviewed changes

hecrj mentioned this pull request Dec 8, 2021

Unable to compile on newer MacOS version on M1 Mac iced-rs/iced#1146

Closed

madsmtm force-pushed the fix-colorsync-linking branch from 106d77c to 007bc57 Compare December 11, 2021 03:31

est31 added a commit to est31/glium-glyph that referenced this pull request Dec 12, 2021

Use windows as second OS instead of Mac OS

b747cf4

Due to the winit bug: rust-windowing/winit#2078

JohnHeitmann mentioned this pull request Dec 13, 2021

Mac builds broken on bleeding edge rustc bevyengine/bevy#3313

Closed

parasyte added a commit to parasyte/cartunes that referenced this pull request Dec 13, 2021

Remove macOS builds from CI

f28424e

- These are failing on: rust-windowing/winit#2078

parasyte mentioned this pull request Dec 13, 2021

Remove macOS builds from CI parasyte/cartunes#13

Merged

parasyte added a commit to parasyte/cartunes that referenced this pull request Dec 13, 2021

Remove macOS builds from CI

e336360

- These are failing on: rust-windowing/winit#2078

parasyte added a commit to parasyte/cartunes that referenced this pull request Dec 13, 2021

Remove macOS builds from CI (#13)

1e0cd28

- These are failing on: rust-windowing/winit#2078

parasyte mentioned this pull request Dec 15, 2021

Update dependencies parasyte/pixels#230

Merged

9 tasks

madsmtm force-pushed the fix-colorsync-linking branch from 007bc57 to c780fc6 Compare January 2, 2022 21:54

ArturKovacs mentioned this pull request Jan 3, 2022

Release 0.26.1 #2125

Merged

3 tasks

madsmtm force-pushed the fix-colorsync-linking branch from c780fc6 to 18d8079 Compare January 3, 2022 20:32

madsmtm force-pushed the fix-colorsync-linking branch from 18d8079 to 83d55c5 Compare January 5, 2022 13:08

madsmtm merged commit 39dd30c into rust-windowing:master Jan 5, 2022

madsmtm deleted the fix-colorsync-linking branch January 5, 2022 13:38

madsmtm added a commit to madsmtm/winit that referenced this pull request Jan 5, 2022

Remove WINIT_LINK_COLORSYNC (no longer needed)

98e5c9e

Since rust-windowing#2078 we link to `ApplicationServices`, which contains the `CGDisplayCreateUUIDFromDisplayID` symbol in all versions.

madsmtm mentioned this pull request Jan 5, 2022

Remove WINIT_LINK_COLORSYNC #2136

Merged

6 tasks

madsmtm added a commit to madsmtm/winit that referenced this pull request Jan 23, 2022

Remove WINIT_LINK_COLORSYNC (no longer needed)

4221a30

Since rust-windowing#2078 we link to `ApplicationServices`, which contains the `CGDisplayCreateUUIDFromDisplayID` symbol in all versions.

madsmtm added a commit that referenced this pull request Jan 23, 2022

Remove WINIT_LINK_COLORSYNC (no longer needed) (#2136)

51bb6b7

Since #2078 we link to `ApplicationServices`, which contains the `CGDisplayCreateUUIDFromDisplayID` symbol in all versions.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `CGDisplayCreateUUIDFromDisplayID` linking (again) #2078

Fix `CGDisplayCreateUUIDFromDisplayID` linking (again) #2078

madsmtm commented Nov 29, 2021 •

edited

madsmtm commented Nov 29, 2021 •

edited

madsmtm commented Dec 1, 2021 •

edited

madsmtm commented Dec 1, 2021

ArturKovacs commented Dec 1, 2021

madsmtm commented Dec 2, 2021

ArturKovacs commented Dec 6, 2021

madsmtm commented Dec 6, 2021

madsmtm Dec 6, 2021 •

edited

A-Helberg Dec 10, 2021

Imberflur commented Dec 7, 2021

icefoxen commented Dec 9, 2021

hecrj commented Dec 17, 2021 •

edited

madsmtm commented Dec 17, 2021

madsmtm commented Dec 17, 2021

ArturKovacs commented Jan 1, 2022

hkratz commented Jan 1, 2022

madsmtm commented Jan 1, 2022

hkratz commented Jan 1, 2022

madsmtm commented Jan 1, 2022

hkratz commented Jan 3, 2022

madsmtm commented Jan 3, 2022

madsmtm commented Jan 5, 2022

Fix CGDisplayCreateUUIDFromDisplayID linking (again) #2078

Fix CGDisplayCreateUUIDFromDisplayID linking (again) #2078

Conversation

madsmtm commented Nov 29, 2021 • edited

madsmtm commented Nov 29, 2021 • edited

madsmtm commented Dec 1, 2021 • edited

madsmtm commented Dec 1, 2021

ArturKovacs commented Dec 1, 2021

madsmtm commented Dec 2, 2021

ArturKovacs commented Dec 6, 2021

madsmtm commented Dec 6, 2021

madsmtm Dec 6, 2021 • edited

Choose a reason for hiding this comment

A-Helberg Dec 10, 2021

Choose a reason for hiding this comment

Imberflur commented Dec 7, 2021

icefoxen commented Dec 9, 2021

hecrj commented Dec 17, 2021 • edited

madsmtm commented Dec 17, 2021

madsmtm commented Dec 17, 2021

ArturKovacs commented Jan 1, 2022

hkratz commented Jan 1, 2022

madsmtm commented Jan 1, 2022

hkratz commented Jan 1, 2022

madsmtm commented Jan 1, 2022

hkratz commented Jan 3, 2022

madsmtm commented Jan 3, 2022

madsmtm commented Jan 5, 2022

Fix `CGDisplayCreateUUIDFromDisplayID` linking (again) #2078

Fix `CGDisplayCreateUUIDFromDisplayID` linking (again) #2078

madsmtm commented Nov 29, 2021 •

edited

madsmtm commented Nov 29, 2021 •

edited

madsmtm commented Dec 1, 2021 •

edited

madsmtm Dec 6, 2021 •

edited

hecrj commented Dec 17, 2021 •

edited