diff --git a/CHANGELOG.md b/CHANGELOG.md index 0797619e..85b3d1c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,24 @@ The latest `## [X.Y.Z]` header must match the `version` field in `punkproject.to Entries are newest-first; one bullet per notable change. See the root `AGENTS.md` "Project Versioning" section for the bump policy. +## [0.27.4] - 2026-07-27 + +- `make.tcl bootsupport` no longer fails to replace a mounted modpod. A zip-based `.tm` + cannot be overwritten in place while any process has it zipfs-mounted, because the mount + memory-maps the archive and windows refuses to overwrite a file with a user-mapped + section open (`ERROR_USER_MAPPED_FILE`). The update now falls back to delete-then-place, + which windows does permit - unlinking a mapped file leaves the holder reading its own + mapping while new content lands at the name. Replacement content is staged to a sibling + temp first, so a mid-sequence failure can never leave the target missing. +- Failures of that kind are now explained instead of being reported as + `error copying ...: invalid argument`. That message is Tcl's catch-all: + `Tcl_WinConvertError` maps only Win32 codes 0..267 and sends everything above them to + `EINVAL`, so every high-numbered windows error arrives with the same uninformative text + (identical in 8.6 and 9.x). `make.tcl` now prints what it actually means and which + process class to look for. +- The mechanism, and the warning not to read `invalid argument` at face value on windows, + is documented in `src/bootsupport/AGENTS.md`. + ## [0.27.3] - 2026-07-27 - `bin/punk-runtime.cmd list` and `use` no longer offer support files as selectable diff --git a/GOALS.md b/GOALS.md index b5f85de4..79961f26 100644 --- a/GOALS.md +++ b/GOALS.md @@ -414,3 +414,7 @@ Detail: goals/G-130-32bit-windows-buildsuites.md ### G-131 [proposed] One runtime, either payload: boot-time container autodetection (metakit or zipfs) Scope: src/vfs/_config/punk_main.tcl and src/vfs/_config/project_main.tcl (container detection - extends the G-129 zipfs derivation to metakit; both files carry the same code); src/buildsuites/suite_tcl86/ and src/buildsuites/suite_tcl90/ (runtimes carrying both container capabilities, which the detection presupposes); src/make.tcl (the source/target kit-type mismatch check, which today assumes a runtime implies one container); src/runtime/mapvfs.config (kit type as a bake-time choice); bin/runtime/win32-ix86/ (zipfs specimens for the no-regression check); src/tests/shell/testsuites/punkexe/ (characterization); src/AGENTS.md + bin/AGENTS.md (what a runtime must provide) Detail: goals/G-131-boot-payload-autodetection.md + +### G-132 [proposed] Draft the Tcl-on-windows errno catch-all report for manual submission +Scope: TEMP_REFERENCE/tcl-windows-errno-catchall-TICKET-DRAFT.md (submission-ready ticket text - STAGING ONLY, unversioned, and the agent does not submit it); this detail file (the tracked record: finding, verbatim reproducer, the submitted URL once the user files it, and the upstream disposition); src/bootsupport/AGENTS.md and src/make.tcl (the existing workaround and its warning - dated, not removed, if upstream fixes it) +Detail: goals/G-132-tcl-windows-errno-catchall-report.md diff --git a/goals/G-132-tcl-windows-errno-catchall-report.md b/goals/G-132-tcl-windows-errno-catchall-report.md new file mode 100644 index 00000000..81b413e7 --- /dev/null +++ b/goals/G-132-tcl-windows-errno-catchall-report.md @@ -0,0 +1,146 @@ +# G-132 Draft the Tcl-on-windows errno catch-all report for manual submission + +Status: proposed +Scope: TEMP_REFERENCE/tcl-windows-errno-catchall-TICKET-DRAFT.md (submission-ready ticket text - STAGING ONLY, unversioned, and the agent does not submit it); this detail file (the tracked record: finding, verbatim reproducer, the submitted URL once the user files it, and the upstream disposition); src/bootsupport/AGENTS.md and src/make.tcl (the existing workaround and its warning - dated, not removed, if upstream fixes it) +Goal: the Tcl-on-windows errno catch-all - every Win32 error above 267 surfacing as "invalid argument" - is written up as a submission-ready upstream report FOR THE DEVELOPER TO FILE MANUALLY, with a minimal reproducer and suggested fixes, and punkshell keeps the tracked record of the finding and whatever upstream decides, so the diagnosis is never re-derived the next time a high-numbered windows error appears as a nonsense message. +Acceptance: the existing core.tcl-lang.org/tcl tracker is searched first and the result recorded here - an existing ticket is a valid outcome, and this goal then only links it and records its state; if unreported, a complete submission-ready ticket text is written to TEMP_REFERENCE/tcl-windows-errno-catchall-TICKET-DRAFT.md carrying title, summary, the minimal reproducer, the source citations (win/tclWinError.c - table extent 0..267 and the errorTable[1] fallback, verified present in both 8.6 and 9.x), the concrete windows code (ERROR_USER_MAPPED_FILE 1224), the cross-platform observation that the same 'file copy -force' succeeds on unix, and the candidate fixes with their trade-offs; THE AGENT DOES NOT SUBMIT IT - no account use, no web form, no API, no mail - submission is the developer's action alone, and the goal is not complete until the developer has submitted or declined; because TEMP_REFERENCE is ignored by both VCS and is one 'git clean -xdf' from gone, the reproducer and the source citations are ALSO carried verbatim in this tracked detail file, so the draft file is a convenience for pasting and never the only copy; once the developer submits, the ticket URL and the upstream response are recorded here with a disposition (fixed-in-release / declined / duplicate / superseded), and if fixed, the fixing release is named so the make.tcl workaround and the src/bootsupport/AGENTS.md warning can be dated; and the punkshell-side workaround is NOT removed by this goal - it must keep working on every runtime punkshell supports regardless of what upstream does. + +## Context + +Found 2026-07-27 while diagnosing a `make.tcl bootsupport` failure that had recurred three +times: replacing the zip-based `punk::mix::templates` modpod in `src/bootsupport` failed +with + + error copying "" to "": invalid argument + +The message is a dead end - it reads as a bad argument, and it names neither the real cause +nor a remedy. Two earlier attempts at characterising it (recorded in commit messages and in +goals/archive/G-124-punkzip-reader.md) were both wrong in different ways, precisely because +the message carries no information. + +The punkshell-side cause is understood and worked around (see the Finding below and +src/bootsupport/AGENTS.md): a zipfs mount memory-maps its archive, and windows refuses to +overwrite a file with a user-mapped section open. That part is ours and is fixed. + +What remains is upstream's: Tcl converts EVERY windows error above 267 to `EINVAL`, so an +entire class of real, common file-operation failures arrives as the same uninformative +message. That is worth reporting, and this goal exists so that the report is drafted +properly and the outcome is recorded - not so that an agent files it. + +## Finding (tracked copy - TEMP_REFERENCE is unversioned) + +Verified 2026-07-27 against `TEMP_REFERENCE/tcl9` (9.1b1) and +`TEMP_REFERENCE/androwish/jni/tcl` (8.6.10). BOTH generations carry byte-identical logic. + +`win/tclWinError.c` holds `errorTable[]`, whose last entry is index 267 +(`ENOTDIR /* ERROR_DIRECTORY 267 */`). The converter is: + + void + Tcl_WinConvertError(unsigned errCode) + { + if ((unsigned)errCode >= sizeof(errorTable)/sizeof(errorTable[0])) { + errCode -= WSAEWOULDBLOCK; + if ((unsigned)errCode >= sizeof(wsaErrorTable)/sizeof(wsaErrorTable[0])) { + Tcl_SetErrno(errorTable[1]); + } else { + Tcl_SetErrno(wsaErrorTable[errCode]); + } + } else { + Tcl_SetErrno(errorTable[errCode]); + } + } + +`errorTable[1]` is `EINVAL` (`/* ERROR_INVALID_FUNCTION 1 */`). So any windows error code +above 267 that is not in the WSA range collapses to `EINVAL` -> "invalid argument". The +affected set is not exotic: + + 303 ERROR_DELETE_PENDING file is in the process of being deleted + 1224 ERROR_USER_MAPPED_FILE cannot be performed on a file with a user-mapped + section open + 1920 ERROR_CANT_ACCESS_FILE the file cannot be accessed by the system + 1921 ERROR_CANT_RESOLVE_FILENAME the name of the file cannot be resolved + 4390 ERROR_NOT_A_REPARSE_POINT not a reparse point + +The specific case that surfaced it: `tclZipfs.c` maps a mounted archive with +`CreateFileMappingW` + `MapViewOfFile` (9.1b1: :1867 and :1875; `UnmapViewOfFile` at :1475), +so any process holding a zipfs mount holds a user-mapped section on that file, and windows +raises 1224 for an in-place overwrite. + +Cross-platform observation worth including in the report: the same `file copy -force` over a +memory-mapped file SUCCEEDS on unix. So the failure is windows-only and silent about it, +which makes the misleading errno costlier than it first appears. + +Measured behaviour, same machine, Tcl 9.0.3: + + file copy -force over the archive, not mounted -> ok + file copy -force over the archive, zipfs-mounted -> invalid argument + file copy -force over the archive, unmounted -> ok + file delete of the archive, while mounted -> ok <-- unlink is permitted + delete-then-place while mounted -> ok, and the holder keeps + reading its own mapping + +## Reproducer (tracked copy) + +Self-contained, no punkshell, windows only. Any zip will do - a kit or a zip-based `.tm` +modpod is convenient. + + set f mycopy.zip + file copy -force somearchive.zip $f + tcl::zipfs::mount $f probe + catch {file copy -force someotherfile $f} e + puts $e ;# error copying ...: invalid argument + ;# actual cause: ERROR_USER_MAPPED_FILE (1224), unrepresented in errorTable + tcl::zipfs::unmount probe + +Expected after a fix: an errno that distinguishes this from a caller error - the message +should not claim the arguments were invalid. + +## Approach + +- Search the tracker BEFORE drafting. This is old code present in 8.6 and 9.x, so a prior + report is entirely plausible and finding one is a complete result for this goal. +- Draft to `TEMP_REFERENCE/tcl-windows-errno-catchall-TICKET-DRAFT.md` in submission-ready + form - a developer should be able to paste it without editing. +- Keep the claim narrow and defensible: the REPORT is about the errno conversion, which is + plainly a defect. The `file copy -force` portability difference is context in the same + ticket, not a second demand - upstream may reasonably decline to change copy semantics + (atomicity, ACL inheritance), and bundling it as a demand weakens the part that is clearly + wrong. +- Offer candidate fixes with trade-offs rather than insisting on one: extend the table; + give the out-of-range branch a better default than `EINVAL` for the sharing/locking family + (`EACCES` is defensible, `EBUSY` arguably closer); or expose the raw windows code so + callers can diagnose. Deciding among these is upstream's call. + +## Alternatives considered + +- File it directly from an agent - rejected, and this is the point of the goal's shape: the + developer submits. Agent-filed reports on a project's public tracker are the developer's + reputation and correspondence to own, and a ticket needs someone who can answer follow-up + questions. +- Fix it locally and say nothing - rejected: the workaround is in punkshell's build tooling + only, and every other Tcl user on windows keeps meeting "invalid argument" for a class of + errors that has nothing to do with arguments. +- Submit a patch rather than a report - deferred, not rejected. It is a table and the change + is small, but choosing the right errno per class is a design conversation, and a report + that names the candidates invites that conversation without presuming its outcome. If the + developer prefers the patch aim, this goal's Acceptance changes rather than the finding. + +## Notes + +- PRECEDENT: G-039 (achieved 2026-07-12) filed a Tcl 9 windows console defect the same way - + agent drafted the full text with repro scripts to `TEMP_REFERENCE/`, the developer + submitted, and the goal recorded the URL + (https://core.tcl-lang.org/tcl/tktview/f10d91c2d3). That draft elided the scripts from the + web submission; whether to do the same here is the developer's call at submission time, + not something this goal presumes. +- The punkshell workaround this goal does NOT touch: `::punkboot::replace_possibly_mapped_file` + falls back from overwrite to delete-then-place, and `::punkboot::mapped_file_hint` explains + the catch-all when a copy genuinely fails. Both live in `src/make.tcl` and are documented + in `src/bootsupport/AGENTS.md`. They must keep working on every runtime punkshell supports + regardless of upstream, so an upstream fix dates them rather than deletes them. +- Related: G-110 - it studies caching of EXTRACTED shared libs, where extraction today is + done by zipfs/vfs::zip. Anything that holds a zipfs mount holds a mapping, so the same + windows constraint applies to any scheme that wants to replace a mounted archive in place. +- The `invalid argument` reading applies to ANY windows file operation in Tcl, not just + copies. Treat it as "some windows error above 267" wherever it appears; that guidance is + recorded in `src/bootsupport/AGENTS.md` because that is where it bit, but it is general. diff --git a/punkproject.toml b/punkproject.toml index 3ad8cca1..97642c56 100644 --- a/punkproject.toml +++ b/punkproject.toml @@ -1,4 +1,4 @@ [project] name = "punkshell" -version = "0.27.3" +version = "0.27.4" license = "BSD-2-Clause" diff --git a/src/bootsupport/AGENTS.md b/src/bootsupport/AGENTS.md index a2944d4f..a2507b9d 100644 --- a/src/bootsupport/AGENTS.md +++ b/src/bootsupport/AGENTS.md @@ -53,6 +53,35 @@ After changing any build-critical module (`punkcheck`, `punk::repo`, `punk::mix` - Do not add compiled extensions or packages requiring dynamic loading to bootsupport. - Check `src/bootsupport/modules/README.md` for the copy-as-module workflow. +### Replacing a mounted modpod: why an overwrite fails and what make.tcl does + +A zip-based `.tm` modpod here (`punk/mix/templates-*.tm` is the one this bites) cannot be +overwritten in place while any process has it MOUNTED. A zipfs mount memory-maps its +archive (`tclZipfs.c`: `CreateFileMappingW` + `MapViewOfFile`), and windows refuses to +overwrite a file with a user-mapped section open - `ERROR_USER_MAPPED_FILE` (1224). Any +holder blocks it: a running punk shell that loaded the modpod, a `src`-mode session, or the +build itself. + +Tcl reports it as **`invalid argument`**, which names nothing useful. That is not a bad +argument: `Tcl_WinConvertError` maps only Win32 codes 0..267 and sends everything above +them to `errorTable[1]` = `EINVAL`, so EVERY high-numbered windows error surfaces with that +one message (identical code in 8.6 and 9.x - `win/tclWinError.c`). Treat `invalid argument` +from any file operation on windows as "some windows error above 267", never at face value. + +`make.tcl` handles it rather than failing: `::punkboot::replace_possibly_mapped_file` +attempts the ordinary `file copy -force`, and on failure falls back to **delete-then-place** +- unlinking a mapped file IS permitted, the holder keeps reading its own mapping, and new +content lands at the name. The replacement content is staged to a sibling +`.punkboot-new` first so a mid-sequence failure can never leave the target missing; +a `broken` return (unlinked and unrestorable) is reported as a build failure naming the +file to restore by hand. A genuine failure additionally prints +`::punkboot::mapped_file_hint`, which explains the catch-all rather than leaving +`invalid argument` bare. + +Practical consequence for agents: a bootsupport update that touches the templates modpod no +longer needs shells closed, and no longer needs the hand-copy-then-rerun dance. If you do +see it fail, the hint line tells you which process class to look for. + ### Moving make.tcl functionality into punkboot::utils `punkboot::utils` (source of truth `src/modules/punkboot/utils-999999.0a1.0.tm`) exists to keep make.tcl from growing unwieldy. Only post-boot functionality may move there — make.tcl loads it lazily from the bootsupport snapshot after boot has set the module paths; anything used during early boot (path setup, `::punkboot::lib` prompts/classifiers, package-availability checks) must stay self-contained in make.tcl. diff --git a/src/make.tcl b/src/make.tcl index df240826..894dcba5 100644 --- a/src/make.tcl +++ b/src/make.tcl @@ -3287,6 +3287,60 @@ proc ::punkboot::get_src_provenance_warnings {projectroot label} { set checked_vcs_roots [dict create] return [list 1 [::punkboot::utils::vcs_dirty_warnings $projectroot checked_vcs_roots $label src]] } +#Replace a file that may be MEMORY-MAPPED by another process (or by this one). +# +#On windows an ordinary 'file copy -force' onto a mapped file is refused: a zipfs mount +#maps its archive (tclZipfs.c CreateFileMappingW + MapViewOfFile), so a zip-based .tm +#modpod that any running shell - including this build - has mounted cannot be overwritten +#in place. Windows reports ERROR_USER_MAPPED_FILE (1224); Tcl's Tcl_WinConvertError maps +#only Win32 codes 0..267 and sends everything above them to EINVAL, so it surfaces as the +#uninformative "invalid argument" (same code in 8.6 and 9.x). +# +#UNLINKING a mapped file IS allowed - the holder keeps reading its own mapping and new +#content lands at the name - so delete-then-place succeeds where overwrite cannot. The new +#content goes to a sibling temp FIRST so a mid-sequence failure can never leave the target +#missing. +# +#Returns one of: ok (plain copy worked), replaced (needed the delete-then-place path), +#failed (target untouched), broken (target was unlinked and could not be restored - the +#caller must treat this as serious). The first copy's error message is written to the +#variable named by firsterrvar. +proc ::punkboot::replace_possibly_mapped_file {srcfile tgtfile firsterrvar} { + upvar 1 $firsterrvar firsterr + set firsterr "" + if {![catch {file copy -force $srcfile $tgtfile} firsterr]} { + return ok + } + if {![file exists $tgtfile]} { + #nothing to unlink - whatever failed, it was not an in-place overwrite + return failed + } + set tmpfile $tgtfile.punkboot-new + catch {file delete -force -- $tmpfile} + if {[catch {file copy -- $srcfile $tmpfile}]} { + catch {file delete -force -- $tmpfile} + return failed + } + if {[catch {file delete -- $tgtfile}]} { + catch {file delete -force -- $tmpfile} + return failed + } + if {[catch {file rename -- $tmpfile $tgtfile}]} { + return broken + } + return replaced +} +#Actionable text for a file-operation failure whose message is windows' catch-all. +#Returns "" when the error does not look like that case. +proc ::punkboot::mapped_file_hint {errmsg tgtfile} { + if {![string match -nocase "*invalid argument*" $errmsg]} { + return "" + } + return "'invalid argument' is Tcl's catch-all for any windows error above 267 (Tcl_WinConvertError), not a bad argument.\ + For a file replace the usual cause is ERROR_USER_MAPPED_FILE: '$tgtfile' is memory-mapped by a process that has it mounted -\ + a zipfs/modpod mount does this, so a running punk shell (or this build) holding that .tm blocks an in-place overwrite.\ + Close shells holding it, or rerun - the update now replaces such files by delete-then-place rather than overwrite." +} #Availability probe + boot-precondition check of an assembled kit vfs (G-125), shared by the #kit gate in the bake loop and the 'check' command report. Same guarded-require treatment as #the provenance check above: a stale or missing punkboot::utils snapshot must not brick the @@ -4370,31 +4424,47 @@ if {$::punkboot::command eq "bootsupport"} { $boot_event targetset_started # -- --- --- --- --- --- puts "\nBOOTSUPPORT module$which update: $srcfile -> $tgtfile" - if {[catch { - file copy -force $srcfile $tgtfile - } errM]} { - #a copy onto a modpod this make.tcl process has mounted (e.g punk::mix::templates) - #fails on windows even when the target content is already current (a prior failed - #record forces a retry). If content is already identical, record OK so the record - #converges instead of churning FAILED forever. - set already_current 0 - if {[file exists $tgtfile] && [file size $tgtfile] == [file size $srcfile]} { - catch { - set src_ck [dict get [punk::mix::base::lib::cksum_path $srcfile] cksum] - set tgt_ck [dict get [punk::mix::base::lib::cksum_path $tgtfile] cksum] - if {$src_ck ne "" && $src_ck eq $tgt_ck} { - set already_current 1 - } - } + #An in-place overwrite is refused when the target is memory-mapped - + #which is what a mounted zip-based .tm modpod (e.g punk::mix::templates) + #is. replace_possibly_mapped_file falls back to delete-then-place, which + #windows does allow. See its comment for why the error reads "invalid + #argument" and why unlinking is safe for the process holding the mapping. + switch -- [::punkboot::replace_possibly_mapped_file $srcfile $tgtfile errM] { + ok { + $boot_event targetset_end OK + } + replaced { + puts stdout " (in-place overwrite refused - replaced by delete-then-place; target was memory-mapped, e.g a mounted modpod)" + $boot_event targetset_end OK -note "overwrite refused ($errM) - replaced via delete-then-place (target memory-mapped)" } - if {$already_current} { - $boot_event targetset_end OK -note "copy failed ($errM) but target content already identical (self-mounted modpod?)" - } else { - puts stderr "BOOTSUPPORT module$which update FAILED: $tgtfile ($errM)" + broken { + puts stderr "BOOTSUPPORT module$which update FAILED: $tgtfile was unlinked and could not be replaced - restore it from $srcfile by hand ($errM)" $boot_event targetset_end FAILED } - } else { - $boot_event targetset_end OK + default { + #target untouched. If its content is already identical, record OK so a + #prior failed record converges instead of churning FAILED forever. + set already_current 0 + if {[file exists $tgtfile] && [file size $tgtfile] == [file size $srcfile]} { + catch { + set src_ck [dict get [punk::mix::base::lib::cksum_path $srcfile] cksum] + set tgt_ck [dict get [punk::mix::base::lib::cksum_path $tgtfile] cksum] + if {$src_ck ne "" && $src_ck eq $tgt_ck} { + set already_current 1 + } + } + } + if {$already_current} { + $boot_event targetset_end OK -note "copy failed ($errM) but target content already identical" + } else { + puts stderr "BOOTSUPPORT module$which update FAILED: $tgtfile ($errM)" + set _hint [::punkboot::mapped_file_hint $errM $tgtfile] + if {$_hint ne ""} { + puts stderr " $_hint" + } + $boot_event targetset_end FAILED + } + } } # -- --- --- --- --- --- } else {