polymonster / premake-android-studio

premake5 module for android-studio and gradle build.
MIT License
34 stars 14 forks source link

Fetch cflags and ldflags from toolset #2

Closed Gaztin closed 5 years ago

Gaztin commented 5 years ago

Premake has preset generators for cflags, ldflags etc. Since we're using gcc/clang we can fetch them instead of having to generate them ourselves!

polymonster commented 5 years ago

Hi, Thanks for contributing..

I am just trying to get my projects working with these changes, I am having trouble with new linker options on windows: --out-implib

what platform are you building on? my projects are working ok on macOS but having to roll back these changes to get parity with windows..

Gaztin commented 5 years ago

I am building on Windows as well, but I'm not running into any problems with the linker flags. How does the premake script look like for that project?

polymonster commented 5 years ago

So the premake lua setup is quite convoluted because we target a lot of platforms and a lot of the premake code is shared, this project is also for my work so I cant just show the whole thing..

Before getting to the linker flags issue there are a couple of issues with cflags and cxxflag as well.. I get "-m32" or "-m64" depending on premake architecture I set. This causes issues with certain abis. We are supporting all abis and all 64bit versions as well, ... We are using android ndk/sdk to determine the architecture and build the correct abi and the -m flag is not required... we recently just shipped a product on all abis without needing this... am I ok to just remove this flag so we do not pass it to cmake?

As for linker flags my problem is building a static lib we are building in the target_link_libraries I get a flag "--out-implib="libname.lib"" firstly -out-implib is not recognised as an option, secondly it's extension should be .a anyway... it is a MinGW flag... again if you are not using this then I could just bypass it?

Also with you changes "-L" libdirs passed into target_link_libraries are relative and not absolute paths, so it is causing some issues for me.. are you using any relative libdirs to the premake5.lua file?

Let me know what you think!

** Update Our issue with the relative path is that we have multiple scripts which are inside different folders for different platforms, the paths here to libs are relative to the lua file that declares the lib dir, so when the get copied into cmake the relative dir is wrong... I can fix this to always make lib dirs absolute paths, is this ok for you?

Gaztin commented 5 years ago

I understand that you cannot share any code from your work. Fortunately this is sufficient. See, I did not add any new flags deliberately. I simply changed it to grab the cflags/cxxflags and ldflags from the toolset embedded in premake. See this file for details. The part that seems to be messing up on your end is caused by line 360:

if cfg.system == p.WINDOWS and not cfg.flags.NoImportLib then
    table.insert(r, '-Wl,--out-implib="' .. cfg.linktarget.relpath .. '"')

Which happens because your system is windows even when generating android-studio files. The reason this isn't happening in my project is because I have this piece of code at the top of my script:

-- Set system to android
if _ACTION == "android-studio" then
    _TARGET_OS = "android"
    system("android")
end

Now, about the -m32 vs -m64 I'm not sure what to do about. Maybe we can insert a hack that simply removes the architecture variable from the toolset.

So after this line: https://github.com/polymonster/premake-android-studio/blob/a834c40b5b380f4e846ea8036f84510ee009b29e/android_studio/android_studio.lua#L240 We add

toolset.architecture = nil
polymonster commented 5 years ago

system "android" fixes the lib issue and also "-m32/-m64".. because premake didn't support android I didn't consider setting the system.

I made a small change to your code for libdirs so that it uses the absolute path, hopefully this is ok for you as well?

Thanks for your help, we are using premake a lot and it is nice to improve the module so it is useful for more people as well 👍

Gaztin commented 5 years ago

That's great! I don't object to absolute paths at all. If there are problems with relative paths on your end, then absolute paths are the way to go! I'm just happy to see an android-studio generator for premake. I'm happy to contribute!