Menu
▾
▴
#4941 file join/normalize bugs violate EIAS
Reproduced on WinXP SP3 with ActiveState 8.4 and 8.6 and Ubuntu 11.04 Tcl 8.5
The following code produces unexpected results :
########################"
# Bug #1 : why $foo1 != $foo2 !?
cd /tmp
set foo1 [file join [pwd] foo/]
set foo2 [file join /tmp foo/]
puts "foo1=*$foo1*"
puts "foo2=*$foo2*"
# Bug #2 : why normalize works differently for the same actual string !?
set fooFromStr "/tmp/foo/"
if { $foo1 eq $fooFromStr } {
puts "Yes, I'm not crazy, foo1 eq fooFromStr !"
}
puts "norm foo1 =*[file normalize $foo1 ]*"
puts "normFormStr=*[file normalize $fooFromStr]*"
########################"
Outputs is :
foo1=*/tmp/foo/*
foo2=*/tmp/foo*
Yes, I'm not crazy, foo1 eq fooFromStr !
norm foo1 =*/tmp/foo/*
normFormStr=*/tmp/foo*
There is two problems :
1.There is no reason why [file join] would return different strings when the same parameter is passed two different ways.
2.There is no reason why two identical strings would produce different results when [file normalize] is called with them as parameters.
Discussion
FWIW, with Tcl 8.5.8 I can reproduce only the second (about file normalize), but not the first (both have trailing slash)
The tip of the trunk can't reproduce Bug#1 either.
Bug#1 is even more strange:
$ cat testfile.tcl
cd /tmp
puts [info patchlevel]
puts "pwd:[file join [pwd] foo/]"
puts "/tmp:[file join /tmp foo/]"
$ tclsh8.5 testfile.tcl
8.5.8
pwd:/tmp/foo/
/tmp:/tmp/foo
$ tclsh8.5 < testfile.tcl
8.5.8
pwd:/tmp/foo/
/tmp:/tmp/foo/
Just in case that SF hoses the formatted text: I make a small script that does cd, and then
puts for the two file join lines. If I start tclsh8.5 with the script as an argument, then the bug
shows up, however if I merely redirect tclsh8.5's stdin from the script, then both variants
of file join do show the trailing slash.
PS: my previous comment also applies to tclsh8.6b2 produced from the thusly-versioned official tarball.
Seems related:
$ tclsh8.5
% info patchlevel
8.5.8
% cd /tmp
% set foo1 [file join [pwd] foo/]
/tmp/foo/
% file normalize $foo1
/tmp/foo/
% string length $foo1
9
% file normalize $foo1
/tmp/foo
Note how the result of "file normalize" changes after taking the "string length" of foo1.
1) Testing results on 8.5.8 are not really
very interesting. 8.5.10 or later please.
2) Yes, it's clear that [file normalize] operates
differently on different intreps, and that's a bug
that needs fixing. We don't need more script
variants provoking shimmering to demo that.
@dgp:
1) Did you miss the test report for 8.6b2 which I posted?
Btw., 8.5.8 is what is installed on Ubuntu 10.10, maybe thus its popularity...
Indeed, the problem depicted here is visible if the provided code is sourced (i.e. code not typed directly within a tcl shell). That is probably why the two first comments state bug #1 is not reproducible.
An experimental branch fixes this by
eliminating the buggy intrep. See
http://core.tcl.tk/tcl/timeline?r=remove-pathappend-intrep
This needs testing to determine whether it
has unacceptable performance impacts. Also,
it causes these test failures:
==== filename-13.1 globbing with brace substitution FAILED
==== Contents of test case:
glob globTest/\{\}
---- Result was:
globTest
---- Result should have been (exact matching):
globTest/
==== filename-13.1 FAILED
==== filename-14.15 asterisks, question marks, and brackets FAILED
==== Contents of test case:
lsort [glob globTest/*/]
---- Result was:
globTest/a1 globTest/a2 globTest/a3
---- Result should have been (exact matching):
globTest/a1/ globTest/a2/ globTest/a3/
==== filename-14.15 FAILED
==== filename-14.30 Bug 2710920 FAILED
==== Contents of test case:
file rootname [lindex [lsort [glob globTest/*/]] 0]
---- Result was:
globTest/a1
---- Result should have been (exact matching):
globTest/a1/
==== filename-14.30 FAILED
Where the results of a [glob] are actually
tested to be sure they have the trailing /
while the bug report here wants it dropped.
A quick trial of this branch vs trunk on
tclbench's file.bench reveals these
apparent problems:
007 FILE glob tmpdir (60 entries) 262.35 137.62
011 FILE glob / dirname 702.31 255.50
014 FILE glob / extension 384.40 267.40
020 FILE glob / rootname 385.44 285.91
022 FILE glob / tail 565.95 255.27
Not everything is slower though.
009 FILE glob / atime 718.00 744.44
012 FILE glob / executable 694.96 714.80
013 FILE glob / exists 693.88 714.44
015 FILE glob / isdirectory 719.39 741.74
016 FILE glob / isfile 720.11 740.94
017 FILE glob / mtime 717.75 746.52
018 FILE glob / owned 744.35 768.85
019 FILE glob / readable 696.89 713.09
021 FILE glob / size 723.59 746.08
023 FILE glob / writable 697.59 712.37
experimental branch now passes all tests.
No significant change to the file.bench results.
The problematic benchmark, it seems to me,
is 007 in the number below which suggests that
a basic
glob -nocomplain -directory $dir *
operation on a directory with 60 entries
(30 file, 30 subdits) is twice as slow
after this change.
Thanks dgp for trying to fix the issue. What is the actual status of your fix ? It is obviously unacceptable to take such a performance hit, although the language would be more sane. Maybe a more elaborate fix could avoid that ?
Everytime I'm forced to dive into tclPathObj.c, it's
to construct a "more elaborate patch". And everytime
I rant that life would be so much easier if we just trashed
all this complexity. Finally got ticked off enough to try it
this time. At least now we know what good it does.
It's not all that hard for me to come up with a "more elaborate"
patch that will work resolving the reported issue.
It's much much harder for me to come with the patch that
fixes the issue that I also have string confidence will not
introduce a new cluster of bugs at the same time. that's what
will hold me back. The code is just that tangled.
See branch bug-3414754 for proposed fix.
Test added.
Before I merge to a release branch, I need
a report of a successful test on Windows to
be sure I've not broken something terribly there.
Joe, can I trouble you to test this branch
on Windows? Thanks!
Jan, is this a test you can run? I'm holding
back 8.5.11RC generation until I see whether
this bug fix is fit for merging into it.
Ran the test on Windows 7, with both mingw
and Visual Studio 10.
Looks OK to me, yes I wouls merge this.
The only thing I encoungered was a test
failure in filesystem-1.3 and filesystem-1.4
(see bug 3075027, the other failures
mentioned there are solved already),
but those two tests are already failing
for ages, I would not hold 8.5.11 for it.