Re: [Podofo-users] Visibility support checking in the build (was: exported symbols)

2018-02-13 Thread zyx
On Mon, 2018-02-12 at 12:00 +0100, zyx wrote:
> -fvisibility=hidden

Hi,
I just recalled, the above is not the best thing to be done. The right
approach is to use CMake, with the visibility variables:
https://cmake.org/cmake/help/v3.1/prop_tgt/LANG_VISIBILITY_PRESET.html
https://cmake.org/cmake/help/v3.1/variable/CMAKE_LANG_VISIBILITY_PRESET.html

That makes it something like:
   set(CMAKE_CXX_VISIBILITY_PRESET hidden)
somewhere at the top of the main CMakeList.txt or better by adding
   CXX_VISIBILITY_PRESET hidden
into
   SET_TARGET_PROPERTIES(podofo_shared PROPERTIES...)
and the like.

I suppose CMake will do the right thing when other compilers and/or
different platforms will be used, like those without
-fvisibility=hidden defined.

Bye,
zyx

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users


Re: [Podofo-users] Visibility support checking in the build (was: exported symbols)

2018-02-12 Thread zyx
On Sat, 2018-02-10 at 22:47 +0100, Matthew Brincke wrote:
> [ typo fixes in quoted text ]

[ not changing history, not feeling good to correct anyone for any
typos, due to being humans, which can make mistakes ]

> I still hope that won't be necessary (@Dom or zyx: could you please
> put the feature test in?).

Hi,
I guess you mean a test for availability of -fvisibility=hidden by the
compiler. You can do it with something like this:

   include(CheckCXXCompilerFlag)

   list(APPEND proposed_cxx_flags
-fvisibility=hidden
   )

   foreach(flag IN LISTS proposed_cxx_flags)
check_cxx_compiler_flag(${flag} cxx_flag_${flag}_supported)
if(cxx_flag_${flag}_supported)
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${flag}")
endif(cxx_flag_${flag}_supported)
unset(cxx_flag_${flag}_supported)
   endforeach()

where the 'proposed_cxx_flags' list can contain any warnings and such
flags. This code should be run somewhere the top of the main
CMakeList.txt, because it modifies CMAKE_CXX_FLAGS, which is used by
the other CMake code/parts.
   
> 
> > bunch of exported symbols, alas not that many, probably more
> > PODOFO_LOCAL and PODOFO_API are still needed through the codebase.
> 
> Yes, probably, I hope that can still be changed after the release
> candidate.

I agree with Mattia, there is no rush for this. It's also a very bad
idea to provide a release candidate and then generate release with
completely changed rules of the build. That can change the game
significantly and all the testing on the previous candidate would be
useless and waste of time. The idea of the release candidate is to
provide something almost stable, with possible changes with "minimal"
impact (like bug fixes).
Bye,
zyx

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users


Re: [Podofo-users] Visibility support checking in the build (was: exported symbols)

2018-02-11 Thread Mattia Rizzolo
On Sat, Feb 10, 2018 at 10:47:41PM +0100, Matthew Brincke wrote:
> The CMake variable to set to express visibility support is now the unset
> PODOFO_HAVE_GCC_SYMBOL_VISIBILITY ("GCC" means "GCC-compatible" here).
> This variable should be set by a feature test in the CMakeLists.txt which
> I'm sorry for not being able to implement right now, I've never done such
> and would really like to contribute other fixes too (@Dom: until which time?).

I asked around this a bit some weeks ago, and it seems that a way to do
it is to use CHECK_C_COMPILER_FLAG / CHECK_CXX_COMPILER_FLAG (which
comes from /usr/share/cmake-3.9/Modules/CheckCXXCompilerFlag.cmake and
is even available for cmake back 2.6, so there should be no need to
vendor that .cmake file).
That doesn't cover a case that for example libssh's variant does:
https://sources.debian.org/src/libssh/0.8.0~20170825.94fa1e38-1/ConfigureChecks.cmake/?hl=40#L37
There it tries to build 'void __attribute__((visibility(\"default\")))
test() {} int main(void){ return 0; }' with -fvisibility=hidden.
Apparently, according to one person from my IRC circle,
ISTR -fvisibility=hidden is quietly accepted by any new enough GCC,
but when you actually try to use the attribute you get warnings if
the target platform doesn't support it.  so to avoid a very noisy
build on such platforms you may want to run a test compile with -Werror


I'll leave to whoever implements this to figure out the best course of
action.

> > bunch of exported symbols, alas not that many, probably more
> > PODOFO_LOCAL and PODOFO_API are still needed through the codebase.
> 
> Yes, probably, I hope that can still be changed after the release candidate.

I don't think there is any need to rush this part, or get crazy about.

-- 
regards,
Mattia Rizzolo

GPG Key: 66AE 2B4A FCCF 3F52 DA18  4D18 4B04 3FCD B944 4540  .''`.
more about me:  https://mapreri.org : :'  :
Launchpad user: https://launchpad.net/~mapreri  `. `'`
Debian QA page: https://qa.debian.org/developer.php?login=mattia  `-


signature.asc
Description: PGP signature
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Podofo-users mailing list
Podofo-users@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/podofo-users