Ticket UUID: | 922727 | |||
Title: | Header reform | |||
Type: | RFE | Version: | None | |
Submitter: | dgp | Created on: | 2004-03-24 21:02:58 | |
Subsystem: | None | Assigned To: | dgp | |
Priority: | 7 High | Severity: | ||
Status: | Closed | Last Modified: | 2004-11-25 03:48:03 | |
Resolution: | Accepted | Closed By: | dgp | |
Closed on: | 2004-11-24 20:48:03 | |||
Description: |
Over time it appears that the system of *.h files in Tcl have drifted away from the recommendations of the Engineering Manual. In particular, the subset relationship of tcl.h < tclInt.h < tclPort.h has emerged, rather than the tcl.h < tclPort.h < tclInt.h that the EM recommmends. This patch restores things to my understanding of the EM. It will require platform testing. After this patch, any C file will only need to #include one of the 3 header files, and should choose the "smallest" one that satisfies. This patch also removes old headers that no longer matter, and merges some that make things simpler. One implication of this patch is that any C file that wants to #include tclInt.h will need the ability to also #include tclPort.h and the suitable tcl(Unix/Win)Port.h to take care of the #include dependencies. The traditional access to tclInt.h within a Tcl source distribution is sufficient to take care of this. Requesting first review from the MacOSX maintainers. | |||
User Comments: |
das added on 2004-07-20 18:26:31:
Logged In: YES user_id=90580 ack, didn't see comment about this staying needing to stay open until 925620 is resolved, reopening. das added on 2004-07-20 18:19:33: Logged In: YES user_id=90580 Committed the attached install-private-headers patches to tcl & tk HEAD dgp added on 2004-05-01 00:30:30: Logged In: YES user_id=80530 Note that this effort exposed a few places where the separation of generic and platform-specific code is not as clean as it ought to be. See Bugs 925620 and 945447. These matters should be cleaned up before we close this report. das added on 2004-04-24 15:55:32: File Added - 84891: tk-install-private-headers.diff das added on 2004-04-24 15:54:35: File Added - 84890: tcl-install-private-headers.diff Logged In: YES user_id=90580 Now that internal headers can be installed outside of the source tree, the attached patches add an new 'install-private-headers' target to the Makefiles, to optionally install the private headers into $(includedir). Any objections to this going in? das added on 2004-04-24 13:01:14: Logged In: YES user_id=90580 committed patches to tcl & tk dgp added on 2004-04-21 21:35:15: Logged In: YES user_id=80530 those patches look fine, and test successfully on my systems. Make it so. das added on 2004-04-21 19:10:11: File Added - 84507: tkPort.diff das added on 2004-04-21 19:08:52: File Deleted - 84358: File Added - 84506: tclPort.diff Logged In: YES user_id=90580 Don, thanks, forgot to diff tk... to better match with TEA, I would suggest using TCL_PLATFORM_DIR instead of TCL_UNIX_DIR, esp. since a similar change is needed to tk/win/ Makefile.in if tclWinPort.h is included without relative path in tclPort.h I've also verified that Tk/X11 still builds fine when tkUnixPort.h is included without relative path in tkPort.h I've checked that cross compiling with win mingw of both tcl and tk still works with these changes. Someone with access to VC++ needs to check that the same is true there. New patches for tcl and tk attached, these now remove all relative paths in (Tcl|Tk)Port.h dgp added on 2004-04-21 00:38:00: File Added - 84407: tk.patch dgp added on 2004-04-21 00:37:57: Logged In: YES user_id=80530 Attractive solutions include: 1) Convert Tk to a TEA extension. 2) Purge Tk of remaining dependence on tclInt.h Short of that, the attached patch is the quick and dirty workaround. dgp added on 2004-04-21 00:00:34: Logged In: YES user_id=80530 I think that patch is on the right track, but it breaks the Tk build on my systems. Does Tk just need the TCL_INCLUDES support you suggest for Itcl? das added on 2004-04-20 15:11:25: Logged In: YES user_id=90580 I should add that I've checked that this change doesn't cause problems with TEA extensions that use private headers, such as incrTcl, as the TEA tcl.m4 has TCL_INCLUDES="-I${TCL_GENERIC_DIR_NATIVE} -I${TCL_PLATFORM_DIR_NATIVE}" i.e. the platform specific directory is already part of the include path and so tclUnixPort.h can be included from tclPort.h without needing a relative path. the same should be true for tclWinPort.h. das added on 2004-04-20 15:00:16: File Added - 84358: tclPort.diff Logged In: YES user_id=90580 Don, sorry for the delay, finally getting back to this. while your patch greatly simplifies the internal header organization, the issue I originally reported on tcl-core remains (broken macosx tk build): tclPort.h cannot be used when it is installed outside of the tcl source hierarchy (e.g. in /usr/local/include) because it #includes the platform port headers via relative paths. I.e. it is currently not possible to #include tclPort.h when it is installed in / usr/local/include; or in Tcl.framework/PrivateHeaders, which is something the OSX Tk build relies on. The attached patch fixes this for tclUnixPort.h, and adds related makefile support. Something similar should be done for tclWinPort.h, can't test that myself though. Note that tkPort.h already doesn't use relative paths when including tkWinPort.h and tkMacOSXPort.h, probably a similar fix for tkUnixPort.h should be applied there as well. If there are no objections I'll check this in (this is essentially the same patch I originally posted to tcl-core). dgp added on 2004-04-07 05:37:53: Logged In: YES user_id=80530 committed to HEAD (8.5a2) for additional testing. dgp added on 2004-03-30 05:39:07: File Deleted - 81823: File Added - 81831: headers.patch Logged In: YES user_id=80530 tested patch, works on Unix and Win. Any additional Mac OS X comments? dgp added on 2004-03-30 04:00:41: File Added - 81823: headers-3.patch dgp added on 2004-03-30 04:00:40: Logged In: YES user_id=80530 Updated patch. Thanks for testing. kennykb added on 2004-03-30 03:23:40: Logged In: YES user_id=99768 tclIOUtil.c needs: #ifdef __WIN32__ #include "tclWinInt.h" #endif near the head of the file. tclWinLoad.c and tclWinSock.c both need #include "tclWinInt.h" tclWinDde.c needs tclInt.h to get TCL_TSD_INIT. Were it not for that (for example, if the TCL_TSD_INIT calls were to be replaced with calls to Tcl_GetThreadSpecificData), it could make do with #define WIN32_LEAN_AND_MEAN #include <windows.h> tclWinReg.c needs tclInt.h for TclWinGetPlatformId - don't know how, offhand, to avoid that. dgp added on 2004-03-30 02:12:36: File Added - 81792: headers-2.patch dgp added on 2004-03-30 02:12:35: Logged In: YES user_id=80530 Here's a second attempt, incorporating the suggestions in the comments. Also, I changed both tclWinDde.c and tclWinReg.c to #include only tcl.h, in the hope that good packages should only need the public Tcl interface. If that's not true, patches to make it true would be welcome. Please test, especially on Windows. jenglish added on 2004-03-26 04:31:03: Logged In: YES user_id=68433 The 2004-03-24 patch merges the contents of tclWinInt.h into tclWinPort.h. It might be worthwhile to keep these distinct. That would help distinguish C source files that are intended to be platform-independent from those that are Windows (/Unix/Mac)-specific. kennykb added on 2004-03-26 03:18:01: Logged In: YES user_id=99768 Per dgp's request, I changed the #include back, but added (in tclInt.decls) the declaration: # Added in 8.5 as part of rationalizing header files declare 29 win { TclFile TclWinMakeFile(HANDLE handle) } With that declaration in place, the compilation gets through tclPort.h without trouble. It still requires tclInt.h, nevertheless, to define the five symbols: 'TclpInitLock', 'TclpInitUnlock', 'TCL_TSD_INIT', 'LONG_MAX', and 'LONG_MIN'. kennykb added on 2004-03-26 02:27:32: Logged In: YES user_id=99768 Patch fails to compile on Windows-VC++. Changing line 15 of tclWinTime.c from #include "tclPort.h" to #include "tclInt.h" appears to fix the problem. No new regression test failures observed. dgp added on 2004-03-25 04:02:58: File Added - 81243: headers.patch |
Attachments:
- tk-install-private-headers.diff [download] added by das on 2004-04-24 15:55:32. [details]
- tcl-install-private-headers.diff [download] added by das on 2004-04-24 15:54:35. [details]
- tkPort.diff [download] added by das on 2004-04-21 19:10:11. [details]
- tclPort.diff [download] added by das on 2004-04-21 19:08:52. [details]
- tk.patch [download] added by dgp on 2004-04-21 00:38:00. [details]
- headers.patch [download] added by dgp on 2004-03-30 05:39:07. [details]