# ticketchng

login: anonymous
mimetype: text/plain
icomment:
  I ran your three test cases through another function that we use for our xmodem client to compute the expected results.  I created a test script and added the expected results for each of the values calculated by crc::sum.  You can run the test script from wish86 console.

  # checksumTest.tcl
  package require sum

  set v1 [string repeat a 1280]
  set expectSum1 58624
  set ckSum1 [crc::sum -sysv -- $v1]
  puts "Test 1. Expected: $expectSum1. Actual: $ckSum1"

  set v2 [string repeat fx 640]
  set expectSum2 11008
  set ckSum2 [crc::sum -sysv -- $v2]
  puts "Test 2. Expected: $expectSum2. Actual: $ckSum2"

  set v3 [string repeat \xfe 1280]
  set expectSum3 62976
  set ckSum3 [crc::sum -sysv -- $v3]
  puts "Test 3. Expected: $expectSum3. Actual: $ckSum3"

  -----
  Test run:
  % source checksumTest.tcl
  Test 1. Expected: 58624. Actual: 58625
  Test 2. Expected: 11008. Actual: 11010
  Test 3. Expected: 62976. Actual: 62980
  %


login: aku
mimetype: text/plain
icomment:
  Thank you for your efforts in getting test results to me.

  I have run your script on my machine here and can confirm that I get different actual results vs the expected. They are the same actual results as produced on your machine.

  As my machine is linux/32 (vs. your Win7/64) the issue does not look to be OS-dependent, nor does it look to be a 32 vs 64 bit issue.

  I have changed the ticket title to reflect this.


login: aku
mimetype: text/plain
icomment:
  Fixed with revision [56e6c4cdf2].
  Committed.
  Pushed.

  Thank you for the report and fix (% vs &).


login: aku
mimetype: text/plain
icomment:
  Patch accepted, with modifications
  - More tests to cover argument handling.
  - Fixed the documentation (missing closing bracket, listified id-list, phrasing).
  Version bumped to 1.4.3.

  Revision [42806b4246].
  Committed.
  Pushed.

  Thank you.


login: aku
mimetype: text/plain
icomment:
  Branch commit first, please.

  This package has no tests :(
  Question is, how do you test this at all ?


login: aku
mimetype: text/plain
icomment:
  Applied patch, except for the "chan close changes, these were not explained in the bug report and therefore left out.
  Made equivalent changes in the coroutine::auto package.

  Bumped versions to 1.1.1 and 1.1.2 respectively.
  Revision [7bed475cb2].
  Committed.
  Pushed.


login: aku
mimetype: text/plain
icomment:
  Confirmed and fixed.
  Fixed another place using "getrect" instead of "get rect" in the method descriptions as well.

  Fixed with revision [ed580c5543].
  Committed.
  Pushed.
  No version bump (no code change).


login: aku
mimetype: text/plain
icomment:
  Read up on https://www.w3schools.com/tags/tag_meta.asp

  It seems that this is best handled with two additional commands, one each for `charset` and `http-equiv`.

  Another command for `name` to make it consistent, and the current command changed to point to the name-command for compatibility.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Done with commit [6a75d04948].

  html bumped to version 1.5. (EF)

  See also ticket [1146362fff], and [1076403fff].


login: anonymous
username: sample yaml code that triggers error
mimetype: text/plain
icomment:
  This should trigger an error with the prepatched code, while being handled properly after the patch.



  ---
  - &foo
  - &bar
  - alpha: 43
    beta:
      houston: [*foo]
      newyork:  [[*bar,[aaa]]]


login: aku
mimetype: text/plain
icomment:
  Thank you for the example.
  This indeed demonstrated both the bug and that it was fixed after patching.

  This is now fixed in revision [06bdef76a4].
  Committed.
  Pushed.

  Package version bumped to 0.3.7.


login: aku
mimetype: text/plain
icomment:
  This is IMHO a problem of "yeti" assuming a specific ordering for the arcs.

  None of the "arcs" or "nodes" methods of the graph package
  promise to deliver a specific ordering or having it consistent
  across implementations.

  The ordering you see/get is based on the internal ordering in the Tcl_HashTables (Tcl implementation) vs. the lists in the C implementation.

  This is similar to how changes to the Tcl_HashTable implementation between Tcl 8.4 and 8.5 affected the ordering of keys returned by "array names". The Tcllib testsuite imposes explicit ordering on results in numerous places to ensure that they are comparable regardless of the Tcl version used to run the tests.

  This issue is the same.

  If the ordering is important for yeti it is better to explicitly impose the desired order instead of assuming to get it from the graph package.


login: aku
mimetype: text/plain
icomment:
  Ah. The 3rd party json-c library.

  Yes, we had this library in Tcllib, for a bit.
  Checking the history it was
  from Dec 10, 2013 (rev [c51875a02a])
  to   Jan 06, 2014 (rev [11390a7baa] dropping it)

  See branch "ak-bug-6efa4f571af052-jsonc" and "trunk" to end date.
  See also ticket [6efa4f571af052].

  This was never in a release.
  (Was added after 1.15, and got removed before 1.16)

  The C code we currently have in Tcllib for JSON parsing is by Mikhail Teterin with modifications by myself, directly using Tcl API, no other data structures, no external library.

  Conclusion:
  - Our releases not affected by the CVE's
  - People directly using one of the revisions in the range mentioned
    above may be affected, and should update.


login: aku
mimetype: text/plain
icomment:
  Right, these files definitely should not be installed.
  Moving them into a sub-directory of their modules should do the trick.


login: aku
mimetype: text/plain
icomment:
  Fixed with revision [1a2e9c5b7b].
  Committed.
  Pushed.
  Version bumped to 0.3.
  Updated documentation
  Testsuite extended to cover the extend syntax.


login: aku
mimetype: text/plain
icomment:
  [13:57]	schelte	The fix is very simple too. Just check for eof and if so, close the socket.
  [14:00]		<silas> looks at the clock and dashes out the door
  [14:01]		<Eof> macht wie eine Banane...
  [14:03]	aku	ok


login: anonymous
mimetype: text/plain
icomment:
  This should fix the issue, testing is simple, telnet to the open comm channel *without* sending any data (no <enter>) and close the telnet. Without patch it keeps looping, with the patch the socket is simply closed.

  --- comm.tcl.ORIG       2014-04-11 23:39:35.000000000 +0200
  +++ comm.tcl    2014-04-25 16:17:14.627764129 +0200
  @@ -929,6 +929,11 @@
   
       # Check if we have a complete line.
       if {[gets $fid protoline] < 0} {
  +       #commDebug {puts stderr "commIncomingOffered no data"}
  +       if {[eof $fid]} {
  +           commDebug {puts stderr "commIncomingOffered eof on fid=$fid"}
  +           catch {close $fid}
  +       }
          return
       }


login: anonymous
mimetype: text/plain
icomment:
  The problem seems to be in computing the left bound of the Gershgorin region of the matrix A in ::math::linearalgebra::eigenvectorsSVD procedure.

  The bound in the procedure is currently the minimum of 2|a_ii|-sum_j|a_ij|, but it should be the minimum of a_ii+|a_ii|-sum_j|a_ij| (the center of the disk is in a_ii, and not in |a_ii|).

  Just replace
  set sum [expr {2.0*abs($aii) - [norm_one $row]}]
  by
  set sum [expr {$aii + abs($aii) - [norm_one $row]}]
  in ::math::linearalgebra::eigenvectorsSVD proc and this bug will be fixed.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fixed in branch "linalg-7f082f8667".

  Revision [c7d7411746].

  Extended testsuite.
  Version bumped to 1.1.5.

  Pushed.

  Merged trunk [837cadf794].

  Merged release [7ef762388b].


login: aku
mimetype: text/plain
icomment:
  Thank you for the report and patch.
  Applied with some small changes
  - Split the big nested expression into multiple commands.
  - Fixed same issue in the finalization, although it would not be that severe there.

  Revision [15c6984635].
  Committed.
  Pushed.
  Package version bumped to 1.1.4.


login: aku
mimetype: text/plain
icomment:
  Fixed with revision [02c0fddd03].
  (Disarm the bad char sequence + new option -comments allowing the user to disable the comment generation for this).
  Committed.
  Pushed.

  Bumped package "pt::peg::to::cparam" to version 1.1.
  Thank you for the report.


login: aku
mimetype: text/plain
icomment:
  Fix extended with revision [ea593a875e].
  Disarm /*-opener as well, for C compilers supporting nested /* ... */ comments.
  Committed.
  Pushed.
  Package bumped to version 1.1.1.
  Thanks for the report.


login: emmanuel
username: emmanuel
mimetype: text/plain
icomment:
  Have changed the message, which will contain an explicit mention that this is an HTTP error and of the numerical code. Hope this helps.

  I close the ticket and open another one explicitely for documentation errors and missing bits and pieces to keep things clear.


login: emmanuel
username: emmanuel
mimetype: text/plain
icomment:
  Since the handler (callbacks) are bound to a (web)socket, and since this error occurs without even having a socket, there are few other ways than the ones that are documented, i.e. returning an empty string as an error indication from ::websocket::open.

  It's a little bit a matter of taste: either we let through the error (which originates from the HTTP library) or (as it is) we catch it, letting the caller testing for the empty string which is being returned. Are there any guidelines around error generation for tcllib that explained these kind of decisions?


login: aku
mimetype: text/plain
icomment:
  > Are there any guidelines around error generation for tcllib ...

  There are currently no guidelines for Tcllib.
  Each package (author) makes up their own mind.

  > It's a little bit a matter of taste

  Not quite. With "letting the caller testing for the empty string" it seems that the larger information about the cause of the error (i.e. error message and error code) are lost. While I can say "something failed" after the test I do not know what.


login: aku
mimetype: text/plain
icomment:
  Thank you for the report and fix.
  Applied.
  Additionally bumped package version to 1.3.4.
  Committed to revision [55876840bd].
  Pushed.


login: emmanuel
username: emmanuel
mimetype: text/plain
icomment:
  Actually, while the websocket package does not explicitely set the origin field, the field can still be set via the -headers option to ::websocket::open. The RFC does not states that the origin field must be set, so I believe that this isn't as severe as it looks like. But I am open to discussing this.

  (And I agree, more interop testing is required and I thank you for your efforts (and reports!))


login: aku
mimetype: text/plain
icomment:
  > ... the field can still be set via the -headers option to ::websocket::open

  True. That is what I did here to get around the issue. Did not need the URLmatcher part because I have the host directly in the higher layers.

  > The RFC does not states that the origin field must be set.

  Huh. I guess the websocket server I am talking to is then stricter than needed :(

  > I believe that this isn't as severe as it looks like

  I have no problem with dropping the severity of the ticket.


login: anonymous
mimetype: text/plain
icomment:
  In my opinion, the websocket client does not need to know about headers other than Connection, Upgrade and Sec-Websocket-*.

  RFC6455 section 1.3:
     This header field is sent by browser clients; for non-browser
     clients, this header field may be sent if it makes sense in the
     context of those clients.

  Users of the library should choose whether it makes sense for them to send an Origin header. They can do so with an interface similar to that of http::geturl, by using the -headers option.

  On the other hand, it would be nice if the websocket _server_ offered some way of checking Origin headers sent by clients.


login: emmanuel
username: emmanuel
mimetype: text/plain
icomment:
  I've dropped the severity of this ticket as a result of the discussion. I hope that it was appropriate.


login: emmanuel
username: emmanuel
mimetype: text/plain
icomment:
  As for server implementations, the current API supposes that the headers sent by the client to the server are pushed further to websocket::test for testing of the upgrade request viability. This means that the current API supposes that the server has taken all the necessary steps to parse the headers of the incoming request, including "Origin" and could segragate if necessary before calling websocket::test or after (but before calling websocket::upgrade)

  In this context, I hope that it is ok to close the bug.


login: aku
mimetype: text/plain
icomment:
  Ah! You are talking about the aclocal.m4 as part of Tcllib's configure(.in) code.
  Had to look at the referenced artifact to understand.

  Have to see if the tcl.m4 in the latest TEA is better in this regard.
  (TEA_PROG_TCLSH).

  Trying to run the tclsh ... makes sense.


login: anonymous
mimetype: text/plain
icomment:
  Here is a patch that provides a --with-tclsh:


  --- tcllib-tcllib_1_16/configure.in     2014-02-11 14:04:18.000000000 -0500
  +++ tcllib-tcllib_1_16.new/configure.in 2014-11-12 23:42:03.000031000 -0500
  @@ -10,8 +10,24 @@
   esac
   AC_SUBST(CYGPATH)
   
  +AC_ARG_WITH(tclsh, [  --with-tclsh=FILE location of a working tclsh executable], with_tclsh=${withval})
  +
   SC_SIMPLE_EXEEXT
  -SC_PROG_TCLSH
  +
  +# allow user to override the tclsh we think we found
  +if test x"${with_tclsh}" != x ; then
  +    AC_MSG_CHECKING([for tclsh])
  +    if "${with_tclsh}" </dev/null ; then
  +        TCLSH_PROG=${with_tclsh}
  +        AC_MSG_RESULT([manually set by --with-tclsh=$TCLSH_PROG])
  +    else
  +        AC_MSG_ERROR([No tclsh at place specified by --with-tclsh (${with_tclsh})])
  +    fi
  +    AC_SUBST(TCLSH_PROG)
  +else
  +    SC_PROG_TCLSH
  +fi
  +


login: aku
mimetype: text/x-fossil-wiki
icomment:
  The patch looks ok.

  Tweaked the messaging a bit.

  Applied with commit [262292fc92].
  Pushed.

  Keeping the ticket open until the same patch has been applied to tklib.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  And applied to Tklib as well,

  Commit: http://core.tcl.tk/tklib/info/ba3b0d913c

  This ticket is now done.


login: anonymous
username: Andy Goth
mimetype: text/plain
icomment:
  Appear to be other issues with the code, for instance using both EXCL and TRUNC open modes.  I just posted a tighter version on the wiki which may assist in refactoring:

  http://wiki.tcl.tk/_/diff?N=772&V=37&D=36&W=0#diff0


login: aku
mimetype: text/plain
icomment:
  Removed the pid reference.
  Revision [0b83eb3dd5].
  Committed.
  Pushed.

  Regarding the referenced code on the wiki, some questions, to understand it a bit more before going for it.

  (1) It apparently removes the loop doing multiple tries. Why ?
  (2) It removes the precheck of file existence in favor of just trying to open it ?
  (3) In case of problems it removes the after-check if the directory is not writable ?
  (4) What is the issue with having both EXCL and TRUNC ?


login: anonymous
username: Andy Goth
mimetype: text/plain
icomment:
  (1) The multiple tries loop is still there. I just moved the $maxtries tunable directly into the loop condition because my goal was to cut the line count.  You don't need to adopt every change I make, so take it or leave it.  I merely offered my code to "assist in refactoring".

  (2) Checking for the file's existence is implied by EXCL, so checking twice is redundant.  Also, the [after 1] serves no discernible purpose.

  (3) Checking for directory writability failure is a minor performance optimization.  Trying and failing ten times takes practically the same amount of time, and it is certainly a rare and fatal situation in which the configured temporary directory is not writable.  The fault I see with my code (which I optimized to be brief, not to be directly suitable for Tcllib) is that unwritable temporary directory is reported as failure to find unused temporary file names.

  (4) EXCL and TRUNC are fundamentally contradictory.  If the file already exists, EXCL says to fail, and TRUNC says to open it with size zero.


login: aku
mimetype: text/plain
icomment:
  (Ad 1) Looking more closely I now see that you are right. You kept the loop and hard-coded the value instead of using a local 'config' variable.

  (Ad 4) Ok. Getting rid of this then.

  (Ad 2) Getting rid of it. ... The after 1 was possible thought as some sort of backing off to let whoever held the found temp file have the time to go away. Except that the next round through the loop will come up with a completely new random suffix and so avoid the found file anyway. Getting rid of that too.

  (Ad 3) So if we wish to report this right the most simplification we can do is to drop the bool flag variable. If it fails it fails immediately, if not, we have 10 more checks which won't fail. ... Hm ... I leave that in for now. 

  Revision [9520c7d9d7].
  Committed.
  Pushed.
  Bumped to version 1.14.8.


login: aku
mimetype: text/plain
icomment:
  Please note that the official documentation now resides at

  https://core.tcl.tk/tcllib/doc/trunk/embedded/www/tcllib/files/modules/log/loggerUtils.html

  Not that it changes the report, this package and documentation have not been touched in some time, and the docs above are the same.


login: anonymous
username: hypnotoad
mimetype: text/plain
icomment:
  I have prepared a patch, and checked it into my private tcllib mirror.

  The fix is available at:
  http://fossil.etoyoc.com/fossil/tcllib/timeline?r=ticket-f03637392d


login: aku
mimetype: text/plain
icomment:
  Pulled from mirror.
  Reviewed. Some things are missing ...
  Action items for missing things ...
  - Update doctools version number (bugfix -> patchlevel increment).
  -- Update documentation (version change).
  -- Update pkgIndex.tcl (version change).
  - Fix analogous errors in the doctools::toc and doctools::idx packages.
  Actions completed.
  Revision is [700345aa09].
  Updated embedded docs, revision [1aebd8a08b].
  Committed.
  Merged to trunk.
  Pushed.


login: aku
mimetype: text/plain
icomment:
  > Why use custom regular expressions in ncgi when we have uri::split?

  IIRC "ncgi" existed before "uri" was written.


login: aku
mimetype: text/plain
icomment:
  Accepted the patch, with slight modifications.
  - Replaced (dict get) with array commands.
  - Keeps the package at needing Tcl 8.4+, instead of bumping it to require Tcl 8.5+

  Fixed in revision [9f2e9488eb].
  Committed.
  Pushed.

  Thank you.


login: aku
mimetype: text/plain
icomment:
  Confirmed. Thank you for the report.

  Ah. This particular type of problem has the name

      "creative writing (of a global variable)".

  Due to a missing 'variable' declaration for X the core's var resolution is in the global namespace when it finally comes to the creation of the variable X.

  Given that we have more Tcllib packages than just "uuid" using the LoadAccelerator pattern to detect and choose among multiple implementations of their functionality this means we may have more instances of this problem lurking. Review of all places in Tcllib with this will be required.


login: aku
mimetype: text/plain
icomment:
  Thank you very much for the report. The final result of the review was that 6 packages were affected by this, including uuid. 14 other packages were not affected. Details in the commit.

  Fixed in revision [64c68f3380].
  Committed.
  Pushed.

  uuid      :: Bumped to 1.0.4.
  sha1      :: Bumped to 1.1.1.
  md4       :: Bumped to 1.0.6.
  ripemd128 :: Bumped to 1.0.5.
  ripemd160 :: Bumped to 1.0.5.
  crc32     :: Bumped to 1.3.2.


login: aku
mimetype: text/plain
icomment:
  Yes. I did quite a lot of work on the parse tools, see the branch
      https://core.tcl.tk/tcllib/timeline?r=pt-work&n=200
  if you are interested in the details.


login: aku
mimetype: text/plain
icomment:
  Thank you for the report and patch.
  The parts about superfluous and uninitialized variables look good and I see no problem with applying them as is.

  The chunks adding cast to (uintptr_t) etc. in various places however will need more work. There are still machines around which do not have inttypes.h, and related header files. IOW, these are not portable.

  It might be useful to check if using the tcl.h macros INT2PTR() and PTR2INT() will work for these.

  I would also be interested in knowing which compiler/platform is throwing the warnings. Because if I have access to such a machine myself I could work things out myself instead of going through you to test any changes I may make.


login: anonymous
mimetype: text/plain
icomment:
  You do have a point about inttypes.h not being universally available. Using INT2PTR is a good idea, provided that critcl compiles the files with -DHAVE_INTPTR_T. The only place where I see that declaration is in tclConfig.sh.

  Anyway, I probably cooked this patch before you committed your long / void * fixes. The current source code compiles without warnings on FreeBSD, amd64, clang 3.4.1.

  So, as long as sizeof(long) == sizeof(void *) all is good.


login: aku
mimetype: text/plain
icomment:
  Just checked and the macros are defined even when HAVE_INTPTR_T is not defined.
  In that case they pretty much look what I did with my casting, i.e. only (void*) etc, without going through the intptr_t et al.

  I guess I simply have to check each chunk if it still applies or not.

  critcl does not check by itself to ascertain what is in the environment or not.
  I.e. it is not autotools.

  The critcl::utils package has some commands which allow the package getting compiled to make such checks. I might look into that. Still have to bring Tcllib's C parts up to critcl 3.x anyway.


login: anonymous
mimetype: text/plain
icomment:
  Perhaps a safer way would be to add a check for sizeof(int) == sizeof(void *) in tclInt.h and if not, cast first using size_t. I conjecture that if there's an integral type the same size of a pointer, that's size_t on any sensible machine nowadays.


  --- generic/tclInt.h
  +++ generic/tclInt.h
  @@ -102,21 +102,31 @@
   #if !defined(INT2PTR) && !defined(PTR2INT)
   #   if defined(HAVE_INTPTR_T) || defined(intptr_t)
   #	define INT2PTR(p) ((void *)(intptr_t)(p))
   #	define PTR2INT(p) ((int)(intptr_t)(p))
   #   else
  -#	define INT2PTR(p) ((void *)(p))
  -#	define PTR2INT(p) ((int)(p))
  +#	if sizeof(int) == sizeof(void *)
  +#	    define INT2PTR(p) ((void *)(p))
  +#	    define PTR2INT(p) ((int)(p))
  +#	else
  +#	    define INT2PTR(p) ((void *)(size_t)(p))
  +#	    define PTR2INT(p) ((int)(size_t)(p))
  +#	endif
   #   endif
   #endif
   #if !defined(UINT2PTR) && !defined(PTR2UINT)
   #   if defined(HAVE_UINTPTR_T) || defined(uintptr_t)
   #	define UINT2PTR(p) ((void *)(uintptr_t)(p))
   #	define PTR2UINT(p) ((unsigned int)(uintptr_t)(p))
   #   else
  -#	define UINT2PTR(p) ((void *)(p))
  -#	define PTR2UINT(p) ((unsigned int)(p))
  +#	if sizeof(unsigned int) == sizeof(void *)
  +#	    define UINT2PTR(p) ((void *)(p))
  +#	    define PTR2UINT(p) ((unsigned int)(p))
  +#	else
  +#	    define UINT2PTR(p) ((void *)(size_t)(p))
  +#	    define PTR2UINT(p) ((unsigned int)(size_t)(p))
  +#	endif
   #   endif
   #endif
   
   #if defined(_WIN32) && defined(_MSC_VER)
   #   define vsnprintf _vsnprintf


login: aku
mimetype: text/plain
icomment:
  > Perhaps a safer way would be to add a check for
  > sizeof(int) == sizeof(void *) in tclInt.h [...]

  You might wish to propose this on the tcl-core mailing list.
  If there are any portability issues with that type of check
  somebody there should know.


login: aku
mimetype: text/plain
icomment:
  Applied the parts about unused variables, missing initialization.
  Not applied the chunks doing casting.

  Commit [5d35be9b3a].
  Pushed.


login: emmanuel
username: emmanuel
mimetype: text/plain
icomment:
  I have an actual usecase for separating the test and upgrade in a server implementation. However, I think that you are right. One solution would be to document the variable created and change the calling/return interface: websocket::test would return fullyqualified name of the variable as an identification of success, otherwise an empty string. It would be up to the caller to remove (unset) the variable if no upgrade is to be performed.

  What do you think?


login: emmanuel
username: emmanuel
mimetype: text/plain
icomment:
  You are right, I had not overseen this in the specification and never have had to use it.

  There is an embryo to an implementation in websocket::test, what would you propose as an API for this?


login: aku
mimetype: text/plain
icomment:
  Received in direct mail to me, instead of commenting here...
  > I am using TCL8.6
  > Regards, Vijay

  Ok. Since Tcl 8.6 the socket command was extended to support IPv6 and I would expect that this automatically conveys IPv6 ability to packages built on top of that, like Tcllib's ftp package.

  In other words, the ability to use IPv6 comes out of the Tcl core in use, and not the ftp package. And the core you are using does support IPv6.

  Given your experience to the contrary I now need more information to reproduce your issue. The best would be if you were to attach a script here (to the ticket) which demonstrates the problem you are seeing.


login: anonymous
mimetype: text/plain
icomment:
  I am running Xlight FTP server over IPv6.

  Here Is the FTP-client using TCL's FTP library:
  -----------------------------------------------
  package require ftp
  set server_ip	"::1"
  set usernm	"anonymous"
  set passwd	"anonymous"
  set ::ftp::VERBOSE 1
  set ::ftp::DEBUG 0

  #Defining Callbacks

  proc Hash {total} {
  	puts -nonewline "#"
  }
  proc ServerReply {id msg state} {
  	puts $msg
  }

  # open a new connection

  set ftp_id [::ftp::Open $server_ip $usernm $passwd -timeout 20 -mode active -output ServerReply -progress Hash]

  #########
  # Till Here everything is going fine. But I am getting error with the below line of code. 
  # Below is the Error I'm getting:
  #08/26/2014 10:46:58 (anonymous ::1<--[::1]:21) "425 PORT doesn't support ipv6 address, use EPRT for ipv6"
  #########

  ::ftp::Get $ftp_id 100MB_file
  ::ftp::Close $ftp_id


login: aku
mimetype: text/plain
icomment:
  Ok, reading

  > (anonymous ::1<--[::1]:21)
  > "425 PORT doesn't support ipv6 address, use EPRT for ipv6"

  it seems to me that the Xlight server expects that the PORT command contains only IPv4 addresses and that IPv6 addresses should be requested via EPRT. Even if it can recognize an IPv6 address in PORT.

  So what we have here is not a low-level IPv6 issue (as I thought initially), but an issue at the level of the FTP protocol itself.

  Searching a bit the relevant RFC seems to be
      http://tools.ietf.org/html/rfc2428

  Is there something in the responses coming from XLight which would allow the FTP package to automatically recognize the need/support for the EPRT command ?

  As an extension of the ftp protocol the package cannot really assume that everybody understands EPRT/EPSV now. Of course, in the worst case use of either PORT or EPRT can be made into an option we can set on an ftp session.


login: aku
mimetype: text/html
icomment:
  Having a second look, I should be able to use the 
  <a href="doc/trunk/embedded/www/tcllib/files/modules/dns/tcllib_ip.html">
  "ip" commands</a>
  to detect use of IPv6 addresses and use that information to automatically activate use of EPRT/EPSV. No user intervention required.


login: anonymous
mimetype: text/plain
icomment:
  Thank you very much for your help.

  AKU:
       Is there something in the responses coming from XLight which would allow the FTP package to automatically recognize the need/support for the EPRT command ?

  ANSWER:
       This morning I tried windows ftp client over IPv6. Below is the response I got from XlightFtp server:
      09/02/2014 12:05:15 (vijay ::1-->[::1]:21) "EPRT |2|::1|58404|"

  Could you let me know how you have used "ip" commands to detect use of IPv6 addresses?


login: anonymous
mimetype: text/plain
icomment:
  FYI

  http://www.juniper.net/documentation/en_US/junos12.1x46/topics/concept/security-alg-ftp-commands.html


login: aku
mimetype: text/html
icomment:
  > Could you let me know how you have used "ip"
  > commands to detect use of IPv6 addresses?

  While I have not done any coding yet the general idea is to use
  <a href="doc/trunk/embedded/www/tcllib/files/modules/dns/tcllib_ip.html#1">
  ip::version</a> on the IP address of the client itself. Depending on the result, either 4 or 6 it should then be possible to have the code doing data connection handling use either PORT or EPRT commands to communicate that address to the server.


login: aku
mimetype: text/plain
icomment:
  > I have gone through the attached ftp.tcl code.
  > Here I found that, this code does not support
  > EPRT or EPSV commands. Please correct me if I
  > am wrong.

  My apologies for not being clear enough in my comments.

  Yes, the code does not support EPRT/EPSV.

  My comments regarding the ip package were thoughts and musings on what changes will have to be made to add such support.


login: anonymous
mimetype: text/plain
icomment:
  Could you let me know what is your next step, I mean are you people going to add EPRT and EPSV commands support to the ftp library in 8.6 itself?


login: aku
mimetype: text/x-fossil-wiki
icomment:
  I managed to make a bit of time now and believe I managed to add EPRT support.
  No EPSV support yet.

  The code has been committed to branch "ak-tkt-b41b6f76b0-ftp-eprt".
  The current commit on that branch is [ad53730d6b].

  Please get yourself these sources (ftp version is 2.5, on that branch),
  and test if they work for your case.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  And with a bit more time some initial work on EPSV support.

  The relevant commit is [eb27679053], again made on the
  branch "ak-tkt-b41b6f76b0-ftp-eprt".

  Please retrieve that commit and test the EPSV support.
  Any problems should be reported in this ticket.


login: aku
mimetype: text/plain
icomment:
  Technically out of date with the elimination of `mimetypes` in version 2 of fumgagic, by pooryorick.

  On the other hand, maybe it was added to `filetypes` in the same set of changes ?

  Nathan, any comments ?


login: aku
mimetype: text/plain
icomment:
  Interesting how fossil is able to handle the unicode chars as is.

  I guess that we are talking about the unicode code-points
      \u4F60  -- Unicode Han 'you, second person pronoun'
  and \u597D  -- Unicode Han 'good, excellent, fine; well'
  respectively, right?

  In the line marked INCORRECT, what is the expected correct result ?
  And again, for the messy string, what was the expected correct result ?

  Need this for my translation of the information in the description into test cases. Also, please write any unicode chars in the expected results using Tcl's \u.... notation. I have to know the code-points, not the ideograms shown to me by the browser.


login: aku
mimetype: text/plain
icomment:
  Can you explain the "O60Y7D" as well ?
  I would have thought that the unquote returns unicode which went into quote, i.e. that they are inverses of each other:

     x = unquote (quote (x))

  The example seems to contradict that.
  Also, is O60Y7D another encoding for a unicode point?

  (You may notice that I am not the author of the package, and are currently working and researching a bit to understand things.)


login: aku
mimetype: text/plain
icomment:
  Reading RFC 2141 referenced in the docs, and glancing over urn-scheme.tcl I believe I begin to see the problem.

  (1) The unicode codepoints should have been converted into UTF8 bytes, and
  (2) each byte should become a %.. sequence, with the hex after % limited to two characters.
  Whereas quote generates a 4byte hex from the exact code-point.

  And unquote should likely reject %-sequences with more than two hex characters. Right now it seems to convert only the first two hex. I am unsure if it converts the next two separately or ignores them.

  And the last unquote was where it got fed a 2 3-byte utf8 characters and converted them wrongly. Likely as separate bytes/chars.

  And the testsuite covered only ASCII, and nothing in the higher unicode points.


login: anonymous
mimetype: text/plain
icomment:
  "你好" together means "how are you?"

  uri::urn::quote 你好 
  the correct result is %E4%BD%A0%E5%A5%BD

  uri::urn::unquote %4F60%597D
  %4F is O, %59 is Y, thus "O60Y7D", not "你好" as I expected.


login: aku
mimetype: text/plain
icomment:
  Ok, the correct result is as I suspected after reading the RFC.
  6 hex sequences, each representing a 3-byte utf-encoded character.

  And now I understand the back-conversion as well, which properly takes only 2 bytes from the hex sequence and treats the remainder as regular characters.

  This side also needs a back conversion from utf-8. That is the reason for the messy string I believe. I.e. you get 6 separate binary bytes back, instead of two utf-8 characters.

  Work on this can commence.


login: aku
mimetype: text/plain
icomment:
  Fixed with revision [9c454867a1].
  Pushed.
  Version bumped to 1.0.3.
  Had to fix the original test cases as well, were broken (made to the bad implementation).

  Thank you for the report and help.


login: aku
mimetype: text/plain
icomment:
  Found that I made some pt code changes on Aug 11, 2014 and forgot to update the tests to match them.

  Fixed with revision [98f00771ae].
  Committed.
  Pushed.
  Thanks again for the report.


login: aku
mimetype: text/plain
icomment:
  Sigh. Yes.

  While I would like to simply disable the whole testsuite when the package is not found the TclOO-dependent pieces are very much in the minority and should not block the entire testsuite.

  Well, at least for pt I am sure of that. Have to check how it looks for tar.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fix, part II (pt, actually also easy, except where I reworked things to properly count the skipped tests).
  Revision [5f6ff5bde3].
  Pushed.

  This completes the set of fixes needed to close the ticket.
  Thanks for the report.


login: aku
mimetype: text/plain
icomment:
  Meh. I wanted to say that this may be a documentation bug, except, these commands are NOT documented at all.

  It seems that 'dict2json' assumes that it only has to quote the keys, and that the values are already properly formatted as JSON.

  The 'list2json' command apparently does the same.

  And the 'string2json' command does not perform proper quoting of all the special characters. Ok, this is a bug.

  At this point I am tempted to say that the best course here is to

  (1) Remove these commands (not documented anyway).
  (2) Point users to the package "json::write" which is documented.
      Which includes notes on which arguments are expected to be
      proper json already.

  Would that work for you ?


login: anonymous
mimetype: text/plain
icomment:
  For me ok.
  For now i understand (probably) problems with JSON -> dict -> JSON conversions.
  I found another solution.
  I would like to add new command to json package: json2huddle.
  With huddle we have solution for this conversions.
  After testing i send RFE patch.


login: anonymous
mimetype: text/plain
icomment:
  Finally my addons to json lib:
  - new proc json2huddle which returns HUDDLE object.

  Simple test: 
  huddle jsondump [json2huddle $jsonText]


login: anonymous
username: gahr@gahr.ch
mimetype: text/plain
icomment:
  I just realized that 8.4 doesn't have the "u" flag char. If compatibility with 8.4 is to be kept, please disregard this ticket.


login: aku
mimetype: text/plain
icomment:
  Actually, given that "aes" is currently assuming Tcl 8.2 it is overdue to be moved to a modern revision of the core.

  I will apply this patch, bump the package to version 1.2 and declare that to require Tcl 8.5.

  At that point please feel free to add more 8.5-isms, if sensible ({*}, lassign, in/ni operators, whatever).


login: aku
mimetype: text/x-fossil-wiki
icomment:
  The aes-modernize patch uses 'lmap'.
  That is a Tcl 8.6 command, and not available for 8.5.
  Six places.

  But we have lassign and can unroll the loops (4 iterations is small enough). Note, this makes use of our knowledge about the size of the list, i.e. that it always has 4 elements.

  And that fixes the test breakage I saw.
  Can't say if that gives us perf gains or not.

  Did you use the aes.bench to benchmark things ?

  For the moment I put the patch + mods on branch "aes-modernize" so that I can benchmark it versus the original using aes.bench.

  Commit [5903319520]
  Pushed.


login: aku
mimetype: text/plain
icomment:
  Config for both variants:
  - Tclsh 8.5.7
  - Repeat 10 (= run 11 times (1+ 10 repeats))
  - Collate min (take best time over all runs)

  Gains about 1-3% it looks like.
  Col 1 = aes/trunk
  Col 2 = aes/modernize
  Normalized to column 1.
  To check against 8.5.16 and 8.6

  +----+-----------------------------+------+------+
  |    | INTERP                      |    1 |    2 |
  +----+-----------------------------+------+------+
  |  1 | AES-128 CBC keyschedule     | 1.00 | 1.00 |
  |  2 | AES-128 ECB decryption      | 1.00 | 1.01 |
  |  3 | AES-128 ECB decryption core | 1.00 | 1.00 |
  |  4 | AES-128 ECB encryption      | 1.00 | 0.99 |
  |  5 | AES-128 ECB encryption core | 1.00 | 0.97 |
  |  6 | AES-128 ECB keyschedule     | 1.00 | 1.00 |
  |  7 | AES-192 CBC keyschedule     | 1.00 | 1.01 |
  |  8 | AES-192 ECB decryption      | 1.00 | 1.01 |
  |  9 | AES-192 ECB decryption core | 1.00 | 1.00 |
  | 10 | AES-192 ECB encryption      | 1.00 | 0.98 |
  | 11 | AES-192 ECB encryption core | 1.00 | 0.98 |
  | 12 | AES-192 ECB keyschedule     | 1.00 | 1.00 |
  | 13 | AES-256 CBC keyschedule     | 1.00 | 1.00 |
  | 14 | AES-256 ECB decryption      | 1.00 | 1.00 |
  | 15 | AES-256 ECB decryption core | 1.00 | 1.01 |
  | 16 | AES-256 ECB encryption      | 1.00 | 0.99 |
  | 17 | AES-256 ECB encryption core | 1.00 | 0.98 |
  | 18 | AES-256 ECB keyschedule     | 1.00 | 0.99 |
  +----+-----------------------------+------+------+


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Duplicate of [b3577ed586], maybe ?

  Regardless, I do not know enough of the oo stuff with delegates.
  This has to go through dkf I believe.


login: aku
mimetype: text/plain
icomment:
  Ok. When you have the patch please attach it to this ticket.

  Please make a small self-contained test case as well, which I can then add to the package's testsuite.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Applied the fix for ticket [b3577ed586].

  This was pushed as oo::util 1.2.1

  Please report if that helped with this problem as well.

  Note that oo::util has the beginnings of a testsuite now.
  Any fix for this should have a self-contained testcase we can add to it.


login: aku
mimetype: text/plain
icomment:
  Pity :(

  You mentioned Sep 18 that you had a likely fix.
  Can you attach this fix here please?

  A self-contained test-case would be good as well.


login: anonymous
mimetype: text/plain
icomment:
  I am a different anonymous (I can't figure out how I can register an account here :( ), so unfortunately I don't have a likely fix.


login: anonymous
username: aspect
mimetype: text/plain
icomment:
  The patch at http://paste.tclers.tk/3540 seems to address this issue cleanly .. I didn't see it prior to reading Ashok's wonderful article on TclOO!

  Apologies for not including the patch here:  I can't attach files as anonymous, and can't get fossil's ticket formatting to play nice.


login: aku
mimetype: text/plain
icomment:
  Hi aspect. I have pulled the patch and attached it to the ticket here.
  Testing it I came across a few issues with it, which I hope you can clarify.

  The base revision I work with is [d29f257cb9], which is trunk head, currently.

  - The change to the ooutil.test file is rejected.
    Looking at the chunk I see pieces of tests which we do not seem to have
    in the repo. It seems that you have more tests in your local version.
    (Would be interested in having these tests)

  - I usually try to confirm the problem, and that new test cases demo it,
    and the fix. I was unable to. IOW the new tests succeed even if I only
    add them, without making the change to the oo::class.Delegate. They
    succeed as well when the Delegate change is made. Thus they do not seem
    to demo the problem and I cannot use them to verify the fix.

  This second part is the most trouble.

  I used Tcl 8.5.18+TclOO 1.0.2, and Tcl 8.6.4 for the testing.
  Both Tcl were near-head of their respective branches.

  Please have a look at these test-cases.


login: anonymous
username: aspect
mimetype: text/plain
icomment:
  - I mistakenly made the patch against the "odie" branch:
    it sounds like you managed to clean up and apply it to
    trunk;  if not, let me know and I'll adjust to suit

  - the provided tests don't actually excercise this issue
    .. I was lazy.  The following does:


  test ooutil-ticket-fe7a0e0a3a {classmethod must not interfere with constructor signatures} -setup {
      oo::class create TestClass {
          superclass oo::class
          self method create {name ignore body} {
              next $name $body
          }
      }
  } -body {
      TestClass create okay {} {}
  } -cleanup {
      rename TestClass {}
  } -result {::okay}


login: aku
mimetype: text/plain
icomment:
  Ok, that explains things.
  - Confirmed, new test demonstrates problem.
  - Confirmed, patch fixes problem, passes new test.
  Commit [874cdb3f3f].
  Pushed.

  Commit [bb0df7cdd1] for the unrelated classmethod tests.
  Pushed.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Attached my variant of oauth, reformatted to fit into Tcllib.
  Notes:
  * Requires Tcl 8.5 (dict cmd, lsearch -nocase, clock milliseconds)
  * Most error returns encapsulated into helper command "Error".
  * Added error-code information to thrown errors.
  * Fixed namespace and version information.
  * Internal date information ($created) switched to use ISO standard date format.
  * Moved https registration outside of command to global.

  Javi, please take the attached code and test if it still works for you.
  It does for me, using the test script you gave me in mail.

  The code is also in the repository, in branch "oauth".
  Revision [7b3a12af1f].


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Documentation received by email.
  Edited.
  Integrated.
  Branch merged.

  Relevant commits are 
  	[0982cecb8c] and 
  	[5f1d4b4301].

  Pushed.
  We are done.
  Closing branch.


login: aku
mimetype: text/html
icomment:
  <p>I wrote a paper on these things in 2009, available at
  <a href="http://www.tclcommunityassociation.org/wub/proceedings/Proceedings-2009/proceedings/TclChannels/rtc.pdf">
  Reflecting and Transforming Channels
  </a>

  <p>Does that help ?

  <p> The referenced examples of that paper became the virtchannel_* modules in Tcllib, i.e. core, base channels, and transforms.
  <p> The exception is the code of the file transfer example in section 5.3.
  That I should dig out and add to Tcllib as well.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Thank you.
  I will try to review ASAP (after I manage to fix up TLS for the POODLE [1]).

  In the mean-time, may I impose on you and ask you to start on a proper testsuite for the packages too ? Note that I would prefer this (testsuite) done on the trunk of Tcllib. That way I can test the current implementation and then later yours, after the merge, or before, from your branch, if you merged that testsuite into it.


  (Ad 1) http://googleonlinesecurity.blogspot.ca/2014/10/this-poodle-bites-exploiting-ssl-30.html


login: hypnotoad
username: hypnotoad
mimetype: text/plain
icomment:
  Will do. This week is a little rough between pumping out papers and a deadline at work, but I will endeavor to get it done sometime after next week.


login: aku
mimetype: text/plain
icomment:
  Thank you. It certainly is not urgent.

  (I know that the package works due to our use in the AT buildsystem. However with big changes I'd rather have a testsuite with which to guard against breakge before merging).


login: aku
mimetype: text/plain
icomment:
  Notes on the commits ...

  * I wonder if it might make sense to test directly for the 'zlib'
    ensemble command instead of testing the version number.

  * I wonder if might make sense to define internal mini-wrapper
    procs for the two cases (native vs Trf/zlibtcl) instead of
    using the variable "::zipfile::decode::native_zip_functs" to
    switch dynamically.

  * The new encoder currently seems to be missing the definitions
    which make it API-compatible with the pre-86 code. Or, IOW, the
    two implementations currently export completely different APIs.


login: hypnotoad
username: hypnotoad
mimetype: text/plain
icomment:
  Notes on the notes to the commits:

  1) Knowing the zlib command exists doesn't tell us enough. Older zlib implementations don't understand Tcl's channel push/pop mechanism. And pre 8.5 tcl doesn't understand Tcl's push/pop mechanism. So at some point which version of Tcl is a decision mechanism.

  2) Definitely. This was a first pass to fix the encode/decode for 8.6 (as I was having a devil of a time getting Trf to work on Windows.) The mission critical piece could be wrapped in a command that tcllib defines at source time depending on conditions.

  3) The new encoder exploits a lot of 8.6isms. And... the older implementation uses an entirely different operating principle based on snit. To reconcile the two would involve massively rewriting the new to target that various zlib extensions and channel wrapping mechanisms in the wild.

  The temporary cheat was to use the new implementation for 8.6+, and the old implementation for < 8.6

  And my own comment:) There is a lot of code that encode and decode could share. I would also create a third package zipfile::common which would implement those functions, and have encode and decode package require zipfile common, and then import them into their respective namespaces.

  Functions like "encode a date in dos time" really should not be implemented twice. Technically three times because decode has one, and encode has two now.


login: aku
mimetype: text/plain
icomment:
  Notes on the notes to the commits:

  > 1) Knowing the zlib command exists doesn't tell us enough.
  > Older zlib implementations don't understand Tcl's channel
  > push/pop mechanism. And pre 8.5 tcl doesn't understand Tcl's
  > push/pop mechanism.

  Ok. This should possibly noted in the code to prevent other people from asking againg, or changing things.

  > 2) Definitely. This was a first pass to fix the encode/decode for 8.6
  > (as I was having a devil of a time getting Trf to work on Windows.)

  I wonder what is going on with that. Build trouble, or runtime ?
  The AT build does not have issues AFAIK.

  > 3) The new encoder exploits a lot of 8.6isms. And... the older
  > implementation uses an entirely different operating principle based on snit.  > To reconcile the two would involve massively rewriting the new to target that
  > various zlib extensions and channel wrapping mechanisms in the wild.

  > The temporary cheat was to use the new implementation for 8.6+, and the
  > old implementation for < 8.6

  The thing is, the user of 'encode' currently has to distinguish 8.6+ and pre-8.6 also, to code to each API.

  Note that I do not mind using TclOO to recreate the object parts of the API,
  and not having any of the logger stuff.

  Another thing to note is that the version numbers of the two packages are 0.x ... If we can agree on a nice API I do not mind changing it around before going 1.x


  > And my own comment:) There is a lot of code that encode and decode
  > could share. I would also create a third package zipfile::common
  > which would implement those functions,

  ++100

  (I think (seem to remember) that the two packages were created at different times and independently. That would explain the duplication.)

  > Technically three times because decode has one

  Wondering now why decode would have an encoding function


login: aku
mimetype: text/plain
icomment:
  Thinking about things a bit more I may make this a doc change, a section talking about protocols and how it is the responsibility of the application to select what should be used, instead of the package using TLS (package).


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Accepted, with some rewriting.
  Put the text block into a new location for reusable text blocks.
  Updated not only oauth, but all packages in Tcllib using TLS (even if optional).

  Commit [a5887963b0].
  Pushed.
  Done.

  Thank you.


login: aku
mimetype: text/plain
icomment:
  Can you tell me more about what I should see when the test script fails ?

  I added a bit of output and code to source the mime package directly out of the checkout, as starting point for a bisection (See newest attachment "bis").

  Then I ran it against the current head of the tcllib trunk branch, and got a nice ::mime::1 token name back. So at first glance it looked to be working to me.

  And when I ran the testsuite it was ok too.

  Hm. It seem I am using Tcl 8.5.14 for my work.
  What version of Tcl are you using in your tests ?


login: anonymous
username: rkeene
mimetype: text/plain
icomment:
  crc::sum version 1.1.0:
    % package require sum
    1.1.0
    % crc::sum -sysv [string repeat X 1489]&
    0

  crc::sum version 1.1.1:
    % package require sum
    1.1.1
    % crc::sum -sysv [string repeat X 1489]&
    65534

  Correct value
    $ wget -O sum.c 'http://heirloom.cvs.sourceforge.net/viewvc/heirloom/heirloom/sum/sum.c?revision=1.2'
    $ gcc -o sum sum.c
    $ tclsh
    % set x [string repeat X 1489]&
    ...omitted...
    % set fd [open x.txt w]
    file5
    % puts -nonewline $fd $x
    % close $fd
    % exec ./sum x.txt
    65535 3 x.txt
    %


login: aku
mimetype: text/plain
icomment:
  Application of the patch breaks the test cases 6.0, 6.1, and 6.2. Checking against the sum.c from the description tells me that the new values seem to be ok. Updated testsuite, and added test for the example given in the description.

  In the future please provide patches containing the necessary doc and test updates as well.

  Version bumped to 1.1.2. Updated docs.

  Commit [232653cd52].
  Pushed.


login: aku
mimetype: text/plain
icomment:
  Ticket created, and implementation patch attached.
  Assigned to myself for integration.

  Asked via mail for test cases to protect the feature against breaking by future changes of the packages.


login: aku
mimetype: text/plain
icomment:
  Patch applied, manually.
  Unclear if the patch was unclean, or the extraction from the mail broke it.
  Added tests.
  Bumped package version to 1.4.
  Updated docs.

  Committed [5dbaabe344].
  Pushed.


login: aku
mimetype: text/plain
icomment:
  Adding a new module should only require extending support/installation/modules.tcl, and possibly .../actions.tcl in case it requires some special stuff.

  Adding a package to an existing module should not require anything.

  Please correct the above should I be wrong.

  Quantum assumptions: Can you list which assumptions you have found ?

  Plugin ... I wonder if a switch to use Kettle would be do-able ? With that we can have a "build.tcl" in each module, and a main "build.tcl" specifying recursion in the main/top directory. ... Of course, Kettle makes assumptions about structure too.


login: hypnotoad
username: hypnotoad
mimetype: text/plain
icomment:
  Agreed, running things through the main installer seems to do the right thing(tm) if the module is properly indexed in support/installation/modules.tcl.

  As far as quantum assumptions go... for taolib I had to rewrite chunks of the SAK installer. Tao has several rather large feature sets that are broken up into multiple files and directories. The SAK installer has no concept that a module could have subdirectories. Also, packages like taotk need to bundle an assortment of image resources for icons, and there isn't a neat way to do that. (Though I was unaware of the ../actions.tcl mechanism, I'll see if I can get around those issues using that.)

  I'm open to evaluating Kettle. My thought is to start off with a feature branch (say... "kettle") and begin the process of porting the installer.


login: aku
mimetype: text/plain
icomment:
  Yes, the _tcl, etc. information in the modules.tcl are procedures in actions.tcl. _tcl is the standard module with plain packages. Some modules already have special actions, like _tcx,, or similar for their special requirements.

  Agreed on using a feature branch "kettle" for initial porting and eval.


login: aku
mimetype: text/plain
icomment:
  I suspect that this change was intentional, right ?

  Is this change documented ?

  If not I would say this is only a doc change.

  Although it can be argued that it was an incompatible API change requiring a major version bump (because it was a change of the public API, not a simple extension).


login: anonymous
username: Adrián Medraño Calvo <amcalvo@prs.de>
mimetype: text/plain
icomment:
  The change was intentional, in the sense of making server and client notifications more consistent, but unintentional with regards to breaking the API.  The incompatibility can be easily fixed with the following patch.  The documentation should be changed marking the event as deprecated.

  diff --git c/websocket.tcl i/websocket.tcl
  index a35f1b1..ef8b642 100644
  --- c/websocket.tcl
  +++ i/websocket.tcl
  @@ -1147,6 +1147,9 @@ proc ::websocket::takeover { sock handler { server 0 } { info {} }} {
       Liveness $sock
   
       # Tell the WebSocket handler that the connection is now open.
  +    if {!$server} {
  +	Push $sock request $info;
  +    }
       Push $sock connect $info;
       
       ${log}::debug "$sock has been registered as a\


login: aku
mimetype: text/x-fossil-wiki
icomment:
  See also https://github.com/dbohdan/tclssg/issues/28
  for a related ticket affected by the requested change.


login: anonymous
mimetype: text/plain
icomment:
  The json::json2dict command returns dictionary or set of key-value pairs. What you see in the command output is its string representation. You're not supposed to use the string representation unless you know what you're doing. Just execute

  % dict get $dict length
  $::x

  and you'll get your initial string.

  This doesn't look like a bug to me.


login: anonymous
mimetype: text/plain
icomment:
  The point is that the original json was an array, not an object containing key:value pairs. "length" is not a key and could be anything. The problem becomes crucial if you do function calls via JSON, and the array is the list of parameters which can normally be given as is to a tcl proc or command

  % set ::x 5
  5
  % set d {1 + \$::x}
  1 + $::x
  expr $d
  6

  works perfectly fine. but:

  % set json [json::write array 1 \"+\" $var]
  [1,"+","$::x"]
  % set a [json::json2dict $json]
  1 + {$::x}
  % expr $a
  can't use non-numeric string as operand of "+"

  This being Tcl, I arguably should be able to use the string representation whenever I want to. There is just no good reason for this to be formatted as a list with a single member. Try it without the $ and it will be just fine.


login: anonymous
mimetype: text/plain
icomment:
  Oops, my bad, I haven't noticed that there was an array and not an object. In this case json::json2dict happens to return a list, not a dictionary. This behavior is reasonable but undocumented, which can be considered a bug.

  As for your examples with [expr], it's just the way lists or dicts work. Their string representations are guarded to make absolutely sure you can extract their items in exactly the same form they were added. So, once more: use string representation of lists or dictionaries only if you know exactly what you're doing. Otherwise it's safer to extract individual items using [lindex] or to iterate using [foreach], or join them into a string via [join].


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Which version of Tcllib is used ?
  Which version of the "tar" package is used ?

  I am asking because of Ticket [2840180fffffffffffff]
  and commit [9cefaa3479] which changed the package behaviour in this area.

  Looking at [fdiff?v1=39116c3beaeef80c&v2=ccc1188a13c75865&sbs=1]

  the code attempts to split a long path at a path separator.
  Your filename is long, but does not have a path separator, so it could not be split.

  Is GNU tar able to handle your file ?
  If yes, then please attach the result tar file to this ticket.
  This would allow me to inspect how GNU tar performs the split.

  Note that this type of example only requires a long file name. The content is not relevant, i.e. this could be done with an empty file.


login: aku
mimetype: text/plain
icomment:
  Note: This change breaks the test cases 1.6, 2.7, and 2.8 of adjust.test.
  These are about strictlength and operating at the same limit.

  Review shows that the test cases are wrong, and have to be fixed as well.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fixed with commit [19a4dd954c].
  Actual change is slightly different from yours, to avoid 8.4 features.
  Package is still 8.2-compatible.

  Pushed.

  Version of textutil::adjust bumped to 0.7.2.


login: aku
mimetype: text/plain
icomment:
  Can I assume that the [gets] command was called without arguments ?

  This looks to be only case where the command goes into the wrong code block.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fixed with commit [45c988bdfc].

  Pushed.

  Version bumped to 1.1.3 (both coroutine & coroutine::auto).

  Done.


login: anonymous
username: heinrichmartin
mimetype: text/plain
icomment:
  pls review priority, this affects valid code, too:

  #!/usr/bin/env tclsh
  package require Tcl 8.6
  package require coroutine::auto
  after 1000

  after -> ::::core_after -> ::unknown


login: aku
mimetype: text/plain
icomment:
  Added commands "distance" and "nextIp".

  Version bumped to 1.3 (new features).
  Extended testsuite and docs.

  Revision [e2be9b2f86].

  Pushed.


login: aku
mimetype: text/plain
icomment:
  Hm.

  I suspect that the event-loop has to be running for this to work.

  That said, the docs definitely do not mention that, and these are the examples from the docs too.

  This needs a check.

  Heh. While the package has a testsuite this (IMHO core) functionality of it is not covered :(

  Could possibly be done using Google's nameservers.

  That begs the question, do you possibly have no nameservers set up ?
  (Which platform ?)

  The output of the [::dns::nameservers] command could help here to see what the package thinks it has available.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  I can now confirm that executing a

    vwait $tok

  just after the dns::resolve command fixes the issue, for dns version 1.3.3.
  For 1.3.4 adding the vwait is a workaround as well.

  So, issue reproduced here.

  Looking into the version 1.3.4 code there should be some 'wait'ing happen automatically. But apparently not, or not correctly.

  Oh. The chosen protocol is UDP (*), and the auto-wait I see in dns::resolve is limited to TCP.

  (Ad *): I have the udp package installed and it got loaded automatically (as per output of [info loaded]). I now suspect that you have the same configuration, with the udp package (or ceptcl) available.

  Making that general ... is a fix. Basically, the vwait of the workaround is done internally, and automatically when -commmand is NOT set, regardless of transport. Very unclear why this was limited to tcp ...

  Hm ... Jan 30, 2003, commit [7109bd29408a8efb]. This added the udp support, and apparently missed the implication of having the auto-wait not extended to it.

  Fixed with revision [ca36641e9f].

  Pushed.

  Version bumped to 1.3.5.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Based on the given examples testcases were added, and resolve modified to make them pass while not failing other cases.

  Considered fixed with revision [a29e0299a8].

  Pushed.

  Version bumped to 1.2.5.


login: aku
mimetype: text/plain
icomment:
  The patch does not contain any changes to the bibtex testsuite.
  Does this mean that it passes the testsuite as is ?

  In that case it it missing a test demonstrating the buggy behaviour and how it is fixed with the patch.

  It should also have new tests for the new feature.

  Is it possibly to supply these to us ?

  Side note: We prefer patches to be a "unified diff" (diff -u format), as that is easier to read than a regular patch.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Branch with patch applied and extended to docs, checker definition (pcx).

  See branch [bibtex-071dbedcc8].

  I plead again for the supplication of testcases demonstrating the bug and the new feature. I do not know enough about bibtex to write these myself.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Merged into 1.17 release branch. Revision [cd1a3ac12d].
  Pushed.

  While resolved as accepted, still keeping this ticket open until we have test cases.

  Modified the ticket title to reflect that.

  Lowering priority.


login: aku
mimetype: text/plain
icomment:
  Michael, can you give me a quick evaluation/opinion on this in general, and if yes, if it should go into 1.17 ?!


login: aku
mimetype: text/plain
icomment:
  Michael's response, in mail:

  > This seems to be quite a reasonable request I see a few corner cases
  > where this could lead to surprises, as there is no 'super root'
  > logger that could act as parent for everything else, unlike some
  > other logging systems that force a single hierarchy, so it should
  > probably do the following

  > 1 If the parent namespace / service already exists and no level is
  >   given, inherit the parents setting

  > 2 If the parent namespace / service does not exist, use the default
  >   setup via logger::setlevel (happens anyway with logger::init)

  > 3 If a level is given use that

  Lets see if I can implement this.


login: aku
mimetype: text/plain
icomment:
  Done with revision [69e306a577].

  Pushed.

  Version bumped to 0.9.4.
  Documentation updated.
  Testsuite extended.

  Merged to release branch with revision [dbfbd27389].


login: aku
mimetype: text/plain
icomment:
  Ah. Had to read the entire example.

  (pre)filter argument is of the same type as the base dir, it looks like.
  I.e. absolute base => absolute (pre)filter, etc.

  I see that the package is still a 0.x series.

  Was tempted to only fix the docs, but with it a 0.x I am now more leaning towards to
  - follow the docs,
  - make the argument absolute in all cases (if easy to implement), and then
  - bump the version to 1.x.
  - That would also come with making Tcl 8.5 the minimum requirement,
  - allowing simplification of the code.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Went not as far as outlined.

  Behaviour is documented => bugfix, bump minor. New version is 0.5.

  Fixed the callback, extended the testsuite, bumped version.
  Nothing else. No switch to 8.5+, for example.

  Fixed with revision [39120747d3].

  Pushed.


login: aku
mimetype: text/plain
icomment:
  Resolved with revision [4fc707c9ef].
  Removed 8.5-isms, restoring 8.4 compatibility as declared.
  Testsuite passes.
  Version bumped to 0.8.1.


login: anonymous
mimetype: text/plain
icomment:
  A recipe for a client blocking the server by refusing to read:

  ---------------->8----------------
  package require websocket;
  namespace eval ::s {};
  proc ::s::accept {sock args} {
      puts "server: new connection $args";
      ::websocket::takeover $sock [list ::s::handler] 1 {};
      ::websocket::send $sock text [string repeat "Welcome" 1000000];
  }
  proc ::s::handler {sock type args} {
      puts "$sock: $type $args";
  }
  proc ::s::canary {interval} {
      puts "canary: Still alive...";
      ::after $interval [list ::s::canary $interval];
  }

  ::s::canary 1000;
  ::socket -server [list ::s::accept] 54321;
  ----------------8<----------------

  In the second shell run the following command:

  ---------------->8----------------
  nc localhost 54321 | { sleep 5s; wc; }
  ----------------8<----------------


login: aku
mimetype: text/plain
icomment:
  Thank you for the submission.
  I welcome future submissions as well.
  Assigned to self for review and integration.
  Bumped priority.


login: aku
mimetype: text/plain
icomment:
  I have put the new code with some basic integration work into branch "huddle-a753cade83". The current revision is [4c78dcbc6f].

  This looks to be incomplete.
  When running the testsuite I see the huddle tests passing, and then failures for yaml and rb tests. These are:

      Aborting the tests found in "yaml.test"
      Error in yaml.tcl: too many nested evaluations (infinite loop?)

  When I looked a bit deeper, using the full stacktrace I saw that yaml contains code like

      proc $procname {command args} [string map "@TYPE@ $type @TAG@ $tag" {
          switch -- $command {
              setting { ; # type definition
                  return {
                      type @TYPE@
                      method {}
                      tag {@TAG@ child}
                      constructor ""
                      str @TAG@
                  }
              }
              default {
                  return [huddle call s $command $args]
              }
          }

  Now, as you changed this in huddle from "setting" to "settings" (added a trailing "s") the unchanged yaml I have goes into the 'default' case of the switch, which in turn goes through the callback to re-invoke this same procedure, ad infinitum.

  When I fixed that I got the next error as

  can't read "isContainer": no such variable
      while executing
  "set types(isContainer:$tag) $isContainer"
      (body of "dict with")
      invoked from within

  which seems to indicate more changes are needed in the yaml package to match it to the changed huddle.

  Can you please check in your environment if you have such changes, and if yes, provide them here ?


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Can you give us a link to the location of the file you are reading ?

  Note that the .man files in Tcllib are plain text / doctools formatted.
  They are the _sources_ from which we derive the actual nroff manpages and HTML documentation.
  They are not intended for reading within a browser.

  Links for HTML docs derived from the rest.man are, for example

  * https://core.tcl.tk/tcllib/doc/trunk/embedded/www/tcllib/files/modules/rest/rest.html
  * http://docs.activestate.com/activetcl/8.5/tcllib/rest/rest.html


login: anonymous
username: cchretie@ciena.com
mimetype: text/plain
icomment:
  I understand that the ".man" files were not meant for reading in the browser. I view the rest.man in a code editor to understand the documentation of the package more clearly.

  https://core.tcl.tk/tcllib/finfo?name=modules/rest/rest.man

  In the rest.man file, the ::rest::create_interface documentation implies a certain organization that the HTML does not publish correctly.

  rest.man
  "[call [cmd ::rest::create_interface] [arg name]]
  TOKENS
       the value is substituted into the url at call time. tokens in the form of %name:default_value% will be an optional argument with a default value.

  url
      the target of the http request
  description
      a string which describes the call. used only for [call describe]
  body
      indicates if arguments are required for the request body or not. should
      be one of none, optional, required, argument or mime_multipart. default is optional.
      if [const argument] is used then the option is parsed as a list where the second value is the name
      of a option. the body will then be used as the value for that option.
      if the value is [const mime_multipart] then the body is required and interpreted as each argument
      representing one part of a mime multipart document. each argument should be a 2 item list with the first being
      a list of header keys and values, and the second being the mime part body.
  ...
  ..
  .
  "


login: aku
mimetype: text/plain
icomment:
  > I understand that the ".man" files were not meant for reading in the browser

  Ok, seems that I misunderstood you a bit.
  Apologies and thank you for your patience with me.

  Thank you for the screenshot as well. Comparing to the sources I can now see that the original writer of the package and documentation apparently failed to use doctools markup in various places, causing confused output.

  This will take me a bit to sort out, as I likely have to read the package code as well to understand what actually is getting documented.

  Luckily the severity of this is only minor. Still, a fix is definitely needed. Thank you for reporting.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Merged work done on branch
  <a href="timeline?r=aku-87e374b7e4-rest-docs">aku-87e374b7e4-rest-docs</a>.
  Revision [2af315d3c6].
  Pushed.
  Updated online documentation.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  We actually have a 'testsNeed' support command for the purpose, whose abort message is understood by the testsuite runner.

  And that command is actually used, but pre-empted by the explicit custom code.
  Fixed by removing the custom code.

  Revision [122c1abff7].
  Committed.
  Pushed.


login: aku
mimetype: text/plain
icomment:
  Also, this cannot be fully handled with only constraints.

  The code loading the package under test must be made conditional too, or the testsuite will abort in that code.

  Simply skipping the entire file via 'testsNeed' is easier.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Replace incomplete use of constraints with a proper 'testsNeed' clause/statement.

  Revision [f705e6446e].
  Committed.
  Pushed.


login: droll
mimetype: text/plain
icomment:
  The fix has been submitted. The 'tkcon' and 'tkcon.tcl' executables are searched without passing via 'csh'.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Miguel, did you see my latest comment in

      https://core.tcl.tk/tcllib/tktview?name=a753cade83

  ?

  Is that solved with the attached sources ?

  Regarding the tags ... Can you support both old-style and new-style tags ?

  Changing the name of a subcommand is possible, however it is also a visible change of the API. That requires a change of the major version number.

  That said, this is pretty much needed already, because your first change (see the a75 ticket I mentioned already, above) already did so, to the point of breaking the yaml parser (as we have it, you may have fixed revision).


login: anonymous
mimetype: text/plain
icomment:
  Hi Andreas,
  thanks for your answer. I didn't read your reply. 

  I have fixed all the problems. Now YAML is not broken and works with this new version of huddle. YAML passes all the tests. I attached all the files. 

  I will not change the tags. I will continue with these tags. But what tags do you think that are better for these other types: boolean, number, null?

  I chose "b" for boolean, "d" for number ( the first letter of double or digit) and "n" for null. Number and null starts with "n" and there was a conflict with the first letter. For this reason I had to choose among both words which one will have assigned the letter "n". Do you have any proposal of tags for these new types?


login: anonymous
mimetype: text/plain
icomment:
  Hi Andreas, finally I think that this modified version of YAML.tcl pass all the tests. Could you please confirm?

  There was a problem with ::huddle::wrap (I changed a little the signature and the behaviour) and with isContainer. New types needs this paramater indicated: isContainer. It is for indicating whether a type can contain other types.

  But I think that there is a bad design in YAML.tcl. It tries to emulate some kind of manual inheritance between types. I will consider this in the future.


login: anonymous
mimetype: text/plain
icomment:
  Any idea when the code attached will make it into an official release of tcllib?  The changes are required to properly format generate json structures.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  I believe I was waiting for some documentation of the new json code from you.
  That said, I have now merged the branch "huddle-a753cade83" into trunk. That means it will be in the next release of Tcllib.

  Merge revision [02ec11eeba21b1a5].

  If that is good enough I will close this bug.


login: anonymous
mimetype: text/plain
icomment:
  Sorry for my late reply. I am developing also firetcl:
  http://sourceforge.net/projects/firetcl/

  It cost me a little to write in english. For this reason I didn't write any manual. I am not fluent in english. Perhaps I will need some help developing the tutorial. Also I am not familiar with the doctools manpage format. Anyway I will try to make an effort. I will appreciate also a help writing the tutorial.

  I attach my last version of huddle.

  Miguel.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Processed latest huddle.zip.

  Committed with revision [5a516799b4].

  Huddle is bumped to v 0.2, given the changes to the visible API.

  This bug is closed.

  Further work on huddle should use a new bug.
  And must be based on the huddle 0.2 now in the trunk.
  Because that will make merging in the changes much easier.
  I had to again fix a number of things in the submitted sources I had fixed before.
  I will not fix them again.


login: aku
mimetype: text/plain
icomment:
  From the mail conversation

  >> This "/sys/devices" directory, is that a special filesystem on linux ?
  >> Or some other platform ?
  >
  > Yes, that's a special file system under Linux. It provides direct
  > hardware access via file I/O (https://en.wikipedia.org/wiki/Sysfs). It
  > also uses a lot of linking inside between drivers, devices, etc. So my
  > suspicion is that you get into an endless loop while following some of
  > these links.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Bisection delivers

    2014-11-27 05:44:34 [4dc136d4389178dc] BAD

    2014-11-27 01:10:32 [83a79da8d2d30a4e] NEXT

    2014-11-27 01:09:11 [e4cc97ecc8ec99c4] CURRENT

    2014-11-19 04:29:54 [d9d86cd9ffe11a23] GOOD

  The revs NEXT and CURRENT exhibit an unrelated bug which breaks the trial script.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  It is checkin [e4cc97ecc8ec99c4] which caused the change, modifying the handling of symlinks. Rev [83a79da8d2d30a4e] is unrelated (sha), and Rev [4dc136d4389178dc] fixes an issue in e4cc...

  Main change from e4cc is that symlinks which are not cycles are now followed. Except, in the heavily symlinked devices tree (ex: subsystem/) has cycles , it looks like, which are not detected, it seems.

  Files seen by find after the change are

    /sys/devices/virtual/tty/tty15/subsystem/tty14/power

    /sys/devices/virtual/tty/tty16/subsystem/tty14/power

  Both represent 
    /sys/devices/virtual/tty/tty14/power

  The "/sys/devices/virtual/tty/*/subsystem" path
  is a symlink up to the "/sys/devices/virtual/tty" directory

  On first glance this should be detected as a cycle to be ignored, and is not.


login: aku
mimetype: text/plain
icomment:
  Ok. I now understand the issue. To wit:

  - The intentioal change e4cc was to move fileutil::find
    (and fileutil::traverse) from a broken regime for
    handling symlinks to one which would handle them
    correctly, allowing find to folllow all but true cyclic
    structures.

    After the change find will enumerate all legal paths
    from underneath the base directory, which are not
    cyclic. Before the use of inodes meant that with
    symlinks present some paths would enumerated, their
    aliases would not be, and it would be effectively
    random wether you would get a path through symlinks, or
    without.

  - The /sys hierarchy, of which /sys/devices is a part of
    is a pathological case for this, due to its extensive
    cross-linking. As a small example just consider the
    tty's. They exist as hierarchy under
      /sys/devices/.../tty/ttyX,
    with each device linking over (via subsystem/) to the
    second hierarchy under /sys/class/tty, which then again
    links back to the tty directories under /sys/devices.
    The end result of such a structure is that we have a
    number of legal paths which are O(n**2) in the number
    of tty's. For the whole of the actual /sys/ tree square
    in the numnber of devices, of which we have a few
    hundred, giving us millions of paths.

  - The original broken regime caused a quick cut-off of
    the links, with the majority not followed, bringing
    things down do roughly O(n) again, if only by accident.

  - Now 'find' is actually traversing the entire millions
    of paths. That alone will be slow. Then there is the
    memory overhead of the entire fifo of paths waiting for
    traversal kept in memory. It actually never shrinks
    until all paths have been traversed. The Tcl interp may
    well go into thrashing.

  I hope that this lets you understand what you were seeing.

  Now, solutions ...

  - First, no, I will not go back to the old scheme. It was
    definitely broken and non-deterministic in what paths
    of the hierarchy you would see and not. Very bad. (I
    noticed originally with my personal photo collection
    which is moderately symlinked for category aliasing
    when paths I knew existed were not listed in the output)

  - Still, bad news and good news ...

    - Bad: "fileutil::find" currently has no wait out, as
      you can specify only a basic path filter, but not a
      directory filter.

    - Good: "fileutil::traverse" (in the package of the
      same name) does have a -prefilter option which allows
      us to cut all symlinks from traversal. Example code:

      ===
      package require fileutil::traverse

      proc IsIdVendorFile {fileName} {
          return [string equal idVendor \
              [file tail $fileName]]
      }

      proc NoLinks {fileName} {
          if {[string equal [file type $fileName] link]} {
              return 0
          }
          return 1
      }

      fileutil::traverse T /sys/devices \
          -prefilter NoLinks \
          -filter IsIdVendorFile
      T foreach p {
          puts $p
      }
      T destroy
      ===

      fileutil::traverse is actually better internally as
      well, treating the list of pending paths as an actual
      stack (find treats it as fifo), reducing memory
      overhead. And with "foreach" it also returns any
      found paths much quicker (incrementally), whereas
      "find" accumulates them before returning all of them
      at once.

      Because of this last (incremental return) it is
      instructive to disable the if command in NoLinks and
      see the effect on the set of returned paths.

  Actions/Todo:

  - Extend the documentation of "find" to note this
    pathological case and the effect of the change from
    version 1.14.9 to 1.14.10. Refer user to
    "fileutil::traverse", and -prefilter.

  - Ditto the same for "fileutil::traverse" (v0.4.4 to
    v0.4.5), except that it does not need a reference to
    itself.

  - Add test cases for "find" and "traverse" which
    demonstrate the basic cross-link structure
    and its effect on traversal and number of paths.

  - No code changes needed, although it might be worth to
    consider extending the "find" api with options, and
    implement a -prefilter.


login: anonymous
username: aspect
mimetype: text/plain
icomment:
  I don't believe this is a bug, as $data is not correctly as a json object:

  % json::json2dict "{ $data }"

  returns the expected response, with keys {message number people}.

  without {}, json2dict parses the first quoted string as a json value and ignores the rest of its input.  This probably ought to be documented.


login: arjenmarkus
mimetype: text/plain
icomment:
  Either documented or there should be an error message stating that it is not a well-formed JSON string. I am not familiar enough with JSON, so I am pretty ignorant about the proper syntax. Also the fact that JSON requires {} _as a part of the string_ is a bit confusing to me, used as I am to Tcl that uses {} as delimiters.


login: anonymous
username: aspect
mimetype: text/plain
icomment:
  JSON syntax is described at http://json.org/ - {} denote an object (dict), [] an array (list) and so forth.

  With the caveat that I don't know how to load the critcl implementation of json on my system, so am unable to test it ... (aku's input would help here!)

  I suggest the following change to modules/json/json.man, which seems to accurately reflect the module's (Tcl implementation) behaviour.


  A "-strict" option which throws an error if the complete input is not consumed seems like it might also be a good idea, but that's a non-trivial change from my position (quite unfamiliar with the module myself!).  Since most users will be consuming a complete JSON object (delimited with {}, so an incomplete/truncated input will error), this is probably not a high-priority change.


  --- json.man    2014-10-08 00:33:09.154819953 +1100
  +++ json.man    2015-05-21 14:33:55.475753629 +1000
  @@ -33,8 +33,8 @@
   Parse JSON formatted text [arg txt] into a Tcl dict and return
   the value.
   
  -[para] If [arg txt] contains more than one JSON entity only the
  -first one is returned.
  +[para] The first complete JSON entity in [arg txt] will be returned, and any
  +trailing text ignored.
   
   
   [call [cmd ::json::many-json2dict] [arg txt] [opt [arg max]]]
  @@ -47,6 +47,9 @@
   limits. A value of "[arg max] == 0" does not make sense and will cause the
   command to throw an error.
   
  +[para] Extra text at the end of [arg txt] which is not formatted as correct
  +JSON will be ignored.
  +
   [list_end]
   [para]


login: hypnotoad
username: hypnotoad
mimetype: text/html
icomment:
  Kevin,
  <p>
  Returning the exit code of a spawned process would require plumbing down on the C level. And non-portable plumbing at that.
  <p>
  Though I agree, it would be really nice.
  <p>
  --Sean "The Hypnotoad" Woods


login: anonymous
username: aspect
mimetype: text/plain
icomment:
  Using [open |] instead of [exec &] would make the child's
  exit status available in the options dictionary at [close]
  time.  Would such a change impede any functionality
  present in this package?


login: hypnotoad
username: hypnotoad
mimetype: text/plain
icomment:
  Well the processman was more intended for "fire and forget" sort of applications. Specifically it is used in the IRM to launch editor instances as well as external programs such as our Fortran based fire model. "Why it died" wasn't as important a question to ask as "Is it still running".

  My gut worry is that the overhead of maintaining a channel is a much higher investment than simply forking a process and checking in on it occasionally via the process table. And as I use it in production already, I don't *really* want to much with that part of it works.

  Let's compromise and build a new method called "launch" (as opposed to "spawn"). A launched application will use the [open |] mechanism, and you would be able to track it via [close]. We just need to provide some connective tissue to allow the rest of the processman API to know how to treat the two varieties.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  This ticket is referenced by the new ticket [c247ed5db4].
  A fix for that ticket was committed, see [8b8f10e2bb],
  on branch [fix-ldap-partial].

  Please check if that fixes your issue as well.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Started investigation branch <a href="timeline?r=tkt-4a4e443ce9-pt-oo-fail">tkt-4a4e443ce9-pt-oo-fail</a>.

  Starting at commit [9034b39a70].
  Placed test grammar into the testsuite.
  Incomplete, most of the files for expected results are not present yet, and those that are will very likely contain the wrong things.

  But it should allow us to start the investigation, especially look at the different implementations for the same grammar. Differential debugging.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Testsuite extension complete, see [5c8a247da3], and confirming the report.
  Further, snit is also affected, but the critcl-based parsers are not.

  evilotto reminded me on the chat of commit [04dad6d262d80b4f] which fixed a similar error. A review of the change however show that this was about the 'error stack', while this problem seems to occur with the 'location stack'. Still, something similar to that is apparently lurking in the RDE engine implementations in snit and TclOO.


login: aku
mimetype: text/plain
icomment:
  Comparing the generated parsers for snit and critcl it is found that they have identical structure, down to the instructions they execute.

  This is another indicator that the implementation of some instruction in snit/oo is at fault. The relevant instructions used by the two parser will now be audited, i.e. compared with each other for functional differences.


login: aku
mimetype: text/plain
icomment:
  The instructions seem to handle the location stack the same across the board.

  Reviewing the failure location again I note that this code is run when more than one AST is on the stack after parsing.

  Now considering that the handling of the AST stack differs between the implementations. Back to auditing the instructions.


login: aku
mimetype: text/plain
icomment:
  > Making the X production a void: eliminates the error
  > (but changes the result of course). 

  Further thought. The X production is used in regular and void contexts
  (X? in ex is regular, !X in ex is void).
  Maybe this is translated wrongly ?

  In the !X context the X generates an AST and the !-handling should drop that because it must not put anything into the result AST.

  If that is not done we have bogus AST elements on the stack.
  Would also explain why void: would prevent the error. X would be translated slightly differently so that it cannot put an AST on the AST stack, and any bogosity in the !-handler would have no effect.


login: anonymous
mimetype: text/plain
icomment:
  The ! method of peg_to_tclparam is generating
  si:value_notahead_start
  call X
  si:void_notahead_exit

  It seems like the latter call does not pop the ast value that should be discarded, and it should be value_notahead_exit.


login: aku
mimetype: text/plain
icomment:
  Yes, I agree. This is also the difference to the C-code generator which gets it right, which is is why critcl works fine.


login: aku
mimetype: text/plain
icomment:
  Fixed.
  Commit [7b5c2a0853].
  Pushed.
  Merged as [06fb32c333].
  Pushed.

  Package pt::peg::to::tclparam modified, version bumped to 1.0.3.


login: aku
mimetype: text/plain
icomment:
  We do not seem to have a patch for this.
  Could this be a core issue with (intra-process) sockets on FreeBSD ?
  Or the Tcl core eventloop.
  Looks to require more instrumentation in the testsuite to see what is going on on Linux, and then compare with FreeBSD.


login: aku
mimetype: text/plain
icomment:
  Forgot to ask, what do you consider to be "the needful".
  This is a term made to mean 3 different things to any 2 people.


login: mi
mimetype: text/plain
icomment:
  "Needful" is whatever is needed to fix the problem. The current stance, that "it is not a problem because it works on Ubuntu" seems unsustainable.


login: anonymous
username: aspect
mimetype: text/plain
icomment:
  I've investigated this a little, and come to the conclusion that the test failure is benign:  as [http://paste.tclers.tk/3523] illustrates and [http://www.unixguide.net/network/socketfaq/2.11.shtml] explains, the assumption that puts will fail on a blocking, unbuffered socket whose remote has closed is not valid.  It seems to be mostly true on Linux, and often false on FreeBSD.

  I don't know how best to alter multiplexer-5.2 such that it accurately tests what it claims to.


login: mi
mimetype: text/html
icomment:
  <blockquote>I don't know how best to alter multiplexer-5.2 such that it accurately tests what it claims to.</blockquote>

  <p align="justify">How about writing a longer text and/or making two writes with an interval between them, that's longer than 0.2 second? Even if the first write succeeds because of Nagle's algorithm, the second one ought to fail...

  <p align="justify">Of course, it would've been best, if Tcl allowed manipulating the socket's parameters (such as setting the <tt>TCP_NDELAY</tt>).


login: aku
mimetype: text/plain
icomment:
    > multiplexer-5.2 such that it accurately tests what it claims to.

  As a non-maintainer/non-author, what does multiplexer-5.2 claim to test ?

  For the record, having read both example and reference now I agree with aspect that the test is apparently sensitive to OS differences in the TCP stack. I further agree with mi that using two puts more than 200 millis apart might be enough to overcome Nagle. aspect, could you test this for us ?


login: anonymous
username: aspect
mimetype: text/html
icomment:
  multiplexer-5.2 ensures that an access filter can deny (immediately close) inbound connections correctly, by checking that the first write from the client fails.

  Adding a second write after 200ms seems to do the right thing:

  <pre>
  $ uname -r
  10.1-RELEASE-p6
  $ ~/bin/tclkit multiplexer.test
  - tcllib::testutils 1.2
  * logger 0.9.4
  * multiplexer 0.2
  multiplexer.test:       Total   9       Passed  9       Skipped 0       Failed  0
  </pre>

  Thanks for the suggestion - I wasn't thinking clearly about Nagle and thought a delay before the first write should be sufficient.

  Patch inline below:

  <pre>
  Index: modules/multiplexer/multiplexer.test
  ==================================================================
  --- modules/multiplexer/multiplexer.test
  +++ modules/multiplexer/multiplexer.test
  @@ -193,22 +193,26 @@
       set ::forever {}
       set mp [multiplexer::create]
       ${mp}::Init 37465
       ${mp}::AddAccessFilter DenyAccessFilter
       set sk1 [socket localhost 37465]
  -    set sk2 [socket localhost 37465]
  -    update
  -    fconfigure $sk1 -buffering none
  -    if { [catch {
  -	puts $sk1 "boom"
  -    } err] } {
  -	set result "socket blocked"
  -    } else {
  -	set result "socket not blocked"
  +    after idle {
  +	update
  +	fconfigure $sk1 -buffering none
  +	if { [catch {
  +	    puts $sk1 "boom"
  +	    after 200	;# delay to overcome nagle - see ticket [ced089d5fe]
  +	    puts $sk1 "tish"
  +	} err] } {
  +	    set ::forever "socket blocked"
  +	} else {
  +	    set ::forever "socket not blocked"
  +	}
       }
  +    vwait ::forever
       ${mp}::destroy
  -    set result
  +    set forever
   } {socket blocked}
   
   
   testsuiteCleanup
   return
  </pre>


login: aku
mimetype: text/plain
icomment:
  Patch applied, into branch "tkt-ced089d5fe-multiplexer".
  Commit [9bfb503d18].
  Pushed.

  Mikhail, can you confirm that this is a fix for the testsuite in your FreeBSD environs ?


login: aku
mimetype: text/plain
icomment:
  First glance review ...

  The string literal setup is broken with respected to multi-threaded Tcl core.
  Tcl_Obj* are wedded to the thread T they are created in. Only T is allowed to 
  to release it, *and* manage the refcount. While the former will never happen the latter (refcount changes) will, that is the point in having the literals shared.

  While a regular threaded core will not complain, expect by possibly crashing unexpectedly, a mem-debug build of a threaded core will generate panic on access to any Tcl_Obj* from a thread outside of its origin thread.

  That all said, this is fixable by making the array[] for the literals a member of a struct which will be allocated per-thread. We have examples of such things in the core itself.


login: mi
mimetype: text/html
icomment:
  <p>The constant objects are created and maintained in the static array inside the "staticobj" function. They aren't wedded to any particular Interpreter nor Thread.
  <p>They begin life with increased refcount and so will never be released. I think...
  <p>The only potential source of problem is when multiple threads try to parse JSON-text and all create a constant object of the same name. This will not cause a crash, but will leak some of the objects.
  <p>Perhaps, the object-creation inside staticobj-function should protected by a mutex, but I don't see a more dangerous problem than that.
  <p>Creating one such array of literals per thread would still be wasteful, though not as wasteful as the bazillion copies resulting from parsing even a moderately-sized JSON-blob. What do you think?


login: aku
mimetype: text/plain
icomment:
  Created branch from the patch, for further review and editing.
  See [<a href='timeline?r=tkt-785d2954d4-jsonc&unhide'>tkt-785d2954d4-jsonc</a>].

  The condition for USE_DICT looks to be a bit loose as well, triggering for Tcl 7.5+, not just 8.5+. And there will be fools who will try.


login: aku
mimetype: text/plain
icomment:
  Commit [fe05de3454].
  - Thread-safe static Tcl_Obj*
  - Stricter dict check
  - Fixed bad data in the numbers.* test due to bad download of patch from my side.

  This passes the json testsuite for me. Please conform for your environs.

  > They aren't wedded to any particular Interpreter nor Thread. 

  Incorrect. Any Tcl_Obj* O allocated (via Tcl_NewObj() and variants) is _implicitly_ (and automatically) tied to the thread T doing that allocation. Access and manipulation of O by some thread T' != T can cause crash and burn of the Tcl core. This is a property of the Tcl core itself.


login: mi
mimetype: text/html
icomment:
  <blockquote>Access and manipulation of O by some thread T' != T can cause crash and burn of the Tcl core.</blockquote>I do not see, how this is possible -- as long as the general rules (respect of <tt>refCount</tt>) are obeyed. And where they aren't, the crash is likely with or without threads...
  <p>Could you describe a hypothetical scenario, where such a crash might occur because of the change I am proposing?
  <blockquote>This is a property of the Tcl core itself.</blockquote>Could you point at documentation or some other reference about this? I don't see any mention of threads in <tt>Tcl_NewObj(3)</tt>...
  <p>I find it hard to distinguish your above statement from something like "Tcl-objects can not be shared between threads," -- which is, of course, not true. If that's not, what you meant, could you explain the difference?


login: mi
mimetype: text/plain
icomment:
  Could you provide a link to the diff, please? So I don't have to wait for the next release here :-)

  Thank you!


login: aku
mimetype: text/x-fossil-wiki
icomment:
  The current implementation of "gEFC" does not contain a "close" command at all.
  Nor do the called procedures (_scan, _exif, and indirectly _format).
  See <a href='http://core.tcl.tk/tcllib/artifact/e6a82ddcda6556f4?ln=370-412'>
  jpeg.tcl e6a82ddcda6556f4 ln=370-412</a>

  Please further note commit [498c380fbbb8bedf] where this issue was fixed in 2013.

  Please re-check that you are testing the correct checkout/revision of Tcllib.


login: mi
mimetype: text/html
icomment:
  <blockquote>The current implementation of "gEFC" does not contain a "close" command at all. Nor do the called procedures (_scan, _exif, and indirectly _format).</blockquote>
  The <tt>close</tt> is invoked by <tt>::jpeg::getExifFromChannel</tt>. The file modules/jpeg/jpeg.tcl, dated April 30, 2015, contains the following line:
  <blockquote><tt>if {$next <= 0} { close $chan ; return }</tt></blockquote>
  When an invalid file is used (as the two tests deliberately do), the $chan will be closed upon the procedure's return.
  <blockquote>Please re-check that you are testing the correct checkout/revision of Tcllib.</blockquote>
  Confirming. The above-quoted test-failure is still present in tcllib-1.17. Please, confirm having run full collection of JPEG-tests without a problem.


login: aku
mimetype: text/plain
icomment:
  Local session inspecting Tcllib release 1.17, from the ground up.

  andreask@tcl-linux-vm:~> pwd
  /home/andreask

  andreask@tcl-linux-vm:~> mkdir T
  andreask@tcl-linux-vm:~> cd T

  andreask@tcl-linux-vm:~/T> fossil clone https://core.tcl.tk/tcllib/home t.fossil
  Round-trips: 6   Artifacts sent: 0  received: 43825
  Clone finished with 1471 bytes sent, 29953160 bytes received
  Rebuilding repository meta-data...
    100.0% complete...
  project-id: 05391902863193f286cc4eeb0b0cd8a8d16b4ec9
  admin-user: andreask (password is [...])

  andreask@tcl-linux-vm:~/T> mkdir dev
  andreask@tcl-linux-vm:~/T> cd dev
  andreask@tcl-linux-vm:~/T/dev> fossil open ../t.fossil
  [...]

  andreask@tcl-linux-vm:~/T/dev> fossil update release
  [...]
  -------------------------------------------------------------------------------
  updated-to:   66ed0de3b320c65ab755a123a89da4b2be101782 2015-04-30 04:21:20 UTC
  leaf:         no
  tags:         trunk, release, tcllib-1-17
  comment:      Tcllib 1.17 Release (user: aku)
  changes:      35 files modified.

  andreask@tcl-linux-vm:~/T/dev> cat modules/jpeg/jpeg.tcl | grep -n next | grep if
  402:            # offset of next exif block (for thumbnail)
  404:            if {$next <= 0} { return }

  andreask@tcl-linux-vm:~/T/dev> lines 403 406 modules/jpeg/jpeg.tcl
              _scan $byteOrder [read $chan 4] i next
              if {$next <= 0} { return }
              # but its relative to start
              seek $chan [expr {$start + $next}] start

  andreask@tcl-linux-vm:~/T/dev> md5sum modules/jpeg/jpeg.tcl
  584f7193f2617d14803f8b5518e81083  modules/jpeg/jpeg.tcl

  I am unable to confirm that jpeg.tcl contains 

      if {$next <= 0} { close $chan ; return }

  For me it does contain

      if {$next <= 0} { return }

  on line 404 of the file.
  The 'lines' command in the session above is a small Tcl script I wrote, it is defined as

  =====
  #!/usr/bin/env tclsh
  package require Tcl 8.5
  set argv [lassign $argv from to]

  foreach f $argv {
      set c [open $f r]
      set n 0
      while {[gets $c line] >= 0} {
          incr n
          if {$n < $from} continue
          if {$n > $to}   continue
          puts  stdout $line
          flush stdout
      }
      close $c
  }
  exit
  =====

  Please walk me through how you retrieved the 1.17 release.
  Something looks to be very wrong in your environment.


login: mi
mimetype: text/plain
icomment:
  I'm looking into:

  https://codeload.github.com/tcltk/tcllib/tar.gz/tcllib_1_17?dummy=/tcltk-tcllib-1.17-tcllib_1_17_GH0.tar.gz (MD5 081ec241c1a9c60730b8c6e80b7ff066).

  The jpeg.tcl inside it has a timestamp of April 30, MD5 of 020552c3698e5afa5444d9be2c5e31f7

  and its line 404 has "close $fh" in it.

  Could it be, the fix was put in after 1.17 was released (and tagged)?


login: aku
mimetype: text/plain
icomment:
  > Could it be, the fix was put in after 1.17 was released (and tagged)?

  Note the following block from my session. I checked _exactly_ the fossil revision which was marked as the tcllib-1-17 release on April 30.

  > andreask@tcl-linux-vm:~/T/dev> fossil update release
  > [...]
  > updated-to:   66ed0de3b320c65ab755a123a89da4b2be101782 2015-04-30 04:21:20 UTC
  > leaf:         no
  > tags:         trunk, release, tcllib-1-17
  > comment:      Tcllib 1.17 Release (user: aku)


  A github download ... Ok. That gives me something to check.
  Namely if my link to github is correct.


login: aku
mimetype: text/plain
icomment:
  The links I put into the 1.17 release technote look ok to me.
  What is coming back from github is definitely not correct.

  ... The main difference seems to be in the embedded docs, where I see old names in use. The only other difference is then this, "jpeg.tcl".

  I fear I have to audit the github mirror, it seems as if something is borken with it.

  :(


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Applied.
  Commit [2b866cf322].
  Pushed.
  Testsuite extended using the script output as demo-file.
  Version bumped to 1.14.11

  Should check what the fumagic packages report.
  They are more like Unix file(1).


login: aku
mimetype: text/plain
icomment:
  Mikhail, please recheck.

  This looks to be another instance of the broken git export we had, like the jpeg problem.

  Looking at the current iso8601.test file I see that the test -1.1 was split into two cases, -1.1.0 for Tcl 8.5.x, and -1.1.1 for Tcl 8.6+, with the proper text, i.e. 'option' vs. 'switch', put into the expected result.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Applied patch into branch
  [<a href='timeline?r=tkt-5613c718c2-cwarnings&unhide'>tkt-5613c718c2-cwarnings</a>].

  Missing the 'return' in "..._lstop". How embarrassing.

  Regarding C99 in 2015, I will check what the native cc's of AIX, HPUX, and Solaris (2.8, 2.10) will say about the patched code. I suspect that at least one of these will choke on the size_t and intptr_t types, being more C89 than C99.


login: mi
mimetype: text/html
icomment:
  <tt>size_t</tt> should not a problem even on the ancient computers. For <tt>intptr_t</tt>, maybe, there is some kind of Tcl_Foo-equivalent?


login: aku
mimetype: text/plain
icomment:
  I suspect a bug in the git export functionality of fossil.

  Still gearing myself up to confirm this. I.e. make a shell script doing just the fossil and git commands "fx" runs, as a separate testable thing I can then use for bisection on fossil.


login: mi
mimetype: text/plain
icomment:
  Maybe, the ultimate solution is to go "back" to hosting the actual release tarballs statically -- rather than rely on GH to automatically (re)generate them upon request?


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Static tarballs are available on SF, in the tcllib files sections.
  For all releases, as part of the release.

  Link is in the
  <a href="technote/56416fe9cf1b0c0f5cd535861225a56af8228999">release technote</a>.


login: aku
mimetype: text/plain
icomment:
  I believe that we should be ok now for Tcllib.

  After quite a few attempts at reproducing the problem I finally realized/determined that the tcllib.fossil file on core needed to be rebuild. Apparently some fossil schema change influenced the git exporter causing this drop of files and dirs.

  I will talk with drh about that to get more details, and then go and refresh all the mirrored repositories, i.e. tklib, tcl, tk, etc. And my personal repos too, of course.

  > Why do you even bother mirroring at GitHub?

  More exposure to a larger group of people, and to those where 'is on github' is a check-mark a project has to tick to be considered.


login: mi
mimetype: text/plain
icomment:
  > those where 'is on github' is a check-mark a project has to
  > tick to be considered.

  The fad-chasing will be the end of you... Are you going to mirror on Facebook and Instagram next? (I'll stop now, I promise!)


login: aku
mimetype: text/plain
icomment:
  Can you confirm that the mirror is now delivering the proper files ?
  Just checked the 1.17 tag and it looks good to me. No differences.


login: aku
mimetype: text/plain
icomment:
  Looking closer I see now that there is text mentioned what constants are there. However it only mentions some, not all, and it does not mention the exact symbolic names in use at all.

  So, the issue is valid even so.


login: hypnotoad
mimetype: text/x-fossil-wiki
icomment:
  Thank you for finding that. The implementation that processman was derived from was originally part of a larger system called odie. All of the other dependencies were sanded out. 

  I have renamed the package as of Checkin [1b6fa7e91f]


login: hypnotoad
username: hypnotoad
mimetype: text/plain
icomment:
  Guilty as charged.

  I will have to see if I can get the same information directly from /proc, or write a hunting algorithm to find the real path to ifconfig. 

  --Hypnotoad


login: aku
mimetype: text/plain
icomment:
  hypnotoad - Are you using auto_execok to find "ifconfig" ?
  If yes we may only have to temporarily extend PATH with a set of common locations for "ifconfig", including /sbin, and the auto_execok does the rest in searching the PATH.


login: anonymous
username: Sergei Golovan
mimetype: text/plain
icomment:
  The same applies to [exec arp -a] in modules/nettool/platform_unix.tcl. In Debian (at least) arp is located in /usr/sbin, which isn't in PATH for non-root.


login: aku
mimetype: text/plain
icomment:
  Accepted with revision [5950a59fa5].

  Still have to convert the session into a proper example I can use in the documentation.

  Version bumped to 0.5.3.

  Thanks for the report and fix.


login: aku
mimetype: text/plain
icomment:
  I am not sure if that is something I can accept.

  Problem being that Tcl core support for surrogates is non-existent as far as I am aware, or quite limited.


login: aku
mimetype: text/plain
icomment:
  Talked with Kevin re surrogate support.
  Accepting the patch does not make us worse off than Java.
  So, schedule for integration.

  Wait, there is no patch here ?
  Where exactly would I found these entity definitions ?
  (I very much would not like to typing these in by myself)


login: anonymous
mimetype: text/plain
icomment:
  Use the attachment gen51entities.tcl to make it from the info grabbed @ http://www.w3.org/html/wg/drafts/html/master/entities.json


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Committed with revision [d646f17b12].

  Version bumped to 1.2.2.

  Added a test to the testsuite.
  Good thing too. Caught that mapEscapes has a limit on the length of acceptable entities (2-7 chars). This is now (2-31) chars based on the max length of names in the new set.

  Thanks to David for report and data.


login: anonymous
mimetype: text/plain
icomment:
  Unable to replicate here .. need more info on your environment:

  % info pa
  8.6.4
  % package require crc16
  1.1.2
  % set crc 0
  0
  % set data 11520
  11520
  % set crc [::crc::crc-ccitt -seed $crc [binary format S1 [expr $data]]]
  28858

  This looks like it was fixed prior to 1.1.2:

  http://core.tcl.tk/tcllib/fdiff?v1=35f521cf41871745&v2=def37b0cc020e30d&sbs=1

  If you're seeing this on 1.1.2, that's very strange!

  If you need to ensure backward compatibility with 1.1.1, use "--":

  % package require crc16
  1.1.1
  % set crc [::crc::crc-ccitt -seed $crc -- [binary format S1 [expr $data]]]
  28858


login: anonymous
mimetype: text/plain
icomment:
  We got two TCL versions:

  - One that is compiled at Questa Sim-64 10.2c (TCL version 8.5.13) that reports the errors detailled at this message. It comes with the CRC16 v1.1.1 package. When I make the test, its seems that I put a wrong auto_path and it used the default versión of the program, sorry for any confusión caused by my fault.
  - We also used ActiveState ActiveTcl 8.5.18.0.298892. This versión comes with the CRC16 v1.1.2. With this program, the first error is not reported (It seems to be fixed) but the second error persist.

  I test with the flag "--" and it works in both versions (I have tested all posible input data from 0 to 65535 and no errors are reported)

  Thanks for your early response


login: patthoyts
username: patthoyts
mimetype: text/plain
icomment:
  This is not a bug in the crc16 package but in the documentation. It is important to use the option termination flag (--) to indicate the end of option processing so that the input data is not parsed as further options.

  The documentation and examples have been modified to emphasise this. The correct form of the command should have been:

      set crc [::crc::crc-ccitt -seed $crc -- [binary format S1 [expr $data]]]


login: aku
mimetype: text/plain
icomment:
  The attached patch is empty.
  Please attach the data. If that still fails please send the patch to akupries at shaw dot ca


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Weird. Here from Linux the patch is ok and usable.
  The previous comment was done from an OSX laptop.

  I can confirm that the new tests fails before the application of the code change, and are ok after. I also confirm that the change does not break any existing tests.

  Accepted patch, with thanks.

  Committed in revision [7e8e8838d6].
  Package version bumped to 1.2.6.


login: aku
mimetype: text/plain
icomment:
  Fix is to go to char classes + collating elements, i.e. [[.$x.]] ... In Tcl then \[\[.$x.]].

  Question now is, since what Tcl version did we have support for that re-syntax ?
  Hm, the package already requires 8.4+.
  I think that is ok, i.e already supports the above re-syntax.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fixed with revision [79c8ed1a87].

  Committed.

  Version bumped to 0.8.1

  Thank you for the report.


login: hypnotoad
username: hypnotoad
mimetype: text/plain
icomment:
  I’ve reviewed the code and it would make a worthy addition to Tcllib. 

  If you are looking for a good place to try out your integration, create a branch on 

  http://fossil.etoyoc.com/sandbox/tcllib

  The “sandbox” is a fossil repository we have set up with self-registration and global admin access. We push changes from the main fossil repo every night, but we never pull. The cool part is that you and I can both can collaborate using that snapshot without the traffic going across to Tcllib itself. When it’s ready to go, I can move the code from one checkout on my machine to a seperate checkout I use for production. But all the while fossil will be tracking our changes.

  I won’t get to it today, but my plan for integration is as follows:
  1) Under /app place the bootstrap program which starts the server
  2) Place the demostrations in the /example file system
  3) Re-useable code will go in modules/firetcl
  4) For now… I would place the javascript in a subfolder in modules/firetcl, and expose that directory as the /js url domain on the server. We may do something slightly different later if we decide to integrate bootstrap into tcllib.
  5) Write up some documentation in Tcllib’s internal format (.man file)

  If you see a firetcl branch by the time you read this, I’ve probaby gotten a start on it. If you don’t, feel free to give it a shot.

  You can reach me at hypnotoad@etoyoc.com.

  —Sean “The Hypnotoad” Woods


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fixed with revision [1735e41b6f].

  Version bumped to 0.5.2.

  Committed.
  Pushed.

  Thank you for the report and fix.


login: aku
mimetype: text/plain
icomment:
  While [info frame] goes to great lengths to get correct location information, the presence of "eval" makes it impossible to get such under all circumstances.

  And for the case here, where the location is not even asked for, but just the command (name) itself ?! It should have never been used from the beginning, IMHO.

  Tcl has [info level] for that.
  [info level 0] gives the currently running command.
  [info level -1] the caller.

  Limitation: [info level -1] may not exist.
  I.e. when rest::simple is called directly, from the top-level of a script, i.e. level #0.
  That can be worked around however.


login: aku
mimetype: text/plain
icomment:
  Sigh. Some crow for me to eat.

  [info level 0] shows the command with aliases resolved. At least for Tcl 8.6.4 I have here.
  So, use of [info frame] seems to be required. I believe I will test this a bit more, because IIRC some older versions actually showed the alias in [info level 0], not resolved. 

  Back to [info frame] ...

  The 'cmd' key exposes the processing done by the interpreter on the command, i.e. the while command sees its up-body with the continuation lines resolved, and so the "put ..." is a proper list.

  With the top-level "put ..." there was no such additional processing before things went to parsing & execution. You see the raw command as found in the sources. IOW if the command is dynamic, through a variable, the 'cmd' key will show something like '$cmd A B', assuming 'set cmd put'.

  At the moment I do not believe that we can get the name of the command it was invoked with (i.e. aliases) in a sure way. ... rest::simple might be doomed.


login: aku
mimetype: text/plain
icomment:
  Trialed 8.5.8 and 8.4.19. Both resolve the alias causing [info level 0] show the definition name.
  If it ever was different it was really way back.

  So, [info level 0] is not usable, contrary to my thoughts. :(

  So, best bet is to work around the issues with [info frame] as much as possible.
  That looks to be quite a bit of work.
  Better do it on a branch.


login: anonymous
username: heinrichmartin
mimetype: text/plain
icomment:
  Well, rest::simple is not doomed; just the aliases ...

  You could merge the config dict with the correct method and tailcall simple instead. (Error out if simple is called without method. Optionally, put the legacy code in a try-block to be backward-compatible.)


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Ok, first attempt at this.

  Revision [8d718b4d0d].

  Branch [rest-817f51002f]

  Please test if that is working for you.


login: anonymous
username: heinrichmartin
mimetype: text/plain
icomment:
  Works for me: my case is #3 from the description. The rest was only found while debugging ...

  Code review for the patch:
  - String match \$* is not enough. I'd consider a hard-coded white-list, i.e. if {$cmd ni $::rest::methods} {return -code error}
  - remove commented code #puts

  HTH


login: aku
mimetype: text/x-fossil-wiki
icomment:
  We have a whitelist in revision [e10a3b6c0fddaec4].
  This is on branch [rest-817f51002f].
  Please test if that is working for you. 

  Note! This revision also contains a fix for ticket [895baa109a] - Merge config content-type into request headers.


login: anonymous
username: heinrichmartin
mimetype: text/plain
icomment:
  Just did a code review. Looks good and the diff is simple (no pun intended) enough to go ahead, merge, and close this ticket; I already tested the previous major changes :-)


login: aku
mimetype: text/plain
icomment:
  Hi Martin.

  Do you have a basic idea where in the rest package the "content-type" should be used ? (I am not the regular maintainer and this is my first time looking into this package (after the issue with rest::simple)).


login: anonymous
username: heinrichmartin
mimetype: text/plain
icomment:
  Hi Andreas,
  I had an error response from the Jira REST API and though that the wrong content type might be the cause. It was not, but content-type is documented - so it should forward it to the http package.
  As for the code, config keys auth, headers, and cookie are merged into headers in simple at rest.tcl:57~68.
  Regards,
  Martin


login: aku
mimetype: text/plain
icomment:
  The section is currently provided by a generic text block.

  I will have a look and see if that can be made more package specific.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fixed in revision [04e098e60c].

  Added a section explaining basic TLS integration immediately before the security considerations (SC). Together they should explain all that is needed, and I won't have to think about templating the SC.


login: hypnotoad
username: hypnotoad
mimetype: text/plain
icomment:
  Well that’s no fun at all. I’ll inject a mutex to at least throw an error if someone attempts that in the future.

  Thanks for catching it!

  —Sean


login: hypnotoad
username: hypnotoad
mimetype: text/plain
icomment:
  I have fixed the bug in checkin [c4c84d18547f70193d0cfc9ff434aebf20d1300c].

  The trick was to add a lock variable that is true while periodic task scripts are actively processed. When
  true, this flag turns cron::wake into a noop. (cron::wake is intended to immediately rouse the system
  when a new task hits the manager.)

  Now when I run the test script I get:

  =========================================================
  baccarat:cron seandeelywoods$ tclsh inf.tcl 
  10 second timer triggered!
  5 seconds triggered!
  =========================================================


login: aku
mimetype: text/plain
icomment:
  Jeff, with apologies for the late-ness.
  The attached file is reported to me as "0 bytes of binary data", IOW as empty.

  As such I am unable to confirm your report yet.

  Please attach again, or send it to me in mail, at akupries at shaw do ca


login: anonymous
mimetype: text/plain
icomment:
  Please see the public gist here for the attachment:
  https://gist.github.com/bovine/fccfb2b8c546d5f64581


login: aku
mimetype: text/plain
icomment:
  I can confirm the RSS growing for Linux as well, Tcl 8.5.14.
  It seems to be much slower however.
  And the TLS does not seem to be required for it either.
  (As a side note, I used sockspy as the proxy)

  I might have to run this through a memory-debug enhanced Tcl core.


login: aku
mimetype: text/plain
icomment:
  I just disabled autoproxy in your test script, and I see the memory still growing, somewhat.

  The modded script simply does http::geturl and http::cleanup in a loop.

  Can you check if you get similar results in your environment and setup as well ?

  I sort of noticed because when I instrumented the local autoproxy code it became clear that no proxy filtering was actually activated (I have no env variables to pick up, and the configure after init does nothing)

  With the above it seems that the leak is more likely in the http package itself.

  When I actually force a proxy by setting host:port as part of the autoproxy::init call, i.e.

      autoproxy::init localhost:8080

  then a handler is properly installed into http, namely "autoproxy::filter".

  That proc is very linear, just checking the provided host for exclusion and returning the proxy config. I cannot really see anything there which would leak memory.

  Given http involvement, mine is 2.7.11, coming with the 8.5.14

  One possibility which might involve autoproxy, would be that activating proxy handling in the http package triggers code branches in it which are more leaky than the code executed without proxies.


login: anonymous
mimetype: text/plain
icomment:
  Just using http::geturl and http::cleanup alone does not have any problem for us. We have many scripts that do that and run continuously for months that way without growing.

  It is only when we need to use autoproxy with HTTPS destinations does the memory leak become unacceptable and we need to make our script self-terminate and restart every few thousand requests to avoid impacting other users on this server.


login: aku
mimetype: text/plain
icomment:
  Hm, so weird ...

  Do you need both autoproxy and TLS for the leak ?
  I.e. do you have a leak just with autoproxy ?
  Or a leak just with TLS active ?
  (Trying to reduce the amount of components)

  Is this with the latest version of TLS ?
  And build against what version of OpenSSL ?

  If TLS is involved, well, sockspy is not good enough to be a proxy in that case, it does not handle TLS connections properly. In that case I would need some recommendations for some proper proxy app I could run locally.


login: anonymous
mimetype: text/plain
icomment:
  You can use Apache httpd with the mod_proxy to enable CONNECT style proxying that you need for HTTPS connections.

  Latest versions is not a requirement--we have have experienced it for more than 3 years over multiple combinations of tcl, tls, openssl, FreeBSD versions.


login: aku
mimetype: text/plain
icomment:
  Thank you. Ok, will have to search for HOWTO guides on apache/mod_proxy setup, my knowledge on these is zilch.

  That said, executive decision: I do not see me getting this investigated, not speaking of fixed, within 2 weeks, when I go on vacation till new year, and I wanted to make a Tcllib release before that.  I will defer this into '16, with the remaining time in this year for setup of the investigation. (Not just apache, valgrind as well, given that openssl is in the mix.). With apologies. (I am slower these days, it seems, in doing things)


login: resuna
mimetype: text/plain
icomment:
  I don't think the test code is actually demonstrating the leak. VSZ is not increasing, and RSS is increasing asymptotically which is typical of any long running program.


login: resuna
mimetype: text/plain
icomment:
  OK, I'm seeing something. I've extended Jeff's test code to track all the variables in all the namespaces. Occasionally, I'm seeing errors like this:

  can't read "state(sock)": no such element in array
      while executing
  "CloseSocket $state(sock) $token"
      (procedure "Finish" line 13)
      invoked from within
  "Finish $token"
      (procedure "http::reset" line 7)
      invoked from within
  "http::reset ::http::1101 timeout"
      ("after" script)
  can't read "state(sock)": no such variable
      while executing
  "eof $state(sock)"
      (procedure "http::Connect" line 6)
      invoked from within
  "http::Connect ::http::1101 https {} /“

  can't read "state(sock)": no such element in array
      while executing
  "CloseSocket $state(sock) $token"
      (procedure "Finish" line 13)
      invoked from within
  "Finish $token"
      (procedure "http::reset" line 7)
      invoked from within
  "http::reset ::http::1124 timeout"
      ("after" script)
  1124 can't read "state(sock)": no such variable
      while executing
  "eof $state(sock)"
      (procedure "http::Connect" line 6)
      invoked from within
  "http::Connect ::http::1124 https {} /“

  After these errors, the instrumented code shows that "[info vars]" says variables ::http::1101 and ::http::1124 exist, but [info exists] and [array exists] both fail for these variable names. I'll attach the instrumented code in a followup.


login: resuna
mimetype: text/plain
icomment:
  OK, I think I found the leak. The ghost variable is an HTTP connection token that hasn't bean cleaned up, probably because the cleanup code errored out on the unset sock array element.

  There's a timeout in the Tcl HTTP library, that's feeding the token to http::finish. I'm guessing that something has already started cleaning up the token and had closed and unset the socket.

  I don't know if the bug is in tcllib's autoproxy, or in tcl's http code ( https://www.tcl.tk/man/tcl8.6/TclCmd/http.htm )

  See tcl/library/http.tcl around lines 205 and 868.


login: aku
mimetype: text/plain
icomment:
  Resuna, Jeff.

  I have now run the bigscanner using `init1` and `init2`, i.e. with and without a proxy. The proxy was `mitmdump`, see link in previous comment.

  No leaks for me.
  However, I am also NOT seeing the `can't read "state(sock)": no such element in array` in the bigscanner output. IOW it seems that I am not managing to trigger whatever makes it happen.

  Packages:
  - http 2.8.11
  - Tcl 8.6.7
  - Tcllib 1.18, autoproxy 1.5.3
  - TclTLS 1.7.13

  I suspect that it is something in the http package, where some callbacks are crossing their wires, i.e. maybe the timeout callback not properly canceled, or a timeout not properly removing fileevent and then code tries to use bad state.

  I suspect that the autoproxy is only part of it in that it makes the mixup somehow more likely, maybe by perturbing event timing due to delays through the proxy.

  I have tried to track things through the code of http and, well, it is getting to complex to hold everything in my mind.

  To hopefully drive this forward I am now attached a copy of `http-2.8.11.tm` to this ticket which has _a lot more_ of calls to `http::Log`, to better trace the internal calls

  Define a non-empty implementation of http::Log, and we should see a nice dump of the activity. And hopefully see what exactly happens when the wires cross each other.


login: aku
mimetype: text/plain
icomment:
  Here is one thing I consider a bug in http, noticed when I aborted a run by killing the proxy:

  CloseSocket takes 2 arguments, socket to close, and optional token.
  Here it is called with the error message in place of the socket.
  The problem is line 657 in the modified package, equivalent to line 611 in the original.

  Interestingly enough, you cannot leave that field undefined. That causes Finish to crash when calling CloseSocket, because it assumes to have a socket. Setting it to the empty string works however.

  ---
  ::http::X1651: Finish ::http::X1651 {} 1 (#1s1c1)
  ::http::X1651: CloseSocket {couldn't open socket: connection refused} ::http::X1651 (#1s1c1)
  Closing socket couldn't open socket: connection refused (no connection info)
  Error: can not find channel named "couldn't open socket: connection refused"
  ::http::X1651: CloseSocket {couldn't open socket: connection refused} ::http::X1651 RETURN
  ::http::X1651: TIMEOUT cancel after#1650
  ::http::X1651: Finish ::http::X1651 {} 1 RETURN
  ---


login: aku
mimetype: text/plain
icomment:
  Looks to be a misunderstanding between the CollectClass and FuseClass procedures in pt_pexr_op.tcl. In the given case CC provides a string as buffer element, and FC always expects a list as element, even if that list contains only a single element. This is important when it comes to characters relevant to Tcl list quoting, i.e "\{".

  Yet changing that breaks about 16 test cases. More is going on.

  The breaking testcases are big grammars, unfortunately. Taking lots of time.
  So, first of the next steps is to translate them into cases for fusechars alone.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fixed with [0616504217].

  It was the issue in CollectClass as described in my previous comment, plus an inverse issue exposed by the fix in FuseClass itself. Changed both places is a full fix. Added test cases.

  Version bumped to 1.0.1


login: aku
mimetype: text/plain
icomment:
  Implementation, inlined into ticket

  ------------------------------------

  ##        Apply a patch in unified diff format
  # @synopsis{Patch directory striplevel patch}
  #
  #
  # @param[in] dir        root directory of the original source tree
  # @param[in] striplevel number of path elements to be removed from the diff header
  # @param[in] patch      output of diff -ru

  proc ApplyPatch {dir striplevel patch} {
          set patchlines [split $patch \n]
          set inhunk false
          set oldcode {}
          set newcode {}
          
          for {set lineidx 0} {$lineidx<[llength $patchlines]} {incr lineidx} {
                  set line [lindex $patchlines $lineidx]
                  if {[string match diff* $line]} {
                          # a diff block starts. Next two lines should be
                          # --- oldfile date time TZ
                          # +++ newfile date time TZ
                          incr lineidx
                          set in [lindex $patchlines $lineidx]
                          incr lineidx
                          set out [lindex $patchlines $lineidx]

                          if {![string match ---* $in] || ![string match +++* $out]} {
                                  puts $in
                                  puts $out
                                  return -code error "Patch not in unified diff format, line $lineidx $in $out"
                          }

                          # the quoting is compatible with list
                          lassign $in -> oldfile
                          lassign $out -> newfile

                          set fntopatch [file join $dir {*}[lrange [file split $newfile] $striplevel end]]
                          set inhunk false
                          #puts "Found diffline for $fntopatch"
                          continue
                  }

                  # state machine for parsing the hunks
                  set typechar [string index $line 0]
                  set codeline [string range $line 1 end]
                  switch $typechar {
                          @ {
                                  if {![regexp {@@\s+\-(\d+),(\d+)\s+\+(\d+),(\d+)\s+@@} $line \
                                          -> oldstart oldlen newstart newlen]} {
                                          return code -error "Erroneous hunk in line $lindeidx, $line"
                                  }
                                  # adjust line numbers for 0-based indexing
                                  incr oldstart -1
                                  incr newstart -1
                                  #puts "New hunk"
                                  set newcode {}
                                  set oldcode {}
                                  set inhunk true
                          }
                          - { # line only in old code
                                  if {$inhunk} {
                                          lappend oldcode $codeline
                                  }
                          }
                          + { # line only in new code
                                  if {$inhunk} {
                                          lappend newcode $codeline
                                  }
                          }
                          " " { # common line
                                  if {$inhunk} {
                                          lappend oldcode $codeline
                                          lappend newcode $codeline
                                  }
                          }
                          default {
                                  # puts "Junk: $codeline";
                                  continue
                          }
                  }
                  # test if the hunk is complete
                  if {[llength $oldcode]==$oldlen && [llength $newcode]==$newlen} {
                          set hunk [dict create \
                                  oldcode $oldcode \
                                  newcode $newcode \
                                  oldstart $oldstart \
                                  newstart $newstart]
                          #puts "hunk complete: $hunk"
                          set inhunk false
                          dict lappend patchdict $fntopatch $hunk
                  }
          }

          # now we have parsed the patch. Apply
          dict for {fn hunks} $patchdict {
                  puts "Patching file $fn"
                  if {[catch {open $fn} fd]} {
                          set orig {}
                  } else {
                          set orig [split [read $fd] \n]
                  }
                  close $fd

                  set patched $orig

                  set fail false
                  set already_applied false
                  set hunknr 1
                  foreach hunk $hunks {
                          dict with hunk {
                                  set oldend [expr {$oldstart+[llength $oldcode]-1}]
                                  set newend [expr {$newstart+[llength $newcode]-1}]
                                  # check if the hunk matches
                                  set origcode [lrange $orig $oldstart $oldend]
                                  if {$origcode ne $oldcode} {
                                          set fail true
                                          puts "Hunk #$hunknr failed"
                                          # check if the patch is already applied
                                          set origcode_applied [lrange $orig $newstart $newend]
                                          if {$origcode_applied eq $newcode} {
                                                  set already_applied true
                                                  puts "Patch already applied"
                                          } else {
                                                  puts "Expected:\n[join $oldcode \n]"
                                                  puts "Seen:\n[join $origcode \n]"
                                          }
                                          break
                                  }
                                  # apply patch
                                  set patched [list {*}[lrange $patched 0 $newstart-1] {*}$newcode {*}[lrange $patched $oldend+1 end]]
                          }
                          incr hunknr
                  }

                  if {!$fail} {
                          # success - write the result back
                          set fd [open $fn w]
                          puts -nonewline $fd [join $patched \n]
                          close $fd
                  }
          }
  }

  -------------------------------------------------


login: aku
mimetype: text/plain
icomment:
  Mail from Christian:
  ~ ~ ~~ ~~~ ~~~~~ ~~~~~~~~ ~~~~~~~~~~~~~
  Hi aku,

  this is the code submitted by me, right? The original code was buggy, in
  the meantime I fixed some bugs. The current code is contained in my
  revised kbs.tcl:

  https://github.com/auriocus/kbskit/blob/master/kbs.tcl#L1363

  This one works for the patches included in the repo, created by "diff
  -ruN" . But it fails for patches generated from "git diff", because git
  adds additional "junk" lines after the "diff" line. This seems to be
  allowed for patch files to include "junk" in between the markers.
  Therefore the code needs revision.

  	Christian


login: aku
mimetype: text/plain
icomment:
  Output from three tools: `diff -ruN`, `fossil diff`, `git diff`, in this order.
  `patch` seem to handle all three forms.
  The diff is for the pending change of the textutil index file.

  It seems that we should treat all lines before a `---` marker as junk.
  The `diff ...` lines look to be more commentary, and not part of the format.

  %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
  diff -ruN old/pkgIndex.tcl new/pkgIndex.tcl
  --- old/pkgIndex.tcl	2019-06-26 11:42:01.194722156 -0700
  +++ new/pkgIndex.tcl	2019-06-26 11:42:43.690791588 -0700
  @@ -11,3 +11,7 @@
   package ifneeded textutil::string   0.8   [list source [file join $dir string.tcl]]
   package ifneeded textutil::expander 1.3.1 [list source [file join $dir expander.tcl]]
   package ifneeded textutil::wcswidth 35.0  [list source [file join $dir wcswidth.tcl]]
  +
  +if {![package vsatisfies [package provide Tcl] 8.5]} { return }
  +
  +package ifneeded textutil::patch 0.1 [list source [file join $dir patch.tcl]]
  %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
  ADDED    modules/textutil/patch.tcl
  ADDED    modules/textutil/patch.test
  Index: modules/textutil/pkgIndex.tcl
  ==================================================================
  --- modules/textutil/pkgIndex.tcl
  +++ modules/textutil/pkgIndex.tcl
  @@ -9,5 +9,9 @@
   package ifneeded textutil::tabify   0.7   [list source [file join $dir tabify.tcl]]
   package ifneeded textutil::repeat   0.7   [list source [file join $dir repeat.tcl]]
   package ifneeded textutil::string   0.8   [list source [file join $dir string.tcl]]
   package ifneeded textutil::expander 1.3.1 [list source [file join $dir expander.tcl]]
   package ifneeded textutil::wcswidth 35.0  [list source [file join $dir wcswidth.tcl]]
  +
  +if {![package vsatisfies [package provide Tcl] 8.5]} { return }
  +
  +package ifneeded textutil::patch 0.1 [list source [file join $dir patch.tcl]]

  %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%
  diff --git a/old/pkgIndex.tcl b/new/pkgIndex.tcl
  index 280f627..88ca6b7 100644
  --- a/old/pkgIndex.tcl
  +++ b/new/pkgIndex.tcl
  @@ -11,3 +11,7 @@ package ifneeded textutil::repeat   0.7   [list source [file join $dir repeat.tc
   package ifneeded textutil::string   0.8   [list source [file join $dir string.tcl]]
   package ifneeded textutil::expander 1.3.1 [list source [file join $dir expander.tcl]]
   package ifneeded textutil::wcswidth 35.0  [list source [file join $dir wcswidth.tcl]]
  +
  +if {![package vsatisfies [package provide Tcl] 8.5]} { return }
  +
  +package ifneeded textutil::patch 0.1 [list source [file join $dir patch.tcl]]


login: aku
mimetype: text/plain
icomment:
  Change code for hunk detection to

  	if {[string match ---* $line]} {
  	    # a diff block starts. Current line should be
  	    # --- oldfile date time TZ
  	    # Next line should be
  	    # +++ newfile date time TZ
  	    set in $line
  	    incr lineidx
  	    set out [lindex $patchlines $lineidx]

  I.e. look for `---`, not `diff`, then tweak to treat current line as `in`, and next line as `out`, matching `+++` as normal.

  This passes my tests.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Please have a look at [89be287714].

  Changes versus your code:

    *  Factored patch parsing into a helper

    *  Replaced `puts` with report callback.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Committed with revision [343fbcce35].

  Thanks to Ashok for the report.

  Version of internal package "pt::rde::critcl" bumped to 1.3.4.

  Version of package "tcllibc" to hold this bumped to 0.3.15.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fixed in commit [81d1d109ad] 

  Version bumped to 1.0.6.

  Tweaked documentation to better highlight the tag part of debug.tag.

  With thanks to Timothy Tomkinson.


login: anonymous
username: dkf
mimetype: text/plain
icomment:
  --- dialect.tcl.orig	2015-11-11 09:53:00.000000000 +0000
  +++ dialect.tcl	2015-11-11 09:51:27.000000000 +0000
  @@ -2,159 +2,189 @@
   # A utility for defining a domain specific language for TclOO systems
   ###
   
  -namespace eval ::oo::dialect {}
  +namespace eval ::oo::dialect {
  +    namespace export create
  +}
   
  -proc ::oo::dialect::push class {
  -  ::variable class_stack
  -  lappend class_stack $class
  -}
  -proc ::oo::dialect::peek {} {
  -  ::variable class_stack
  -  return [lindex $class_stack end]
  -}
  -proc ::oo::dialect::pop {} {
  -  ::variable class_stack
  -  set class_stack [lrange $class_stack 0 end-1]
  +# A stack of class names
  +proc ::oo::dialect::Push {class} {
  +    ::variable class_stack
  +    lappend class_stack $class
  +}
  +proc ::oo::dialect::Peek {} {
  +    ::variable class_stack
  +    return [lindex $class_stack end]
  +}
  +proc ::oo::dialect::Pop {} {
  +    ::variable class_stack
  +    set class_stack [lrange $class_stack 0 end-1]
   }
   
   ###
  -# This proc will generate a namespace, a "mother of all classes",
  -# and a rudimentary set of policies for this dialect
  -###
  -proc ::oo::dialect::create {name {parent {}}} {
  -  if {[string index $name 0] eq ":"} {
  -    set name [string trimleft $name :]
  -    set NSPACE ::${name}
  -  } else {
  -    set NSPACE [uplevel 1 [namespace current]]::$name
  -  }
  -  ::namespace eval $NSPACE {}
  -  ::namespace eval ${NSPACE}::define {}
  -  ###
  -  # Build the "define" namespace
  -  ###
  -  if {$parent eq {}} {
  -    ###
  -    # With no "parent" language, begin with all of the keywords in oo::define
  -    ###
  -    foreach command [info commands ::oo::define::*] {
  -      set procname [namespace tail $command]
  -      proc ${NSPACE}::define::$procname args "oo::define \[::oo::dialect::peek\] $procname {*}\$args"
  -    }
  -    # Create an empty dynamic_methods proc
  -    proc ${NSPACE}::dynamic_methods class {}
  -    set ANCESTORS {}
  -  } else {
  -    ###
  -    # If we have a parent language, that language already has the oo::define keywords
  -    # as well as additional keywords and behaviors. We should begin with that
  -    ###
  -    if {[string index $parent 0] eq ":"} {
  -      set parent [string trimleft $parent :]
  -      set pnspace ::${parent}
  +# This proc will generate a namespace, a "mother of all classes", and a
  +# rudimentary set of policies for this dialect.
  +###
  +proc ::oo::dialect::create {name {parent ""}} {
  +    set NSPACE [NSNormalize $name]
  +    ::namespace eval $NSPACE {::namespace eval define {}}
  +    ###
  +    # Build the "define" namespace
  +    ###
  +    if {$parent eq ""} {
  +	###
  +	# With no "parent" language, begin with all of the keywords in
  +	# oo::define
  +	###
  +	foreach command [info commands ::oo::define::*] {
  +	    set procname [namespace tail $command]
  +	    interp alias {} ${NSPACE}::define::$procname {} \
  +		::oo::dialect::DefineThunk $procname
  +	}
  +	# Create an empty dynamic_methods proc
  +	proc ${NSPACE}::dynamic_methods {class} {}
  +	namespace eval $NSPACE {
  +	    ::namespace export dynamic_methods
  +	    ::namespace eval define {::namespace export *}
  +	}
  +	set ANCESTORS {}
       } else {
  -      set pnspace [uplevel 1 [namespace current]]::$parent
  -      if {![namespace exists $pnspace]} {
  -        set parent [string trimleft $parent :]
  -        set pnspace ::${parent}
  -      }
  -    }
  -    ::namespace eval ${pnspace} [list ::namespace export dynamic_methods]
  -    ::namespace eval ${pnspace}::define [list ::namespace export *]
  -    ::namespace eval ${NSPACE} [list ::namespace import ${pnspace}::dynamic_methods]    
  -    ::namespace eval ${NSPACE}::define [list ::namespace import ${pnspace}::define::*]
  -    set ANCESTORS ${pnspace}::object
  -  }
  -  ###
  -  # Build our dialect template functions
  -  ###
  -  uplevel #0 [string map [list %NSPACE% $NSPACE %name% $name %ANCESTORS% $ANCESTORS] {
  -###
  -# create the %NSPACE%::define command
  -###
  -proc %NSPACE%::define {class args} {
  -  ::oo::dialect::push $class
  -  if {[llength $args]==1} {
  -    namespace eval %NSPACE%::define [lindex $args 0]
  -  } else {
  -    %NSPACE%::define::[lindex $args 0] {*}[lrange $args 1 end]
  -  }
  -  %NSPACE%::dynamic_methods $class
  -  ::oo::dialect::pop
  +	###
  +	# If we have a parent language, that language already has the
  +	# [oo::define] keywords as well as additional keywords and behaviors.
  +	# We should begin with that
  +	###
  +	set pnspace [NSNormalize $parent]
  +	apply [list parent {
  +	    ::namespace export dynamic_methods
  +	    ::namespace import ${parent}::dynamic_methods
  +	} $NSPACE] $pnspace
  +	apply [list parent {
  +	    ::namespace import ${parent}::define::*
  +	    ::namespace export *
  +	} ${NSPACE}::define] $pnspace
  +	set ANCESTORS [list ${pnspace}::object]
  +    }
  +    ###
  +    # Build our dialect template functions
  +    ###
  +
  +    interp alias {} ${NSPACE}::define {} \
  +	::oo::dialect::Define $NSPACE
  +    interp alias {} ${NSPACE}::define::current_class {} \
  +	::oo::dialect::Peek
  +    interp alias {} ${NSPACE}::define::aliases {} \
  +	::oo::dialect::Aliases $NSPACE
  +    interp alias {} ${NSPACE}::define::superclass {} \
  +	::oo::dialect::SuperClass $NSPACE
  +
  +    ###
  +    # Build the metaclass for our language
  +    ###
  +    ::oo::class create ${NSPACE}::class {
  +	superclass ::oo::dialect::MotherOfAllMetaClasses
  +    }
  +    # Wire up the create method to add in the extra argument we need; the
  +    # MotherOfAllMetaClasses will know what to do with it.
  +    ::oo::objdefine ${NSPACE}::class \
  +	method create {name {definitionScript ""}} \
  +	[list next $name ${NSPACE}::define $definitionScript]
  +
  +    ###
  +    # Build the mother of all classes. Note that $ANCESTORS is already
  +    # guaranteed to be a list in canonical form.
  +    ###
  +    uplevel #0 [string map [list %NSPACE% [list $NSPACE] %name% [list $name] %ANCESTORS% $ANCESTORS] {
  +	%NSPACE%::class create %NSPACE%::object {
  +	    superclass %ANCESTORS%
  +	    # Put MOACish stuff in here
  +	}
  +    }]
   }
   
  -###
  -# title: Specify other names that this class will answer to
  -###
  -proc %NSPACE%::define::current_class args {
  -  return [::oo::dialect::peek]
  -}
  -    
  -###
  -# title: Specify other names that this class will answer to
  -###
  -proc %NSPACE%::define::aliases args {
  -  set class [::oo::dialect::peek]
  -  set %NSPACE%::cname($class) $class
  -  foreach name $args {
  -    set alias ::[string trimleft $name :]
  -    set %NSPACE%::cname($alias) $class
  -  }
  +# Support commands; not intended to be called directly.
  +
  +proc ::oo::dialect::NSNormalize {qualname} {
  +    if {![string match ::* $qualname]} {
  +	set qualname [uplevel 2 {namespace current}]::$qualname
  +    }
  +    regsub -all {::+} $qualname "::"
  +}
  +
  +proc ::oo::dialect::DefineThunk {target args} {
  +    tailcall ::oo::define [Peek] $target {*}$args
   }
   
   ###
  -# Create a superclass keyword which will enforce the inheritance
  -# of our language's mother of all classes
  -###
  -proc %NSPACE%::define::superclass {args} {
  -  set class [::oo::dialect::peek]
  -  dict set %NSPACE%::class_info($class) superclass 1
  -  set result {}
  -  foreach item $args {
  -    set Item ::[string trimleft $item :]
  -    if {[info exists %NSPACE%::cname($Item)]} {
  -      if {$%NSPACE%::cname($Item) ni $result} {
  -        lappend result $%NSPACE%::cname($Item)
  -      }
  -    } else {
  -      if {[info commands $Item] ne {}} {
  -        if {$Item ni $result} {
  -          lappend result $Item
  -        }
  -      } else {
  -        if {$item ni $result} {
  -          lappend result $item
  -        }
  -      }
  +# Implementation of the languages' define command
  +###
  +proc ::oo::dialect::Define {namespace class args} {
  +    Push $class
  +    try {
  +	if {[llength $args]==1} {
  +	    namespace eval ${namespace}::define [lindex $args 0]
  +	} else {
  +	    ${namespace}::define::[lindex $args 0] {*}[lrange $args 1 end]
  +	}
  +	${namespace}::dynamic_methods $class
  +    } finally {
  +	Pop
       }
  -  }
  -  if {$class ne "%NSPACE%::object" && "%NSPACE%::object" ni $result} {
  -    lappend result %NSPACE%::object
  -  }
  -  oo::define [::oo::dialect::peek] superclass {*}$result
   }
   
   ###
  -# Build the metaclass for our language
  +# Implementation of how we specify the other names that this class will answer
  +# to
   ###
  -oo::class create %NSPACE%::class {
  -  superclass oo::class
  -  constructor {definitionScript} {
  -    %NSPACE%::define [self] {
  -      superclass
  +
  +proc ::oo::dialect::Aliases {namespace args} {
  +    set class [Peek]
  +    namespace upvar $namespace cname cname
  +    set cname($class) $class
  +    foreach name $args {
  +	set alias ::[string trimleft $name :]
  +	set cname($alias) $class
       }
  -    %NSPACE%::define [self] $definitionScript
  -  }
   }
  +
   ###
  -# Build the mother of all classes
  +# Implementation of a superclass keyword which will enforce the inheritance of
  +# our language's mother of all classes
   ###
  -%NSPACE%::class create %NSPACE%::object {
  -  superclass %ANCESTORS%
  -    # Put MOACish stuff in here
  +
  +proc ::oo::dialect::SuperClass {namespace args} {
  +    set class [Peek]
  +    namespace upvar $namespace class_info class_info cname cname
  +    dict set class_info($class) superclass 1
  +    set unique {}
  +    foreach item $args {
  +	set Item [NSNormalize $item]
  +	if {[info exists cname($Item)]} {
  +	    set item $cname($Item)
  +	} elseif {[llength [info commands $Item]]} {
  +	    set item $Item
  +	}
  +	dict set unique $item $item
  +    }
  +    set root ${namespace}::object
  +    if {$class ne $root} {
  +	dict set unique $root $root
  +    }
  +    tailcall ::oo::define $class superclass {*}[dict keys $unique]
   }
  -  }]
  -  
  -  
  +
  +###
  +# Implementation of the common portions of the the metaclass for our
  +# languages.
  +###
  +
  +::oo::class create ::oo::dialect::MotherOfAllMetaClasses {
  +    superclass ::oo::class
  +    constructor {define definitionScript} {
  +	$define [self] {
  +	    superclass
  +	}
  +	$define [self] $definitionScript
  +    }
   }
  -package provide oo::dialect 0.1
  +
  +package provide oo::dialect 0.1.1


login: hypnotoad
username: hypnotoad
mimetype: text/plain
icomment:
  I have performed a series of tests with the patch, and find that it produces functional code. It has been integrated into the mainline code as well as the branches I test from. 

  Thanks!


login: scorriere
mimetype: text/plain
icomment:
  I took some more time to investigate this today.  I attempted to use callbacks to get the $result data along with the HTTP return code.  It appears that the _callback code uses a different callback if there is an HTTP error.  The body of the response is not passed to the callback, just the HTTP return code (And location if 302).


login: scorriere
mimetype: text/plain
icomment:
  I forgot to mention that I opened a pull request for this feature.  Please see:
  https://github.com/tcltk/tcllib/pull/6


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fixed with commit [87b07c73c6].

  **Note** The code generating the retList had a bug, for redirection errors the location information got lost. This part of the patch was rewritten. The rewrite also made use of the new httpCode variable. The original set it, and then made no use of it.

  The change is currently on branch [rest-tkt-284fd715e4].

  Please review the commit and my changes to the patch.
  Please get a tarball from the branch (*) and test that the changes actually work in your environment.

  (Ad *) You must be logged in as anon to see the links for tarball or zip download in the info page for the commit.


login: scorriere
mimetype: text/plain
icomment:
  The changes look good to me.  I have tested in my environment.  I have had successful and error requests work appropriately with error-body unset and with error-body set.  I get the correct return in all 4 cases.

  The code changes look good to me (Damn, I missed the httpCode thing when I wrote it).

  Thanks for handling this aku!


login: aku
mimetype: text/plain
icomment:
  What is the output of

    puts $::errorInfo

  immediately after the failing 'package require' ?
  This might gives us a clue where the package tries to read argv0.

  Note, argv0 is one of Tcl's global variables, automatically defined by the tclsh/wish. It contains the name of the command running the Tcl script.

  Might be that imap simply tries to use it without either ::-prefix, or without brining it into scope via global argv0.


login: anonymous
mimetype: text/plain
icomment:
  The error output is :
  can't read "argv0": no such variable
      while executing
  "if {[info script] eq $argv0} {
      # set imap4::debug 0
      set FOLDER INBOX
      set port 0
      if {[llength $argv] < 3} {
          puts "Usage: imap4...."
      (file "/usr/share/tcltk/tcllib1.17/imap4/imap4.tcl" line 1297)
      invoked from within
  "source /usr/share/tcltk/tcllib1.17/imap4/imap4.tcl"
      ("package ifneeded imap4 0.5.2" script)


login: aku
mimetype: text/plain
icomment:
  Thank you. That code is apparently in imap4.tcl to detect if the package is run as the main script, and then to make it run as a simple example imap client application.

  This is at the toplevel of the script, so that variable should exist, as far as I know. You are using tclsh to run your example, right ? (Or wish). Which version ? Can you show us the entire script you use which has the problem ? If not, then maybe a suitably shrunken version which still has the problem without leaking any confidential details of your setup and code ? ... I am wondering if you are using 'package require imap' from inside of a procedure. That might cause this issue.

  Regardless of the answers, you have a workaround available to you.

  Open the file mentioned in the stack trace you gave us, go to the end, the line 1297 where the if condition in question lurks, and disable the condition with a "0 &&" added to it, i.e like

     if {0 && ([info script] eq $argv0)} {

  That disables the condition and code, and should allow you to use the package.
  An alternate possible fix would be

     if {[info exists argv0] && ([info script] eq $argv0)} {

  i.e. check if the variable exists.

  That however feels a bit like simply papering over the problem. Which might be the tclsh you use somehow not creating the variable. Or maybe the 'require' weas invoked from inside a proc. The package code does not go to the global stack level. If that is the issue simply putting a

     global argv0

  before the if statement could solve the issue as well.

  Ther proper fix (in my opinion) would be to move that entire piece of code out of the package into a separate and proper example application instead.

  That however will have to wait until I am back from my x-mas vacation.


login: anonymous
mimetype: text/plain
icomment:
  Hello aku,
  Sorry for answering late.

  I tried " if {[info exists argv0] && ([info script] eq $argv0)} {" but still getting : can't read "argv0": no such variable
  The other wordaround works perfectly by the way.

  I am using this in eggdrop which embeds Tcl, and therefore doesn't set argv0 at all, including the global variable. I agree that the test code should probably be moved somewhere else.

  Regards.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Technically speaking this is not a bug, because the callback argument is documented as

  * The value of this option is the __name__ of a procedure

  Practically speaking however all callbacks should be handled as command prefixes, and a restriction to a single command name, that is nonsense.

  Luckily a command prefix is a simple extension of a command name, so I can treat this as a backward-compatible extension of the current API and get away with a minor version change.


login: aku
mimetype: text/plain
icomment:
  Exif is a deprecated and excluded module (superceded by jpeg). Part of the code making the embedded docs took this into account, and other parts did not.

  Commit [fed6333392] fixes the parts which did not. exif is now completely gone from the docs


login: droll
mimetype: text/plain
icomment:
  Good catch! The following patch for TEPAM 0.5 will correct the identified issue:

  '''Patch Begin'''
  1398a1399
  >                   set PureProcName [PureProcName]
  1412a1414
  >                   set PureProcName [PureProcName]
  '''Patch End'''

  I will add a test to the regression environment and perform a few additional verification in the coming days before I submit the bug fix to the repository.


login: aku
mimetype: text/plain
icomment:
  Based on the size of the .tcl files, i.e.

     747 modules/tepam/tepam_doc_gen.tcl
    2762 modules/tepam/tepam.tcl

  I deduce that this is applied to "tepam.tcl". Except, looking at the given lines the match does not look that good. On the third hand, both locations seem to be inside of proc ProcedureHelp, so that fits.

  A unified diff (diff -u) might be a better choice of output for comment and attachment as that format for patches provides context, i.e. a few lines around the actual change, making matching easier, both for humans and the patch command.


login: droll
username: Andreas Drollinger
mimetype: text/plain
icomment:
  Fixed with commit ac49d84e58a4b7283524dc2dc0dad212e5497a7c on August 14, 2016. The new TEPAM revision is 0.5.2.


login: arjenmarkus
mimetype: text/plain
icomment:
  The suggested code has been incorporated in the repository.

  One test case added now. I will add the complete set as well. Bumped the package to 1.2.1 (there were some other additions as well).


login: arjenmarkus
mimetype: text/plain
icomment:
  You're absolutely right. The argument stdev is used as if it were the variance. The easiest way to solve this is to adapt the actual implementation.


login: arjenmarkus
mimetype: text/plain
icomment:
  Hm, I see there are tests already, as there should be, but they do not detect the misinterpretation of the stdev argument. I will have to check that more closely.


login: arjenmarkus
mimetype: text/plain
icomment:
  This has been solved, note added to the documentation as this has consequences for users. Of course, the new implementation is better as it does what it says it does.


login: arjenmarkus
mimetype: text/plain
icomment:
  The difference between solveGauss and solvePGauss is that the first uses the Gauss-Seidel algorithm in a straightforward way to solve the system and the second uses partial pivoting to do this. The effect of pivoting is that the solution is slightly more stable. I do not know how ill-conditioned the matrix is, but looking at the solution (almost the null vector) and the wide variation in coefficients, I would hazard to say that the matrix is quite ill-conditioned.

  I agree that you would expect a permutation of the solution's components, but there is no guarantee given the finite precision of floating-point operations.

  Can you provide an example with a solution vector that is not near zero?


login: arjenmarkus
mimetype: text/plain
icomment:
  The described issue is actually not a problem as such. The given example is extreme, as the entries in the matrix differ by many orders of magnitude and the solution is very near to the null vector. The example suffers from the finite precision that is inherent in floating-point arithmetic.

  Nonetheless, I have added a test case to demonstrate that consistent permutation of the rows does not lead to a different solution.


login: aku
mimetype: text/plain
icomment:
  Fixed with commit [a69bb7c2123135de0205bc6441e971c597de484e].
  Visible documentation fixed with commit [95352867142f1a4eb510731538805f5e514cb747]


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Googling around I found

    * https://www.gnu.org/software/bison/manual/html_node/Pure-Decl.html

    * https://www.gnu.org/software/bison/manual/html_node/Pure-Calling.html


login: aku
mimetype: text/x-fossil-wiki
icomment:
  New branch [jsonc-tkt-715944a1f7]

  Commit [56dcef567957f409ffcc03303c96ca13e3719a4d]

  Declared the parser as pure/re-entrant.
  Regenerated with bison 3.0.2.
  Fixed build issues.
  Ran testsuite. Passed.

  So, the referenced branch and commit are not worse off.
  Hopefully it is now thread-safe.

  Please try in your environs.

  To get a zip-archive or tarball of the commit log in with your account, or as anon,
  then go to the commit's info page and use the appropriate "Download" link.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Confirmed working and fixing the issue.

  Merged to trunk with commit [3870a868297e67addf77feef289ab968ac95587b].


login: aku
mimetype: text/plain
icomment:
  Do you have a patch ?

  As far as I can tell the 'init', where everything happens is simply one-way, with no concept of undoing anything it is setting up.

  I suspect that changing this will be considerable work.


login: patthoyts
username: patthoyts
mimetype: text/plain
icomment:
  Added the ability to reset this (without manipulating the autoproxy::options array directly) by using "autoproxy::configure -basic --" to reset any existing details.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Merged, commit [5105ca9c418157f6e03d933a9011bf2f70819133].
  Added documentation for pt::tclparam::configuration::nx.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  `token::shell` was written to help with command line completion (See `cmdr` package).

  It does not have a concept of files. While it sees words it does not know which of them could be files or file patterns.

  I believe that this goes way beyond the scope of package.

  With thanks for the submission, I decline.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  This report does not apply to Tcllib.
  The proper repository is

     *   <a href='http://core.tcl.tk/bwidget/home'>http://core.tcl.tk/bwidget/home</a>


login: oehhar
username: oehhar
mimetype: text/x-fossil-wiki
icomment:
  Thank you. Transfered to bwidget tracker:

  [https://core.tcl.tk/bwidget/tktview/72a5727d1b7fb76b32cea032eb7d4bf7c6fa28bf]


login: aku
mimetype: text/plain
icomment:
  Trouble reduced to 8.6.4 only by now.

  tclsh running the tests has the `udp` package.

  The tests use 3 processes (main, server, and other).
  Somewhere in there one of the processes is unable to get networking information.

  =================================
  @+
  @|Could not locate nns@f4:6d:04:53:dd:e4 on the network
  @|    while executing
  @|"error "Could not locate $rcpt on the network""
  @|    (procedure "::cluster::ping" line 15)
  @|    invoked from within
  @|"::cluster::ping nns@$macid"
  @|    (file "/home/aku/Play/Tcllib/dev.tcllib-tests/modules/udpcluster/udpcluster.test" line 168)
  @|    invoked from within
  @|"source [pSet file]"
  @-
  @@ EndFile 1509574573
  =================================

  That is a toplevel command just before testcase cluster-comm-3.0.
  The preceding test cases (cluster-comm-{1.0 ... 2.4} all passed.
  I am unable to say if they use cluster::ping too.


login: arjenmarkus
mimetype: text/plain
icomment:
  Changed the required patch level for math::geometry to 8.5. I do not have a Tcl version 8.5 installed at the moment, so it is not that easy to check the failures.

  Again, will have to look into this.


login: aku
mimetype: text/plain
icomment:
  I rechecked (because I have 8.4.20, 8.5.18, and 8.6.4 installed).

  While I still see fails for 8.4.20 (10), the fails for 8.5 look to be gone.

  <pre>
    [8.4.20] math ~~ FAILS T  1363 P  1353 S     0 F    10
    [8.5.18] math ~~ Skip  T  2291 P  2291 S     0 F     0
    [8.6.4]  math ~~       T  2489 P  2489 S     0 F     0
  </pre>


login: kjnash
username: kjnash
mimetype: text/plain
icomment:
  The 44 tests of uri-rfc2396.test are skipped (known bug).

  The test that fails is likely uri.tcl, test uri-8.0; the test itself is incorrect.

  This test is patched, along with bugfixes to uri.tcl, in ticket [7b5c6cb477].


login: kjnash
username: kjnash
mimetype: text/plain
icomment:
  (correcting an error in the previous comment)

  The 44 tests of uri-rfc2396.test are skipped (known bug).

  The test that fails is likely uri.test, test uri-8.0; the test itself is incorrect.

  This test is patched, along with bugfixes to uri.tcl, in ticket [7b5c6cb477].


login: kjnash
username: kjnash
mimetype: text/plain
icomment:
  Commit [55226b97ac] fixes this bug by applying ticket [7b5c6cb477].  The faulty test uri-8.0 from file uri.test was rewritten.


login: aku
mimetype: text/plain
icomment:
  With latest fixes to make it actually run the tests this now hangs itself with 8.6.4, after a lot of error output.


login: hypnotoad
username: hypnotoad
mimetype: text/plain
icomment:
  The tests were designed on Windows, and hadn't accounted for differences in the representation of \n on Unix (at least on the test suite side.) Along the way streamlined the http header parser and added a timeout to the coroutine::util::gets_safety function.

  All tests now work on Unix. And they don't hang. Yeah.


login: aku
mimetype: text/html
icomment:
  This does not belong to the Tcllib tracker.
  Moved to the Tklib tracker.
  Ref: <a href="http://core.tcl.tk/tklib/tktview/5e324065bfb46f73dcbaa6ef1c7cc24890968602">http://core.tcl.tk/tklib/tktview/5e324065bfb46f73dcbaa6ef1c7cc24890968602</a>


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Branch [dt-html-fixes].

  Commit [83742e01e4].

  Merged with commit [463f99dd29].
  Bumped versions of the packages using the code (doctools, doctools::toc, doctools::idx), with commit [0feca7d152].

  Truly, please nag me.
  Just came across this while closing out merged branches still marked open.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Confirmed reported behaviour/issue for the Tcl implementation.

  The critcl implementation on the other hand is OK.

  Working branch: [tkt-39ab616d8f-ak]


login: aku
mimetype: text/plain
icomment:
  Fix was easy, always check a node if it was visited, before performing actions.
  While the check I used at the end prevented visited neighbors from being added to the work stack it did nothing for nodes which already were on the stack as unvisited neighbors of an earlier node, and then got visited before work reached them.

  Merged with commit [e1f5469305]
  Bumped version to 2.4.1.

  Did a double merge because I added a second testcase to ensure that the Critcl variant was truly ok. Because of the differences in the exact walk order the original new test actually did not catch the situation for it, so passing that did not mean anything. Luckily it was definitely ok.

  Thank you for the report, and for the patience. Next time please do not hesitate to nag us.


login: aku
mimetype: text/plain
icomment:
  Do you have a patch for the issue ?
  Alternatively it would be good to have an example of the response itself.

  I am asking these because I do not have an imap server to play with.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  I am sorry, I am unable to apply this patch.

  Using the patch command <code>patch oauth.tcl PATCH</code> I get
  <pre>
    patching file oauth.tcl
    Reversed (or previously applied) patch detected!  Assume -R? [n] y
    Hunk #2 FAILED at 168.
    Hunk #3 FAILED at 171.
    Hunk #4 FAILED at 242.
    Hunk #5 FAILED at 254.
    Hunk #6 FAILED at 256.
    patch: **** '<' expected at line 22 of patch
  </pre>

  Please provide me with a <strong>unified diff</strong> (the result of a <code>diff -u ...</code>). This is a more readable form I can also apply manually if needed.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  While I still had to apply it manually (*) with the context of an unidiff it was now at least possible.

  Did some additional cleanup and simplification (Moved common code to helper procs, use of 8.5 lassign, use of 8.5 {*}). Bumped to version 1.0.1.

  See commit [a0ec57b1bd].

  Not merged yet. As I do not have testsuite to run I would like you to test the change and give your approval (or not).


  (Ad *) Copying the patch out of the description seems to break something in the formatting. For the future please consider to attach the patch instead of placing it in the description. When you have the ticket view open this is the left-most button in the secondary navbar.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Nothing heard back. Accepted as working.

  Merged into `trunk` and `tcllib-1-19-rc`.

  Relevant merge commits:
     *   [738baecbc3] = `trunk`
     *   [52bd859b74] = `tcllib-1-19-rc`


login: anonymous
mimetype: text/plain
icomment:
  This change introduced an opening brace in the regexp in line 286 in ::oauth::Split, which prevents the package from loading.


login: anonymous
mimetype: text/x-fossil-wiki
icomment:
  Also, in line 275 in ::oauth::QuoteValues the variable header does not exist and should be $params, otherwise it causes a runtime error.


login: anonymous
mimetype: text/x-fossil-wiki
icomment:
  OK, can you explain to me how adding a brace in front of an otherwise fine regular expression is supposed to work? I'm not sure what you're trying to accomplish here. My scripts don't work when there is a { in front of a new-line capturing expression. I suggest deleting the \{ sequence entirely.


login: chrstphrchvz
username: chrstphrchvz
mimetype: text/x-fossil-wiki
icomment:
  There was an issue opened on the GitHub mirror a few years ago that I'm wondering might have been for this same issue this ticket addresses: [https://github.com/tcltk/tcllib/issues/7]. Can anyone confirm?

  (Note that issue trackers on GitHub repositories can be disabled.)


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Unclear how I missed the May 5, 2018 response.
  Deep apologies.

  Finally done the removal of the bogus \{ sequence.

  Looked over things a few times and I am wholly unclear why it was written with it. My best guess this late is that it was an accidental insertion I then missed, and then not thought about on the first "fix" (See [aec286d43d]).

  Done with with commit [a136e80afe].

  Version bump to 1.0.3.

  Thank you for your patience here.
  Hopefully this now truly done.


login: aku
mimetype: text/plain
icomment:
  Regarding
  <pre>
    puts [tar::get t.tar 0b10]         ;# "number two" - expected error
  </pre>

  https://core.tcl.tk/tcllib/artifact/ccc1188a13c75865?ln=215

  shows that a missing file results in an empty string, not an error.

  Not that this behaviour is truly documented, i.e. the docs for <code>tar::get</code> simply say nothing about what happens when the requested file does not exist.

  And as we are not yet at the v1 (still 0.10) we can change this, IMHO.

  As I would have expected an error as well I will change the code to that.

  Oh, and tar::get is not really tested either, at all.
  Only indirect in the test for <code>tar::add</code>

  Ok, out of scope here. I will make a new ticket for the incomplete testsuite.


login: aku
mimetype: text/plain
icomment:
  Fix merged with commit [21cf0660cc1f5a28].

  Version bumped to 0.11

  Thank you for the report, and your patience.


login: anonymous
mimetype: text/plain
icomment:
  This was a good opportunity to add a whole number of procedures which included several relating to this question. version 1.1 of math::numtheory.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Note that <code>rest::put</code> is implemented as an interp alias to <code>rest::simple</code>. (rest.tcl, lines 88-92 for the whole block).

  If that is enough to implement PATCH, then yes, that is trivial.

  Can you confirm?

  For testing see branch [tkt-997c8b4e67-aku], 
  or commit [3b0925ca00].

  This implements the alias for <codee>patch</code>, documents it, and bumps rest to version 1.3.

  It is not merged yet, waiting on your approval.

  Thank you for the report, and your patience.


login: anonymous
mimetype: text/plain
icomment:
  Hi aku,

  yes, that would be enough. PATCH works similar to PUT, but PATCH is a partial change while PUT is a full replacement of a resource.

  There are different ways to use PATCH, e. g. RFC 6902 (JSON Patch), but the application also may use its own format, e. g. a reduced XML which only contains the values which should be updated. But the application layer is not focus of the Tcl's rest library.

  Thank you for your commitment.


login: anonymous
mimetype: text/plain
icomment:
  Hi aku,

  I just saw that you forgot to adjust the line 

    if {$cmd ni {get delete head post put}} {

  in rest.tcl

  It should say

    if {$cmd ni {get delete head post put patch}} {

  Thank you


login: anonymous
mimetype: text/plain
icomment:
  Wait, there is another problem:

  Before the line

     if {$cmd ni {get delete head post put patch}}

  $cmd has the value ::rest::patch or rest::patch instead of patch (as expected). This also affects the other functions.

  This causes the error "Unable to determine rest::simple method, found "::rest::patch" (or "rest::patch") Please specify it explicitly." always occur.

  Maybe this is because [info frame -2] reflects the command with the namespace.

  I am using Tcl 8.6.6

  None of the following is working

    ::rest::patch $url $query $args

    rest::patch $url $query $args

  What am I doing wrong?


login: aku
mimetype: text/plain
icomment:
  This might be differences between Tcl 8.5 and Tcl. 86, with regard to the result returned by [info frame].

  Re-opening the ticket now, will have another look.

  This really needs a testsuite.


login: anonymous
mimetype: text/plain
icomment:
  This is also the case with tcl 8.5. I do not know the reason why.

  However, I found a way to extract the method name from the string.

  Sure, the coding is not as nice, but it finds ::rest::get, rest::get and get with any leading spaces and trailing payload.


      if {![dict exists $loc cmd]} {
          return -code error "Unable to determine rest::simple method in the current context ([dict get $loc type]). Please specify it explicitly."
      }
      set cmd [dict get $loc cmd]
  #    if {[catch {
  #       set cmd [lindex $cmd 0]
  #    }]} {
  #       # Not a proper list. String processing.
  #       # Simple: Assume name without spaces.
  #       # TODO: Quoted literal.
          regexp {^(?: *)(?:::)?(?:rest::)?([a-zA-Z0-9_]+)(?: )*?(?:.*)$} $cmd -> cmd
  #    }


  You may also use ^(?:.*)(get|delete|head|put|post|patch)(?:.*)$ to make it even easier. This would also save the if {$cmd ni { ... }) below.


login: anonymous
username: Tobias
mimetype: text/plain
icomment:
  In rest 1.0.1 there was this line which extracts the command and stores it under the "method"-key:

     dict set config method [namespace tail [lindex [dict get [info frame -1] cmd] 0]]

  You'll notice the "namespace tail" which should eliminate all leading "::rest::" or "rest::".

  In rest 1.0.2 this namespace-line comes AFTER the command is compared to "get|delete|head|..." which makes no sense at all. Wouldn't it better be placed BEFORE?

     if {$cmd ni {get delete head post put}} {
        return -code error "Unable to determine rest::simple method..."
     }
     set cmd [namespace tail $cmd]
     if {$cmd eq "simple"} { set cmd get }

  Also the following comparison for "simple" will never be true because "simple" isn't part of the previous comparison "$cmd ni ..."


login: géballin
mimetype: text/plain
icomment:
  I corrected that for my use by changing line 105 from
  set cmd [lindex $cmd 0]
  to
  set cmd [lindex [split [lindex $cmd 0] :] end]

  So I split by : and take the last element.

  But I think it's really not a good idea to add patch method because it's not a standard method (rejected in RFC 5789) and not Restful.

  I think a better idea should be to add a rest::accept_method method where you could declare non-standard HTTP methods that you would like to use (I already did it for me, but need to do the diff for create a patch ticket). And of course, redesign the method storage for a more factorized way and add all the standard HTTP methods (no options, no trace, no connect… which are all standards HTTP methods widely used in Rest APIs).


login: anonymous
username: PATCH in OData 3.0
mimetype: text/plain
icomment:
  The PATCH method is also used in OData 3.0 and later, where it replaces the deprecated non-standard method MERGE.

  See http://www.odata.org/documentation/odata-version-3-0/odata-version-3-0-core-protocol/ for more information.


login: anonymous
mimetype: text/plain
icomment:
  While OData 3.0 allows MERGE, but strictly says that PATCH should be preferred by the client, OData 4.0 [1] does not mention MERGE anymore. Because MERGE is only needed by legacy OData implementations, we should only support the standard verb PATCH [2].



  [1] http://docs.oasis-open.org/odata/odata/v4.0/odata-v4.0-part1-protocol.html
  [2] https://tools.ietf.org/html/rfc5789


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Re geballin, about `set cmd [lindex [split [lindex $cmd 0] :] end]`.

  This `split/lindex` combination is equivalent to the `namespace tail`, Tobias mentioned.

  I have now moved this and the code forcing `get` for `simple` to before the cmd check. That should now work and be sensible.

  This is commit [997c8b4e67]
  on branch [tkt-997c8b4e67-aku].

  The package is now at version 1.3.1.

  Regarding the comments about OData 3.
  Does this need code changes ?

  If yes, please attach a patch to this ticket,
  based on commit [997c8b4e67].

  Regardless, please validate that the latest changes do what I believe they do.


login: anonymous
mimetype: text/plain
icomment:
  I think, when we add PATCH, we should also add OPTIONS, CONNECT and TRACE, to complete the standard HTTP methods (https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods)


login: anonymous
mimetype: text/plain
icomment:
  Sorry, I misunderstood. TRACE and CONNECT are not used in Rest at all, and OPTIONS normally does not occur in productive code, as it is used to check server capabilities.


login: anonymous
mimetype: text/plain
icomment:
  As you see here, almost nobody uses OPTIONS and if it is used, it mostly returns the description of the RESTful service, which is useful to discover the service for the first time: http://zacstewart.com/2012/04/14/http-options-method.html


login: géballin
mimetype: text/plain
icomment:
  Options is widely used in HATEOS API since it's the way you discover which method you can apply to the ressource. Any good programming REST API must give an OPTIONS method to each url.
  Options method is too widely used to know if a ressource is available or not by returning 200 or 404, it's very important for asynchronous API.

  Even if you don't use them personnaly, every standard methods should be available in the client REST (which include WebDAV methods) but like said before, rest.tcl is badely written in that part, I just need little time to send you my updated version (and find a REST server tu use to do a test file).

  Thanks aku, namespace tail is the good way to do it, I agree.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Reading through the documentation I see no references made to sharing of sub structures between different records, and circular references are documented as not allowed.

  These were my main thoughts about ways on how objects for nested structures could be made to survive the death of their container.

  As is nested structures look to be definitely owned by their container.
  As such they should definitely be destroyed when the container is destroyed.

  Accepted for fixing.

  This will need additional tests, and the documentation might need clarification as well, in places.


login: pda
mimetype: text/plain
icomment:
  Best patch IMHO :
  change line 414 to:
     -request 1 -server 0 -require 0 -ssl2 no -ssl3 no

  And let tcltls/OpenSSL choose the best TLS version.


login: pda
mimetype: text/plain
icomment:
  The following patch solves the problem (I did not found the way to attach a patch in the ticket Web interface, sorry):

  --- ldap.tcl.bak        2017-03-21 11:01:54.716595089 +0100
  +++ ldap.tcl    2017-03-21 11:02:25.956334530 +0100
  @@ -738,7 +738,10 @@
                       set conn(pdu,partial) 1
                       append conn(pdu,received) $type
                   }
  -                }
  +           }
  +           partial {
  +               return
  +           }
               eof {
                   CleanupWaitingMessages $handle
                   set conn(lastError) "Server closed connection"


login: pda
mimetype: text/plain
icomment:
  The attached patch may fix tickets:
  b9a39c02c7855865fb48c32d1d4dcbf8ee22750b
  5f4f30667b4bc066593e0aa37ddf88b62e1830b5


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Make refs actual links.

  * [b9a39c02c7855865fb48c32d1d4dcbf8ee22750b]

  * [5f4f30667b4bc066593e0aa37ddf88b62e1830b5]


login: aku
mimetype: text/plain
icomment:
  Your fix is now applied in commit [8b8f10e2bb],
  on branch [fix-ldap-partial].

  It is not merged to trunk yet, as I would like to ping the reporters of the two tickets you referenced to see if their issues are solved by this too.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Ok. I Have not heard back from the submitters of the other tickets, for about 2.5 months. No need to hold back anymore.

  This fix is now merged into both `trunk` and `tcllib-1-19-rc` (Latter is where the 1.19 release is currently getting prepared). The relevant merge commits are

     *   [0826350f8d] = `trunk`
     *   [2f3369f04b] = `tcllib-1-19-rc`


login: aku
mimetype: text/x-fossil-wiki
icomment:
  The first two chunks (`ldapx::ldap` --> `ldapx::entry`) where already present in my instance of the manpage. I only had to apply chunk three (Adding `c destroy`).

  See commit [31128791fd].

  Note, still have to regenerate the visible documentation.
  That will come late in the release-prep.


login: aku
mimetype: text/plain
icomment:
  To me the issue is less that the 0.1 is accepted (because it simply is a superset of the spec) and more that it is converted incorrectly. That is a problem.

  Of course, if the easy fix is to disallow the form of input I will likely go for that. But we will see.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  The conversion issue affects only the Tcl implementation.
  The Critcl implementation converts correctly.

  ... It is something in the RE to split the json into tokens.
  ... Deeper, something in the "json::numberRE".

  Test script:
  <pre>
  lappend auto_path [pwd]

  puts [package require json]=[package ifneeded json [package require json]]
  puts [info patchlevel]
  puts *\t[join [info loaded] \n*\t]

  puts ""
  foreach {fp} {
      1.1
      0.1
      .1
      1.
      1
  } { 
      set m [regexp --          $json::numberRE $fp match]
      regexp -indices -- $json::numberRE $fp matchi

      puts ($fp):\t${m}\t[json::json2dict $fp]\t($match)\t($matchi)
  }
  </pre>

  Results:
  <pre>
  1.3.3=source /home/aku/Play/Tcllib/dev.tcllib/modules/json/json.tcl
  8.5.18
  *

  (1.1):  1       1.1     (1.1)   (0 2)
  (0.1):  1       0.1     (0.1)   (0 2)
  (.1):   1       1       (1)     (1 1)  ***
  (1.):   1       1       (1)     (0 0)
  (1):    1       1       (1)     (0 0)
  <pre>

  The .1 is interesting, see ***. It matches only "1", ignoring the leading dot.

  The relevant code in the package is
  <pre>
  proc ::json::json2dict_tcl {jsonText} {
      variable tokenRE
      set tokens [regexp -all -inline -- $tokenRE $jsonText]  
  [...]
  </pre>
  where numberRE is an alternative in tokenRE.

  With the given input it will also ignore leading dot.

  Part of the problem is that the pattern is not anchored.
  However because the code uses -all it cannot be anchored, doing so would prevent the matches after the first token.

  And do not forget that the trailing dot for "1." is ignored as well.

  I might have to create specialized number patterns for these cases to get them matched correctly.
  (I.e. patterns for fractionals, and cardinal with trailing dot, but no fractional)


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fix merged with commit [dbdaf3dff4].

  Version bumped to 1.3.4

  Extended testsuite.

  Went with additional number patterns.
  Essentially when parts of the number are missing other parts become mandatory, and that was not handled by the existing single pattern for numbers.

  Thank you for the report, and your patience.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  This actually depends on how you created the matrix form the CSV file.

  I suspect that you used <a href="https://core.tcl.tk/tcllib/doc/trunk/embedded/www/tcllib/files/modules/csv/csv.html#5"><strong>read2matrix</strong></a>
  with the <strong>expand</strong> argument set to <strong>auto</strong>.

  In this mode the reader automatically expands the matrix to fit the max number of columns found in the file.

  You could try the other modes to see if they are more suitable.

  Generally note that a matrix is a table, like in a database. Each row will have the same number of columns, even if some are empty. A matrix where each row has a different number of columns is not supported, and I do not intend to support that.

  Does this help ?


login: anonymous
mimetype: text/plain
icomment:
  Corrected the [conforming] procedure - it did not take proper care of this case. Also added testcases for the procedure. Bumped version to 1.1.6


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Patch accepted.

  Branch [tkt-e4cfcc0f96-ak]

  Merged fixes with commit [bf5f60bebe].

  Version bumped to 0.3.

  Thank you for reporting, and your patience.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Branch [tkt-7d93822003-ak].

  Fix in commit [5f4fac2410].

  Not merged yet.

  Please check that this change works for you now.


login: aku
mimetype: text/plain
icomment:
  Looks mostly good.
  Passed tests.

  Still had to bump the version of number of "pt::tclparam::configuration::nx".

  Wanted to bump "pt::rde::nx" as well, given that you changed it.
  Saw that is was tied to the version of "pt::rde::oo".
  Can see the reason for that, the "nx" is using "oo" as the base it is converting internally.

  OTOH, your change is a bugfix in the "nx" machinery and I would like to have that reflected in the version.

  Best idea so far is to have additional version numbers added at th end which reflect nx-specific things, i.e. write

      package provide pt::rde::nx [package req pt::rde::oo].1.0

  What do you think ?


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Accepted with changes. See commit [41e67fa77f].

  The issue was the missing extraction of the `key` for the single-argument case.
  I went for a minimal change simply adding that extraction.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fix accepted and merged. Se commit [98a32c3965].

  Added test demonstrating the issue.
  Bumped version to 0.8.
  Applied the same fix to the Tcl 8.2- implementation.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Accepted with changes.

  See commit [b09616777a].

  The `_setfileenc` (`SFE`) inside `_loadfile` (`LF`) was a no-op (`enc` could never exist at that point). Importing `enc` causes failures in the testsuite. The command was used only in two places, one which did its own `SFE`, and `revert`. Moved the `SFE`  from `LF` to `revert`. That seems to work, at least the testsuite passes.

  Note to future: Please submit a patch which extends the testsuite of `ini` and demonstrates the new behaviour and handling of encodings.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fixed, commit [232ea3ae18].
  One puts ? A plethora actually.

  Should possibly add in use of the `debug` package for tracing, instead.


login: aku
mimetype: text/plain
icomment:
  Bah. These channels do not have testsuites, at all.

  For now I will make something which just gets me this ticket fixed.
  Then a new ticket will point to the lack of a testsuite.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fixed with commit [bdce7f7a76].
  Merged with commit [501a905042].
  Versions bumped.
  Initial testsuites made for the troubled channels.


login: aku
mimetype: text/plain
icomment:
  Hi Jack (Glenn?).

  Are you asking for a documentation change ?
  Or are you asking for a code change ?

  While both are trivial a code change implies a version bump (patchlevel).
  A documentation change keeps the version.

  (And I assume your code is now based on the current return value instead of the documented).


login: avl42
username: avl42
mimetype: text/plain
icomment:
  I've also got a couple further changes for that file...

  mostly changing the dreaded "foreach {v1 v2 ...} $list break" idiom to lassign, changing some sqrt(pow(...,2)+pow(...,2)) to hypot(...,...), and avoiding a pow( sqrt(...) ,2) detour where the sqrt isn't needed, and last but not least avoiding a comparison of each a number and a list with a non-numeric string within a loop.

  I'm humbly asking for write access to commit it on a branch for further checking.


login: aku
mimetype: text/plain
icomment:
  Currently taking myself, saw the branch, making for easy review.

  Fixed a missing requirement of 8.5 in the code. Now necessary, with `lassign` used. Package index and testsuite already did this, just the code itself did not.


login: aku
mimetype: text/plain
icomment:
  Test added.

  The areaPolygon was not tested at all.
  Need a ticket to verify coverage of geometry testsuite.

  Further bumped version (bugfix -> patchlevel increment, now at 1.2.3).

  Now ready for merge.


login: aku
mimetype: text/plain
icomment:
  Hm.

  In the testsuite some of the expected results are
  ```
    <!-- Copyright &copy; .COPYRIGHT.                                                                                                                                                        
       -->
  ```
  With the proposed change that becomes
  ```
    <!-- Copyright &amp;copy; .COPYRIGHT.
       -->
  ```
  Is that an expected change ?

  It seems that I should/have to go over the uses of `ht_comment`
  and check for parts which are already escaped, and protect them.

  Hm. There are indications that the &copy; should have been protected already.

  I believe I will have to make a test case for your case.

  As a side question, html formatting inside of a comment is not allowed, must be escaped into proper entities ?

  Heh. One way of protection, put the (c)-symbol in as the regular character, and let the new escape do the work now. Instead of using the entity.


login: gjr.timmer
mimetype: text/plain
icomment:
  The reason for this patch is the following. I've made several patches for doctools, my company is using is to generate documentation for several company packages. Within it we are using for example a URI in the Copyright statement as described above. The ht_comment without escaping causes html malformation.

  When ht_comment uses escaping all HTML tags are generated correctly.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Patch applied, see commit [1a19fa44a5].

  Updated test results to match the breakage in the comment.

  Bug is important, the breakage in a comment is minor.

  Considering this as fixed now.

  Keeping version from previous ticket [82d77f6e1f] and fix [71c513a5bc].


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Patches applied.
  Package versions bumped.
  Testsuite fails with patch
  ... html results needed an update.
  Done. Tests pass.

  Commit [71c513a5bc].

  Docs regenerated. All html files touched, doctype added.


login: aku
mimetype: text/html
icomment:
  The intent behind writing `coro::auto` was to allow pre-existing functionality/procedures to be run either normally, or within a coro context, without having to rewrite them.

  The intent behind the particular rewrite of `global` was to allow multiple coros based on the same procedure without having them interfere in each other's state.

  Given the trouble with that in regular code, like `clock`, I would say that this experiment has failed.

  As such I would rather disable/deprecate/remove `coro::auto` entirely, i.e. give up on automatic support for code in both regular and coro contexts, and be explicit when writing functionality about the context it should be in.

  On the other hand, some of the ideas apparently do work (i.e. channel IO support). Given that a middle position would be to keep the support which works, and drop the other things, i.e. `global`, or rework them to not interfere with the standard builtin.

  That actually matches the proposed `real fix` in the description of the ticket.

  In that case the package is however not really `auto` anymore.

  Partially yes, for IO, and partially not, for scoping and variables.

  Do we have a better name for the package modified this way ?

  On the issue of refering to a cor-global outside of the coro ... AFAIK that is not possible at all.


login: tom@wildturkeyranch.net
username: tomturkey
mimetype: text/plain
icomment:
  I am not sure I would agree on the "not really  'auto'".  I found that 
  the package with out the 'global' was very useful. (I am working with 
  vwait and after to do waits that time out and refer to the global for 
  vwait as ::foo)

    I did not mention the additional issue around 'global' of a global 
  being referenced as "::foo" as well as "global foo".

  On the issue of referring to a coroutine variable from outside of the coro I think what may do the trick is to lean on "namespace", possibly such that a coroutines globals (corglobals) are in the namespace that seems to be created for each command already. One might then write:
  proc ::coroutine::util::global {args} {
    foreach corglobal $args {
       variable [info coroutine]::$corglobal
     }
  }
  This would create a variable that can be accessed from anywhere including as a textvariable. And if invoked while not in a coroutine, creates a simple global.
  I realize this is not "auto" but it is clear to me that corglobals are NOT the same thing as our other globals. I think this also solves the problem of referring a coroutine global from the true global space.

  This is, of course, NOT part of "auto" but of coroutine. Also I would suggest renaming "::coroutine::util::global" to "coroutine::util::corovariable" just to make it clear that this is a coroutine variable.


login: aku
mimetype: text/plain
icomment:
  Ok. So this is about eliminating symbol chains.

  Do you have a patch implementing this feature ?
  (preferably with tests ?)

  This looks to be quite a lot of work to be done from meta grammar to interpreter and the various coded engines. Especially for something which can be implemented as a post-parse tree-transformation.


login: ricardo-andre
username: about "puts [expr 8.2+0.2] ..."
mimetype: text/plain
icomment:
  Hi.
  I noticed that the corrections were published, except for the

  puts [expr 8.2+0.2] ; # => will output 8.399999999999999
  puts [expr 8.2-0.2] ; # => will output 7.999999999999999

  Can you tell if it was intentional or a error with the patch or ... and why?
  Thanks.


login: aku
mimetype: text/plain
icomment:
  Pulled the relevant commits from the closed PR in.


  use -nocase switch in regsub commands to make regex shorter 

  ---- modules/ncgi/ncgi.tcl
  @@ -271,11 +271,11 @@ proc ::ncgi::decode {str} {

      set str [string map [list + { } "\\" "\\\\" \[ \\\[ \] \\\]] $str]

      # prepare to process all %-escapes
  -   regsub -all -- {%([Ee][A-Fa-f0-9])%([89ABab][A-Fa-f0-9])%([89ABab][A-Fa-f0-9])} \
  +   regsub -all -nocase -- {%([E][A-F0-9])%([89AB][A-F0-9])%([89AB][A-F0-9])} \
  	$str {[encoding convertfrom utf-8 [DecodeHex \1\2\3]]} str
  -   regsub -all -- {%([CDcd][A-Fa-f0-9])%([89ABab][A-Fa-f0-9])}                     \
  +   regsub -all -nocase -- {%([CD][A-F0-9])%([89AB][A-F0-9])}                     \
  	$str {[encoding convertfrom utf-8 [DecodeHex \1\2]]} str
  -   regsub -all -- {%([0-7][A-Fa-f0-9])} $str {\\u00\1} str
  +   regsub -all -nocase -- {%([0-7][A-F0-9])} $str {\\u00\1} str

      # process \u unicode mapped chars
      return [subst -novar $str]
  ----
  improved handling of one byte encodings

  In practice www-url-encoded POST params can use encodings other than UTF-8
  (think of legacy Tcl applications that use one of the ISO-8859-x charsets). In
  this case URL parameters can contain references to 8-bit code points (in the
  form of `%[A-F0-9][A-F0-9]`) that are not valid UTF-8 code points.

  For example, `%DC` can be used as a percent encoding for the german umlaut `Ü`
  (if a Tcl application is based on ISO-8859-1). Currently, the `decode`
  procedure does not decode `%DC` as all one byte UTF-8 code points must start
  with `[0-7]`.

  This commit improves the handling of one byte percent encoded non-ASCII
  characters. It allows to use ncgi in application contexts that do not use
  UTF-8 as the base encoding.

  ---- modules/ncgi/ncgi.tcl
  @@ -275,7 +275,7 @@ proc ::ncgi::decode {str} {

  	$str {[encoding convertfrom utf-8 [DecodeHex \1\2\3]]} str
      regsub -all -nocase -- {%([CD][A-F0-9])%([89AB][A-F0-9])}                     \
  	$str {[encoding convertfrom utf-8 [DecodeHex \1\2]]} str
  -   regsub -all -nocase -- {%([0-7][A-F0-9])} $str {\\u00\1} str
  +   regsub -all -nocase -- {%([A-F0-9][A-F0-9])} $str {\\u00\1} str

      # process \u unicode mapped chars
      return [subst -novar $str]
  ----


login: pooryorick
mimetype: text/x-fossil-wiki
icomment:
  This is a task for user agents, and mime is fit for purpose in this regard.  User agents are free to add arbitrary headers and to provide arbitrary content types using the -canonical option.  For incoming messages the mime module faithfully parses these parts and passes the information along to the user agent.


login: aku
mimetype: text/plain
icomment:
  I suspect that the `$sstot` in the expression is 0, making it a div-by-zero issue. Handing to Arjen.


login: arjenmarkus
mimetype: text/plain
icomment:
  Yes, the problem occurred in the calculation of the correlation coefficient. I have introduced a special case for this situation, returning a value of 1.


login: aku
mimetype: text/plain
icomment:
  What does the error look like ?

  A 'package require foo 1.0' should be ok to load an '1.1' implementation
  (because -exact is not given later versions are acceptable).


login: bovine
mimetype: text/plain
icomment:
  This is what we are seeing logged in our /var/log/httpd.log under Apache Rivet with a global error handler:

  ERRINFO: can’t find package md4c 1.0 while executing “package require md4c 1.0”


login: bovine
mimetype: text/plain
icomment:
  $ tclsh

  tclsh8.6 [/tmp] package require md4c 1.0
  can't find package md4c 1.0
  while evaluating package require md4c 1.0
  tclsh8.6 [/tmp] package require md4c
  can't find package md4c
  while evaluating package require md4c
  tclsh8.6 [/tmp] package require tcllibc
  0.3.15
  tclsh8.6 [/tmp] ::md4::md4c hello
  d7ËzyKÎ+rzÌbî'


login: aku
mimetype: text/plain
icomment:
  Reading

  > $ tclsh
  > tclsh8.6 [/tmp] package require md4c 1.0
  > can't find package md4c 1.0
  > while evaluating package require md4c 1.0

  What happens if you as for `md4`, not `md4c` ?

  I believe that it should find and load `md4`, and that should look for and load `tcllibc`, getting you the C implementation of md4.

  > tclsh8.6 [/tmp] package require tcllibc
  > 0.3.15
  > tclsh8.6 [/tmp] ::md4::md4c hello

  The `make critcl` target builds `tcllibc` which contains all the C parts of Tcllib.

  I don't think that I have Makefile targets to build just the `md4c` part, etc.
  Although that might be doable via `sak.tcl critcl md4c`.

  Tclhttpd looks to be out of date in this regard, trying to get just `md4c`.


login: bovine
mimetype: text/plain
icomment:
  That works.

  $ tclsh
  tclsh8.6 [/tmp] package require md4
  1.0.6
  tclsh8.6 [/tmp] ::md4::md4c hello
  d7ËzyKÎ+rzÌbî'

  So either the pkgIndex.tcl is missing a line for md4c, or the ineffective "package require md4c" should be changed/removed from modules/md4/md4.tcl


login: aku
mimetype: text/plain
icomment:
  Removing the ineffective `package require` command.
  Review of the sources shows that md4 has a `LoadAccelerator` command which will be invoked at the end to conditionally look for boosting C code (`critcl`, `cryptkit`), and the `critcl` part looks for both `tcllibc` and `md4c` anyway.

  Bumping to version 1.0.7.

  Changed with commit [dba79cf013].

  Thank you for the report. (And your patience).


login: ssoberni
mimetype: text/plain
icomment:
  As hinted at in IRC, it turned out that the FFP was erroneously dropped due to using the equivalent of i_error_clear_push rather than of i_error_push in three out of four *_branch (choice) super-instructions:

  si:voidvalue_branch
  si:valuevoid_branch
  si:valuevalue_branch

  This was fixed for the TclOO and Snit RDE (the C one was not affected, naturally). NX benefits from the TclOO fix directly, as it derives from the TclOO engine.

  Branch ssoberni-ffp-3ed39a451f [*] contains a fix plus test. The test is WIP, because I don't exactly know how to best instrument the test runner to cover for this failure case? Here, the combined "failure" condition would be (as signalled by parset):

  - matched prefix ne input /xor/ suffix ne ""
  - the content of myerror with the FFP

  Pls. advise.

  [*] https://core.tcl.tk/tcllib/timeline?r=ssoberni-ffp-3ed39a451f


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Just pushed a commit on the branch which fixes the tests.

  See [d58dafb1140c96d6].

  It uses the "!." construction to force the parser to reject anything which does not consume the entire input ("!" = negative look-ahead, "." = any character, i.e. at the given point are "no more characters allowed").

  After that I fixed the ok example. It was a copy of the fail example, so I just had to add the missing semicolon.

  In the error results the report location was not quite right, got fixed.

  Due to my grammar changes I had to update the ok result as well.

  If you like that I will merge into trunk.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Ok, and merged. Commit [623801c583].

  Running the other branch (pt-container-ssoberni) through this ticket as well, keeping it open.


login: aku
mimetype: text/plain
icomment:
  For the future, please use unified diffs, i.e `diff -u`.
  These are easier to apply.

  Fixed with commit [5006d62d3e].

  Bumped version to 0.4.


login: aku
mimetype: text/plain
icomment:
  Review started.
  Don't seem to have a Test tclsh with TclTLS installed.
  May have to run against the user tclsh (an AT) for this.


login: aku
mimetype: text/plain
icomment:
  Notes:
  - Bump version of crc16 -- Bugfix - Patchlevel
  - Bump version of autoproxy - While done new (backward compatible) functionality prompts a change of the minor version, not patchlevel.


login: aku
mimetype: text/plain
icomment:
  Testing ok ...

  hephaistos:(621) ~/Play/Tcllib/dev.tcllib-pat > ./sak.tcl test run -l X crc http comm pop3
  [    ]  Starting ...                                                   
  [    ]  [8.4.20] crc  ~~       T   135 P   135 S     0 F     0
  [    ]  [8.4.20] http ~~       T    16 P    16 S     0 F     0
  [    ]  [8.4.20] comm ~~       T    22 P    21 S     1 F     0
  [    ]  [8.4.20] pop3 ~~       T    45 P    44 S     1 F     0
  [    ]  [8.5.18] crc  ~~       T   135 P   135 S     0 F     0
  [    ]  [8.5.18] http ~~       T    16 P    16 S     0 F     0
  [    ]  [8.5.18] comm ~~       T    22 P    21 S     1 F     0
  [    ]  [8.5.18] pop3 ~~       T    45 P    44 S     1 F     0
  [    ]  [8.6.4]  crc  ~~       T   135 P   135 S     0 F     0
  [    ]  [8.6.4]  http ~~       T    16 P    16 S     0 F     0
  [    ]  [8.6.4]  comm ~~       T    22 P    21 S     1 F     0
  [    ]  [8.6.4]  pop3 ~~       T    45 P    44 S     1 F     0

  hephaistos:(622) ~/Play/Tcllib/dev.tcllib-pat > ./sak.tcl test run -l X -s /home/aku/opt/ActiveTcl/bin/tclsh crc http comm pop3
  [    ]  Starting ...                        
  [    ]  [8.5.14] crc  ~~       T   210 P   210 S     0 F     0
  [    ]  [8.5.14] http ~~       T    16 P    16 S     0 F     0
  [    ]  [8.5.14] comm ~~       T    22 P    22 S     0 F     0
  [    ]  [8.5.14] pop3 ~~       T    45 P    45 S     0 F     0

  The second run is with a tclsh having tls. The others are without tls.


login: aku
mimetype: text/plain
icomment:
  Various failures in the full test which have nothing to do with the change, it looks like.

  Will merge.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Confirmed with testing on trunk that all the failures are on trunk, and caused by the changes.

  Merged. Fixed doc issues, and regenerated docs.
  Fixed a split in the trunk due the merge.

  Commit [ca0b7e688f is final merge result.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Reviewed. Looks very sensible and ok to me.
  The original code definitely had the list missing, and the bad argument ref (`verify` vs `verify_cert`). See also `ldap::secure_connect`, where it was done correctly.

  Looking at the test suite it only checks the wrong#args of the various commands, but not their actual operation, which explains how this managed to fly through. :(

  Created ticket [95cf373829] for this flaw.

  Version bumped to 1.9.1.

  Fixed with commit [510aa7c21d].


login: aku
mimetype: text/plain
icomment:
  Commit [fb859d7803] fixes all the places using `$math::` in `math::exact`.
  Further fixed relative proc definitions to be absolute.

  Unclear if that fixes all the places.

  Currently not running tests against 9.0a0. Actually I do, and see how the change of the major version throws all the `package require` commands and version checks out of wack. So, no usable results. And making the entirety of Tcllib friendly to 9 will be a major undertaking.

  Keeping ticket open, at lower priority.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Commit [d50864197b] fixes all the places using `$yaml::` (and associated `info exists` commands.

  Unclear if that fixes all the places.

  Currently not running tests against 9.0a0. Actually I do, and see how the change of the major version throws all the `package require` commands and version checks out of wack. So, no usable results. And making the entirety of Tcllib friendly to 9 will be a major undertaking.

  Keeping ticket open, at lower priority.


login: aku
mimetype: text/plain
icomment:
  Placed this under `Documentation` as the nearest category.
  While `Website` might be better we currently don't have that - TODO also.

  As this is something which should go into the feedback section of the manpages as well `Documentation` fits too.


login: aku
mimetype: text/plain
icomment:
  Category `Website` added.

  `New ticket` form updated.

  Still to do the manpage feedback section.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Updated manpage feedback. See commit [00329f8b8f].

  This completes the actions needed for this ticket.


login: oehhar
username: oehhar
mimetype: text/x-fossil-wiki
icomment:
  For the records, the patch was tested with success by Harald and Alexandru (see the clt thread).

  Thus I favor to merge it.

  Thank you,
  Harald


login: aku
mimetype: text/plain
icomment:
  Re-opened. Still to merge. Currently investigating Linux testsuite failures.
  Notes:

  - Tweaked the doc setup to have the custom tls note part of the second, and not before

  - Added Tcl 8.5 requirement to have `in`/`ni` operators.


login: aku
mimetype: text/plain
icomment:
  I see no difference between the tests autoproxy 2.0.0 and 2.0.1, except the title and constraints. Setup, body, and cleanup are identical. Ditto for 2.1.0 and 2.1.1

  I further see multiple -setup declarations in these tests.
  I do not believe that both will be run, only the second.

  I wonder if that may be the issue, i.e. packageReset missing.


login: aku
mimetype: text/plain
icomment:
  Pushed a fix for the multiple -setup clauses.

  This did not fix the linux test failures.

  Investigation continues


login: aku
mimetype: text/plain
icomment:
  I believe I know.

  The last test to make a successful configuration change is 1.2.4.1, setting tls_package to twapi.

  And for two, the `packageReset` procedure does not reset the new tls_package information.

  Thus it leaks to the 2.0.0 test. And as the 2.x tests do not configure the package for themselves they all think to use twapi.

  That works of course works in an environment which has twapi.

  Fixing now ...


login: gold
username: gold
mimetype: text/plain
icomment:
  gold, Possibly related ticket, i am looking in stacks.
  Attachment "extrafuncs.tcl" to ticket [641143ffff] added by dkf on 2006-03-16 16:27:34.


login: arjenmarkus
mimetype: text/plain
icomment:
  I have used this to implement a full set of trigonometric functions and their inverses, as well as some additional hyperbolic functions and their inverses. They can be found in the math::trig package in Tcllib's math module.


login: arjenmarkus
mimetype: text/plain
icomment:
  What I forgot to mention is that the implementation of the degrees version of sin, cos, etc. is such that multiples of 90 degrees will result in exact values, that is: sind(180) = 0, cosd(90) = 0, sind(90) = 1.

  This is achieved by reducing the angle to an interval -45 to 45 degrees first and keeping track of the quadrant (rotated over 45 degrees) in which the original angle lies. Then converting to radians and applying basic trigonometric formulae.

  No such claim of exactness is made for other angles for which one or more of the sine/cosine/... functions yield a rational value.


login: anonymous
username: gold
mimetype: text/plain
icomment:
  to akupries 
  subj. are examples needed on wiki or man page?
   
  Thank you. This seems really exciting work, which wiil keep TCL in pace with
  some of the other brand name (math oriented) languages. However, are examples needed on wiki or TCL/TCLLIB/UNIX man page?


login: arjenmarkus
mimetype: text/plain
icomment:
  A drawback of this method is that roundoff errors will accumulate if the angles are large. Furthermore, perhaps more important, the calculation can be done quite efficient by simply calculating the number of 2pi's that are needed.


login: arjenmarkus
mimetype: text/plain
icomment:
  I have now implemented a more direct version of such a reduction procedure for arguments in both radians and degrees. Part of the new math::trig package that contains a lot more mathematical functions - see ticket  https://core.tcl-lang.org/tcllib/tktview/21fef042b91ca252fc2a20ead5cd05c2b51b1776

  The package is part of Tcllib's math module.


login: anonymous
username: gold
mimetype: text/plain
icomment:
  to akupries 
  subj. are examples needed on wiki or man page?
   
  Thank you. This seems really exciting work, which will keep TCL in pace with
  some of the other brand name (math oriented) languages. However, are examples needed on wiki or TCL/TCLLIB/UNIX man page? Probably I should revamp cited wiki page to reference your recent TCCLIB contribution. Thanks again, gold


login: aku
mimetype: text/plain
icomment:
  From the description this ticket does belong with https://core.tcl.tk/tcl/
  instead of here. I also recommend to provide them with a short snippet/excerpt of the C code where you have the trouble.


login: zakhmak
username: C code example
mimetype: text/plain
icomment:
  Hello,
  You will find a C code example that reproduced this problem.

  Ok I trust you, it's just that it concerns a C function of the Tcl API, I thought the tcllib was the right place for this. 
  So should I recreate a duplicate ticket at tcl section or is there any possibility to transfer it automatically? Both belong to core.tcl.tk/ website apparently.

  Greetings

  #include <tcl.h>

  int
  Test_ObjPrintfObjCmd(
      ClientData clientData, Tcl_Interp *interp,
      int objc, Tcl_Obj *CONST objv[])
  {
      Tcl_DString dynstr;
      Tcl_Obj *obj = NULL;
      char *text = NULL, buftxt[128] = "";
      static const char *modes[] = {"without_ObjPrintf","with_ObjPrintf",NULL};
      static const char formatstr[] = "answer is at %p and its value is %d";
      int ok, imode = 0, answer = 42;

      Tcl_DStringInit(&dynstr);

      if (objc != 2) {
          sprintf(buftxt, "Usage : Test_ObjPrintf mode");

          Tcl_WrongNumArgs(interp,0,objv,buftxt);
          return TCL_ERROR;
      }

      ok = Tcl_GetIndexFromObj(interp, objv[1], modes,
              "unknown mode", TCL_EXACT, &imode);
      if (ok == TCL_ERROR) return TCL_ERROR;

      switch (imode) {

        case 0: /* without_ObjPrintf */

          sprintf(buftxt, formatstr,&answer, answer);

          obj = Tcl_NewStringObj(buftxt,-1);

          break;

        case 1: /* with_ObjPrintf */

          obj = Tcl_ObjPrintf(formatstr,&answer, answer);

          break;
      }

      if (obj == NULL) return TCL_ERROR;

      text = Tcl_GetStringFromObj(obj,NULL);
      if (text == NULL) return TCL_ERROR;

      Tcl_DStringAppend(&dynstr,text,-1);
      Tcl_DStringResult(interp,&dynstr);

      return TCL_OK;
  }

  int
  Pb_objprintf_Init(Tcl_Interp *interp) {

      Tcl_Command cmd;

      cmd = Tcl_CreateObjCommand(interp, "Test_ObjPrintf",
          Test_ObjPrintfObjCmd,
          (ClientData) NULL, (Tcl_CmdDeleteProc *) NULL);
      if (cmd == NULL) return TCL_ERROR;

      return TCL_OK;
  }


login: aku
mimetype: text/html
icomment:
  <pre>
  > Ok I trust you, it's just that it concerns a C function of the Tcl API,
  > I thought the tcllib was the right place for this. 
  </pre>

  <p> No. Apologies for the confusion. If you go looking at the
  <a href="https://core.tcl.tk/tcllib/doc/trunk/embedded/www/toc.html">Tcllib documentation</a> you will find that <code>tcllib</code> is a big bag of useful packages written in Tcl.

  <p> The <code>TclLib</code> you were thinking of is the C API of Tcl itself, see its <a href="https://www.tcl.tk/man/tcl8.6/TclLib/contents.htm">manpages</a>. This is an integral part of the <code>Tcl</code> interpreter sources and thus found at <a href="https://core.tcl.tk/tcl">https://core.tcl.tk/tcl/</a>

  <p> I will go and see what I can do to make the descriptions on <a href="https://core.tcl.tk/">https://core.tcl.tk/</a> better and less confusing.

  <pre>
  > So should I recreate a duplicate ticket at tcl section
  </pre>

  <p> Yes please.

  <pre>
  > or is there any possibility to transfer it automatically?
  </pre>

  <p> Unfortunately not. They are different fossil repositories with independent Ticketing systems.

  <pre>
  > Both belong to core.tcl.tk/ website apparently.
  </pre>

  <p> Yes. Like various other Tcl-related projects.

  <pre>
  > https://core.tcl.tk/tcl/tktview/e5691de2eb49302428e77726a057449a06f499da
  </pre>

  <p> Thank you.


login: usmankhalid
mimetype: text/plain
icomment:
  From here on page 5 it is already mentioned the correct way of parser handling such space.

   https://tools.ietf.org/html/rfc2849

  2)  Any non-empty line, including comment lines, in an LDIF file
            MAY be folded by inserting a line separator (SEP) and a SPACE.
            Folding MUST NOT occur before the first character of the line.
            In other words, folding a line into two lines, the first of
            which is empty, is not permitted. Any line that begins with a
            single space MUST be treated as a continuation of the previous
            (non-empty) line. When joining folded lines, exactly one space
            character at the beginning of each continued line must be
            discarded. Implementations SHOULD NOT fold lines in the middle
            of a multi-byte UTF-8 character.


login: usmankhalid
username: proc Lexical {selfns} in ldapx.tcl might need to correct
mimetype: text/plain
icomment:
  Hello 

  just an additional hint. I beleive the problem might be in this proc in ldapx.tcl file. thanks.

  proc Lexical {selfns}


login: usmankhalid
username: working
mimetype: text/plain
icomment:
  Hello
  Thanks for the quick fix, it is working fine. 
  Will you release it? 
  How can I get this updated ldap directory? will there be a new release of tcllib where this patch will be available as well.
  another question. why ldapx new version written 0.2.5 but when you see in latest tcllib it is 1.0. In older tcllin it was 0.2.4. So this is confusing. Could you explain briefly. thanks.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Patch applied, and tested (testsuite extended).

  Commit [41b91c0848

  Thank you to usmankhalid for the report, and to Pierre for the fix.

  Questions:
  - We are currently working on making release 1.19.
    The commit is done on the branch with the release work
    It is planned to make an RC tarball later this weekend.

  - I suspect there was some mixup with version numbers in the past.
    The new version with the fix is 1.1 and that should be in all places.
    One of the changes I made was to update the .man file with all the
    correct numbers.


login: aku
mimetype: text/plain
icomment:
  So the commit and it looked easy enough.

  The change to the debugging can be seen as bug fix, with patchlevel version increment. You actually describe it as such (... broken ... I fixed ...)


login: ssoberni
mimetype: text/plain
icomment:
  Agreed. I bumped the patch level. I also refactored some [expr] calls in [f94401b3e2], I hope this is fine with you?


login: anonymous
username: stacy.doss@gamil.com
mimetype: text/plain
icomment:
  I discovered that the code uses error to exit RT.CacheConfigureCommand, the client code I am working with aliases error and does some extended processing.

  So the error is introduced by the client code.

  However it may make more sense to use the return -code error syntax.

  #Stacy


login: pooryorick
mimetype: text/x-fossil-wiki
icomment:
  The functionality was already incorporated into fileutil::magic::filetype, and the documentation was changed to reflect that.


login: anonymous
username: dzach
mimetype: text/plain
icomment:
  I think I missed the point that the value is a leaf, therefore a key ending in a ':' would direct 'dict print' to print the value as is. 

  Therefore this is not a bug.

  dzach


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Comments in the code:

  https://core.tcl.tk/tcllib/artifact/a4ca98d41fd10afe?ln=592-595

  indicate that ports should be matched to servers, i.e. indeed be iterated together.

  The increment at
  https://core.tcl.tk/tcllib/artifact/a4ca98d41fd10afe?ln=677
  looks indeed to be much too late, what with the two `continue`s at
  https://core.tcl.tk/tcllib/artifact/a4ca98d41fd10afe?ln=652
  and
  https://core.tcl.tk/tcllib/artifact/a4ca98d41fd10afe?ln=669

  Moving the increment just after
  https://core.tcl.tk/tcllib/artifact/a4ca98d41fd10afe?ln=633
  should work

  The proposed nested iteration will not match servers and ports as documented. It would try each port with each server, i.e do a cartesian product of both, like a SQL join.

  However

      foreach server  $options(-servers) $port $options(-ports) { ... } 

  might do what we want, if it fills missing elements with the empty string when on of the lists is too short.

  That would avoid all the index calculations entirely.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Committed a tentative fix to branch [smtp-init-tkt-d9be31a488].

  This is commit [9c0c77acf0].

  Please get this branch or commit and check if that fixes the problem you have with the command.


login: aku
mimetype: text/plain
icomment:
  Franco, can you please make a unified diff (`diff -u`) of your changes, and attach them here ? (The `attach` button is the left-most in the secondary navigation bar. You may have to log in first). In case of trouble with that, make it a comment. The important part to me is to get the patch as `unified diff`, because that makes application much easier. A patch as given in the description I would have to apply very much manually, i.e. search the locations, etc.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Thank you. I have applied it locally now, although not yet committed, nor pushed.

  While working on the documentation to describe your new option I had a thought and question:

  How are you handling when the TLS package is not present ?

  The code seems to assume that TLS is already present, and simply fails if not.

  The other parts of smtp.tcl (support for STARTTLS) attempt to `package require tls` and the -tlspolicy option then governs what happens if that fails (error vs continue insecure).

  With -tlsimport / native TLS I do not see how it could continue unsecure, so I guess -tlspolicy will not be relevant. However attempting to load tls is something I believe could be added.

  ... Read the code around the new `tls::import` some more. It is in a catch, so a failed `tls::import` (whether through missing package or failed cert) will close the socket and continue trying with the next server. Like tlspolicy `secure`. Good. I believe I will put a `package require tls` in that place to make the error better when the package is missing, vs negotiation issues.

  ...

  Ok, this is now in branch [smtp-tlsimport-tkt-83d674b2dd], commit [657782a889].

  Please retrieve this and verify that it still works for you as intended.


login: anonymous
username: Testing
mimetype: text/plain
icomment:
  I agree with your comments. I have tested succesfully the module downloaded from the trunk. Thank you. Franco.


login: aku
mimetype: text/plain
icomment:
  Agreed on (1), run connection setup non-async.
  Agreed on (2).
  Agreed on (3).
  Agreed on (4). Depending on how the arguments are passed / options are handled this might be automatic, i.e. with secure_connect accepting new options the connect will do as well. -- I haven't looked into the code yet.

  Looking for patches.


login: aku
mimetype: text/plain
icomment:
  > Looking for patches.

  To clarify: Please provide patches, and/or a branch of Tcllib containing the changes.


login: pda
mimetype: text/x-fossil-wiki
icomment:
  1) this problem does not occur when using ldap::connect then ldap::starttls

    ... but I don't understand why...


login: aku
mimetype: text/x-fossil-wiki
icomment:
  I have placed the patch into commit [337c7e5654] on branch `ldap-60160205fe-tls`.

  The only thing I see missing are basic tests for the new command (`::ldap::tlsoptions`), and if possible the extended syntax of the changed commands (`starttls`, `secure_connect`). I say `basic` because we currently do not have anything better than that for the changed commands.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Patch containing new test reviewed and applied.

  Branch completed, merged to trunk.

  Merge commit [5e2cff31b4].

  Closing the ticket.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Hi Jerry. Thank you for the report.

  Are you referring to the following sentence in the RFC ?

  <verbatim>... characters that must be escaped: quotation mark, reverse solidus, and the control characters (U+0000 through U+001F). ...</verbatim>

  If yes, note also the immediately following sentence:

  <verbatim>Any character may be escaped.</verbatim>


login: Jerry
mimetype: text/plain
icomment:
  Hi aku,

  Thanks for the reply!

  Yes, I was, but I'm a bit confused now. Is there then a reason as to why this list is escaping only this specific set of characters?

  Maybe I'm missing something.

  I guess I will have to handle those characters separately.


login: oehhar
username: oehhar
mimetype: text/x-fossil-wiki
icomment:
  Here are some hints and thoughts, no help for implementation etc.

  The work done for TIP490 "oo for msgcat" may be helpful here: [http://core.tcl.tk/tips/doc/trunk/tip/490.md].

  See the section "Implementation" of the TIP. It contains the following function to detect the oo-environment of the callers-caller:

  <verbatim>
  proc ::msgcat::PackageNamespaceGet {} {
      uplevel 2 {
  	# Check for no object
  	switch -exact -- [namespace which self] {
  	    {::oo::define::self} {
  		# We are within a class definition
  		return [namespace qualifiers [self]]
  	    }
  	    {::oo::Helpers::self} {
  		# We are within an object
  		set Class [info object class [self]]
  		# Check for classless defined object
  		if {$Class eq {::oo::object}} {
  		    return [namespace qualifiers [self]]
  		}
  		# Class defined object
  		return [namespace qualifiers $Class]
  	    }
  	    default {
  		# Not in object environment
  		return [namespace current]
  	    }
  	}
      }
  }
  </verbatim>

  This is for TCL8.7 and only tcloo. Additional work might be necessary for tcl 8.5-6 and IncrTCL.

  Hope this may help for an implementation,
  Harald


login: aku
mimetype: text/plain
icomment:
  Sean, can you look into this, please ?
  This might have to be reported to Caius as well.
  Or maybe a fix ported over if they already have one.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fixed with commit [b39206d300].

  Thanks for the report and fix. Also added sha1, base64 to the support.

  No version bump.


login: hypnotoad
username: hypnotoad
mimetype: text/plain
icomment:
  That patch also needs to ignore any source code in a directory named /build that contains a file named build.tcl. All of those files are ingredients to produce the final module file, and may very well contain patterns that will frighten and confuse an automated package indexer.

  The concept of the amalgamator for certain modules is to break the implementation up into smaller logical chunks, as well as provide a means to produce the tweaks needed to build repetitive regression tests as well as compile data files into source code snippets.

  These modules are rebuilt but running tclsh build/build.tcl

  The output is the $module.tcl file as well as the pkgIndex.tcl file and (sometimes) the $module.test file.

  The proper patch would be:

  if {[file isdirectory $f] && [file tail $f] eq "build"} {continue}

  This matches a similar existing exclusion in the sak installer code.


login: hypnotoad
username: hypnotoad
mimetype: text/x-fossil-wiki
icomment:
  It appears that trunk missed a lot of updates from the hypnotoad branch that clarify to the sak.tcl system which parts of the file system to ignore.

  I would propose a merge of trunk tag:

  [c3a46560db6c7860df7989ebaa03ede47ff2d310]

  With the latest hypnotoad checkin:

  [e42366acb6804168f3aec267d82379222e1b01f1]


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fixed with merge commit [5b0076422d] bringing in hypnotoad changes, especially a restructuring which hides the sources and build code of his modules from sak and installer.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Thank you for the additional test cases.

  Had to put in some fixes to make them work, not much however: Disabled forgotten debug output in minimize, added forgotten pt_pexpr_op.tcl (pt::pe::op) to the set of supporting packages.

  Commit [8436048fee].


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Added some more tests, comments, notes.

  Commmit [d907079d5b].

  Regarding documentation question: I see no problem adding disclaimers about transform limitations and will accept PRs.

  Wrt start set for `drop unrealizable` ... I can see that this can ignore unreachable. Even if they are realizable they would either go away later in the minimize, or simply be ignored during execution of either grammar interpreter or a generated parser.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Branch integrated, see commit [3720e40747].

  And commit [4bbe140a79] makes the change to the `all` set of `drop unrealizable`.

  For the disclaimer in the docs please provide a PR/branch.

  Closing this ticket now as done.

  Thank you for your patience.


login: aku
mimetype: text/plain
icomment:
  Commit [b908e8c70e] is in preparation of work on this ticket.
  Basically lots of code changes were made to the derived files instead of the true source. This has been corrected.

  Now I can open a branch and add the reported issue as test cases.


login: aku
mimetype: text/plain
icomment:
  Ok.

  The primeFactors procedure does know that we should search only as far as sqrt(number) for prime factors. See the `primesLowerThan` call (pLT) at the beginning.

  But then the search loop runs while number > 1, allowing it to run over the end of the list of known prime numbers computed by pLT before. That happens if the number has a prime factor above the sqrt(number) limit.

  This is technically also influenced by the order of numbers we are factorizing. For our examples, factorize 208850, whose sqrt is above 457, and now the list is large enough that 914 and 964 factor without problem, as 451 and 457 are in it.


  The fix ... Check for `p == {}` i nthe loop and then add the remaining number as the last primefactor.


login: aku
mimetype: text/plain
icomment:
  >The Example section references the Examples of the ldap package as an example

  I do not see that, with apologies.

  Looking at the referenced asn doc page the relevant sentence in the example section says `found in the implementation of`. I.e. the code of ldap makes use of the asn package and command.

  And I have now confirmed that this is indeed (still) true.

  So, this report looks invalid to me.

  Even so, thank you for reporting, and please continue reporting things you believe are trouble.


login: anonymous
mimetype: text/x-fossil-wiki
icomment:
  > Looking at the referenced asn doc page the relevant sentence in the example section says `found in the implementation of`. I.e. the code of ldap makes use of the asn package and command.

  That is just plain ignorant documentation.  The link pointed to in that quoted sentence is `https://core.tcl.tk/tcllib/doc/trunk/embedded/www/tcllib/files/modules/ldap/ldap.html` which is the documentation for ldap and not the implementation of the asn code in the repository.  Something needs to change to show an example on the documentation page.


login: arjenmarkus
mimetype: text/plain
icomment:
  Hm, it turns out that the file numtheory.tcl contains only a small part of the original package. I will need to examine this more closely. This requires a new ticket, as I cannot currently apply this rightaway.


login: arjenmarkus
mimetype: text/plain
icomment:
  I was a trifle hasty with my conclusion. The procedures I was looking for are contained in the file primes.tcl. 

  Added the above convenience procedure and updated the tests and the manual.

  (Note to self: still a bunch of tests to be added!)


login: aku
mimetype: text/x-fossil-wiki
icomment:
  This fix has never merged to trunk so far. (It was done on branch `pooryorick`, was followed by lots of other changes. I remember then quite a lot of discussion around API (in)compatibility).

  The main line fix is commit [b2ae85b606] in response to ticket [57909d2e1c].
  Commit here is arguably a nicer fix.

  I should have a review of the other mime.tcl changes, see how much can be brought into trunk before running into the parts which got argued about.


login: arjenmarkus
mimetype: text/plain
icomment:
  I will create a new package for this proc and the ticket regarding combined_market_rate. These are fairly specialised algorithms and to make them recognisable, I think a "market" package would be better than merely putting them in one of the existing packages.


login: andrewm
username: amangogna
mimetype: text/plain
icomment:
  I'm assigning this bug to myself. The submitter has included the fix, so it will be a matter of applying it.

  Andrew Mangogna


login: aku
mimetype: text/plain
icomment:
  Thanks for that, and thanks for everybody's patience.
  Currently running the entire test suite as the last check.
  I do not expect to see issues, the small run just on the module was good, and I am not aware of dependent modules in Tcllib.


login: aku
mimetype: text/plain
icomment:
  At the moment the title comes across as 'this is a bug' (due to the phrase 'not work correctly' in it).

  The description on the other hand feels more like a feature request: 'make cmdline more similar to getopt'.

  Please provide an example which demonstrates what Tcllib's cmdline did versus what you expected it to do.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Fixed by Kevin. Applied by me (IIRC).

  See commits [c261a6a0eae8b4d4], and [74b6db9a0d].

  Branch [https://core.tcl-lang.org/tcllib/timeline?r=kbk-refactor-disjointset&c=2018-11-22+05%3A22%3A09] except for the final integration (c261a).


login: ethanp
mimetype: text/plain
icomment:
  To fully reproduce the huddle v0.1 behavior, the fix should be:
  proc ::huddle::wrap { args } {
      set num [llength $args]
      if {$num == 1} {
        return [list HUDDLE [lindex $args 0]]
      } elseif {$num == 2} { 
        # Below behavior is from huddle::wrap v0.1
        if {[lindex $args 0] ne ""} {
          return [list HUDDLE $args]
        } else {
          return [list HUDDLE [lindex $args 1]]
        }
      } else {
        error "huddle wrap expects 1 or 2 arguments"
      }
  }


login: aku
mimetype: text/x-fossil-wiki
icomment:
  I see a commit ([31f9e28d7c490954]) with an experimental fix, submitted Dec 31.
  How much is that fix tested ?


login: aku
mimetype: text/x-fossil-wiki
icomment:
  From kevin a more focused link:

  https://core.tcl.tk/tcllib/artifact?udc=1&ln=98-142&name=918f0d4f8ba645ae


login: hypnotoad
mimetype: text/plain
icomment:
  Based on the information from ink, I was able to craft a complete solutions to the problem I was trying to solve using existing tools in the library.

  Which pretty much negates the need to add any new features. So I will be closing this ticket.

  Thanks Andreas and Kevin for your help.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  I am unable to find "createLogXYPlot" in Tcllib.

  Is that maybe something for the Tklib tracker ?

  Checking ... Yeah, looks to be Tklib's "plotchart" package.

  Please enter into the tracker at https://core.tcl.tk/tklib/ticket

  Assigned to Arjen as heads up.

  Set to `not applicable here`, leaving closing of ticket to Arjen.


login: aku
mimetype: text/plain
icomment:
  Hi Jasper

  Do you have an example .sml file which does not look to contain private data with which I can test things ?

  If yes, please attach to the ticket.

  (See left-most button in the secondary navbar. Same navbar which has "Edit").


login: aku
mimetype: text/plain
icomment:
  Thank you very much.

  (Although I believe I will reduce the base64 part by quite bit when making the proper test case)


login: aku
mimetype: text/x-fossil-wiki
icomment:
  This trouble looks to have come in with commit [913f7d92c5c35342].

  From the annotation

  <pre>
  913f7d92c5 2013-11-01  674:          if {(!$blankP) && ([string last \r $line] == {$x - 1})} {
  </pre>

  No, we are not using { and } to brace sub-expressions. 

  The original code was

  <pre>
  if {(!$blankP) && ([string last "\r" $line] == [expr {$x-1}])} {
  </pre>

  The removal of the superfluous expr forgot to convert the braces into proper parens.

  Making this conversion and the new test case passes.

  Now to clean up up my debugging code and making this a proper fix.

  Why this would work on Windows ? My best guess is that the package used on the Windows machine in question is from before the commit introducing the problem. IOW different versions of the mime package, with and without the issue.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  And done with commit [b2ae85b606].

  Version bump: mime 1.6.1

  Thank you for the report and help in getting this tested.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Some cross-references.

  - See also ticket [a16b1095974e071d294ccbc0cf4671148bddab76].

  - Associated fix is commit [074ec6a961081540] on branch `pooryorick`,
    never merged. Arguably a nicer fix.

  - Should review the work on that branch, see what can be brought into trunk.


login: aku
mimetype: text/plain
icomment:
  Error is thrown by `::struct::graph::CheckE`, an internal argument validator.

  Likely from the use within the `nodes` method.

  4 calls of _nodes in _walk.

  As `_nodes $dir $node`, with $dir in -in, -out.

  The processing of -in, -out, etc. is not properly managing the node names.
  It should pull them directly when processing the filter instead of looping.
  Except, as it is documented we are allowed to say

      -in node1 node2 ... -value foo

  And now a node name like -bar is difficult to distinguish from an invalid option.

  Right now we default to treat it as invalid filter option.

  Not sure yet what trouble we may get into if we default the other way, i.e. treat as node name

  Actually we can influence that by the option we have seen, i.e. change treatment only after one of the filter options taking nodes and change back when reaching one of the other options.

  That might be our best bet for a heuristics.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Work is going on in branch https://core.tcl.tk/tcllib/timeline?r=doc-overhaul

  See https://core.tcl.tk/tcllib/doc/doc-overhaul/embedded/md/tcllib/files/devdoc/tcllib_installer.md

  Specifically
  https://core.tcl.tk/tcllib/doc/doc-overhaul/embedded/md/tcllib/files/devdoc/tcllib_installer.md#subsection5


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Commit [6612c2b8a881d7dd] brings the documentation work into trunk, making it official.

  With respect to this ticket, now see Section <a href="https://core.tcl.tk/tcllib/doc/trunk/embedded/md/tcllib/files/devdoc/tcllib_installer.md#subsection5">Critcl & Accelerators</a>
  of the Installation Guide.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  The xml package, i.e "TclXML" is not a part of Tcllib and has to be installed separately.

  References:

   * https://wiki.tcl-lang.org/page/TclXML

   * https://sourceforge.net/projects/tclxml

  Various linux distribution provide binaries

  Ubuntu:

      > wajig search tclxml

      tclxml - Tcl library for XML parsing

      tclxml-dev - Tcl library for XML parsing - development files

  OpenSUSE

  https://build.opensuse.org/package/show/devel:languages:tcl/tclxml


login: aku
mimetype: text/plain
icomment:
  Fixed with commit [5f3611f96a].

  Patch accepted as is for the fix.
  Added a testsuite for mkzip.
  Well, the seed of one.
  Currently covers only the basic wrong#args and this issue.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Done with commit [aeb3bec0da].

  Accepted with slight changes: 
  - Comment tweaking, and
  - Restoring the early bailout in `read` when no channels to handle.
  - Added test suite for tcl::chan::cat.

  It seems that just changing `fileevent` to `chan event` does the trick.
  And I am completely boggled why.
  These should have been equivalent.
  And they are not.
  I confirmed, I added a test case based on the given example, and the `vwait` used as event loop is stuck without the change, exactly as described.


login: aku
mimetype: text/plain
icomment:
  Hi Sean.

  Results of my testing against 8.4.20, 8.5.19, 8.6.9 (*).

  1. hypnotoad branch as is (365742ea93)

     Lots of test failures, looking to be issues fixed on trunk

  2. Merged trunk into hypnotoad. Staged local, not committed, not pushed.

     Still test failures, limited to 8.6.9, httpd:

  grep FAILED ../MS.faildetails 
  ==== modules/httpd/httpd.test httpd-client-0006 FAILED =========
  ==== modules/httpd/httpd.test httpd-proxy-0006 FAILED =========
  ==== modules/httpd/httpd.test scgi-client-0006 FAILED =========

  All have the same delta

  > ---- Result was:
  > Line 0 differs {HTTP/1.0 500 Server Internal Error} != {HTTP/1.0 200 OK}
  > Line 3 differs {Content-Length: 26} != {Content-Length: 134}
  > Line 5 differs {500 Server Internal Error} != {}
  > Line 6 differs {} != {if {![package vsatisfies [package provide Tcl] 8.6]} {return}}
  > Line 7 differs {} != {package ifneeded httpd 4.3.2 [list source [file join $dir httpd.tcl]]}
  > ---- Result should have been:
  >

  I am unsure were to look in the httpd sources to find the issue.
  An unexpected Tcl version ?
  That said the system Tcl versions I have are /usr/bin/tclsh 8.6.5
  and /home/aku/opt/ActiveTcl/bin/tclsh 8.6.7

  The testsuite was run by
  /home/aku/Play/Tcllib/1.TestSupport/8.6/Install/bin/tclsh8.6 8.6.9 (*)

  Note, this issue also happens when running just the httpd testsuite instead of everything.

  Full log for the scgi case at the end (**)


  Conclusion:

  - Not quite ready, but nearly so.

  (*) Code references:
      tcl-8.4/COMMIT = 649ee985ae
      tcl-8.5/COMMIT = 523e36b232
      tcl-8.6/COMMIT = 60d2391840

  (**)
  ---- scgi-client-0005 start
  ++++ scgi-client-0005 PASSED
  ---- scgi-client-0006 start
  ACTUAL {Status: 500 Server Internal Error
  Content-Type: text/plain
  Connection: close
  Content-Length: 26

  500 Server Internal Error
  }
  CORRECT {Status: 200 OK
  Content-Type: text/plain
  Connection: close
  Content-Length: 134


  if {![package vsatisfies [package provide Tcl] 8.6]} {return}
  package ifneeded httpd 4.3.2 [list source [file join $dir httpd.tcl]]

  }


  ==== scgi-client-0006 Return a file FAILED
  ==== Contents of test case:

  set reply [::scgi::test::send 10003 {REQUEST_METHOD GET REQUEST_URI /file} {}]

  set checkreply "Status: 200 OK
  Content-Type: text/plain
  Connection: close
  Content-Length: [string length $checkfile]

  $checkfile"
  ::httpd::test::compare $reply $checkreply

  ---- Result was:
  Line 0 differs {Status: 500 Server Internal Error} != {Status: 200 OK}
  Line 3 differs {Content-Length: 26} != {Content-Length: 134}
  Line 5 differs {500 Server Internal Error} != {}
  Line 6 differs {} != {if {![package vsatisfies [package provide Tcl] 8.6]} {return}}
  Line 7 differs {} != {package ifneeded httpd 4.3.2 [list source [file join $dir httpd.tcl]]}

  ---- Result should have been (exact matching):

  ==== scgi-client-0006 FAILED


login: aku
mimetype: text/plain
icomment:
  Wrt 1: Checking around I would say it is an issue with the markdown generator. The stars/asterisks are markdown syntax for emphasis. They are also limited to inline use, i.e. cannot cross lines, while the .man [emph] command is ok with application on multi-line text. The generator is not taking this into account and simply emits the md syntax without breaking lines apart and marking each separately.

  Wrt 2: The trailing bracket is actually present. It just somehow moved to the next line, after the example. Checking ... In the .man it is definitely part of the example.  And in the generated .md it is separated from the example by a blank line. So the input is ok, and the fossil markdown rendering does the correct thing too. The issue is in the markdown generator. Again.

  So, both problem are issues with the new markdown generator, and not with the manpage.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Ref 2: A missing flush of accumulated expander text in `example_end` caused the bracket to become part of the plain text after the example instead of being part of it.

  Ref 1: Fixed as envisioned

  Fixed with commit [f5786278a4].

  Version bump 1.5.5.

  Thank you for the report.


login: anonymous
mimetype: text/plain
icomment:
  --- blowfish-1.0.4.tcl	2019-06-06 09:52:21 +0200
  +++ blowfish-1.0.5.tcl	2019-06-06 10:02:41 +0200
  @@ -509,18 +509,27 @@
       if {[eof $in]} {
           fileevent $in readable {}
           set state(reading) 0
  -    }
  -
  -    set data [read $in $chunksize]
  -    # FIX ME: we should ony pad after eof
  -    if {[string length $pad] > 0} {
  -        set data [Pad $data 8]
  -    }
  -
  -    if {$out == {}} {
  -        append state(output) [$state(cmd) $Key $data]
  +        set data $state(remainder)
  +        # Only pad at the end of the stream.
  +        if {[string length $pad] > 0} {
  +            set data [Pad $data 8 $pad]
  +        }
       } else {
  -        puts -nonewline $out [$state(cmd) $Key $data]
  +        set data [read $in $chunksize]
  +        puts "Chunk: reading [string len $data] bytes"
  +        set data $state(remainder)$data
  +        # If data is not a multiple of 8, state(remainder) will hold
  +        # excess bytes for the next round.
  +        set pagedlen [expr {([string length $data] / 8) * 8}]
  +        set state(remainder) [string range $data $pagedlen end]
  +        set data [string range $data 0 $pagedlen-1]
  +    }
  +    if {[string length $data] > 0} {
  +        if {$out == {}} {
  +            append state(output) [$state(cmd) $Key $data]
  +        } else {
  +            puts -nonewline $out [$state(cmd) $Key $data]
  +        }
       }
   }
   
  @@ -585,7 +594,7 @@
   proc ::blowfish::Pad {data blocksize {fill \0}} {
       set len [string length $data]
       if {$len == 0} {
  -        set data [string repeat $fill $blocksize]
  +        # do not pad an empty string
       } elseif {($len % $blocksize) != 0} {
           set pad [expr {$blocksize - ($len % $blocksize)}]
           append data [string repeat $fill $pad]
  @@ -682,6 +691,7 @@
               set state(cmd) Decrypt
           }
           set state(output) ""
  +        set state(remainder) ""
           fileevent $opts(-in) readable \
               [list [namespace origin Chunk] \
                    $Key $opts(-in) $opts(-out) $opts(-chunksize) $opts(-pad)]


login: arjenmarkus
mimetype: text/plain
icomment:
  I have added my own implementation - missed the attached diffs. There are two new procedures, one that produces random numbers with a "skewed" triangular distribution and one that produces random numbers with a symmetric triangular distribution as per the JCGM document.


login: arjenmarkus
mimetype: text/plain
icomment:
  The implementation is slightly less general than the patches by Emiliano, which I had missed. For now, though, the functionality should be enough, so closing the ticket.


login: rkeene
mimetype: text/plain
icomment:
  This is ultimately a documentation issue  the [::pki::sign] command returns
  a PKCS#1 v1.5 signature which embeds the hashing algorithm as an ASN.1
  header to the data    UNLESS "raw" is specified as the "algorithm" for
  [::pki::sign] (in which case the data is only padded before being signed).

  The [::pki::verify] command knows to expect this PKCS#1 v1.5 structure so that
  the relevant hashing information can be obtained and stripped from the
  input so you don't need to specify the algorithm *UNLESS* it does not contain
  the PKCS#1 v1.5 header, in which case the hashing algorithm has to be specified.
  Since the only case where you have to specify the hashing algorithm is in the
  case where the prefix is missing, if you specify the hashing algorithm for the
  [::pki::verify] function it won't strip the header that [::pki::sign] added
  because it assumes you specified the algorithm because there wasn't a header, so
  it doesn't even check for a header.

  Summary:  Don't specify the "algorithm" parameter for [pki::verify] unless you
  specified "raw" as the algorithm for [pki::sign].


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Updated the documentation, see commit [3f868874e7]

  See https://core.tcl-lang.org/tcllib/doc/trunk/embedded/md/tcllib/files/modules/pki/pki.md#3
  for the updated documentation.


  Restructured the testsuite a bit, moving the input assets into a separate directory, and large results into asset files as well, under the same directory.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  A few questions to collect more data points

    - Was <b>tcllibc</b> installed ?

    - Alternatively, after the <b>package require struct::graph</b>, does the output of *info loaded* contain a line about <b>tcllibc</b> ?

    - What happens when the demo code is run in a plain <b>tclsh</b> instead of Tkcon ?

    - Similarly, what happens for a plain <b>wish</b> ?

  Note, I do not have a Windows machine, nor access.

  All of the above said, I tried to repro on my linux box and saw

  <pre>
    andreask@ten:~/fast/aku> tclsh
    % info patch
    8.6.7
    % package require struct::graph
    set g [struct::graph]
    $g node insert n n
    2.4
    % ::graph1
    % struct::graph(c) gn_new - tried to use duplicate name for new node
    Aborted (core dumped)
  </pre>

  <pre>
    andreask@ten:~/fast/aku> tclsh
    % package require struct::graph
    2.4
    % info loaded
    {/home/andreask/opt/ActiveTcl/lib/tcllibc/linux-x86_64/tcllibc.so Tcllibc}
    % exit
  </pre>

  So, same Tcl, plain tclsh, tcllibc is installed. Similar error (core dump).

  Disabling tcllibc makes the core dump go away.

  This means the issue is in the C implementation of struct::graph available through Tcllibc.

  A workaround thus should be to go to the pkgIndex.tcl for Tcllibc and diable it by adding a <b>return</b> before everything else. Of course, with Tcllibc disabled things like struct::graph will become slower, using the pure Tcl implementation instead. That may be unwanted. On the third hand, it may be bearable during dev.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Another datapoint

  <pre>
  andreask@ten:~/fast/aku> tclsh
  % package require struct::graph
  2.4
  % set g [struct::graph]
  ::graph1
  % $g node insert n
  n
  % $g node insert n
  node "n" already exists in graph "::graph1"
  % $g node insert n n
  node "n" already exists in graph "::graph1"
  % $g node insert k k
  struct::graph(c) gn_new - tried to use duplicate name for new node
  Aborted (core dumped)
  </pre>

  The crash only happens if the two dup nodes are inserted in the same call of <b>node insert</b>.

  Using separate calls properly detects that the name exists already.

  This gives us another workaround, do not insert multiple nodes at once.
  This is nicer than disabling all the C (booster) implementations.


login: aku
mimetype: text/x-fossil-wiki
icomment:
  Relevant code at

  https://core.tcl-lang.org/tcllib/artifact?udc=1&ln=1989-1994&name=5d9774f9acc3d2f9

  It checks all the nodes to insert against the existing graph.
  It does not check for duplicates within the list itself.

  Easiest change is to merge the loops, i.e. "check & insert, one after the other"; instead of "check all, then insert all". The not so nice thing about this proposed change: When a dup is encountered the graph may already be changed. In the existing code the graph changes only if no error would occur.

  If I want to keep that I either have to undo all previously added nodes when I run into a dup, or complexify the validation to detect dups in the list of arguments.

  Generally, I should check other methods for similar errors.

  ... I can confirm that the Tcl implementation exhibits the same flawed type of check, except it does not crash the system.

  https://core.tcl-lang.org/tcllib/artifact?udc=1&ln=1848&name=38cb023553938f18


login: aku
mimetype: text/x-fossil-wiki
icomment:
  <b>$g node delete</b> looks to be in similar straights.
  Ditto <b>$g arc delete</b>.

  While they check that you are not trying to delete an unknown node/arc they will happily try to delete the same node/arc twice, if referenced in a single call.

  For <b>node delete</b> see
  https://core.tcl-lang.org/tcllib/artifact?udc=1&ln=1827-1830&name=5d9774f9acc3d2f9

  This one has a simpler fix. Simply skip deleted node/arc in the deletion loop a few lines down. Of course that then does not match the documented behaviour, that deleting an unknown node/arc throws an error. So, again either merge the validation and editing loops, or make the validation more complex to recognize the dup deletion from the list of arguments.


login: anonymous
username: Harm Olthof
mimetype: text/plain
icomment:
  Re your questions:
  tcllibc is loaded:
  Main console display active (Tcl8.6.7 / Tk8.6.7)
  (bin) 49 % package require struct::graph
  2.4.1
  (bin) 50 % info loaded
  {C:/ActiveTcl/lib/tcllibc/win32-x86_64/tcllibc.dll Tcllibc} {{} Tk} {C:/ActiveTcl/lib/reg1.3/tclreg13.dll Registry}
  (bin) 51 % 

  If you run the example script from a plain tcl shell the results are the same(the ceash included), except there is no pop up msg box: the error message is send to stdout.


login: anonymous
username: Harm Olthof
mimetype: text/plain
icomment:
  Why not deduplicate the list of input nodes first. ([lsort -unique, or maybe something a little bit more cleverer to preserve the order of input nodes).
  Then the rest of the code can left untouched.


login: aku
mimetype: text/plain
icomment:
  Thanks for confirming that tcllibc is involved in your setup as well.

  Deduplication: Yes, something like that, just at C level. I.e. have an additional transient hashtable to hold node/arc names already seen and use that as source as well, not just the graph itself as-is.


login: aku
mimetype: text/plain
icomment:
  I confirm.

  The issue looks to be that the link in the markdown

      [Tcl Community - Kind Communication](md/tcllib/files/devdoc/tcl_community_communication.md)

  is not rendered as

      https://core.tcl-lang.org/tcllib/doc/trunk/embedded/md/tcllib/files/devdoc/tcl_community_communication.md

  as expected, but as

      https://core.tcl-lang.org/tcllib/wiki?name=md/tcllib/files/devdoc/tcl_community_communication.md

  instead.

  I suspect that this is an issue of Richard having updated the fossil handling the repository to a very new version where link handling has changed.

  I see `Fossil 2.10 [2da704c5a1] 2019-08-21 12:32:20` at the bottom of the generated page, that fossil was made yesterday.


login: aku
mimetype: text/plain
icomment:
  Talked to Richard, and he found and fixed the issue with Fossil.

  The site now runs `Fossil 2.10 [3b10e64468] 2019-08-23 11:07:53`
  which renders the links correctly again.

  Relevant fossil commit is
  https://www.fossil-scm.org/fossil/info/3b10e644686bc279

  Thanks to Gregor for the report.


