Skip to content

Conversation

@ArthurPKFX
Copy link

Commit d363e35 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?

  • 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
  • Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • Follow our coding conventions
  • Minimize the number of commits
  • Align documentation to your changes

Commit d363e35 broke visibility by not updating internal code to use new location
@nickclark2016
Copy link
Member

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

@alex-rass-88
Copy link
Contributor

Yes, I guess I overlooked it. My mistake.

@ArthurPKFX
Copy link
Author

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

@nickclark2016
Copy link
Member

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
Copy link
Member

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

@nickclark2016
Copy link
Member

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

@alex-rass-88
Copy link
Contributor

Now change need in one file:

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

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

@alex-rass-88
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants