Menu
▾
▴
[Tcl-bugs] [ tcl-Bugs-2902965 ] stub related changes cause tclkit built to break
|
From: SourceForge.net <no...@so...> - 2009-11-27 22:32:12
|
Bugs item #2902965, was opened at 2009-11-24 07:56 Message generated for change (Comment added) made by nijtmans You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=2902965&group_id=10894 Please note that this message will contain a full copy of the comment thread, including the initial issue submission, for this request, not just the latest update. Category: 40. Dynamic Loading Group: development: 8.6b1.1 Status: Open Resolution: Fixed Priority: 5 Private: No Submitted By: Jan Nijtmans (nijtmans) Assigned to: Jan Nijtmans (nijtmans) Summary: stub related changes cause tclkit built to break Initial Comment: 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. ---------------------------------------------------------------------- >Comment By: Jan Nijtmans (nijtmans) Date: 2009-11-27 23:32 Message: 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! ---------------------------------------------------------------------- Comment By: Pat Thoyts (patthoyts) Date: 2009-11-27 22:39 Message: 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. ---------------------------------------------------------------------- Comment By: Kevin B KENNY (kennykb) Date: 2009-11-27 22:09 Message: 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. ---------------------------------------------------------------------- Comment By: Andreas Kupries (andreas_kupries) Date: 2009-11-27 22:02 Message: Yes, I object. 8.5.8 is the stable branch, and these changes you are doing currently look anything but stable. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2009-11-26 13:20 Message: 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? ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2009-11-26 10:21 Message: Fixed in HEAD (/win and /unix). Tk still to be done. (Remarks/critics/suggestions still welcome!) ---------------------------------------------------------------------- Comment By: Pat Thoyts (patthoyts) Date: 2009-11-25 01:34 Message: > 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. ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2009-11-24 21:20 Message: >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. ---------------------------------------------------------------------- Comment By: Don Porter (dgp) Date: 2009-11-24 20:41 Message: ??? 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 ?!? ---------------------------------------------------------------------- Comment By: Jan Nijtmans (nijtmans) Date: 2009-11-24 12:23 Message: 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! ---------------------------------------------------------------------- You can respond by visiting: https://sourceforge.net/tracker/?func=detail&atid=110894&aid=2902965&group_id=10894 |