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

MACOSX/IOS: Various updates from PR #1128 #1307

Closed
wants to merge 8 commits into from

Conversation

@ccawley2011
Copy link
Member

commented Aug 19, 2018

Originally from PR #1296, and modified to keep the iOS 3-6 packaging rules.

@criezy
Copy link
Member

left a comment

I am not confortable at this point with getting rid of the old macOS packaging code (commit 51e33cf), since this was still what I used for the last release, and whoever does the next one (possibly me) might still want to use it. The commit mentioned that in a controlled environment where you only have the static libraries having a separate static build target is not needed, which is true. But in most environment you will have some dynamic libraries and developers might want to generate a static build without having to use the Docker image that allows it. Commit 4dfa61e might mitigate this, but I would like more time to check and verify those changes.

And as noted in my review I would also like more time to verify the change to the link flags in 830edf3.

append_var LIBS "-framework AudioUnit -framework AudioToolbox -framework Carbon -framework CoreMIDI"
# SDL2 doesn't seem to add Cocoa for us.
append_var LIBS "-framework Cocoa"
append_var LIBS "-framework AudioUnit -framework AudioToolbox -framework Cocoa -framework CoreMIDI"

This comment has been minimized.

Copy link
@criezy

criezy Aug 20, 2018

Member

SDL 1.2 needs Carbon, so we cannot just blindly remove it here. Actually SDL2 also still depends on it as well. But interestingly I do seem to get all of those when I use sdl2-config --static-libs, which is used in the ports/mk file. So I am not actually sure we need any of these here.

I would like some more time to check this.

csnover and others added some commits Oct 20, 2017

BUILD: Fix case-sensitivity issues with macOS ScummVMDockTilePlugin
This happened to work on case-insensitive filesystems only.
BUILD: Get rid of old macOS packaging code
Manually adding all the “correct” static libraries is error-prone
and broken. The CI service will not have dynamic libraries to link
so there is no concern about making the bundle work there.

If it is really necessary to allow some normal system with dylibs
generate a fully statically linked build some system to convert
all the -l flags in LDFLAGS into absolute library paths would be a
far better choice.
BUILD: Add partial option for statically linking system libraries
Eventually this should probably also use -static or -Wl,-Bstatic
for non-Darwin platforms. For now, it only does these things:

1. Switches to use --static-libs/--static when getting dependencies
   from the third-party libraries with configuration scripts, since
   this is needed to get the correct -framework flags from SDL2 and
   extra dependencies from FreeType2;
2. Rewrites linker flags from -lfoo to $staticlibpath/lib/libfoo.a,
   since this is required in order to get the Apple linker to do
   static linking whenever there is a shared library available.

This commit changes the recently added --enable-static flag name
since that flag DOES NOT actually generate static builds, it only
changes which dependencies are requested from third party
library configuration scripts.

@ccawley2011 ccawley2011 force-pushed the ccawley2011:macosx-ios-1 branch from dac46fb to d0169e7 Nov 13, 2018

@bluegr

This comment has been minimized.

Copy link
Member

commented Aug 4, 2019

Any news on this?

@criezy

This comment has been minimized.

Copy link
Member

commented Aug 4, 2019

I have merged the uncontroversial commits manually.

I don't want to merge commit 80bc3ae as is, and as a result I cannot merge commits 2c44681 and d0169e7. However those commits contain some nice change, but I will need more time to try to extract them if possible. I made a note of that and I am closing this PR now.

@criezy criezy closed this Aug 4, 2019

@bluegr

This comment has been minimized.

Copy link
Member

commented Aug 5, 2019

Thanks :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.