Make wxWindow::GetContentScaleFactor() work for GTK3

[ojwb]

This is a partial backport of f95fd11.

[vadz]

I'm not sure if we really need to add the new function to version-script.in, but it should do no harm, shouldn't it?

[ojwb]

The obvious *wxWindow*GetContentScaleFactor* would also match the pre-existing wxWindowBase::GetContentScaleFactor() wouldn't it?

I see the last stanza attempts to handle a similar case:

@WX_VERSION_TAG@ {
        # Explicitly mention this one as otherwise it would be caught by
        # wxDataViewRenderer*FinishEditing wildcard in 3.0.3 above.
        *wxDataViewRendererBase*FinishEditing*;
        *;
};

But I don't understand how that can actually work - if it's the last match in this file which wins for a given symbol then that * would always win; if the first match wins, then *wxDataViewRendererBase*FinishEditing* would need to be at the top to work.

Poking with nm on the resulting shared libraries shows it does seem to work though:

$ nm --dynamic --with-symbol-versions lib/libwx_gtk3u*-3.0.so*|grep 'wxDataViewRender.*FinishEditing'
000000000016ec20 T _ZN18wxDataViewRenderer13FinishEditingEv@@WXU_3.0.3
00000000000d06a0 T _ZN22wxDataViewRendererBase13FinishEditingEv@@WXU_3.0
000000000016ec20 T _ZN18wxDataViewRenderer13FinishEditingEv@@WXU_3.0.3
00000000000d06a0 T _ZN22wxDataViewRendererBase13FinishEditingEv@@WXU_3.0
000000000016ec20 T _ZN18wxDataViewRenderer13FinishEditingEv@@WXU_3.0.3
00000000000d06a0 T _ZN22wxDataViewRendererBase13FinishEditingEv@@WXU_3.0

So I'm very confused. Is * somehow special as a pattern or something?

I tried to RTFM but the ld man page unhelpfully refers me to a VERSION section which doesn't seem to actually exist.

[ojwb]

A related question - if a new symbol is versioned, what should I do when backporting the patch to the Debian package?

If I leave the version on, it's versioned as WXU_3.0.5 in a 3.0.4 (+ patches) build - is that going to cause a problem?

[ojwb]

I found https://sourceware.org/binutils/docs/ld/VERSION.html#VERSION which seems to be where the man page is meaning to point to.

That doesn't seem to say what happens when a symbol matches more than one pattern, except for indicating that * only matches if nothing else does:

You can bind all otherwise unspecified symbols to a given version node by using ‘global: *;’ somewhere in the version script

But given this works for existing cases, I guess I'll just copy that and not worry about how or why.

[vadz]

To answer your question (better late than never...): I think that the version script file is read from top to bottom and so wxDataViewRendererBase::FinishEditing() is assigned WXU_3.0.3 version initially, but it's overwritten later with WXU_3.0 when the contents of the last section is applied. So your changes do look correct to me, however there is an extra twist in this case: it looks like the shared library doesn't export wxWindowBase::GetContentScaleFactor() at all (because it's inline, presumably), so it doesn't have any version because it doesn't appear in nm -D output in the first place. I think it still does no harm to have the entry in the version script for it even so, though, so I won't change anything here and will (squash) merge your changes as they are in a moment.

Thanks again!

[ojwb]

I think that the version script file is read from top to bottom and so wxDataViewRendererBase::FinishEditing() is assigned WXU_3.0.3 version initially, but it's overwritten later with WXU_3.0 when the contents of the last section is applied

Right, but that can't also be true for the * pattern - I guess that's just a special case.

[vadz]

Right, but that can't also be true for the * pattern - I guess that's just a special case.

You're right, and this page confirms it. I'd like to quote it here because this information is really valuable and I don't want to lose it if this page disappears, so here is the relevant excerpt:

---

This is the result:

• If there is an exact match, then we use the first tag in the version script where it matches.
  • If the exact match in that tag is global, it is used.
  • Otherwise the exact match in that tag is local, and is used.
• Otherwise, if there is any match with a global wildcard pattern:
  • If there is any match with a wildcard pattern which is not *, then we use the tag in which the last such pattern appears.
  • Otherwise, we matched *”. If there is no match with a local wildcard pattern which is not *”, then we use the last match with a global *. Otherwise, continue.
• Otherwise, if there is any match with a local wildcard pattern:
  • If there is any match with a wildcard pattern which is not *”, then we use the tag in which the last such pattern appears.
  • Otherwise, we matched“*, and we use the tag in which the last such match occurred.

---

I've also learned that we could use extern "C++" to just specify the name directly instead of using the ugly and error-prone wildcards. We definitely should do it at least for 3.2 (and yes, I do hope it will be released one day...).

[ojwb]

Ah yes, that blog entry is much more helpful.

I've also learned that we could use extern "C++" to just specify the name directly instead of using the ugly and error-prone wildcards.

Yes, though the ld manual warns:

Demangled names may contains spaces and other special characters. As described above, you can use a glob pattern to match demangled names, or you can use a double-quoted string to match the string exactly. In the latter case, be aware that minor differences (such as differing whitespace) between the version script and the demangler output will cause a mismatch. As the exact string generated by the demangler might change in the future, even if the mangled name does not, you should check that all of your version directives are behaving as you expect when you upgrade.

But then it also warns against using wildcards in the way wx currently does:

Note that it’s slightly crazy to use wildcards in a global spec except on the last version node

(AIUI all wx's specs are implicitly global.)

[vadz]

First of all, I have no idea why, but d0ab581 didn't result in this PR being closed.

Second, I've checked the output of nm and using demangled symbol names in the version script results in exactly the same version info in the libraries, so I've committed ebb046f which removes the wildcards.

Thanks for looking into this!

[ojwb]

First of all, I have no idea why, but d0ab581 didn't result in this PR being closed.

I can answer that - see https://help.github.com/en/articles/closing-issues-using-keywords (my emphasis):

When a pull request or commit references a keyword and issue number, it creates an association between the pull request and the issue. When the pull request is merged into your repository's default branch, the corresponding issue is automatically closed.

So essentially only merging to master will auto-close, because that's the default branch.