Ticket UUID: | ad7bafd5e11cfa09e9ed91ae4acff706438705e1 | |||
Title: | auto_path/tcl_library consistency for zipfs filesystem | |||
Type: | Bug | Version: | 9 | |
Submitter: | juliannoble2 | Created on: | 2024-10-06 19:13:24 | |
Subsystem: | - New Builtin Commands | Assigned To: | jan.nijtmans | |
Priority: | 5 Medium | Severity: | Important | |
Status: | Open | Last Modified: | 2024-11-01 07:01:12 | |
Resolution: | None | Closed By: | nobody | |
Closed on: | ||||
Description: |
The default location for init.tcl and hence tcl_library that is setup in tclZipfs.c is //zipfs:/app/tcl_library This is not directly an immediate issue - but then init.tcl adds the parent dir to the auto_path. This ends up as //zipfs:/app Aside from //zipfs:/app and //zipfs:/app/tcl_library - there aren't any other //zipfs:/.. entries in the auto_path by default. A user setting up a tclkit-like system then has to store library folders directly in the first usable dir ie somewhere like //zipfs:/app/mypackage1 //zipfs:/app/mypackage2 tclPkgUnknown will scan all base user folders for pkgIndex.tcl files eg things like //zipfs:/app/doc //zipfs:/app/modules which aren't applicable and while fast enough in zipfs - doesn't seem efficient/scalable If the user tries to organise libraries under something like //zipfs:/app/lib - it's only going to work if they adjust the auto_path - or create a //zipfs:/app/lib/pkgIndex.tcl to point to the folders therein. //zipfs:/app is analogous to the normal tcl install folder (on windows at least) containing things like apps,bin,doc,include,lib,licenses,samples etc. The fact that zipfs:/app/tcl_library is at the same level as those (rather than something like //zipfs:/app/lib/tcl9.0) is no big deal in itself - but the default of having to store each library folder at the same level as docs etc (without fiddling things) seems confusing. It also makes the filestructure for //zipfs:/ less compatible/sensible for those trying to bring things across from old tclkits. Creating a wrapped executable as per the instructions in tip 430 works - but the resultant executable now depends on init.tcl etc from the external filesystem, or for the user to add that back in for the exe to be independent. To add that in - the first inclination would be to just bring in the lib folder from the tcl installation dir as a whole - but that won't work as init.tcl still won't be found. The user could then move the lib/tcl9.0 to be tcl_library at the root of the .vfs and it will work - including finding things like lib/tcl9/9.0/platform-1.0.19.tm - but any pkgIndex.tcl based libraries at lib/whatever won't be found as mentioned above. If a tool equivalent to the old sdx gets made for zipfs based executables to easily 'unwrap' and 'wrap' a .vfs folder - the situation could be much improved - but I'm tempted to consider this as a bug in consistency and usability that doesn't seem necessary(?) Before anyone attempts to build a modern SDX equivalent for zipkits and distribute it - can we review this? Any other opinions? | |||
User Comments: |
juliannoble2 added on 2024-10-10 01:42:13:
This has gotten way off topic from the original issue. My comments regarding writing to the mounted zipfs should be disregarded - writing to an existing file is unaffected by the offset fiddling - and isn't persisted anyway which is as documented. Regarding the offset issue - which I still see as anomalous on windows at least - I'll try to follow up with some proper test-running on some other ticket. juliannoble2 added on 2024-10-09 13:43:53: I've tried the same unzip -l tests using a tclsh90 from magicsplat rather than my own builds, and also an installer from some other software that uses zip. They give the same warning - so I think it's reasonable to believe this is normal on windows at least. The mention of what unzip -l does/doesn't display in ticket [fc65ff1b663fd16e7582] was incidental I think. The main issue was that the reporter said no exe was produced by 'zipfs mkimg' From my perspective 'correcting' the offsets to get rid of the unzip warning just breaks the ability to write mounted exes - and breaks the ability to use zip tools to write it as well - and I suspect it's the wrong thing to do on other platforms too - but we need more feedback I guess. I don't understand why 'zipfs mkimg' produced only a zip file for the bug reporter - but I note there were also other changes related to offsets in that commit. juliannoble2 added on 2024-10-09 13:17:44: ok.. so opening pretty much any exe with tools like 7zip or peazip shows resource data folders and files - but I think that's a red herring and it's not zip based. Still.. perhaps there are platform differences as to why zip -l tclshwithzipfs.exe | head shows that warning juliannoble2 added on 2024-10-09 13:07:53: C:\buildtcl\2024zig\build_tcl90\zig-out90j>cat tclsh90s.exe mkzipfs.zip > test.exe C:\buildtcl\2024zig\build_tcl90\zig-out90j>unzip -l test.exe | head warning [test.exe]: 2405888 extra bytes at beginning or within zipfile (attempting to process anyway) That tclsh90s.exe has no tcl zipfs attached. But.. I just realized it still has windows resource data - which.. now that I look at it I think is zip based? That's umm... interesting :/ jan.nijtmans added on 2024-10-09 13:00:36: > but so does a cat-ed tclsh + zip anway - so I think we *should* see that message. > It doesn't stop any zip utils from dealing with it that I can see. I don't think so. Starting with tclsh + zip, running "zip -A" gives this message. Then running "zip -A" again doesn't give the message any more, because the offsets are corrected. I think using "zipfs mking" followed by "zip -A" shouldn't give this message any more. That was the original bug-report. juliannoble2 added on 2024-10-09 12:59:25: oh.. just realize that's a test branch You also need to pass startDataOffset around.. and add back into ZipFsMkZipOrImg: dataStartOffset = Tcl_Tell(out); It's not all that much.. but should I just generate a diff? juliannoble2 added on 2024-10-09 12:52:29: Sorry.. confused myself with offsets :P I meant that those 2 are *also* meant to be relative - so you have to subtract the offset to account for the file-data coming before the zip-archive. juliannoble2 added on 2024-10-09 12:47:38: Yes. It may be that at the time - that fixed it - but there is such confusion in naming (IMHO) regarding all the various offsets that it was later broken? I know that with my changes the: unzip -l test.exe | head shows warning [test.exe]: xxxx extra bytes at beginning or within zipfile but so does a cat-ed tclsh + zip anway - so I think we *should* see that message. It doesn't stop any zip utils from dealing with it that I can see. I haven't been able to run proper test suites - but from my perspective so far the situation is much improved with those subtractions added back in. Most offsets are meant to be relative to the zip-start - but I believe those 2 need to be absolute to the beginning of the file. Not sure about the password stuff - but I wasn't able to verify it worked/did anything even before I changed things. jan.nijtmans added on 2024-10-09 12:29:25: You mean [194c178dba50929e|this]? juliannoble2 added on 2024-10-09 09:11:54: >There was already a ticket on this problem: [fc65ff1b663fd16e7582], which is closed as "fixed". Yeah, I didn't understand how that patch could work without the dataStartOffset being subtracted: e.g in SerializeCentralDirectorySuffix ZipWriteInt(start, end, buf + ZIP_CENTRAL_DIRSTART_OFFS, directoryStartOffset - dataStartOffset); The central directory offset has to be relative to the start of the zip. So I edited the latest tclZipfs.c to put it back in for SerializeLocalEntryHeader and SerializeCentralDirectoryEntry - and tried it out. I can now create a kit with zipfs mkimg that is easily editable using standard zip tools. Also - previously if I did: zipfs mount tclshwithzipfs.exe tclshwithzipfs.exe I couldn't write to it. After the dataStartOffset being added back in I can. I still need to understand the bug that 2021 commit was supposed to fix though. jan.nijtmans added on 2024-10-08 13:46:55: > Before I separately bugreport .... There was already a ticket on this problem: [fc65ff1b663fd16e7582], which is closed as "fixed". I never tested the fix myself (because I simply use "zip -A"), but this ticket suggests that it's not really fixed yet. "zipfs mkimg" should calculate the same offsets as "zip -A" does. juliannoble2 added on 2024-10-08 11:56:27: My initial zipkits were made using the 'zipfs mkimg' command as I was trying to avoid external executable dependencies. Such tclsh exes with a zipfs attached run fine and can load libraries including binaries - but cannot be edited in place with 'zip -A' - or with other tools such as peazip or 7zip on windows. The zip -A command on an executable created with zipfs mkimg shows: zip warning: central dir not where expected - could not adjust offsets My worry is that as this is builtin, others may use this method to build kits that seem to work but are much harder to work with. I was under the impression zipkits couldn't be updated directly until I tried one built with plain cat. Before I separately bugreport: Can anyone with a more standard build system verify? Mine is a little curly (zig based) so I'm not sure if there are other factors. Same on other platforms? juliannoble2 added on 2024-10-07 11:06:43: Really it seems like a namespacing issue to me. BI kits without a main.tcl will need to have various package folders at the base. Just looking at tcllib - these are often just folder names without a version number - so whether a folder is part of the application or a lib is not glance-obvious. An application developer has to consider the namespace of all tcl libraries just in choosing names for their own data/application folders that won't collide now or in the future - or know to put all their stuff under a single highly unique name (deeper dir structure responsibility pushed on to app dev) - or rearrange the supplied kit and neaten it up by putting the pkgIndex.tcl libraries under lib and remember to customise auto_path themselves. jan.nijtmans added on 2024-10-07 11:04:49: > What about a special case for adding //zipfs:/app/lib to the ::auto_path by default? ... I don't see a problem with that. But I don't want to rush making any change, before more people had the chance to chime in. Keeping this ticket open for a while, so others can add their experiences here. juliannoble2 added on 2024-10-07 10:48:21: Ok, as ubiquitous as zip is - i'm not as familiar as I should be (more used to tar etc I guess) Thanks for the -A tip. I think there is some assumed knowledge that might be obvious to most, but worthy of documenting. Playing with cookfs,zipkit,metakit and UPX I'm trying to get an understanding of what can coexist (if possible at all) attached to an executable and still be mountable. Sticking to one such as zipfs and using generic tools does seem practical. While I feel the defaults are a little weird, and more likely to result in variant layouts that are harder for other tools to deal with - it's not a showstopper for zipkit builders as any main.tcl can manipulate auto_paths etc as needed. What about a special case for adding //zipfs:/app/lib to the ::auto_path by default? Useful for the case of zipkits that are batteries included but without an application so no main.tcl? I hear you don't see the need to add 'any deeper directory-structure' - but I really think having everything at the same level as package folders is messy. You're just pushing the 'deeper directory-structure' on to the application developer to avoid the chaos at the base. I don't mean the 2 tcl_library,tk_library at base - that's not unreasonable - I mean the case when you have 50 libraries and some application specific folders scattered amongst them. Just my opinion - feel free to close this ticket if the core team is happy with it as is. jan.nijtmans added on 2024-10-07 08:20:59: > tclPkgUnknown will scan all base user folders for pkgIndex.tcl files eg things like //zipfs:/app/doc //zipfs:/app/modules I think the directory structure should be as simple as possible. The top-directory //zipfs:/app contains everything in the zip-file belonging to the executable. //zipfs:/app/tcl_library and //zipfs:/app/tk_library should be kept as-is, other subdirectories are free for anyone else's use. If someone wants its own module xxx to be added, it can be stored in //zipfs:/app/xxx, I see no need to add any deeper directory-structure to it. There is no need to unwrap and wrap a .vfs folder. The "zip" utility has a "-A" option which can add entries to an existing zip-file, even being attached to an executable. So adding your own "main.tcl" can be done like: zip -A tclsh90.exe main.tcl >Creating a wrapped executable as per the instructions in tip 430 works ... It would be useful to re-write those instructions using the "zip" utility. |
Attachments:
- tclZipfs.patch [download] added by juliannoble2 on 2024-10-09 14:03:37. [details]