On 04/29/2016 06:49 PM, Yury Gribov wrote:
On 04/29/2016 04:28 PM, Emil Velikov wrote:
On 28 April 2016 at 15:28, Yury Gribov <[email protected]> wrote:
On 04/28/2016 04:59 PM, Emil Velikov wrote:
On 27 April 2016 at 08:48, Yury Gribov <[email protected]> wrote:
On 04/26/2016 02:14 PM, Emil Velikov wrote:
On 26 April 2016 at 07:54, Yury Gribov <[email protected]> wrote:
On 04/20/2016 10:03 AM, Yury Gribov wrote:
On 04/19/2016 06:35 PM, Yury Gribov wrote:
On 04/18/2016 06:21 PM, Adam Jackson wrote:
On Mon, 2016-04-18 at 09:23 +0300, Yury Gribov wrote:
Does below look like a sane approach?
1) get all deps via
$ apt-cache rdepends libice6 libice-dev
2) unpack each of the above .debs and scan for ELFs
3) for each ELF see if one of now hidden symbols is UND
That sounds good to me.
I've cooked a simple script (attached, reviews welcome) and
applied
it
to Ubuntu 14. It showed that there are additional uses for
_IceTransNoListen (mentioned by Alan in separate email) but
nothing
else.
Note that this does not handle transitive dependencies e.g.
if some
weird library links static version of libICE and the re-exports
it's
symbols as part of new lib's public interface.
It'd be possible to scan for this too I suspect. Look for
packages
that BuildRequire libice6-static, scan the resulting binaries for
exports. I suspect there are zero such packages.
I have postponed this activity for now (especially given that this
behavior would be a serious packaging abuse).
Here's an updated patch with added _IceTransNoListen. Does this
look
better now?
Ping.
Pong. Sending patches as attachments is not the most productive
way to
get things reviewed ;-)
I see. Strangely the Content-Disposition in my previous email is
properly
set to "inline". I've both inlined and attached the new patch to be
sure.
Close but no cigar :-P Please follow the suggestions in the wiki
http://www.x.org/wiki/Development/Documentation/SubmittingPatches/
I've checked them (from the very beginning actually) and there seems
to be
no strong requirement for attachments (it even says "Always attach
patches
as plain text files - if emailed then either attached or in-line.").
Looks like you read more than me - I never got past the first 15 lines
(the ones mentioning git send-mail). Sorry about that had no idea that
the docs are so ... interesting
+have_visibility=disabled
+if test x$SYMBOL_VISIBILITY != xno; then
+ AC_MSG_CHECKING(for symbol visibility support)
+ if test x$GCC = xyes; then
+ VISIBILITY_CFLAGS="-fvisibility=hidden"
+ else
+ if test x$SUNCC = xyes; then
+ VISIBILITY_CFLAGS="-xldscope=hidden"
+ else
+ have_visibility=no
If Cygwin/Mingw does support the visibility flags (see below) I'd just
warn/error out in here.
IMHO if a specific platform/compiler is not handles we better get a
report and sort it out.
But what would we do if there are problems with some
compiler/version/platform (except for configure-time testing)?
Sort it out accordingly as we know a bit more about the users.
Which reminds me -> one should update the .pc file as well -> add
xproto to the Requires.private section.
In general I would not bother with anything more than
- PKG_CHECK_MODULE(XPROTO, xproto) + wire up the XPROTO_CFLAGS (both
missing)
- Add the above "if test $compiler set VISIBILILTY_FLAGS" +
propagate
the latter throughout (both done)
- Add (now missing) #include <X11/Xfuncproto.h> in the respective
places and annotate the functions (already done)
No extra configure toggles (above), no configure time testing. Unless
cygwin/mingw requires the latter.
So the latter is needed. Fair enough.
+ fi
+ fi
+ if test x$have_visibility != xno; then
+ save_CFLAGS="$CFLAGS"
+ proto_inc=`$PKG_CONFIG --cflags xproto`
+ CFLAGS="$CFLAGS $VISIBILITY_CFLAGS $proto_inc"
+ AC_COMPILE_IFELSE([AC_LANG_PROGRAM([
+ [#include <X11/Xfuncproto.h>
+ #if defined(__CYGWIN__) ||
defined(__MINGW32__)
+ #error No visibility support
Are you sure that think this is correct ? Afaict things should work,
despite that xserver sets SYMBOL_VISIBILITY=no.
Jon, any ideas why xserver disables -fvisibility=hidden ? Should we
remove that line or there's something subtle that requires the
behaviour ? For example: relying on symbols that are not marked as
exported or alike.
Note that _X_EXPORT is simply not defined for Cygwin and Mingw so
there are
basically no annotations at all. They ignore visibility annotations
and warn
on them by default:
visi.c:0: warning: visibility attribute not supported in this
configuration; ignored
Most OSS projects disable visibility on Cygwin/Mingw for this reason.
Yes, as mentioned earlier, one should not be using attribute
visibility but __declspec dllexport/dllimport for Windows.
As to why they're missing is beyond me.
So all in all, even though some of my suggestions did not work out
there's still a few bits missing in the patch.
Thanks
Emil
Emil,
Please check if I've properly addressed all your suggestions.
I actually started to dislike the current X11 approach to visibility
enabling. We seem to detect it twice - in package's configure.ac and in
generic Xfuncproto.h, in totally different and probably sometimes
incompatible ways (optimistic compile check in configure vs. rigid
compiler version check in Xfuncproto.h).
Ping :)
_______________________________________________
[email protected]: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel
