LevelXStudios/OpenGameCompressor
4
2
Fork 0
Code Issues Pull requests Projects Releases 2 Packages Wiki Activity Actions

Fix the remaining review findings and settle the open decisions #4

Merged
SkyfaR merged 54 commits from remaining-findings into main 2026-09-30 18:46:52 +02:00
Conversation 0 Commits 54 Files changed 99 +3425 -578
SkyfaR commented 2026-09-30 18:32:10 +02:00
Owner
Copy link

The remaining findings of the code review: 11 of medium and 22 of low severity, one commit per finding, plus the three
decisions taken on the points the review left open. Pull requests #1 to #3 covered the critical and the high ones.

Migration

  • btrfs-convert is asked to keep the label; device-mapper volumes (LUKS, LVM) are found under either of their names
    and known by their name alone.
  • A finished migration gives way to a new plan; an unfinished one is never replaced, and an unreadable saved
    migration is shown instead of overwritten.
  • Steam is reached when it is installed as a Flatpak.
  • Before a re-download formats the drive, the drive is compared with the plan and the backup once more. A folder,
    a Steam library or a game that appeared after the plan was made, or a file that was changed, added or deleted after
    the backup, ends the run with its path and asks for a new plan. Nothing is changed on the drive. Files whose times
    differ are read and compared, so a backup drive that cannot store a time is no reason to refuse. A drive the helper
    left unmounted has to be mounted first. The step is announced and can be cancelled.
  • A backup makes room where it copies to, so a new plan can use the backup folder of a discarded one; the space
    check counts what is replaced as free, and nothing else in the folder is touched. The restore copies back only the
    Steam files of its own plan. A copy gets the time its file had before it was read. A folder that cannot be read
    counts as data, never as empty.
  • The top folder of the new filesystem goes to the user when they could not write to the old one (the
    mkfs.ext4 case: root owns the top folder, the user the folders below). Until now, copying the backup back failed
    with "permission denied" after the drive had been formatted. The run says so in a warning.

Compression

  • Files their owner cannot write to are compressed (Proton ships many as 0555); a game folder that is a symlink is
    accepted.
  • A changed zstd level is applied to files that are compressed already, and recorded on kernels that ignore it.
  • The marker is written only after the data is on disk and is bound to the file it was written for; files the
    compressor leaves alone count no savings.
  • A game that was reinstalled or moved at the same build is compressed again; Steam's state is asked again right
    before each game.
  • ogc compress without --level keeps the level recorded for a game instead of rewriting it with level 3, and
    reads the records after it has waited for another run to finish.

Desktop app

  • A game is analysed again after the wizard, the timer or the CLI compressed it; a cancelled compression is no longer
    shown as completed; library rows keep their state across a reload.
  • The data step shows only the result of the latest scan; the run step keeps its state on a visit; the phase list
    follows a running migration.
  • A changed zstd level reaches the timer unit; two background analyses no longer wait for each other.

Tests, CI and packaging

  • CI runs the image tests, and all tests as an unprivileged user.
  • Translations: wrong variables are caught, keys are checked wherever they are used, LANGUAGE takes precedence over
    the locale.
  • Rust 1.92 and Pango 1.56 are declared as requirements; the Arch package builds with a redirected cargo target
    directory; the VM test works with rootless podman and installs qemu without a partial upgrade.

Checked

  • cargo fmt --check, cargo clippy --all-targets -- -D warnings with and without the desktop app
  • all tests, including the GUI tests on a headless display and the image tests
  • the VM test with all five scenarios (convert redownload label resume mapper); the redownload scenario now also
    covers a folder created after the plan, a drive the helper left unmounted, a save game rewritten and a file deleted
    after the backup, and a second plan whose backup goes over the first
  • the new checks were mutated one at a time (53 mutants); a test fails for 52 of them, the exception being one of
    two cancel checks in the same function
  • two rounds of adversarial review of the three decisions and of the fixes that followed
  • the check job of the CI workflow in a fresh Arch container

Not checked here

These need a real desktop or real hardware: the changes in the running desktop app, a compression run over Proton's
read-only files in a real library, Steam as a Flatpak, a real LUKS or LVM volume, reinstalling a game at the same
build, and a re-download on a drive whose top folder belongs to root (the owner change is covered by unit tests only,
because the VM test runs as root).

The remaining findings of the code review: 11 of medium and 22 of low severity, one commit per finding, plus the three decisions taken on the points the review left open. Pull requests #1 to #3 covered the critical and the high ones. ## Migration - `btrfs-convert` is asked to keep the label; device-mapper volumes (LUKS, LVM) are found under either of their names and known by their name alone. - A finished migration gives way to a new plan; an unfinished one is never replaced, and an unreadable saved migration is shown instead of overwritten. - Steam is reached when it is installed as a Flatpak. - **Before a re-download formats the drive, the drive is compared with the plan and the backup once more.** A folder, a Steam library or a game that appeared after the plan was made, or a file that was changed, added or deleted after the backup, ends the run with its path and asks for a new plan. Nothing is changed on the drive. Files whose times differ are read and compared, so a backup drive that cannot store a time is no reason to refuse. A drive the helper left unmounted has to be mounted first. The step is announced and can be cancelled. - A backup makes room where it copies to, so a new plan can use the backup folder of a discarded one; the space check counts what is replaced as free, and nothing else in the folder is touched. The restore copies back only the Steam files of its own plan. A copy gets the time its file had before it was read. A folder that cannot be read counts as data, never as empty. - **The top folder of the new filesystem goes to the user when they could not write to the old one** (the `mkfs.ext4` case: root owns the top folder, the user the folders below). Until now, copying the backup back failed with "permission denied" after the drive had been formatted. The run says so in a warning. ## Compression - Files their owner cannot write to are compressed (Proton ships many as 0555); a game folder that is a symlink is accepted. - A changed zstd level is applied to files that are compressed already, and recorded on kernels that ignore it. - The marker is written only after the data is on disk and is bound to the file it was written for; files the compressor leaves alone count no savings. - A game that was reinstalled or moved at the same build is compressed again; Steam's state is asked again right before each game. - **`ogc compress` without `--level` keeps the level recorded for a game** instead of rewriting it with level 3, and reads the records after it has waited for another run to finish. ## Desktop app - A game is analysed again after the wizard, the timer or the CLI compressed it; a cancelled compression is no longer shown as completed; library rows keep their state across a reload. - The data step shows only the result of the latest scan; the run step keeps its state on a visit; the phase list follows a running migration. - A changed zstd level reaches the timer unit; two background analyses no longer wait for each other. ## Tests, CI and packaging - CI runs the image tests, and all tests as an unprivileged user. - Translations: wrong variables are caught, keys are checked wherever they are used, `LANGUAGE` takes precedence over the locale. - Rust 1.92 and Pango 1.56 are declared as requirements; the Arch package builds with a redirected cargo target directory; the VM test works with rootless podman and installs qemu without a partial upgrade. ## Checked - `cargo fmt --check`, `cargo clippy --all-targets -- -D warnings` with and without the desktop app - all tests, including the GUI tests on a headless display and the image tests - the VM test with all five scenarios (`convert redownload label resume mapper`); the `redownload` scenario now also covers a folder created after the plan, a drive the helper left unmounted, a save game rewritten and a file deleted after the backup, and a second plan whose backup goes over the first - the new checks were mutated one at a time (53 mutants); a test fails for 52 of them, the exception being one of two cancel checks in the same function - two rounds of adversarial review of the three decisions and of the fixes that followed - the `check` job of the CI workflow in a fresh Arch container ## Not checked here These need a real desktop or real hardware: the changes in the running desktop app, a compression run over Proton's read-only files in a real library, Steam as a Flatpak, a real LUKS or LVM volume, reinstalling a game at the same build, and a re-download on a drive whose top folder belongs to root (the owner change is covered by unit tests only, because the VM test runs as root). 
btrfs-convert was called without --copy-label. By its documentation
and by what it prints ("Target filesystem: Label:" is empty), the new
filesystem then has no label, while formatting has always passed the
old one on. A drive without an fstab entry, which the desktop mounts
under /run/media/<user>/<label>, would come back under its UUID after
the next login, and the Steam library path would be gone.

btrfs-progs 7.1 happens to keep the label anyway, so nothing was lost
with that version. The option is passed now instead of relying on it,
the image test creates its ext4 image with a label and expects it
after the conversion, and the VM scenarios `convert` and `label` check
it on the real drive.
A record of the helper finds its drive again by partition UUID and by
the ID of the disk, and formats a drive that an interrupted run left
empty only with such an ID. A partition takes the disk's ID from the
disk it is on, and so did every other device with a parent: an LVM
volume or an opened LUKS container got the ID of the disk below it.

That ID would make one such volume pass for another. They have no
partition UUID, so a record of one volume matched any other volume of
the same size on that disk. With the recorded volume removed or
renamed after an interrupted format, the next run would have taken an
empty sibling for it and formatted that. The helper could not work on
such volumes so far; the next change lets it.

Only partitions take the disk's ID now. A device-mapper or RAID
volume is found under its recorded name only, and an empty one is
formatted only while it is the same kernel device in the same boot,
like a filesystem on a whole disk that has no ID.
lsblk lists an opened LUKS container or an LVM volume as
/dev/mapper/<name>, which udev makes a link to /dev/dm-<n>. The helper
resolved the path it was given and then looked for a device that lsblk
lists under exactly the resolved name. For such a volume there is
none, so every migration of one ended with "is not a block device
known to lsblk": nothing was changed, but the drive had been offered
and its backup had already been made.

The lookup now compares what both names resolve to, and so does the
check that a continued run is asked for the drive of its record. The
fstab entry of such a volume usually names /dev/mapper/<name> as well;
it is compared with the resolved device and is recognised too. The
containers themselves, crypto_LUKS and LVM2_member, are refused as
before.

The VM test gains the scenario `mapper`, a conversion on a linear
device-mapper volume that fstab names by its path. It is not among
the scenarios that scripts/vm-e2e.sh runs by default.
The plan file stays after a migration has finished, because
`ogc migrate delete-backup` and `status` still need it. `ogc migrate
plan` refused to write another one as long as any plan existed, and
`abort` refuses once the drive was changed, so after the first
migration no second drive could be planned without deleting the file
by hand.

A plan in the phase Finished is now replaced by the new one. The
backup of the old migration is not deleted. If it is still there, the
command says where it is and that the folder has to be deleted by
hand, since `delete-backup` works on the new plan from then on. Plans
in any other phase stop the command as before.
After a re-download the runner started Steam and sent it the
steam://validate links by running `steam`. The Flatpak puts no such
program on PATH, although its libraries are found and its drives can
be migrated. Both calls failed, the plan stayed in the phase Restored,
where it can neither be finished nor discarded, and the drive stayed
locked for compression.

The client is now run as `steam` where that exists, and otherwise as
`flatpak run com.valvesoftware.Steam` if flatpak and the Flatpak's
Steam folder exist. If neither does, the error says so instead of
"cannot run steam".

A plan without any Steam game no longer starts Steam at all: there is
nothing to download, and such a drive may be on a machine that has no
Steam.
Two confirmed defects of the re-download method are not fixed yet,
because each needs a decision on how the migration should behave.

Copying the backup back fails when the top folder of the old
filesystem belonged to root and only folders below it to the user:
the helper gives the new top folder the same owner. And a run that is
continued long after its backup was made formats the drive without
looking at it again, so what changed since is lost.

Both are listed under the known limitations, with the way out.
The plan of a finished migration stayed on disk for good. Every visit
to the MIGRATE page, also after a restart, resumed it and showed
"Migration finished" with no way back to the first step, so a second
drive could not be migrated until migration.json was deleted by hand.

The finished screen now offers MIGRATE ANOTHER DRIVE. It removes the
record, which is only done for a finished migration, empties the run
page and opens the wizard at its first step. A backup that still
exists is kept, and a message says where it is.

The buttons of the finished screen move to a file of their own. The
helper that runs GTK tests on one thread moves out of the confirm step
so that the tests of other files can use it.
The analysis cache was keyed by the Steam build id alone. When the
migration wizard, the timer or `ogc compress` compressed a game whose
build the app had analysed before, the report from before was used
again: the game showed "0 B saved" next to its full potential, and the
total on the dashboard did not move, also after a restart or RELOAD.

Each cached report now remembers what games.tsv recorded for the game
when it was made, the level and the build id, and is used only while
that record is the same. Compressing a game changes the record, so the
next reload analyses it again. Lines of older cache files count as
made for an untracked game; tracked games are analysed once more.
Scanning a drive can take minutes, and BACK stays available
meanwhile. The data step showed the result of every scan that ended,
whichever ended last, and added its items below those already listed.

After NEXT, BACK, NEXT the list was there twice. Only the upper copy
was read, so ticking the lower "workshop" or "shadercache" box did
nothing, and a re-download then formatted the drive without that
backup. After picking another drive, the plan of the first drive could
arrive last and was handed on together with the check boxes of the
second.

Scans are now counted, and a result is taken only from the latest one.
Showing a result replaces the list instead of extending it.
After CANCEL the worker skips the files it has not reached and does
not record the game, but it reports the run like any other: skipped
files are no failures. The row read "ZSTD:3 · … FREED", the game
counted among the compressed ones on the dashboard and became the
LAST ACTION, until the next reload showed that nothing had recorded
it.

The window now remembers that CANCEL was pressed during the running
job. The run that ends then puts its row back to what it showed
before, and is neither counted nor stored as the last action. The
worker's own flag is not read for this, because the worker clears it
for the next request, which can be before the window has handled the
events of the cancelled job.
A row was named by its position in the list, and jobs report by that
name. A reload builds the list anew, also while a job is queued or
running: RELOAD during the first scan, or the reloads the migration
page triggers. When a game had been installed or removed in between,
every row behind it had moved, and the job's results went to the
neighbouring game. An analysis was then stored in the cache under the
wrong app id and stayed there until that game was updated.

A game now keeps its key, given to its app id and folder when it is
first seen, for as long as the app runs. Rows are looked up by key
instead of by position, so a result reaches its own game or, if that
is gone, no row at all.
Two defects in how the last step of the wizard keeps its state.

Every visit to the MIGRATE page showed the saved migration anew. While
a migration waited for downloads, for the rollback to be removed or
after an error, the text gave way to "continue where it stopped", and
one more CONTINUE and DISCARD PLAN were added each time. A step that
shows the saved plan already is now left alone; a plan that changed on
disk is shown with its buttons replaced instead of added to.

A waiting screen schedules a re-check after a minute, and only the
next waiting screen cancelled it. After CHECK NOW and CANCEL the
re-check still ran and started compressing again, and after FINISH
WITHOUT THEM it ran the finished migration once more. A re-check now
runs only while the screen that scheduled it is still shown.
The list of phases was drawn when a run started and again when it
stopped. In between, the runner is on the worker thread: the redraw on
every reported step looked for it on the page, found nothing and did
nothing. A run that went from the backup through the conversion to the
restore in one go showed "Back up … RUNNING" and everything else as
pending the whole time.

The worker thread now reports each phase it has reached, and the page
keeps a copy of the plan for the time of the run and moves its phase
along. The redraw that could never happen is gone. Drawing the list
moves to a file of its own.
The service unit of the timer names the level for newly installed
games, and it was written only when one of the two switches was
toggled. Raising the level in the same settings card changed the
sidebar and gui.conf, but `ogc auto --include-new` went on with the
level from the moment the timer was enabled.

Changing the level now writes the unit again if the timer is enabled,
compresses new games and names another level. This waits until the
stepper has been at rest for a moment, because writing the unit asks
systemd and blocks the card for that time.
When migration.json was damaged or written by another version, the
error went to the run step, which was not shown: the wizard stayed at
its first step with an empty drive list and no explanation. After
REFRESH it could be walked through, and START saved the new plan over
the file, which may have described a migration stopped between the
backup and the restore.

The page now switches to the run step, which shows the error and
offers nothing, so the wizard is out of reach while the file is there.
START checks the file once more and refuses to replace one it cannot
read, as `ogc migrate plan` does. The message says why no new
migration can be started and that the file can be moved away if none
is unfinished.
MIGRATE ANOTHER DRIVE removed whatever plan was saved at that moment.
With the finished screen left open while another drive was planned
and run on the command line, that was the plan of the new migration,
which could then no longer be finished.

The saved plan is removed only if it is the finished one that is
shown. Anything else stays, and the wizard shows it.
START saved the new plan over whatever was saved. With the wizard left
open while a drive was planned and run on the command line, that
replaced the plan of a migration that may be half done. The command
line refuses in that case.

The wizard now shows the unfinished migration instead of starting the
new one. A finished one may still be replaced.
A background analysis makes way for any other request and queues its
rest again. When the other request was a background analysis too, for
example after RELOAD during the scan at startup, the two made way for
each other forever. Nothing was analysed any more, and the worker sent
millions of job events, which the window handled without ever
returning to the main loop.

A background analysis now takes other waiting background analyses into
its own list and goes on. It still makes way for what the user asks
for.
The defragment ioctl and the marker both need write permission on the
inode. For a file without the owner write bit the kernel answers
EPERM, the file counted as a failure, and a game with one such file
was never recorded as compressed: the command line reported an error
and the timer tried the same game again every hour. Proton ships a
third of its files as 0555.

Such a file now gets the owner write bit while it is rewritten and
marked. The mode is put back on every way out, also when the rewrite
fails, and a mode that cannot be put back is reported as a failure.
Size and mtime do not change, so the marker still matches. Files of
other owners are left as they are and fail as before.
A game compressed at level 3 and compressed again at level 12 kept
nearly all of its data at level 3: the marker no longer matched, but
the extent map showed the files as compressed, and that check cannot
see the level. The run still ended without failures, so the new level
was recorded and shown for the game. The desktop app never forces a
rewrite, so a new level could not be applied from there at all.

The marker now tells two cases apart. A file that is unchanged since
it was processed at another level is rewritten at the new one. A file
without a marker, or one that changed since, is still skipped when
its extents are compressed. On kernels that ignore the level, the
first rewrite shows that, and the remaining files are skipped as
before instead of being written again with the same result.

This means that compressing a library with another level than the
one it was compressed with rewrites it, also when that level is the
default of the command line.
A game folder moved to another drive and linked back into
steamapps/common passed the checks before compressing, which follow
the link, and was then refused by the scanner with "is not a
directory". Analysing and compressing that game failed in the command
line, the desktop app and the timer, while --dir on the same path
worked because it resolves the path first.

The scanner now follows a link at the root. Links below it are still
not followed, and the walk still stays on the filesystem of the
folder the link points to.

Such a link can lead onto a drive with an unfinished migration, so
the migration lock is now also checked for the resolved path.
The analysis estimated every file with its own sample at the chosen
level. The compressor decides differently for files of 1 MiB and
more: it compresses 16 chunks at level 1 and does not rewrite a file
that saves less than 3 % there. For packed data with a few
compressible chunks, the analysis therefore promised savings that
compressing never delivered. The desktop app analyses again after
compressing and kept showing that potential for the game.

The analysis now asks the compressor's own rule for every file it
would otherwise count savings for, and estimates such a file at its
present size. This costs one more sample of at most 2 MiB for large
files that are not compressed yet.
The defragment ioctl starts the rewrite and returns; the kernel
compresses and switches the extents later. The marker was set right
after the call, and the only sync came at the end of the directory.
After a crash or power loss in between, a file could keep its old
uncompressed extents together with a marker that matched. Every later
run skipped it at the marker check and recorded the game as
compressed.

compress_files now returns the markers of the rewritten files instead
of setting them, and compress_dir sets them after the sync succeeded.
A crash before that leaves files without a marker, which are looked
at again. Files that are left alone as incompressible are still
marked at once, because nothing is pending for them.
The marker held the level, the size and the mtime. A copy made with a
tool that keeps extended attributes and times, such as cp -a, rsync
-aX or mv across filesystems, therefore carried a matching marker.
On a drive mounted without compress= the copy is not compressed, but
every file was skipped at the marker check and the game was recorded
as compressed. Only --force helped, which the desktop app does not
offer.

New markers also name the inode and its birth time, which a copy
cannot keep, and start with v2. A copy no longer matches and falls
through to the extent check like a file without a marker.

Markers in the v1 form are still honoured, so existing libraries are
not compressed again. When such a file is seen the next time, its
marker is replaced by the v2 form. Copies made of a library before
that happened are not detected.
On a kernel that ignores the zstd level (older than 6.15), a file that
was compressed at another level is left alone once the first rewrite
has shown that the level has no effect. Its marker kept the old level,
so every later run rewrote the first files of each folder again to
find that out, and never settled.

The marker of such a file now records the level that was asked for:
nothing more can be done for it on that kernel. This path cannot be
reached on a kernel that applies the level, so no test covers it.
A file its owner may not write to gets the owner's write bit while it
is rewritten. A run that is killed in between cannot put the mode
back; the architecture notes and the known limitations now say so.
games.tsv remembered a compressed game by its app id and build id
only. A game that was uninstalled and installed again, or moved to
another library, while Steam still offered the same build had new,
uncompressed files but the old entry. "ogc auto" then reported
nothing to do every hour until the next update, which for many games
never comes, and --include-new did not pick the game up either. The
migration skipped such a game the same way.

An entry now also records the manifest's LastUpdated time and the
install folder, and no longer describes a game when either differs.
"ogc auto" treats that like a new build and keeps the recorded level.
Files that still carry their marker are skipped as before, so an
unneeded run costs little.

The two values are appended as a fourth and fifth column. A file of
an older version still loads; its entries are compared by build id
alone until the game is compressed again. An older version reads the
first three columns of a new file as before.

One entry cannot describe two installs of the same app, for example
in native and in Flatpak Steam. For those only the build id is
compared, as it was, so that they do not take turns on every run.
The service unit doubled every "$" in the path of ogc, as one does
for the arguments of ExecStart. systemd expands no variables in the
program path, so it looked for a file with "$$" in its name. With
ogc installed below such a path, "ogc timer enable" reported success
and the service then failed every hour.

The path is now written with its "$" unchanged. Quotes, backslashes
and "%" are still escaped.
"ogc compress" and "ogc auto" read all manifests once at the start
and checked every game against that snapshot. "ogc compress" then
waits for the compression lock, possibly for a long time, and both
work through their games one after the other. A game Steam began to
update in the meantime still counted as fully installed and was
compressed while Steam rewrote it. The build id recorded afterwards
was the old one, too.

library::refresh reads the manifest of one game again. Both commands
call it before the checks of each game and use the result for the
checks and for the record. A game that has been uninstalled since, or
whose manifest now names another folder, is skipped.
The desktop app and the migration still check a game against the
manifests they read earlier, the app shows a reinstalled game as
compressed until it has been compressed again, and two installs of
one app share a single entry in games.tsv. These are listed under the
known limitations.
An entry written by an older version holds only the build. It matched
any install of that build, so a game that was compressed before the
upgrade and reinstalled afterwards was still never recompressed, and
games that were already stuck stayed stuck.

Such a game is now planned once. If it is still compressed, the run
only reads the markers of its files; afterwards its entry records when
it was installed and where.
The image-conversion tests looked for their tools by running "which",
which the CI container does not have. Every tool then counted as
missing, and the three tests passed without running mkfs.ext4,
btrfs-convert or mkfs.btrfs, although all of them are installed.

The tests now look the tools up in PATH themselves. With
OGC_TEST_REQUIRE_TOOLS set, as in CI, a missing tool is a failure, so
these tests cannot silently skip there again.

CI also ran the tests as root, where the tests that rely on read-only
modes skip themselves. The test step now creates a user and runs
cargo test, together with the Broadway display, as that user.
Cargo.toml and the README promised Rust 1.88, but the default build
cannot work with it: gtk4 0.11.4 and the glib, gio, pango, cairo,
gdk4, gsk4, graphene and gdk-pixbuf crates in Cargo.lock declare
rust-version 1.92, and cargo refuses to build them with an older
compiler. Nothing noticed, because CI uses the current Rust of Arch.

1.92 is the highest rust-version among the locked crates; the next
one below is 1.85. Only the build without the desktop app stays
below it.
The README listed GTK 4.12 and libadwaita 1.6 only. The app also
needs Pango 1.56, for FontMap::add_font_file, which registers the
bundled fonts: the pango crate is built with its v1_56 feature, and
the build fails in the pkg-config check on an older Pango. A
distribution with GTK 4.16, libadwaita 1.6 and Pango 1.54 met the
documented requirements and still could not build the app.
The formatting test passed every variable found anywhere in a
language's own files to each of its messages. A translation that
misspells a variable, or uses one of another message, brought its own
name along and formatted without an error; at runtime the caller does
not pass it, and the user sees the unresolved placeholder.

Each message now gets only the variables its English message uses,
read from the parsed Fluent files. All thirteen languages pass.
The test that looks for the keys used in the code searched for a
call directly followed by the key. rustfmt puts the key on the next
line in nineteen tr!() calls, and the "wiz-waiting-" keys and the two
progress keys of the runner reach translate() through a variable. A
mistyped or renamed key in any of these places went unnoticed, and
the user would have seen the raw key.

The test now skips the whitespace after the opening parenthesis, and
checks every literal that starts with "wiz-waiting-" or "run-" the
way it already did for "wiz-step-". That replaces the two progress
keys the test listed by hand, which only proved that English has
them, not that the code spells them the same way.
The language came from the first of LC_ALL, LC_MESSAGES, LANG and
LANGUAGE that was set. LANG is set in every desktop session, so
LANGUAGE was never read: with LANG=en_US.UTF-8 and LANGUAGE=de:en,
which shows every gettext application in German, ogc, the timer and
the app with the language "System" were English.

The selection now follows gettext. The locale is the first of
LC_ALL, LC_MESSAGES and LANG; the first language in LANGUAGE that is
shipped goes before it, and "C" in that list means English. Without
a locale and in the "C" locale LANGUAGE does not count, so a lone
LANGUAGE no longer selects a language.
The runner handed its work folder to the uid and gid of the user who
started the test. That is right with rootful docker. With rootless
podman, which vm-e2e.sh prefers, the container's root already is
that user, and uid 1000 inside the container is one of the user's
subordinate ids: the chown took the folder away from the user, who
could no longer delete target/vm-e2e/work. Without subordinate ids
the chown failed and turned a passing run into a failed one.

The runner now gives the files to the owner of the mounted folder as
it sees it, which is the right user in both cases, and a failing
chown no longer changes the result. HOST_OWNER is not needed any
more.
The runner stage was built on the guest stage and added qemu with
"pacman -Sy". The guest's fully upgraded layer comes before the
binaries are copied and stays in the build cache, while the qemu
layer comes after them and is rebuilt on every code change. Weeks
later that installed the current qemu onto the system libraries of
the cached layer, which Arch does not support: qemu can then fail to
start, and every scenario reports FAIL without a result line.

The runner is now its own Arch image, fully upgraded together with
qemu before anything is copied into it. That layer no longer depends
on the binaries, and it is never an install onto an older system. It
only needs qemu and mkfs.ext4; the guest filesystem is copied in as
before.
build-arch-package.sh took the source tarball from target/package,
but cargo writes it below its target directory, which CARGO_TARGET_DIR
or build.target-dir in a cargo configuration can move elsewhere. The
script then failed with "cannot stat", or, when an older tarball of
the same version still lay in ./target/package, packaged those stale
sources without a word.

cargo package now gets the directory the script reads from, as the
PKGBUILD and vm-e2e.sh already do for their builds.
`ogc compress` used level 3 whenever `--level` was not given. Since a
changed level is applied to files that are compressed already, a plain
`ogc compress --all` rewrote a library that the desktop app had
compressed at another level, and the app then rewrote it back.

Without `--level`, each game now keeps the level it was compressed
with. Games that were never compressed, and folders given with
`--dir`, still get 3.
A re-download that was planned or backed up earlier and continued later
formatted the drive and restored that backup. What had changed on the
drive in between was lost, and a folder created after the plan was made
was neither backed up nor listed as unprotected.

Before the backup starts, and again right before the helper is called
for a drive that is still untouched, the drive is scanned again: a Steam
library or irreplaceable data the plan does not know ends the run. Once
backed up, every selected item is also compared with its copy by name,
kind, link target, size and modification time. The run then names the
path and asks for a new plan; nothing on the drive is changed.

The VM test covers both cases in the redownload scenario.
After a re-download the helper gave the top folder of the new filesystem
the owner and mode of the old one. Where that was root with mode 755 and
only the folders below belonged to the user, which is what mkfs.ext4
leaves, copying the backup back failed with "permission denied" on every
attempt, after the drive had been formatted.

The client now asks the helper for the invoking user as owner when the
recorded owner and mode would keep that user from creating anything in
the top folder. The group is kept if the user is a member of it, and the
run reports the change as a warning. A top folder the user could write
to, and a conversion, keep what they had.
The runner was at the 500 lines a file may have. No change in behaviour.
A discarded plan keeps its backup, and the next plan for the drive gets
the same folder. The new backup was copied over the old one: a file that
had been deleted on the drive stayed in the copy, which then failed its
verification on every attempt, and a folder where a file had been could
not be copied at all. The space check also counted the old backup as
used although it was about to be replaced.

This was the way a run now points to when it finds the drive changed
after its backup, so it has to work: the backup empties its own "data"
and "steam" folders first, and the space check adds what they hold to
the free space when that is short.
The times of a copied file were taken from the source after the copy.
A file that a program rewrote in place while it was being copied passed
its new time on to a copy that held the old beginning, and the
comparison before formatting took the two for the same.
A folder the scan could not list, such as another user's, was measured
as empty. An empty item is never listed as unprotected, and the scan
before formatting did not report it either, so a re-download erased it
without having backed it up or asked. It now counts as one byte at
least: selected, its backup fails with the reason; left out, its loss
has to be accepted.
ogc compress read games.tsv before it waited for the desktop app, the
timer or a migration to finish. What those recorded in the meantime was
not seen, and a game they had just compressed at level 9 was rewritten
at 3 by a run that named no level.

The README said that a game keeps its level without --level. That holds
for Steam games; a folder given with --dir has no record and gets 3. A
test pins that the command line names a level only when asked.
A review of the comparison a re-download makes before it formats found
ways past it, and ways in which it refused a drive for no reason:

- A drive the helper had left unmounted in an interrupted run was not
  compared at all, and the helper continued on it. The run now ends and
  asks for the drive to be mounted.
- A file whose time the backup drive cannot store (FAT knows none before
  1980) differed from its copy right after the backup, on every attempt.
  Files whose times differ are now read and compared.
- A game installed after the plan was made was not asked for again, and
  a file in a library's steamapps folder that the backup does not hold
  was lost. Both end the run.
- The scan and the comparison can take minutes and showed nothing. They
  are announced as a step and can be cancelled; a cancel pressed before
  the helper is called ends the run there.
- The messages said "was added" and "was changed" for paths that were
  deleted, or that had been there all along, and named the action to
  take with other words than its button in Korean, Turkish, Chinese and
  Japanese; Spanish switched from usted to tú.

The comparison moves into a module of its own, and the part of the check
that needs no Steam gets a test. The VM test covers the unmounted drive
and a file deleted after the backup.
The owner of the new top folder is decided when the helper is first
called, and recorded by it. A run that continues from such a record
announced the change from its own arguments, which the helper ignores
then. The warning now appears only for a drive that is still untouched,
and says that the folder also becomes writable, which is all that
changes when the user owned it already.

The tests now pin the mode in the helper's arguments, all three
permission bits, and that the user's groups are read. What remains when
a run is continued by another user is listed as a limitation.
To start from an empty folder, a backup emptied "data" and "steam" in
its backup folder as a whole. A plan of version 0.1.0 can name a folder
of the user's there, and a folder called "steam" on a backup drive is
not far-fetched.

Only the place of each selected item and the files of each library are
removed now, after links among an item's parents were replaced, so that
nothing is deleted through a link an earlier backup stored. An item
whose path is not one below the mount point, as only a plan that was
tampered with can name it, ends the backup before anything is removed.
The space check counts the same places.
A second review of the backup's new removal found four things:

- The restore copied back everything below "steam", also what an earlier
  plan had left there for a library that is an item by now. Its old
  manifest made the item hold one file more than its copy, and the
  restore never verified, on a drive that was formatted already. The
  restore now copies the files of the plan's libraries only.
- The space check counted the room of all old copies, but each was
  removed only when its item's turn came. A large item that came first
  could fill the backup drive with the old copy of a later one still
  there. Room is now made for all items before the first is copied.
- Only item paths were checked for leaving the drive. A library path
  from a tampered plan had the backup delete files outside its folder.
  Both are checked before anything is removed, in the restore as well,
  and links on the way to a library's copy are replaced like those on
  the way to an item's.
- delete-backup removed a plain file called "data" or "steam", which
  the backup never writes.

The architecture notes follow, also on when the owner of the new top
folder is recorded.
Name the file a comparison cannot read
All checks were successful
CI / Format, lint and test (pull_request) Successful in 2m4s
Details
CI / Arch package (pull_request) Successful in 2m12s
Details
CI / Format, lint and test (push) Successful in 2m4s
Details
CI / Arch package (push) Successful in 2m8s
Details
635937793b 
A file that could not be opened or read while it was compared with its
copy ended the run with the bare error of the system, and nothing said
which of thousands of files it was.

The Japanese, Korean and Chinese texts for a path the plan does not
cover read as if the plan were about to be erased. The roadmap said the
owner of the new top folder is fixed by the first call of the helper;
it is recorded each time the helper finds the drive untouched.
SkyfaR merged commit 635937793b into main 2026-09-30 18:46:52 +02:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
LevelXStudios/OpenGameCompressor!4
No description provided.