Opened 8 years ago

Closed 7 years ago

Last modified 7 years ago

#52537 closed defect (wontfix)

hatari - fix for build on Snow Leopard without Xcode 4.2

Reported by: ken-cunningham-webuse Owned by: macports-tickets@…
Priority: Normal Milestone:
Component: ports Version: 2.3.4
Keywords: haspatch maintainer Cc: emuslor (James Wilkinson), mojca (Mojca Miklavec)
Port: hatari

Description

building the hatari GUI on Snow Leopard requires Xcode 4.2. Add logic to select proper build depending on Xcode version installed and selected.

Attachments (5)

patch-fix-SL-Xcode326.diff (1.2 KB) - added by ken-cunningham-webuse 8 years ago.
hatari-3.2.6-GUI-build-fail.log.zip (14.0 KB) - added by ken-cunningham-webuse 8 years ago.
xcode 3.2.6 GUI build failure log
Portfile (4.3 KB) - added by ken-cunningham-webuse 8 years ago.
hatari revision_1 replacement Portfile
Portfile-hatari-rev1-update.diff (3.7 KB) - added by ken-cunningham-webuse 8 years ago.
new portfile diff with fix and enhancements
Portfile-hatari-rev2-update-by-mojca.diff (3.4 KB) - added by mojca (Mojca Miklavec) 7 years ago.
my suggested (untested) revision of the rev1 patch for hatari

Download all attachments as: .zip

Change History (28)

comment:1 Changed 8 years ago by ken-cunningham-webuse

I wasn't completely sure about the revision bump. I suspect it is actually _not_ needed, as no installed hatari ports need to be touched. This only fixes the failed build on 10.6 with stock Xcode 3.2.6.

comment:2 Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)

Correct, there's no reason to increase the revision.

Requiring Xcode 4.2 to build the GUI on Snow Leopard is not great, since Xcode 4.x for Snow Leopard cannot be obtained by anyone who has not paid Apple for a developer program membership.

comment:3 Changed 8 years ago by ken-cunningham-webuse

The requirement for Xcode >= 4.2 to build the GUI is an upstream decision.

By default, stock Snow Leopard will go ahead and build the command line version, like 10.4 and 10.5

But if you have 10.6 and have Xcode 4.2, you can have the GUI - why not, I thought? Seems a more egalitarian choice. (and perhaps influenced by the fact that that's what I have, myself :> ).

The only other option (other than re-writing hatari) would be to disable the GUI for all 10.6 across the board -- even if your system could build it without trouble. And that doesn't seem in keeping with the spirit of what we're aiming to do.

Last edited 8 years ago by ken-cunningham-webuse (previous) (diff)

Changed 8 years ago by ken-cunningham-webuse

Attachment: patch-fix-SL-Xcode326.diff added

comment:4 Changed 8 years ago by ken-cunningham-webuse

New diff file, deleting the rev bump. Otherwise I think it's good to go.

comment:5 Changed 8 years ago by ryandesign (Ryan Carsten Schmidt)

Do you know which part of Xcode 4.2 is required? I'm guessing it's not something we have an alternative implementation of in MacPorts?

comment:6 Changed 8 years ago by ken-cunningham-webuse

The GUI uses a feature of ObjectiveC (ARC,) that wasn't added until 4.2. And there was one other ObjC feature that wouldn't build without 4.2 as well that I can't recall. I'm happy to try rebuilding it with Xcode 3.2.6 and put the log up for you, if you have time to dig into it that far.

Have you been trying hatari out? I haven't seen StarGlider running since 1987! What a blast.

Last edited 8 years ago by ken-cunningham-webuse (previous) (diff)

Changed 8 years ago by ken-cunningham-webuse

xcode 3.2.6 GUI build failure log

comment:7 Changed 8 years ago by ken-cunningham-webuse

Here you are - relevant part of the Xcode 3.2.6 GUI build failure log is here, full log attached above. Xcode 4.2 builds it without trouble. I'd be delighted to learn there is some kind of alternative macports fix for this. --Best, K

:info:build In file included from /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_emulators_hatari/hatari/work/hatari-1.9.0/src/gui-osx/PrefsController.m:18:
:info:build /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_emulators_hatari/hatari/work/hatari-1.9.0/src/gui-osx/PrefsController.h:78:5: error: 
:info:build       unknown type name '__unsafe_unretained'
:info:build     __unsafe_unretained IBOutlet NSStepper *TTRAMSizeStepper;
:info:build     ^
:info:build /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_emulators_hatari/hatari/work/hatari-1.9.0/src/gui-osx/PrefsController.h:78:44: error: 
:info:build       expected ';' at end of declaration list
:info:build     __unsafe_unretained IBOutlet NSStepper *TTRAMSizeStepper;
:info:build                                            ^
:info:build /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_emulators_hatari/hatari/work/hatari-1.9.0/src/gui-osx/PrefsController.h:79:5: error: 
:info:build       unknown type name '__unsafe_unretained'
:info:build     __unsafe_unretained IBOutlet NSTextField *TTRAMSizeValue;
:info:build     ^
:info:build /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_emulators_hatari/hatari/work/hatari-1.9.0/src/gui-osx/PrefsController.h:79:46: error: 
:info:build       expected ';' at end of declaration list
:info:build     __unsafe_unretained IBOutlet NSTextField *TTRAMSizeValue;
:info:build                                              ^
:info:build /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_emulators_hatari/hatari/work/hatari-1.9.0/src/gui-osx/PrefsController.h:83:5: error: 
:info:build       unknown type name '__unsafe_unretained'
:info:build     __unsafe_unretained IBOutlet NSButtonCell *bCell68060;
:info:build     ^
:info:build /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_emulators_hatari/hatari/work/hatari-1.9.0/src/gui-osx/PrefsController.h:83:47: error: 
:info:build       expected ';' at end of declaration list
:info:build     __unsafe_unretained IBOutlet NSButtonCell *bCell68060;
:info:build                                               ^
:info:build /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_emulators_hatari/hatari/work/hatari-1.9.0/src/gui-osx/PrefsController.m:1042:6: error: 
:info:build       use of undeclared identifier 'TTRAMSizeValue'
:info:build     [TTRAMSizeValue setIntValue: [sender intValue]];
:info:build      ^
:info:build 7 diagnostics generated.
:info:build make[2]: *** [src/CMakeFiles/hatari.dir/gui-osx/PrefsController.m.o] Error 1
:info:build make[2]: *** Waiting for unfinished jobs....
:info:build make[2]: Leaving directory `/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_emulators_hatari/hatari/work/build'
:info:build make[1]: *** [src/CMakeFiles/hatari.dir/all] Error 2
:info:build make[1]: Leaving directory `/opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_emulators_hatari/hatari/work/build'
:info:build make: *** [all] Error 2


comment:8 Changed 8 years ago by mojca (Mojca Miklavec)

My suggestion would be to commit that change and try to figure out a solution for older Xcode versions afterwards, if possible at all.

The changes seem totally reasonable to me.

Minor "typo":

so blacklist it on 10.6+

this is probably on 10.7+ now :)

comment:9 Changed 8 years ago by mojca (Mojca Miklavec)

Cc: mojca@… added

Cc Me!

comment:10 in reply to:  7 Changed 8 years ago by larryv (Lawrence Velázquez)

Replying to ken.cunningham.webuse@…:

:info:build In file included from /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_emulators_hatari/hatari/work/hatari-1.9.0/src/gui-osx/PrefsController.m:18:
:info:build /opt/local/var/macports/build/_opt_local_var_macports_sources_rsync.macports.org_release_tarballs_ports_emulators_hatari/hatari/work/hatari-1.9.0/src/gui-osx/PrefsController.h:78:5: error: 
:info:build       unknown type name '__unsafe_unretained'
:info:build     __unsafe_unretained IBOutlet NSStepper *TTRAMSizeStepper;
:info:build     ^

__unsafe_unretained is an ARC variable lifetime qualifier, which requires Xcode 4.2.

https://developer.apple.com/library/prerelease/content/releasenotes/ObjectiveC/ObjCAvailabilityIndex

You could try blacklisting { clang < 211 } so that one of our compilers is used. I think Snow Leopard’s Objective-C runtime is new enough to handle it.

comment:11 Changed 8 years ago by ken-cunningham-webuse

Larry & Ryan, you were right. SnowLeopard will build the MacOS GUI with xcode 3.2.6 installed, but with a newer version of clang.

In the end I forced it to macports-clang-3.7 or newer, as most people would / should likely have that clang version on Snow Leopard. (3.8 doesn't work correctly, and 3.9 won't compile on SL unless you have installed my as-yet-not-published libSnowLeopardFixes port.

I must admit, the blacklisting / fallbacking took a while to figure out right, and I'm not certain even now I have it fully correct.

Even when I blacklisted { clang < 211 } the system kept choosing xcode's clang. I tried bumping the number up to 500, and it still chose xcode's clang, even though i have macports-clang-3.7 installed. Only when I blacklisted all clang did it ignore xcode's clang, and then it started installed macports-clang-3.4 (even though macports-clang-3.7 was there). I understand in a rudimentary way how the logic for this goes, but if you're going to force a clang install on SL, it might as well be 3.7.

Ultimately I blacklisted everything up to macports-clang-3.6, and then added a fallback for 3.7. I suppose I might just have whitelisted macports-clang-3.7 and achieved the same effect.

New portfile fix coming shortly. Let me know if I got it right this time. --K

comment:12 in reply to:  11 ; Changed 8 years ago by larryv (Lawrence Velázquez)

Replying to ken.cunningham.webuse@…:

In the end I forced it to macports-clang-3.7 or newer, as most people would / should likely have that clang version on Snow Leopard.

This is not a safe assumption to make. Given that 3.7 is (a) not in the default fallback list for Xcode 3.2 and (b) requires manual bootstrapping, 3.4 is almost certainly more likely to be present.

Even when I blacklisted { clang < 211 } the system kept choosing xcode's clang. I tried bumping the number up to 500, and it still chose xcode's clang, even though i have macports-clang-3.7 installed. Only when I blacklisted all clang did it ignore xcode's clang

This doesn’t sound right. Did you add the compiler_blacklist_versions-1.0 portgroup? Base doesn’t support version blacklisting on its own.

and then it started installed macports-clang-3.4 (even though macports-clang-3.7 was there).

MacPorts does not just use whatever compiler happens to be there. It follows the combination of blacklist/whitelist/fallback list, which is deterministic.

I understand in a rudimentary way how the logic for this goes, but if you're going to force a clang install on SL, it might as well be 3.7.

This isn’t the place for this conversation, but I disagree because 3.7 requires manual bootstrapping.

I suspect you just didn’t use compiler_blacklist_versions-1.0. If you add that, blacklisting {clang < 211} should work fine.

comment:13 in reply to:  12 Changed 8 years ago by ken-cunningham-webuse

did you compiler_blacklist_versions-1.0 portgroup?

Ah -- of course -- the 'ol compiler_blacklist_versions-1.0 portgroup

Last edited 7 years ago by ken-cunningham-webuse (previous) (diff)

comment:14 Changed 8 years ago by ken-cunningham-webuse

I believe this should do it.

Last edited 7 years ago by ken-cunningham-webuse (previous) (diff)

Changed 8 years ago by ken-cunningham-webuse

Attachment: Portfile added

hatari revision_1 replacement Portfile

Changed 8 years ago by ken-cunningham-webuse

new portfile diff with fix and enhancements

comment:15 Changed 8 years ago by ken-cunningham-webuse

Are we thinking of committing this someday?

Last edited 7 years ago by ken-cunningham-webuse (previous) (diff)

comment:16 Changed 7 years ago by ken-cunningham-webuse

now that we're on github, should I move this patch over there?

comment:17 in reply to:  16 Changed 7 years ago by larryv (Lawrence Velázquez)

No. Please don’t open GitHub pull requests for existing Trac tickets.

Changed 7 years ago by mojca (Mojca Miklavec)

my suggested (untested) revision of the rev1 patch for hatari

comment:18 Changed 7 years ago by mojca (Mojca Miklavec)

I attached some suggestions for improving the patch:

  • Moved the darwin-specific code to a single block and use replace rather than delete and append
  • Moved compiler blacklisting inside if {![variant_isset commandlineapp]} {...}
  • if { ${os.major} >= 10 } could of course be replaced by else, it's up to you. I felt the two things (whether or not to build the gui and whether to use libsdl or libsdl2) were not related in any way. Then, if one day gui stops working on 10.6, you could accidentally disable the else part for 10.6.
  • Added some random minor whitespace changes to comply with "multiples of 4" rule (I didn't include your changes of lines, but that's not to say they shouldn't be there, I just didn't want to make the patch too big). I'm not sure whether or not line & whitespace changes warrant a separate commit in this case or not. This is usually critical when the whole file changes. Just a few minor changes should be fine.

Personally I would make gui an option rather than commandline and make it default on 10.6+, but that's just "cosmetics" and up to maintainer to choose. I guess I would make libsdl2 default on (at least) 10.7+ and libsdl as the only option on 10.5 and lower. I don't see any reason to keep it as a variant on 10.7+, but again that's up to you to decide, maybe the functionality differs or needs more testing? You could keep the variant on 10.6 if you want and stay at libsdl as the default one.

These are just a few random thoughts. Please test and provide your final decision/patch.

comment:19 Changed 7 years ago by ken-cunningham-webuse

Thanks! Will look this over and we can finalize it shortly. I appreciate your efforts and advice. - K

comment:20 Changed 7 years ago by ken-cunningham-webuse

Mojca & others - hatari has been updated to version 2.0.0 now. A new Portfile is finished, incorporating your ideas, and I suppose it would make the most sense to post it up to github. I think you can close this ticket off now as a 'wontfix' as we move on.

comment:21 Changed 7 years ago by mojca (Mojca Miklavec)

You can also attach it here if you want.

comment:22 Changed 7 years ago by l2dy (Zero King)

Resolution: wontfix
Status: newclosed

comment:23 Changed 7 years ago by kencu (Ken)

Yeah, I gotta upload the the new portfile. Thanks for reminding me!

Note: See TracTickets for help on using tickets.