Tcl Source Code

View Ticket
Login
2024-09-10
12:14 Closed ticket [7db9574a06]: Undocumented features of zipfs implementation plus 6 other changes artifact: 0451855ba3 user: tberg
2024-08-06
06:38 Ticket [75291b89b3] Documentation of zipfs in tclsh(1) status still Open with 3 other changes artifact: f15b975a2c user: tberg
06:22 Ticket [7db9574a06] Undocumented features of zipfs implementation status still Open with 3 other changes artifact: e06bdc2f0f user: tberg
06:20
resolution of ticket [7db9574a06] check-in: d97273f74c user: Torsten tags: zipfs-consolidation
2024-08-05
23:10 Ticket [7db9574a06] Undocumented features of zipfs implementation status still Open with 3 other changes artifact: c7415756b2 user: tberg
2024-08-03
20:41 Ticket [7db9574a06]: 3 changes artifact: c24e37918d user: cjmcdonald
2024-08-02
21:58 Ticket [7db9574a06]: 3 changes artifact: efc5152433 user: tberg
2024-07-31
03:11 Ticket [7db9574a06]: 3 changes artifact: 65ed84edd5 user: apnadkarni
2024-07-30
08:35 New ticket [7db9574a06]. artifact: e4251f432b user: tberg

Ticket UUID: 7db9574a06b1994c5b3f8cdf13e6ffcbea072d4b
Title: Undocumented features of zipfs implementation
Type: Bug Version: 9.0
Submitter: tberg Created on: 2024-07-30 08:35:22
Subsystem: - New Builtin Commands Assigned To: nobody
Priority: 5 Medium Severity: Important
Status: Closed Last Modified: 2024-09-10 12:14:08
Resolution: Fixed Closed By: tberg
    Closed on: 2024-09-10 12:14:08
Description:

Here are some undocumented features of zipfs (implementing TIP 430):

1. zipfs mount_data

There is a command zipfs mount_data but it is undocumented. Should it be documented in the manual page?

2. ::tcl::zipfs::tcl_library_init

The C source code defines a ::tcl::zipfs::tcl_library_init procedure that is also undocumented. Should it be documented and what does it do? I guess it is only for internal use.

On the other hand, TIP 430 defines a public subcommand zipfs tcl_library. This command is not implemented but seems to do the same.

3. zip archives attached to tclsh/wish or to libraries

TIP 430 describes what happens when a zip archive is attached/appended to tclsh/wish or to a library file. In this context, it also mentions the specific mount points ZIPFS_ROOT/app and ZIPFS_ROOT/lib/PGKNAME plus some files and directories inside to look for during startup.

This is documented in the tclsh(1) manual page. However, the documentation does not mention the specific mount points. The example at the end of TIP 430 should then also be included in the manual as it clarifies nicely how to exactly use that feature as it is difficult to put in words alone. This could be done in zipfs(n) to not overload the tclsh(1) manual page.

Also, the TIP defines something like tclsh install .... This is not documented in tclsh(1) and I am not sure if this is even implemented.

4. General consistency with TIP 430

In general, the manual page is not consistent with the proposal in the TIP, meaning that zipfs was implemented differently from what the TIP says. Maybe that is OK, I am not sure whether the TIP should always be in line with the implementation, but here are some of the differences:

  • zipfs canonical has some options not mentioned in the TIP
  • zipfs mount can use a password option not documented in the TIP
  • zipfs lmkimg has one optional pair of options (... ?password infile?) while the TIP has them as two separate options (... ?password? ?infile?). This is somewhat related to this ticket

When we refer to the TIP as "further reading" from the manual page, this could at least increase confusion.

User Comments: tberg added on 2024-09-10 12:14:08:

As the TIP is now also edited and contains an additional "Amendment" section, this ticket is consequently closed.


tberg added on 2024-08-06 06:22:52:

The changes are now implemented with commit d97273f74c2dfecc (apart from the TIP amendment)


tberg added on 2024-08-05 23:10:02:

After discussion with TCT members, the following was agreed:

  1. Rename the subcommand to zipfs mountdata and document it in the manual page. The renaming has already been done as of commit 731b44b4c730e74d. (This can be done without a TIP as zipfs mount_data was not part of TIP 430 anyway).

  2. Nothing to do, the command is internal and stays undocumented in the manual page

  3. The respective manual pages will be clarified. tclsh install ... will not be documented as it is not implemented.

  4. An amendment to TIP430 is to be written to clarify which parts of the TIP are actually implemented and which not. A future TIP might then be issued for remaining subcommands etc. that are still missing.


cjmcdonald added on 2024-08-03 20:41:15:

Regarding zipfs mount_data, note that a corresponding C level interface TclZipfs_MountBuffer has been made public and documented in the man pages at https://www.tcl.tk/man/tcl9.0/TclLib/zipfs.html. So we have the strange situation where the C interface has been documented, but not the Tcl command, and yet there are a set of Tcl tests.

I'd like to see the Tcl zipfs mount_data (or mountdata) command documented, rather than made undocumented and internal. I think that there is a useful use case, where a zip archive is transferred to a program from a server via a socket. Without mount_data it would be necessary to write that received archive to a temporary file on disk in order to mount it, whereas with mount_data it could be opened directly from memory.


tberg added on 2024-08-02 21:58:00:
  1. When invoking zipfs xxx to provoke an error message about the possible subcommands, zipfs mount_data is also listed. I am not sure whether this already qualifies for the subcommand as candidate for being documented. At least it feels awkward to have a subcommand being listed but not being documented. As for the name, I would propose zipfs mountdata then. We still have the chance to rename it now.

  2. OK, I get the point. If zipfs mount_data should stay undocumented, I nearly believe this is for internal use also. Then that one should probably also be moved into the ::tcl::zipfs namespace.

As for the TIP, I guess the process should be to first look at the zipfs source code now in core and make sure the documentation (manual pages zipfs(n) and interp(n)) is in line with the implementation. Then, the TIP should be brought in line with those as well ...


apnadkarni added on 2024-07-31 03:11:28:
1. Not sure why it is undocumented. It does not appear to be used anywhere within the Tcl core. The test suite does cover it with the same test cases as for `mount`. If we do decide to include it, the name `mount_data` is a little incongruous as I don't think any other ensemble subcommands use snake case (e.g. getwithdefault, not get_with_default) but that may be bike shedding.

2. `tcl_library_init` is internal use, it locates the Tcl support scripts. Should not be documented. In fact the whole tcl::zipfs:: namespace should be treated as private. As far as the TIP `zipfs tcl_library` reference goes, it should be removed from the TIP as it does not appear anywhere in the source and is not needed.

3. Have to look at this closer but as far as `tclsh install` goes, it is not implemented and needs to be removed from the TIP.

4. In a nutshell, the TIP needs to be updated or rewritten to match the implementation and manpage.