Tk Source Code

Re: recent change to unix/Makefile.in:

| Author: das <das>  2008-03-19 10:21:47
| Committer: das <das>  2008-03-19 10:21:47
|    ttkStubLib.o needs to be in tk library as well as stub library

Why is this needed?  

See also #1819422.  From talking with dgp, I think the consensus is that extensions should _either_ compile with -DUSE_FOO_STUBS and link with libfoostub.a, _or_ compile without -DUSE_FOO_STUBS and link directly against libfoo.a.

There is no need, as far as I can tell, for TtkInitializeStubs or the rest of the stub mechanism to be included in libtk.so, and in fact this will break other linkage mechanisms (in particular any dependent extensions that link against Tile's stub library instead of Tk's).

Logged In: YES 
user_id=90580
Originator: NO

Hi Joe, apologies for the delay

I have now worked around this issue in Ffidl by including a private (renamed) copy of TtkInitializeStubs, as I had already been doing with Tk_InitStubs. I had originally wanted to get away from that approach and use the public functions in libtkstub (resp. libtk) but given the difficulties in this area it is easier to continue doing things this way... (BTW, this is all for an update to Ffidl for 8.5 that has not yet been released).

Even though it now no longer affects me, I maintain that there continues to be a problem with the presence of tkStubLib.o in both libtk and libtkstub, which prevents linking with both libtk and libtkstub in some circumstances due to duplicate symbol definition errors from Tk_InitStubs et al (e.g. when linking with -force_flat_namespace in Mac OS X 10.4 and earlier). The same is of course true with tclStubLib.o in both libtcl and libtclstub.

One way around the need to link with both libtk and libtkstub was to place the symbols only present in libtkstub (the ttk stub symbols) into libtk, thus obviating the need to link with libtkstub...

This was only ever intended to be a workaround; as you say, the correct solution is for Tk_InitStubs et al to be removed from libtk (resp Tcl_InitStubs from libtcl), I certainly completely agree with that. 
At the same time these symbols should be made MODULE_SCOPE again so that simply linking with libtkstub.a does not cause {Tcl,Tk}_InitStubs et al to be exported from your extension, which causes other duplicate symbol problems (c.f. the dance I have to do in TK_SHLIB_LD_EXTRAS in tk/unix/configure.in to avoid exporting the tcl stub lib symbols from libtk...)

In short given the problems that my workaround caused with tileqt, I'm fine with the revert, and agree that we should move forward more aggressively with [Patch 1819422], given that is now again impossible to link with the ttk stub symbols in some circumstances.

While experimenting with all this, I came across two small issues with that ttk stub support that I just committed fixes for:
- the definition of Ttk_InitStubs when USE_TTK_STUBS is not defined was incorrect (missing 4th arg to Tcl_PkgRequire).
- when building Tk with --disable-shared, the ttkStubLib.o in libtkstub lib would reference Tcl symbols without going through the tcl stubs table, making it impossible to link extension by linking only with libtclstub and libtkstub.

Logged In: YES 
user_id=68433
Originator: YES

| What isn't supported is "stubbed but static"

Actually that's how *kits are usually built, including tclkit.  (This is partly because TEA does not provide a convenient way to turn off -DUSE_TCL_STUBS; there may be other reasons.)

That should continue to work if we move the stub table pointer and stub loader out of the main libraries; however *kit build systems will need to be updated to include -ltclstub at link-time if they're not doing so already (it looks like tclkit does not).

Logged In: YES 
user_id=79902
Originator: NO

FWIW, I'm in agreement with Joe. There are two supported modes for building things that link against Tcl (and Tk, which uses an equivalent mechanism):

1) Stubbed (-DUSE_TCL_STUBS plus -ltclstub) and which must be dynamically loaded

2) Unstubbed (-UUSE_TCL_STUBS plus -ltcl) which can be either dynamically loaded (relying on the platform to link stuff right) or statically linked

What isn't supported is "stubbed but static"; that is not the purpose of the stubs mechanism.

Logged In: YES 
user_id=68433
Originator: YES

More:

> I have detailed my reasons here, you OTOH have not bothered to explain how
> this change is supposed to break any "other linkage mechanisms",

In particular any dependent extensions that compile against Tile headers and link against Tile's stub library.  For example, tileqt.  (As of 8.5.1, this is the only way that tileqt can be built, due to other MODULE_SCOPE and stubs table related screwups).

> so I don't quite understand why you want this to be reverted so badly.

Because it's a mistake, and it gives us yet another thing to undo.

Also because until you mentioned ffidl, I was not aware of anything that would be broken by reverting it, (other than maybe `make checkstubs`).

Logged In: YES 
user_id=68433
Originator: YES

More: 

Re-re-re-reading the initial problem report for #1716117:

(a) static wish8.5 wouldn't link
(b) because libtk8.5 includes Tk_InitStubs
(c) which references tclStubsPtr
(d) which was no longer publically visible from libtcl.so
(e) to make `make checkstubs` work

The fix (2007-05-16, generic/tclStubLib.c r1.15) reverted (d).

Changing (b) would have also worked: libtk8.5 does not need to, and should not, have a copy of Tk_InitStubs().  This is what [Patch 1819422] proposes to do.  I suggest we move forward with this.

Daniel -- will this fix the ffidl problem?

Logged In: YES 
user_id=68433
Originator: YES

Further analysis:

There are three parts to the stubs mechanism: the stub table, the stub loader, and the stub table pointer.  Client code may either link directly against the main library, or compile with -DUSE_XXX_STUBS and link against the stub library.

The stub table (e.g., tclStubs, defined in tclStubInit.c) holds statically-initialized pointers to all the exported functions; it belongs in the main library.  

The stub loader (e.g., Tcl_InitStubs, defined in tclStubLib.c) is responsible for storing the address of the stub table into the stub table pointer (e.g., tclStubsPtr, also defined in tclStubLib.c).  The stub loader and the stub table pointer belong in the stub library.

In code compiled with -DUSE_XXX_STUBS, public entry points are #defined as indirect function calls through the XXX stub table pointer.  Code compiled with -DUSE_XXX_STUBS must therefore (1) call the stub loader at initialization time and (2) link with the stub library.

By historical accident, Tcl and Tk have included an extra copy of their stub loader and stub table pointer in their main libraries. Because of this, code compiled with -DUSE_XXX_STUBS can get away without doing (2); and on most Unix systems it can even get away without doing (1).   (On traditional ELF systems everybody shares the same copy of tclStubsPtr; so extensions that fail to call Tcl_InitStubs() will still work.)

Logged In: YES 
user_id=68433
Originator: YES

> I have a use case that needs this, it does not involve any legacy
> extension but a new one (ffidl updated for 8.5) that cannot work without
> -DUSE_TTK_STUBS.

OK.  What breaks, and how, and on what platforms?

-DUSE_TTK_STUBS should only affect code that #include's tkTheme.h or ttkTheme.h.  ffidl does not appear to do so, so this is either a red herring or something else has gone wrong that I don't see.

Logged In: YES 
user_id=90580
Originator: NO

I was referring to the following comment by jenglish in 1716117:

> This affects more than just Tk: building an extended tclsh / Big Wish with
> statically-compiled, stubs-enabled extensions and linking against libtcl.so
> no longer works either.  (Reported by Rolf Ade on the chat).
>
> Option (1), export tclStubsPtr from libtcl.so again, is the right
> solution.


(same here for the lateness, excuse any testiness that may have come through as a result, probably better that I leave this along for tonight...)

Logged In: YES 
user_id=79902
Originator: NO

I did before posting. (It's very late here though.)

I don't get at all why that file has to be in the main library. That's a crucial fact that's missing. As far as I can see, neither issue report describes the reason.

Logged In: YES 
user_id=90580
Originator: NO

dkf: now you need to go and reread 1716117 ...

that bug was all about re-exposing the symbols in tclStubLib.o and tkStubLib.o from the libtcl and libtk shared libraries after they were made MODULE_SCOPE
Indeed tclStubLib.o _is_ part of libtcl.so, same as tkStubLib.o is part of libtk, that's exactly the point of 1716117 and 1819422

dgp and jenglish made the decision in 1716117 that this state of things should remain for 8.5; IMO a necessary (if uninteded) consequence of that decision is that the ttkStubLib.o symbols now also need to be in libtk, for the reasons explained in my previous comments.

Note that I have nothing against having the ttkStubLib.o symbols in libtkstub only, as long as the tkStubLib.o symbols are also not in libtk (but as mentioned, it was decided that the latter could not happen for 8.5)

Logged In: YES 
user_id=79902
Originator: NO

das: tkStubLib.o should never have been part of libtk.so, for exactly the same reason that tclStubLib.o is not part of libtcl.so. If someone's been foolish enough to put it in there, Stop That. It breaks things. If your code requires it, your code is wrong.

Logged In: YES 
user_id=90580
Originator: NO

oh come on! have you even read my previous comment?
it is _not_ possible to compile with -DUSE_TTK_STUBS and link with libtkstub for a static extension in an app linked with libtk (because of the legacy presence of tkStubLib.o in libtk, as explained), this is exactly as in the case described in 1716117...  
I have a use case that needs this, it does not involve any legacy extension but a new one (ffidl updated for 8.5) that cannot work without -DUSE_TTK_STUBS.
I have detailed my reasons here, you OTOH have not bothered to explain how this change is supposed to break any "other linkage mechanisms", so I don't quite understand why you want this to be reverted so badly.
As mentioned, the only other option to use ttk stubs in this situation is to build a separate libttkstub lib, if you're going ahead with the revert, are you going to implement that then?

Logged In: YES 
user_id=68433
Originator: YES

Summarizing discussion on the tcl'ers chat: it is the intent that if you compile with -DUSE_FOO_STUBS you're supposed to link with -lfoostub.  So including a duplicate copy of the Ttk stub loader in libtk.so is not desirable; and since there are no legacy  extensions that expect it to be there it's not necessary either.

Moving it back out again.

Logged In: YES 
user_id=68433
Originator: YES

| reread your own comments in 1716117...

The issue in #1716117 was existing extensions that (contrary to recommendations) compiled with -DUSE_{TCL|TK}_STUBS but did not link with -l{tcl|tk}stub.  AIUI, the resolution for 1716177 was that this mode of operation does not need to (and should not) be supported, but that 8.5 was the wrong time to break this particular mode of operation.

Fortunately (sort of), there are no legacy issues wrt. the Ttk stubs table -- the only known extant extension dependent on the Ttk stubs table does not work with Tk 8.5.0 or Tk 8.5.1 anyway (see #1863007).

Logged In: YES 
user_id=90580
Originator: NO

reread your own comments in 1716117...

while I certainly agree with the general sentiment that extensions "_either_ compile with -DUSE_FOO_STUBS and link with libfoostub.a, _or_ compile without -DUSE_FOO_STUBS and link directly against libfoo.a" this does not work for static linking with Tk currently due to the legacy presence of tkStubLib.o in libtk, which makes it impossible to link with both -ltk and -ltkstubs due to duplicate symbols from the tk stubs tables. This is not a mode of linking supported for other extensions, so the general stub lib rules don't really apply...

So the only way to get the symbols from ttkStubLib.o when needing to link with -ltk is for them to be included in the tk library itself, hence my change (you reverted my tclStubLib and tkStubLib MODULE_SCOPE changes in 1716117 yourself for exactly that same reason...)

It's an all or nothing affair, once we remove the tkStubsPtr vars from libtk in 8.6, ttkStubsPtr can go as well, but it does not work to only have some but not all symbols from libtkstub in libtk...

The only other possibility would be to build ttkStubLib.o into its own static library libttkstub.a and doc that this needs to be linked along with -ltk (but _not_ -ltkstub until 8.6) when linking statically with tk and extensions using ttk stubs, not a good solution IMO...

I don't quite see how this change could break the other linkage mechanisms you mention, can you elaborate?

Logged In: YES 
user_id=80530
Originator: NO


This is similar to 1819422, I think.

Our plan was to get the *StubsPtr variables
out of the shared libraries during 8.6
development, but the project had too much
need for trial and error and testing to attempt
in a patch release.

I think it's a mistake to copy this pattern,
and give us yet another thing to undo.  Don't
put ttkStubsPtr into libtk without a really
good reason and no other reasonable solution.