premake / premake-core

Premake
https://premake.github.io/
BSD 3-Clause "New" or "Revised" License
3.22k stars 620 forks source link

Outputting MACOSX_DEPLOYMENT_TARGET with xcode11 #1336

Closed baconpaul closed 5 years ago

baconpaul commented 5 years ago

Hello! I am not 100% sure this is a bug, but it is a problem for me, and I may have a solution but I'm not sure.

My computer auto-updated to Xcode 11 last night (I know, right?) and I found my project didn't run anymore. The reason was that Xcode 11 default builds to target osx 10.15 and I need to target earlier than that, since I'm not running 10.15.

To do that I need to add MACOSX_DEPLOYMENT_TARGET = 10.14 to my Xcode project. Basically xcode11 ignores the -mmacosx-min-version flag in favor of that.

So I added to my premake5.lua

    xcodebuildsettings
    {           
       ["MACOSX_DEPLOYMENT_TARGET"] = "10.14";
    }

which I thought would do it. But using stock premake5 alpha14 bin from your website my build settings turn out to have the 10.14 as an array. Here's my project.pbxproj

                        buildSettings = {
                                ALWAYS_SEARCH_USER_PATHS = NO;
                                CONFIGURATION_BUILD_DIR = target/au/Release;
                                DEBUG_INFORMATION_FORMAT = "dwarf-with-dsym";
                                GCC_DYNAMIC_NO_PIC = NO;
                                INSTALL_PATH = /usr/local/lib;
                                MACOSX_DEPLOYMENT_TARGET = (
                                        10.14,
                                );
                                PRODUCT_NAME = Surge;
                        };

Turns out that that actually doesn't work. So what I need to do is get rid of the ( ). So I forked your repo and built a local copy. In modules/xcode/xcode_common.lua around line 222 is where you output this and it turns out that you force even single values into an array. You have code commented out for the single value case. So if I uncomment that, by applying this diff

diff --git a/modules/xcode/xcode_common.lua b/modules/xcode/xcode_common.lua
index 9abfe396..454960c4 100644
--- a/modules/xcode/xcode_common.lua
+++ b/modules/xcode/xcode_common.lua
@@ -222,8 +222,8 @@
                        value(level, name)
                elseif type(value) ~= 'table' then
                        _p(level, '%s = %s;', stringifySetting(name), stringifySetting(value))
-               --elseif #value == 1 then
-                       --_p(level, '%s = %s;', stringifySetting(name), stringifySetting(value[1]))
+        elseif #value == 1 then
+                       _p(level, '%s = %s;', stringifySetting(name), stringifySetting(value[1]))
                elseif #value >= 1 then
                        _p(level, '%s = (', stringifySetting(name))
                        for _, item in ipairs(value) do

then run that premake so I don't get () in my project, everything works.

So OK there must be a reason you don't do that; and there may be a way for my syntax in my file to tickle that table branch. but I couldn't find it. So my question is

  1. Is there a way to get the single value behavior without that diff
  2. If not, would you consider uncommenting those single value lines or would you want to use a different approach to tagging single-ness?
  3. If you would prefer a different approach, can you sketch it for me? I had thought about adding an 'xcodetarget' keyword which inserted this specially into settings for instance.

I'm glad to try and code this in and send a PR of course. I just am not sure what the right sort of PR is so I figured I would open this issue first.

Thanks very much for your attention! Hope this issue is clear. And I would love it if there's a fix I can just make in my premake5.lua which tickles the right branch.

Best

samsinsane commented 5 years ago

1003 adds support for MACOSX_DEPLOYMENT_TARGET via systemversion, if you want to pull out the change for systemversion and then add some tests for it, that'd be great. Uncommenting those lines will likely break anything that needs to be a list and only has 1 item, and I don't believe there's a way to improve xcodebuildsettings. Also, I'm not sure there's a huge amount of value in improving that when we could just add proper support for more settings.

baconpaul commented 5 years ago

Oh thanks I’ll look this week!

baconpaul commented 5 years ago

Hey that was super easy. Firing in a PR now.