Tcl Source Code

View Ticket
Login
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: