Tcl Source Code

View Ticket
Login
Ticket UUID: 1b8a893ded5f2a9b0c70a5893c7a2be3429d8033
Title: TCL_PACKAGE_PATH path is (wrongly) braced.
Type: Bug Version: 8.6.14
Submitter: avl42 Created on: 2024-03-03 18:32:18
Subsystem: 53. Configuration and Build Tools Assigned To: jan.nijtmans
Priority: 5 Medium Severity: Minor
Status: Closed Last Modified: 2024-03-05 12:28:33
Resolution: Fixed Closed By: oehhar
    Closed on: 2024-03-05 12:28:33
Description: (text/x-fossil-plain)
Recent changes (somewhere since 8.6.10) in file  unix/configure.in  lines 871 and 873  cause extra braces to be added to the value of TCL_PACKAGE_PATH

These extra braces spoil installation of tcltls, which attempts to use the value of TCL_PACKAGE_PATH to determine install location.

At this point I cannot yet tell, if this is a regression of tcl, or if tcltls is doing something wrong by using TCL_PACKAGE_PATH from tclConfig.sh .
User Comments: oehhar added on 2024-03-05 12:28:33: (text/x-fossil-wiki)
Hi Jan,
we have multiple usage:

   *   TCL_PACKAGE_PATH is a configure option of TCL
   *   TCL_PACKAGE_PATH is passed for module compilation via tclConfig.sh

Its "official" use targets a c embedded interpeter.

The only one I know is KitDLL with KitCreator:
I scanned the trunk of the distribution
https://kitcreator.rkeene.org/fossil/info/bf07147147174383
and found:

<verbatim>
function install() {
	local filename newFilename

	mkdir -p "${installdir}/lib" || return 1
	${MAKE:-make} tcllibdir="${installdir}/lib" TCL_PACKAGE_PATH="${installdir}/lib" "${make_extra[@]}" install || return 1

</verbatim>

What I understand is, that the Variable TCL_PACKAGE_PATH is used to inform TCL (or other packages) about the package path and it is again equal to ${installdir}/lib.

Nevertheless, within the embedding process, it is not used.

Here is my old howto for reference:
https://wiki.tcl-lang.org/page/Embedding+TCL+program+in+DLL

I think in embedding, the libraries are anywahy handled differently.

Long post, not much content.
I would favor to remove all this from 9.0 ;-).

If it is used, it is used the wrong way.

Take care,
Harald

jan.nijtmans added on 2024-03-05 11:10:47: (text/x-fossil-wiki)
In this case, I recommend not to document TCL_PACKAGE_PATH. It's meant to change the paths where packages can be found like this:
<pre>
./configure TCL_PACKAGE_PATH=<xxxx>
</pre>
but ... it's not recommended to change it from the default value "${prefix}/lib". So documenting it would only provide bad ideas .....

I don't know why it's included in tclConfig.sh. We could simply remove it in Tcl 9.0, that would force extension writers to use "${prefix}/lib" in stead, which is what it should be.

Noting that there is a TCL_MODULE_PATH as well, which is specific to the "tm" machinery, adding to the confusion. This one shouldn't be modified either, so better leave it undocumented .....

oehhar added on 2024-03-05 10:20:48: (text/x-fossil-wiki)
Any idea were to document this?
I have not found anything within the current documentation?

The path separator is now ":" on MacOS/Unix and ";" on Windows.
That is the right way to go, but documentation would be great.

Or it is programmer-like: the best documentation is source code ;-).

Take care,
Harald

P.S.: I have opened a TCLTLS ticket here:
[https://core.tcl-lang.org/tcltls/tktview/c9a4f87325d68cf830bf897846b47eff26468ba6]

jan.nijtmans added on 2024-03-05 09:44:24: (text/x-fossil-wiki)
> After `test -z "$TCL_PACKAGE_PATH" && ... ` the value of TCL_PACKAGE_PATH is "probably" empty, so it doesn't really make sense to me to add this empty value afterwards.

Indeed! That's corrected now. In Tcl 8.6.15 there will be no trailing space any more here.

> If we could just set the variable to  ${prefix}/lib  , then we wouldn't even need to change tcltls, as there wouldn't be any colon in the value, then...

In most cases, $TCL_PACKAGE_PATH is just "${prefix}/lib" now. I also set it to "${prefix}\lib" on Windows now, just in case, even though TCL_PACKAGE_PATH is not actually used in Windows

TCLTLS should be changed to use simply "${prefix}/lib", that's how TEA-compatible extensions are supposed to determine where they should be installed.

Hope this helps,
        Jan Nijtmans

oehhar added on 2024-03-05 07:35:07: (text/x-fossil-wiki)
Andreas,
how handles TCLTLS the fact, if there is a space within the path?
I suppose, the leading "{" (what caused the bug) should then also be handled.

How is a space to be specified?

   *   "/first path" "/second path" /nospacepath
   *   {/first path} {/second path} /nospacepath
   *   /first\ path /second\ path /nospacepath
   *   /first path:/second path:/nospacepath

At least, I suppose, that the column-version is the easiest to handle.

I am doubtful to make such a change in 8.6.15. A clear definition is required. TCLTLS should anyway be changed. I think, the ":" version is the must easiest useing for example sed.

Any opinion appreciated,
Harald

avl42 added on 2024-03-04 23:21:12:
I prefer the colon to the braces :-)

But...

After `test -z "$TCL_PACKAGE_PATH" && ... ` the value of TCL_PACKAGE_PATH is "probably" empty, so it doesn't really make sense to me to add this empty value afterwards.

If we could just set the variable to  ${prefix}/lib  , then we wouldn't even need to change tcltls, as there wouldn't be any colon in the value, then...

tcltls currently hast code in it to strip away the blank after the value, but it wouldn't yet be prepared to remove a colon, iirc.

jan.nijtmans added on 2024-03-04 21:05:53: (text/x-fossil-wiki)
Fixed [c2a94d79f639037e|here]. Although - actually - it's not a bug, it's a logical change: on UNIX, it's custom to use ':' as path separator in environment variables so why not do it the same in TCL_PACKAGE_PATH. The Tcl core is capable to convert this to a Tcl list for internal usage.

If TCL_PACKAGE_PATH contains a single path not containing spaces (which will be the usual case) nothing actually changes. Otherwise this is a *** potential incompatibility ***.

jan.nijtmans added on 2024-03-04 10:24:37: (text/x-fossil-wiki)
Proposed fix [4427b7e16885d94e|here]. Not tested on MacOS yet.

Since this - actually - works as designed (tcltls should _not_ use TCL_PACKAGE_PATH for this purpose), lowered "Severity" to Minor.

jan.nijtmans added on 2024-03-04 08:34:47:
Possible solution: we can change TCL_PACKAGE_PATH, not to be a Tcl list any more, but a path separated by ':' (as is usual in UNIX). Then the curly braces can be removed

jan.nijtmans added on 2024-03-04 08:22:42: (text/x-fossil-wiki)
This is the commit which made the change: [6acb955a47696047]. Comment: "Fix determination of TCL_PACKAGE_PATH if it contains multiple directories, and at least one of them contains a space"

So, this change is made because TCL_PACKAGE_PATH is a Tcl list (multiple entries separated by spaces), and ${libdir} and/or ${prefix} could contain spaces itself. Without the extra braces, paths containing spaces would be split.

I don't think TCL_PACKAGE_PATH is suitable to determine the install location. tcltls should be installed in ${prefix}/lib, and that's determined by "configure --prefix=<prefix>".

oehhar added on 2024-03-04 08:16:55: (text/x-fossil-wiki)
Thanks, Andreas, for the ticket.
Please allow me to copy the information from the clt thread titled "tcltls install error..." started 3.3.2024 16:39 UTC.

<H2>First post</H2>
After building latest tcl/tk 8.6.14, I also freshened up a few extra
libraries, in particular  tcltls-1.7.22

It seemed to compile well, but the last few lines of build-log show:
<verbatim>
...
objcopy --discard-all shared-tcltls.so
mv shared-tcltls.so tcltls.so
/usr/bin/install -c -d '{/usr/local/lib}/tcltls1.7.22'
/usr/bin/install -c tcltls.so '{/usr/local/lib}/tcltls1.7.22'
/usr/bin/install -c -m 644    pkgIndex.tcl '{/usr/local/lib}/tcltls1.7.22'
</verbatim>
Yes, there are some strange extra curlies, and - indeed - it didn't
install into /usr/local/..., but instead into a local directory "{".

Is it just me...?   (I'll go deeper into analysis, if this isn't an
already known issue.)

Last time I built tcltls was version 1.7.21 on Solaris, and I didn't
have these extra curlies back then.

<H2>Andreas addition 1</H2>

Well, I did look further into it, and the culprit is not tcltls.

In  /usr/local/lib/tclConfig.sh  from fresh tcl-8.6.14 install
it says literally: TCL_PACKAGE_PATH='{/usr/local/lib} '

All other  tclConfig.sh on my system all still have the whitespace,
but no curlies in that line.

My last tcl build on that machine is a bit aged, so I cannot yet
tell, how "fresh" the regression is - if it isn't caused by some
other oddity on my system, anyway.

<H2>Andreas addition 2</H2>

It seems to be a regression inside tcl-8.6 that happened somewhere
between 8.6.10 (correct) and 8.6.14 (wrong)

in 8.6.10 the unix/configure.in had this line 871:
<verbatim>
    test -z "$TCL_PACKAGE_PATH" && TCL_PACKAGE_PATH="${prefix}/lib ${TCL_PACKAGE_PATH}"
</verbatim>

in 8.6.14, that line (now 873) has changed to:
<verbatim>
    test -z "$TCL_PACKAGE_PATH" && TCL_PACKAGE_PATH="{${prefix}/lib} ${TCL_PACKAGE_PATH}"
</verbatim>

Note: There is a similar line shortly above, with a similar change
and two now-braced items, which may affect other unix-platforms.

Either, there was a wrong assumption about what language code
   would actually use that tclConfig.sh entry,
or tcltls is wrong by using it at all.  I don't know.