#334 Header reform

closed-accepted
None
7
2004-11-24
2004-03-24
Don Porter
No

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.

Discussion

1 2 > >> (Page 1 of 2)
  • Kevin B KENNY

    Kevin B KENNY - 2004-03-25

    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.

     
  • Kevin B KENNY

    Kevin B KENNY - 2004-03-25

    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'.

     
  • Joe English

    Joe English - 2004-03-25

    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.

     
  • Don Porter

    Don Porter - 2004-03-29
    • assigned_to: das --> patthoyts
     
  • Don Porter

    Don Porter - 2004-03-29

    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.

     
  • Kevin B KENNY

    Kevin B KENNY - 2004-03-29

    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.

     
  • Don Porter

    Don Porter - 2004-03-29

    Logged In: YES
    user_id=80530

    Updated patch. Thanks for testing.

     
  • Don Porter

    Don Porter - 2004-03-29
    • priority: 5 --> 7
    • assigned_to: patthoyts --> das
     
  • Don Porter

    Don Porter - 2004-03-29
     
  • Don Porter

    Don Porter - 2004-03-29

    Logged In: YES
    user_id=80530

    tested patch, works on Unix and
    Win. Any additional Mac OS X
    comments?

     
  • Don Porter

    Don Porter - 2004-04-06
    • assigned_to: das --> dgp
    • status: open --> closed-accepted
     
  • Don Porter

    Don Porter - 2004-04-06

    Logged In: YES
    user_id=80530

    committed to HEAD (8.5a2) for
    additional testing.

     
  • Daniel A. Steffen

    • status: closed-accepted --> open-accepted
     
  • Daniel A. Steffen

    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).

     
  • Daniel A. Steffen

    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.

     
  • Don Porter

    Don Porter - 2004-04-20

    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?

     
  • Don Porter

    Don Porter - 2004-04-20

    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.

     
  • Don Porter

    Don Porter - 2004-04-20
     
  • Daniel A. Steffen

    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

     
  • Don Porter

    Don Porter - 2004-04-21

    Logged In: YES
    user_id=80530

    those patches look fine, and
    test successfully on my systems.

    Make it so.

     
  • Daniel A. Steffen

    Logged In: YES
    user_id=90580

    committed patches to tcl & tk

     
  • Daniel A. Steffen

    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?

     
1 2 > >> (Page 1 of 2)

Get latest updates about Open Source Projects, Conferences and News.

Sign up for the SourceForge newsletter:

JavaScript is required for this form.





No, thanks