Tcl Source Code

View Ticket
Login
Ticket UUID: 2902965
Title: stub related changes cause tclkit built to break
Type: Bug Version: obsolete: 8.6b1.1
Submitter: nijtmans Created on: 2009-11-24 06:56:29
Subsystem: 40. Dynamic Loading Assigned To: patthoyts
Priority: 9 Immediate Severity:
Status: Closed Last Modified: 2009-12-26 09:20:36
Resolution: Wont Fix Closed By: sf-robot
    Closed on: 2009-12-26 02:20:36
Description:
Pat reported me the following. I didn't expect those changes
to cause such problems. This issue is meant to work
out a solution, for everybody to comment on.

Pat Thoyts:
I'm not sure this is correct. You now force the dde and registry
extensions to use stubs at all times - even when building static. tclkit
links static versions of these into the tclkit executable so now I get a
link error for unresolved symbol _tclStubsPtr from tclWinDde.obj and
tclWinReg.obj when linking the tclkit executable. Obviously I can add
the tcl stubs library to the linker command line but really should I
need to? Surely in a static binary like this I should only be exposing
the stubs interface - not making use of it internally.
The original bug you were fixing is irrelevant to the tclkit build as
when we construct the vfs we manufacture a static pkgIndex so that
package require does the right thing. ie:
 package ifneeded dde 1.n.n {load {} dde}

I could well be wrong, but surely when building a static version of
dde/registry I don't need the macro getting undef'd on me.

Also, if this breaks tclkit I think we can be sure it will break other
builds of embedded tcl elsewhere.
User Comments: sf-robot added on 2009-12-26 09:20:36:

allow_comments - 1

This Tracker item was closed automatically by the system. It was
previously set to a Pending status, and the original submitter
did not respond within 14 days (the time period specified by
the administrator of this Tracker).

nijtmans added on 2009-12-11 17:57:17:
Thinking more about it, the change that caused TclKit
build to break is that the tcldde and tclreg packages
are now compiled with TCL_USE_STUBS, which
they were not before. That means that any executable
which wants to include those two packages in static
form, now needs to link with the stub library. This is
exactly the kind of POTENTIAL INCOMPATIBILITY
as what was caused by the removal of tclStubsPtr
from the tcl library.

I tried to fix that by including the stub objects in
libtcl.a, but it was (rightfully) reverted by das,
because it caused multiply
defined symbols on older OSX.

The conclusion should be: The tclkit build should be
changed to link with the Tcl stub library in this case:
It was already needed when linking with tk.lib, now it
is needed when linking it with either tcldde.lib or
tclreg.lib. Therefore the "Wont Fix"

Assigning it to Pat Thoyts for evaluation.

nijtmans added on 2009-11-30 20:31:27:
Whooooah. Found the real cause of the problem on OSX: In 3 files
(tkConsole.c, tkWindow.c and tkMain.c), there are direct calls to
Tcl_InitStubs().If USE_TCL_STUBS is not definded, Tcl_InitStubs
is a macro which is supposed to be used when Tcl is compiled
statically. However, Tk is always compiled with USE_TCL_STUBS,
whether Tcl is compiled statically or not.
So Tk violates Tcl's rules in this respect!.

Hmmm. That's a completely separate problem which has
to be fixed first.....

nijtmans added on 2009-11-30 05:25:46:
I don;t have an older OSX to try this on, but - yes - breaking a
build is bad! I now synced the win/tcl.m4's from Tcl and Tk,
hoping (and expecting) everything is OK now. Actually, I
don't understand what's the problem including tclSubLib.o
in both libtcl.a and libtclstub.a: Linking archives only
results in linking the objects which contain symbols
which are unresolved yet. (?). But I beleave das on
his word that there's a problem on older OSX (yes, I
know there are strange machines in this world!)

So, I'll leave the tclStubsPtr subject for now
until I fully understand what's going on.

das added on 2009-11-29 16:30:55:
also reverted the unix/tclAppInit.c change

after re-reading some of your comments again to try and figure out what this was supposed to be fixing, I think you are confused re tclStubsPtr:

> No! For shared libraries nothing changes.
> Previously, there were two tclStubPtr variables,
> one in the shared library, another in the static
> stub library.

NO! there is no tclStubsPtr in the tcl shared library anymore, nor in the static tcl library (that was the whole point of the recent work to get stubsPtr out of libtcl), nor in tclsh, the only place to get tclStubsPtr from is the static stubs library.

das added on 2009-11-29 15:46:27:
I am reverting the unix tcl.m4 change, it breaks static tk builds on older OSX due to multiply defined symbols (cannot link against both libtcl and libtclstub with this change), MAKE_LIB should _not_ include ${STUB_LIB_OBJS} !

(Also note that we've been keeping tcl and tk unix/tcl.m4 in sync, please test any tcl.m4 change made in tcl/unix with tk/unix as well...)

If you need to link with the stub objects, explicitly link with the static stub library, that's the only place STUB_LIB_OBJS should be included, they should not be present in the main static lib.

We spent a lot of time not that long ago getting this right, I don't understand what this change is supposed to be fixing, but it's undoing all that work for no reason I see explained in this bug as far as unix is concerned (I don't pretend to understand the windows issue, your change may well be valid there).

Also, as previously mentioned, tclsh should _not_ link with the stubs library, reverted that change to unix/Makefile.in

nijtmans added on 2009-11-28 05:32:12:
Point noted. However, the difficult point is to assure that it works with ALL builds (makefile.vc, win/Makefile, unix/Makefile) before. Kevin rightfully reverted the win/tclAppInit.c change (I was planning to do that myself), but the real problem was a dependancy problem, not the change in tclAppInit.c itself. On Windows, you cannot reference to a variable in a static executable from a dll, so this change was useless!

patthoyts added on 2009-11-28 04:39:06:
In my opinion only bug fixes should be going into 8.5. Certainly not anything that might interfere with peoples build systems as this will likely do.

The change here seems to be to add the stubs library back into the tcl library when that is being built as a static library. It seems to me that if a build requires the stubs library to be available then it should specify this when linking. So it is that tclkit links with -ltcl8.5 and -ltclstubs. We went though this when dgp removed the stubs objects from the main tcl dll for 8.6a0. We had to fix a load of builds then and I don't much want to do it all again.

kennykb added on 2009-11-28 04:09:43:
Please do not backport. The changes do not fix bugs, they simply (allegedly) improve build cleanliness. 

They also have made me confront one broken build after another, and I am getting sick and tired of finding that the HEAD fails to build every time I update - and spending hours figuring what you did and restoring it.  You have been committing these changes entirely untested.

andreas_kupries added on 2009-11-28 04:02:47:
Yes, I object. 8.5.8 is the stable branch, and these changes you are doing currently look anything but stable.

nijtmans added on 2009-11-26 19:20:06:
I would like to backport the fix (see patch below) to
Tcl/Tk 8.5 as well. Just this patch, nothing else (no
changes to the tclreg/dde/tcltest build, no changes
to the tclStubPtr variable in libtcl.so!). Reason: It
would move away the use of tclStubsPtr from
libtcl.so, reducing the chance of any ill-behaviour.
If some (ill-compiled) extension previously would
find tclStubPtr in libtcl.so, now it will find it in
tclsh/wish and it is guaranteed to be initialized.

Any objections?

nijtmans added on 2009-11-26 16:21:39:
Fixed in HEAD (/win and /unix). Tk still to be done.
(Remarks/critics/suggestions still welcome!)

patthoyts added on 2009-11-25 07:34:24:
> make it possible to build both a static tclreg12.lib
> and a tclreg12.dll next to each other 
When you build a dll you get a link library with it - so creating a tclreg12.dll also creates a tclreg12.lib.
The static lib is also named tclreg12.lib although we usually try to add a suffix to help discriminate this from the link library so getting tclreg12s.lib. So creating the two types of library 'next to each other' is something to take care with.

nijtmans added on 2009-11-25 03:20:29:
>Should I conclude then, that after
>we had completed taking tclStubsPtr
>out of libtcl, you are putting it back in
>again ?!?
No! For shared libraries nothing  changes.
Previously, there were two tclStubPtr variables,
one in the shared library, another in the static
stub library. That is dangerous, because when
static and shared libraries are mixed, it is
system dependant what will be the result.
There were two different object files which
declared the same variable, and that is
dangerous.

What I want to do is putting the stub library in
the static library as well. Still it depends on the
situation which objects is included in the
final executable: the one from the stub
library or the one from the static library. But
this time it doesn't matter because both
object files are exactly the same!

So, the dangerous situation from before
is still gone. We can mix static and shared
libraries at will, it simply will work always.

dgp added on 2009-11-25 02:41:53:
???

Haven't examined the patch yet.
Just reacting to the last comment.

Should I conclude then, that after
we had completed taking tclStubsPtr
out of libtcl, you are putting it back in
again ?!?

nijtmans added on 2009-11-24 18:23:44:
OK, here is the fix.

Explanation: The reason for the stub changes is to
make it possible to build both a static tclreg12.lib
and a tclreg12.dll next to each other (and the same
for tcldde13.lib/tcldde13.dll). Then we can do the
same with the future tcltest86.lib/tcltest86.dll.
We would need to compile two tclWinReg.obj
files, one compiled without and one compiled
with stubs. It's a lot easier to reduce this to only
one, it has the advantage that tclreg12.lib
can be linked in any tclsh, no matter it is
compiled against tcl86.dll or tcl86.lib. Any
combination of static and shared libraries
will work! The disadvantage is that every
executable which includes a single static
library will need to link with the stub table
as well.

Solution: Include the stub library in any
static library we produce. Then linking with
any static library will automatically include
the stub library objects as well. This fixes
the tclkit build.

proposed patch attached to this issue (/win
directory only, in /unix similar changes have
to be made)

The modification to tclAppInit.c is not
necessary, it only makes that tclsh not
is forced to link with the stub library, it
is completely initialized as well. So,
even extensions that are compiled with
TCL_USE_STUBS bug forget to call
Tcl_InitStubs() will still work. It is meant
as an extra safety net.

With those changes, it's impossible to
produce non-working static executables!

nijtmans added on 2009-11-24 18:23:23:

File Added - 352316: tcl.patch

Attachments: