premake-core icon indicating copy to clipboard operation
premake-core copied to clipboard

Fix visibility setting on android and gcc VS

Open ArthurPKFX opened this issue 1 year ago • 8 comments

Commit d363e35f5b335b15694bd6e8e96166fcf51e510d broke visibility by not updating internal code to use new location

What does this PR do?

Fixes the visibility setting for android and gcc VS projects (untested for gcc VS, just noticed it used the same old location for visibility)

How does this PR change Premake's behavior?

Doesn't error anymore

Anything else we should know?

Wanted to add a unit test but no idea in which file to add it

Did you check all the boxes?

  • [x] Focus on a single fix or feature; remove any unrelated formatting or code changes
  • [ ] Add unit tests showing fix or feature works; all tests pass
  • [x] Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • [x] Follow our coding conventions
  • [x] Minimize the number of commits
  • [x] Align documentation to your changes

ArthurPKFX avatar Nov 22 '24 14:11 ArthurPKFX

Can you add a unit test that demonstrates the bug (and by proxy, the fix)?

nickclark2016 avatar Nov 23 '24 09:11 nickclark2016

Yes, I guess I overlooked it. My mistake.

alex-rass-88 avatar Nov 24 '24 15:11 alex-rass-88

@nickclark2016 As stated I wasn't sure which test file to put it in. Please tell me where to put it 🙂

ArthurPKFX avatar Nov 25 '24 11:11 ArthurPKFX

For the VS module - vc2010/test_compile_settings.lua For the Android module - There does not appear to be a good file for it currently. Feel free to add a new file in the module's test directory (make sure to add it to the _tests.lua file)

nickclark2016 avatar Nov 25 '24 14:11 nickclark2016

Hi, just wanted to poke on this to see if any progress has been made on UTs

nickclark2016 avatar Dec 10 '24 18:12 nickclark2016

@alex-rass-88 Just wanted to poke on this again, before we freeze the master branch for beta5's release this coming weekend.

nickclark2016 avatar Feb 11 '25 16:02 nickclark2016

Now change need in one file: https://github.com/premake/premake-core/blob/801e6a0d0b21bdd0483cbbaf28e469c190b23140/modules/vstudio/vs2010_vcxproj.lua#L4448

table.insert(opts, p.tools.gcc.cxxflags.visibility[cfg.visibility]) ==> table.insert(opts, p.tools.gcc.shared.visibility[cfg.visibility])

alex-rass-88 avatar Feb 12 '25 06:02 alex-rass-88

For tests need add to modules/vstudio/tests/android/test_android_project.lua:

        function suite.checkVisibilityFlagsHidden()
                 visibility "Hidden"
                 prepare()
                 test.capture [[
<ClCompile>
	<PrecompiledHeader>NotUsing</PrecompiledHeader>
	<Optimization>Disabled</Optimization>
	<AdditionalOptions>-fvisibility=hidden %(AdditionalOptions)</AdditionalOptions>
</ClCompile>
                 ]]
        end

        function suite.checkVisibilityFlagsInternal()
                 visibility "Internal"
                 prepare()
                 test.capture [[
<ClCompile>
	<PrecompiledHeader>NotUsing</PrecompiledHeader>
	<Optimization>Disabled</Optimization>
	<AdditionalOptions>-fvisibility=internal %(AdditionalOptions)</AdditionalOptions>
</ClCompile>
                 ]]
        end

        function suite.checkVisibilityFlagsProtected()
                 visibility "Protected"
                 prepare()
                 test.capture [[
<ClCompile>
	<PrecompiledHeader>NotUsing</PrecompiledHeader>
	<Optimization>Disabled</Optimization>
	<AdditionalOptions>-fvisibility=protected %(AdditionalOptions)</AdditionalOptions>
</ClCompile>
                 ]]
        end

alex-rass-88 avatar Feb 12 '25 07:02 alex-rass-88