Repository navigation
feat(iso): create ISO images from folders and archives - #1195
Conversation
KnifeLemon
left a comment
There was a problem hiding this comment.
Thanks @Feraris20, this is a solid piece of work, and the tests around failure and cancellation are very welcome. The in-process writer, the refusals and the archive route all follow what we agreed in #632.
Must fix
1. A shrinking file gives a broken image that counts as success
If a file gets smaller while the image is written (a log file rotating, for example), image.Read ends early, the loop stops and File.Move replaces the destination with a cut-off image. I got 67 MB from 96 MB of content, and it can't be opened. Please fail when copied != image.Length, before the move (FolderIsoConverter.cs:86).
2. Old and far-future file dates
A file dated before 1900 fails the whole image with "Year is out of range" and no file name. A file dated 2200 reads back as 1944. Please clamp the date to what ISO 9660 can hold, 1900 to 2155 (FolderIsoConverter.cs:68).
3. Mark of the Web is lost for folders
JobQueue reads the mark from the source path, and a folder has none. A folder with a downloaded setup.exe becomes an unmarked ISO, while the same file in a marked ZIP gives a marked ISO. If any file going into the image is marked, the image should be marked too.
4. A deleted Folders donut leaves nothing
The donut can be deleted in Settings, and after that a dropped folder opens a donut with no slices. Please fall back to the normal profile choice (RadialViewModel.cs:164).
5. A dropped drive root
Dropping C:\ fails with "The path is empty. (Parameter 'path')". Either give the image a real name or refuse with a clear message.
Please take out of this PR
- Plain ISO 9660. We settled on ISO 9660 + Joliet only in #632, so please remove the option, its editor card and its strings.
- Folder ZIP, 7Z and TAR actions. They're a reasonable idea, but they weren't part of the plan and they deserve their own PR. They also add six presets to the library, and
folder-zipshows up as a second "ZIP". When they come back, 7Z needs a fix: 7-Zip follows a junction inside the folder and packs files from outside it, while ZIP and TAR skip it. - The notification change. Showing the full error is a good change, so please send it as its own small PR. One thing to fix there: each failed card grows from about 195 to 309 px, and the toast has no height limit, so at 1080p about four failed jobs push the newest card and its close button off the top of the screen. A smaller limit on the error text, or a limit on the whole stack, would solve it.
Smaller
- Refusal messages: they're hard-coded in English.
ConversionRefusedExceptionwith an i18n key would translate them. Linked_entries_are_rejected_without_following_them: it fails on Windows without Developer Mode, becauseCreateSymbolicLinkthrowsIOExceptionthere. A junction would test the same thing without that privilege.docs/ENGINES.md: the new section sits above the intro. Please move it further down.
I'll handle these
- Merging main, including moving your migrations to one step on the next free schema number, since main has moved past v70.
- The red CI.
DefaultPlacementTests.InputsOfbuilds inputs from extensions, so the Folders donut has none. I'll add thefolderinput there.
Please leave those two to me and I'll push them after your changes.
|
Thanks for the review. The fixes are pushed in b9d929d.
The plain ISO option, folder archive presets, and notification change have been removed from this PR. Joliet is now the only filesystem, and the Folders donut has one ISO image action. Archive-to-ISO remains available without being enabled on the Archives donut by default. Refusal messages are translated in all 20 languages. The Windows link tests now use junctions, and the ISO images documentation sits immediately after Archives. Validation:
|
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Main took schemas 70 to 72 (jif, java, Metadata), so the Folders donut step becomes the next free one and runs after them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
InputsOf built inputs from extensions, so the Folders donut had none and its ISO slot could never pass. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
fa and bn-BD came in from main after the PR translated its keys into the other languages. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks @Feraris20, this round covers everything from the last review: the shrink check, the clamped dates, the web mark for folders, the donut fallback and the drive-root refusal all work, and the removed parts are gone. I tried a few extra cases too (a file in the middle shrinking, deep trees, case-only names) and they behave well. I've merged main and pushed three small commits on top: the migration is now schema 73 after main's 70–72, One thing to fix, please: Optional, if you have time:
Please pull the branch before your next push, since it has our commits now. |
|
One more from CI: on both Linux jobs, 1901-12-13T20:45:52Z is the smallest 32-bit Unix time, so it's the runner's file system that can't hold the 1800/1900 source date: the file never gets the date the test sets, and the image correctly carries what the file has. Reading the date back after setting it and comparing against |
|
Thanks for checking the extra cases. Both reported test issues are fixed: the archive availability test skips without bundled 7-Zip, and the timestamp test compares against dates the filesystem actually stored. The optional improvements are included too: migration places ISO before ZIP, the Joliet name limit is 62 characters, and Focused checks passed:
|
Main took schema 73 for the Upscale tool (KnifeLemon#1064), so folder inputs' ISO donut is now the v74 step, right after it. The i18n files keep main's text with the ISO keys added; the preset-order test leaves out to-iso as well as Metadata and Upscale. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
KnifeLemon
left a comment
There was a problem hiding this comment.
Thanks @Feraris20: the 7-Zip skip, the preset placement before ZIP, the Joliet name cap and the Linux date test are all in, and CI is green on every platform. Main took schema 73 for Upscale (#1064) in the meantime, so I merged main once more and moved the ISO step to 74. Merging.
What does this change?
Adds ISO creation from folders and supported archives, using ISO 9660 with Joliet by default and offering plain ISO 9660 as an alternative. Folder profiles also offer ZIP, 7Z, TAR, TAR.GZ, TAR.BZ2 and TAR.XZ. ISO can be added to the Archives donut manually; it is not enabled there by default.
Conversion failure notifications now show the complete error message instead of cutting it off after four lines. The text can be selected and copied, and longer messages can be scrolled within the notification. This makes errors such as unsupported Joliet filenames readable in full.
Existing destination files are preserved when ISO creation fails or is cancelled. Password-protected archives are rejected with a clear error; password entry is not added. Individual files must be smaller than 4 GiB. Related to #632.
How was it tested?
98 focused tests passed on Windows x64, with no failures or skips, covering the ISO additions, folder archive actions, migrations, CLI, donut/editor behavior and notification details. Formatting verification passed. The full project suite is not claimed complete.
The portable ZIP was extracted and tested separately, including direct application startup, folder and archive conversions, both ISO standards, and safe rejection of an encrypted ZIP. A 2,001-file tree with Unicode names and 20 empty directories round-tripped with matching hashes. A file one byte below 4 GiB converted and extracted with a matching hash; an exact 4 GiB file was rejected without replacing an existing image.
Generated archive conversion fixtures cover ZIP, JAR, 7Z, TAR, TAR.GZ, TAR.BZ2, TAR.XZ, ALZ, EGG, GZ, BZ2, XZ, LZMA and LZIP. Other supported archive formats have route checks, but not new end-to-end fixtures. Linux and macOS runtime testing has not been performed.
Scope
This includes plain ISO 9660, archive-to-ISO conversion and folder archive actions in addition to the original Joliet-only request in #632. Those additions are called out here for review. Portable-storage initialization changes from the local testing build are excluded from this PR.