iTerm2 @3.4.15_3: SafeAreaInsets are still breaking builds on older systems

[Gandoon (Erik Hedlund)]

The other day a new revision (3) of iTerm2 was released and it still gives some of us users of older systems problems due to SafeAreaInsets, which seems to be related to the notch on newer Macs.

As the iTerm2 project development seems to have left the 3.4-era and moved on to working on v3.5, it may be possible that a submission to George Nachman would not necessarily be expediently addressed for source builds, I submit here a workaround source code- and Portfile patch that removes any use of SafeAreaInsets which were identified in now closed 63824. This solves the build problems for at least MacOS 10.15, but will likely solve similar problems on 10.14 as well.

There have been indications (see the closed ticket above) that also MacOS 11 may have this issue, so maybe the initial tests for ${os.major} in the Portfile should be set to > 20 instead of ${os.major} > 19, but I do not have a MacOS 11 machine to test this on, so I played safe and put what I can test there. If someone wants to test on a MacOS 11 system and incorporate these fixes, feel free to just test that, it may work if you have issues building.

[Gandoon (Erik Hedlund)]

Source code patch – Removes uses of safeAreaInsets on systems older than MacOS 11

[Gandoon (Erik Hedlund)]

Portfile patch – applies the source code patch for systems older than MacOS 11 (could be amended to do that for MacOS 11 as well)

[Gandoon (Erik Hedlund)]

A quick note. It may be better to somehow check for the presence of the symbols, but as far as I know, the only notched MacBook Pros out there with the notch are the 2021 models onwards. They all shipped with MacOS 12 from the start, so it is probably safe to comment out the portions on MacOS 11 versions as well. It would be unlikely to find a machine that has the notch running a MacOS 11 or earlier system outside perhaps Apple's own development circles. Hence, I doubt the code that gets commented out does anything useful on earlier machines anyway. So the presented source patch is probably safe to use in most cases, provided that it is not applied to any system running MacOS 12.

Despite not having any proof for the statement, I have a strong feeling that the safeAreaInsets symbol actually only exists in MacOS 12.

[kencu (Ken)]

to get these newer codebases to compile against older SDKs, we usually use the availability macros to wrap the new code.

It works for a while, but then becomes too hard to maintain eventually.

compiling against a newer SDK, like current SDK, is really the only robust solution to this. It’s being worked on.

[kencu (Ken)]

Replying to Gandoon:

Despite not having any proof for the statement, I have a strong feeling that the safeAreaInsets symbol actually only exists in MacOS 12.

The code you are commenting out says exactly that.

[Gandoon (Erik Hedlund)]

Replying to kencu:

Replying to Gandoon:

Despite not having any proof for the statement, I have a strong feeling that the safeAreaInsets symbol actually only exists in MacOS 12.

The code you are commenting out says exactly that.

Yes, I know. But the thing is that the compile system complains about the symbols being absent and refuses to build. The removal of the checks altogether fixes this on MacOS 10.15, and most likely on MacOS 11 as well, if the symbols are not present there either.

[Gandoon (Erik Hedlund)]

Replying to kencu:

to get these newer codebases to compile against older SDKs, we usually use the availability macros to wrap the new code.

It works for a while, but then becomes too hard to maintain eventually.

compiling against a newer SDK, like current SDK, is really the only robust solution to this. It’s being worked on.

On legacy hard- and software it may be hard to impossible to do so unfortunately, so I solved this issue for myself like this. I share my experience here to the benefit of anyone with the same issues. Hopefully someone might benefit from it, either by someone from MacPorts incorporating my solution, or by individual direct use by end users who prefer building rather than pulling binaries.

Cheers

[kencu (Ken)]

Replying to Gandoon:

Replying to kencu:

Replying to Gandoon:

Despite not having any proof for the statement, I have a strong feeling that the safeAreaInsets symbol actually only exists in MacOS 12.

The code you are commenting out says exactly that.

Yes, I know. But the thing is that the compile system complains about the symbols being absent and refuses to build. The removal of the checks altogether fixes this on MacOS 10.15, and most likely on MacOS 11 as well, if the symbols are not present there either.

I can't tell if you are agreeing with me or disagreeing with me :>

The code you are commenting out calls features that only exist in newer SDKs. So old SDKs don't have it, so the build fails.

The fix is to use the availability macros to block the code out on old SDKs

here's an example of how to do it:

​https://github.com/macports/macports-ports/blob/master/aqua/qt5/files/patch-qt515-highsierra1.diff

[kencu (Ken)]

if you want to do something like that and generate a PR for it, that would be very much appreciated. MacPorts relies on contributors.

Otherwise this can sit here until somebody else does it.

[Gandoon (Erik Hedlund)]

Replying to kencu:

I can't tell if you are agreeing with me or disagreeing with me :>

No, no… I don't disagree with you. I am just not very well versed in the usage of the availability macros you mention. I have mainly been active here, submitting tickets, fairly recently as things have started to break much more often for me. Possibly because I am still using a legacy OS. I just shared what solutions I applied to solve the problem for me. The solution is in no way, shape, or form made out to be an ideal solution, but it works. I personally consider it a workaround.

The code you are commenting out calls features that only exist in newer SDKs. So old SDKs don't have it, so the build fails.

Indeed, that was the point of getting rid of it :)

The fix is to use the availability macros to block the code out on old SDKs

here's an example of how to do it:

​https://github.com/macports/macports-ports/blob/master/aqua/qt5/files/patch-qt515-highsierra1.diff

As for the availability macros, thank you for the example. I will have a look at your suggested reading for the next time something like this comes around. It would probably be good to learn how to do this.

(And if I have the time later today or tomorrow I may revisit this and see if I can figure out how to do this for iTerm2. Unless someone else beat me to it.)

[Gandoon (Erik Hedlund)]

So if I understand the use of those macros correctly, something along the lines of the content of patch-safeAreaInsets-fixv2.diff (appended to the ticket and shown below) should be ok? It builds on my machine with that patch applied instead of the previously suggested one. Please, do let me know if this is more in line with what you had in mind.

I here use the #if MAC_OS_X_VERSION_MIN_REQUIRED >= 120000 to do the same I did before by explicitly commenting out of the relevant sections. It is still unnecessary to apply it to MacOS 12 targets though, but if it works as it should, it shouldn't break anything for them either.

--- sources/NSScreen+iTerm.m.orig       2021-12-05 01:21:52.000000000 +0100
+++ sources/NSScreen+iTerm.m    2022-06-19 19:06:28.781367951 +0200
@@ -93,9 +93,11 @@
 }

 - (CGFloat)notchHeight {
+#if MAC_OS_X_VERSION_MIN_REQUIRED >= 120000
     if (@available(macOS 12.0, *)) {
         return self.safeAreaInsets.top;
     }
+#endif
     return 0;
 }

@@ -107,11 +109,13 @@
 }

 - (CGFloat)it_menuBarHeight {
+#if MAC_OS_X_VERSION_MIN_REQUIRED >= 120000
     if (@available(macOS 12, *)) {
         // When the "current" screen has a notch, there doesn't seem to be a way to get the height
         // of the menu bar on other screens :(
         return MAX(24, self.safeAreaInsets.top);
     }
+#endif
     return NSApp.mainMenu.menuBarHeight;
 }

--- sources/iTermRootTerminalView.m.orig        2021-12-05 01:21:52.000000000 +0100
+++ sources/iTermRootTerminalView.m     2022-06-19 19:04:11.922745994 +0200
@@ -1252,11 +1252,13 @@
     if (fakeHeight > 0) {
         return fakeHeight;
     }
+#if MAC_OS_X_VERSION_MIN_REQUIRED >= 120000
     if (@available(macOS 12, *)) {
         // self.safeAreaInsets is all 0s on a notch Mac. Why the hell doesn't anything work right?
         const NSEdgeInsets safeAreaInsets = self.window.screen.safeAreaInsets;
         return safeAreaInsets.top;
     }
+#endif
     return 0;
 }

[Gandoon (Erik Hedlund)]

Source code patch – Removes uses of safeAreaInsets on systems older than MacOS 11, variant using the availability macros as suggested

[Gandoon (Erik Hedlund)]

Portfile patch – simplified variant that simply applies the source code patch for systems newer than 10.14

[kencu (Ken)]

getting there, yes! thanks so much.

Indeed, macports prefers patches that are always applied, rather than conditional patches, for various reasons.

you test for MAC_OS_X_VERSION_MAX_ALLOWED if are testing the SDK version (which you are wanting to do here). It’s a detail we don’t need to get into now.

We can work with this, thanks!

[kencu (Ken)]

I tried this on 10.14 and it got past the insets issue, however the build soon failed for other reasons:

/opt/local/var/macports/build/_opt_macports-ports_aqua_iTerm2/iTerm2/work/gnachman-iTerm2-ffafb06/sources/PseudoTerminal+WindowStyle.m:125:25: error: property 'titlebarSeparatorStyle' not found on object of type 'NSWindow *'
            self.window.titlebarSeparatorStyle = NSTitlebarSeparatorStyleNone;
                        ^
/opt/local/var/macports/build/_opt_macports-ports_aqua_iTerm2/iTerm2/work/gnachman-iTerm2-ffafb06/sources/PseudoTerminal+WindowStyle.m:125:50: error: use of undeclared identifier 'NSTitlebarSeparatorStyleNone'
            self.window.titlebarSeparatorStyle = NSTitlebarSeparatorStyleNone;
                                                 ^
/opt/local/var/macports/build/_opt_macports-ports_aqua_iTerm2/iTerm2/work/gnachman-iTerm2-ffafb06/sources/PseudoTerminal+WindowStyle.m:127:25: error: property 'titlebarSeparatorStyle' not found on object of type 'NSWindow *'
            self.window.titlebarSeparatorStyle = NSTitlebarSeparatorStyleAutomatic;
                        ^
/opt/local/var/macports/build/_opt_macports-ports_aqua_iTerm2/iTerm2/work/gnachman-iTerm2-ffafb06/sources/PseudoTerminal+WindowStyle.m:127:50: error: use of undeclared identifier 'NSTitlebarSeparatorStyleAutomatic'
            self.window.titlebarSeparatorStyle = NSTitlebarSeparatorStyleAutomatic;
                                                 ^

so at least on that system there is more work to be done to build against that older sdk.

[kencu (Ken)]

I tried building against the 11.3 SDK on 10.14, but that was also a failure as the Xcode clang compiler doesn't work well with the 11.3 SDK, it seems.

I then tried to force a newer compiler, but that failes as this port uses xcode to build, and that is harder to force to use a newer compiler.

[Gandoon (Erik Hedlund)]

Replying to kencu:

getting there, yes! thanks so much.

Indeed, macports prefers patches that are always applied, rather than conditional patches, for various reasons.

you test for MAC_OS_X_VERSION_MAX_ALLOWED if are testing the SDK version (which you are wanting to do here). It’s a detail we don’t need to get into now.

We can work with this, thanks!

I am not quite sure what you mean here, I test for #if MAC_OS_X_VERSION_MIN_REQUIRED >= 120000 not MAC_OS_X_VERSION_MAX_ALLOWED, as I just want the code to be there only on MacOS 12 or later. I may be misunderstanding exactly how these macros are meant to be used, and if so, I would be grateful for a quick breakdown of how they are supposed to be used. I was trying to find further documentation of them online, but I mainly found quite old references to the availability macros, say MacOS 10.12 or thereabouts.

For information, I am building with an Xcode 12.4 build 12D4e. However, I am in fact doing so on a 10.15.7 system. The reason for this is that I have previously had some backward compatibility issues with some to MacPorts unrelated software that made me opt to not upgrade at the time. (I may upgrade at some point to 11, but that would be when I manage to acquire a new suitable backup drive.)

This is the SDK configuration I have available at the moment:

$ xcodebuild -showsdks

.   .   .

macOS SDKs:
	DriverKit 20.2                	-sdk driverkit.macosx20.2
	macOS 11.1                    	-sdk macosx11.1

.   .   .

Unfortunately I do not have neither a 10.14 nor an 11 system to try this on live, but judging from the outcome on my 10.15 system and the fact that it is built with the 11.1 SDK, I hope it is not too far a stretch to expect it to work on MacOS 11. I guess there needs to be a few more of these tests yet for the older SDKs.

Either way, thanks for pointing me towards the macro solution, I learnt something from this, and hopefully my input will solve issues for others going forward :)

[kencu (Ken)]

This is a bit long, and I was going to avoid discussing it here, but a) you asked and b) might as well get it right. I sense you are a person who appreciates a good amount of verbage, so I will try to be both long winded and complete here.

The way Apple has approached supporting code for older deployment targets changed about five years ago, so things you may read on the web about the way to use the Availability Macros might have changed now and will be out of date perhaps.

The way Apple wants you to do this now is as follows: use the most current MacOS system, use the most current Xcode, use the most current MacOSX SDK, and then set your deployment target to the be the oldest system that you are prepared to support with your code.

In your code, when you are trying to use features that don't exist on older systems that you still want to support, you guard those newer features like this:

if (@available(macOS 12, *)) {
  // use some newer feature in 12.x

} else {
  //either don't use it, or use a fallback implementation

}

The compiler will change that @available call into code that tests the system your application is actually running on, and follow the right code path for whatever system you are running on.

If you try to use a feature that doesn't exist on your deployment target, the compiler will even tell you that you need to add an @availability guard for it -- isn't that helpful? Great!

NOW -- we take MacPorts. We don't want to exactly follow Apple's plans here. We want to compile this code against an older SDK, say 10.15, but that older SDK doesn't know anything about the newer features in the 12.x SDK.

So when you try to compile code that stumbles across the newer features, even if they are guarded by an @availability block, the code will not compile. This is what you stumbled across with safeAreaInsets.

We (MacPorts) want to get this code to compile against the older SDK. To do that, we need to completely block out the part that the older SDK doesn't know about, and skip all of Apple's @availability tricks.

So we want to test the version of the SDK we are building against, and not try to compile that code if the SDK is too old to manage it. The system we are running on will taken care of by the @available block, if we can compile it.

OK so far?

Now, MAC_OS_X_VERSION_MAX_ALLOWED is set in the SDK to the version of the SDK. So this tells us if the SDK will know about the new features or not. This then tells us if it is safe to try to compile the new code surrounded by the @available block. If our SDK is too old, compiling will fail. If the SDK is new enough, compiling will succeed. So that is the one we want to use.

MacPorts modification to Apple's procedure, to allow compiling on older SDKs:

#if MAC_OS_X_VERSION_MAX_ALLOWED >= 120000
if (@available(macOS 12, *)) {
  // use some newer feature in 12.x

} else {
#endif

  //either don't use it, or use a fallback implementation

#if MAC_OS_X_VERSION_MAX_ALLOWED >= 12000
}
#endif

We don't ever care (with this method) about MAC_OS_X_VERSION_MIN_REQUIRED as the @available code takes care of that issue, and it is not relevant here for us to deal with.

This lets the code block work properly on newer systems (compiling the code, even for older deployment targets) and also for older systems (with older SDKs that don't support compiling the code at all).

In practice, MAC_OS_X_VERSION_MIN_REQUIRED and MAC_OS_X_VERSION_MAX_ALLOWED are often set to the same thing, so the point is moot. But it could be that someone tries to build this code on a newer system with an older deployment target, and in that case, using MAC_OS_X_VERSION_MIN_REQUIRED would have broken the whole @available mechanism (by not compiling it in at all, which is wrong).

In the old days, in old code, before the @available mechanism was used, we used to use MAC_OS_X_VERSION_MIN_REQUIRED to do the thing that @available() does now. I believe that people found it very very confusing, and I have seen tremendous variability in how it was coded, so I think Apple sat down and came up with a newer, more clear, method.

In code that uses @available, there will be almost no need I can think of to ever use MAC_OS_X_VERSION_MIN_REQUIRED.

[Gandoon (Erik Hedlund)]

Replying to kencu:

This is a bit long, and I was going to avoid discussing it here, but a) you asked and b) might as well get it right. I sense you are a person who appreciates a good amount of verbage, so I will try to be both long winded and complete here.

Haha, maybe you are half right there :) I at least try to be as clear as possible when I express myself to make sure I am not misunderstood.
Thank you for the long-windedness.

The way Apple has approached supporting code for older deployment targets changed about five years ago, so things you may read on the web about the way to use the Availability Macros might have changed now and will be out of date perhaps.

The way Apple wants you to do this now is as follows: use the most current MacOS system, use the most current Xcode, use the most current MacOSX SDK, and then set your deployment target to the be the oldest system that you are prepared to support with your code.

In your code, when you are trying to use features that don't exist on older systems that you still want to support, you guard those newer features like this:

if (@available(macOS 12, *)) {
  // use some newer feature in 12.x

} else {
  //either don't use it, or use a fallback implementation

}

The compiler will change that @available call into code that tests the system your application is actually running on, and follow the right code path for whatever system you are running on.

If you try to use a feature that doesn't exist on your deployment target, the compiler will even tell you that you need to add an @availability guard for it -- isn't that helpful? Great!

NOW -- we take MacPorts. We don't want to exactly follow Apple's plans here. We want to compile this code against an older SDK, say 10.15, but that older SDK doesn't know anything about the newer features in the 12.x SDK.

So when you try to compile code that stumbles across the newer features, even if they are guarded by an @availability block, the code will not compile. This is what you stumbled across with safeAreaInsets.

We (MacPorts) want to get this code to compile against the older SDK. To do that, we need to completely block out the part that the older SDK doesn't know about, and skip all of Apple's @availability tricks.

So we want to test the version of the SDK we are building against, and not try to compile that code if the SDK is too old to manage it. The system we are running on will taken care of by the @available block, if we can compile it.

OK so far?

Yes, I am with you…

Now, MAC_OS_X_VERSION_MAX_ALLOWED is set in the SDK to the version of the SDK. So this tells us if the SDK will know about the new features or not. This then tells us if it is safe to try to compile the new code surrounded by the @available block. If our SDK is too old, compiling will fail. If the SDK is new enough, compiling will succeed. So that is the one we want to use.

MacPorts modification to Apple's procedure, to allow compiling on older SDKs:

#if MAC_OS_X_VERSION_MAX_ALLOWED >= 120000
if (@available(macOS 12, *)) {
  // use some newer feature in 12.x

} else {
#endif

  //either don't use it, or use a fallback implementation

#if MAC_OS_X_VERSION_MAX_ALLOWED >= 12000
}
#endif

We don't ever care (with this method) about MAC_OS_X_VERSION_MIN_REQUIRED as the @available code takes care of that issue, and it is not relevant here for us to deal with.

This lets the code block work properly on newer systems (compiling the code, even for older deployment targets) and also for older systems (with older SDKs that don't support compiling the code at all).

In practice, MAC_OS_X_VERSION_MIN_REQUIRED and MAC_OS_X_VERSION_MAX_ALLOWED are often set to the same thing, so the point is moot. But it could be that someone tries to build this code on a newer system with an older deployment target, and in that case, using MAC_OS_X_VERSION_MIN_REQUIRED would have broken the whole @available mechanism (by not compiling it in at all, which is wrong).

Aha‚ I guess this is what I misunderstood… This explanation clarifies the use case perfectly. Great!

In the old days, in old code, before the @available mechanism was used, we used to use MAC_OS_X_VERSION_MIN_REQUIRED to do the thing that @available() does now. I believe that people found it very very confusing, and I have seen tremendous variability in how it was coded, so I think Apple sat down and came up with a newer, more clear, method.

In code that uses @available, there will be almost no need I can think of to ever use MAC_OS_X_VERSION_MIN_REQUIRED.

Once again, thank you for the explanations. This makes a lot of sense, and having this explanation I feel that I have a lot more meat on my bones regarding both how to work with these bits that seems to be very well intentioned by Apple.

Well this exercise was interesting and as my iTerm2 now builds as intended, and I hopefully have provided the backbone for a fix. Plus you have given me a fairly comprehensive explanation as to how and why this is done the way it is done.

I just tried it again with the MAC_OS_X_VERSION_MIN_REQUIRED swapped directly for MAC_OS_X_VERSION_MAX_ALLOWED and verified that it worked, so I can confirm that it works on MacOS 10.15.7 with the 11.1 SDK.

Cheers :)

[Gandoon (Erik Hedlund)]

Source code patch – Removes uses of safeAreaInsets on systems older than MacOS 11, using the correct MAC_OS_X_VERSION_MAX_ALLOWED as per discussion before.