Conversation
#1899) Refs #1898 On Unix, `podup update` used to rename the running binary to `podup.old` before writing the new one. That lost the operator's mode (every update came out `0755`, because the mode was read from a path that no longer existed) and left the path empty while the new binary was written. `move_target_aside` now hard-links the target to `.old` on Unix and leaves the target where it is. The staged file is still renamed over the target atomically, so the path never disappears and `write_temp` reads the real mode. I went with a hard link rather than a copy after trying the copy first, which broke three things: it followed a symlink planted at `.old`, it could not read a `0111` binary, and a failed staging rolled back to a copy without the original's owner, ACLs, xattrs or capabilities. A hard link keeps the original inode, needs no read permission, and `link(2)` refuses to follow a symlink at the destination. When linking fails (a filesystem without hard links) it falls back to the old rename, so that case behaves exactly as before. `restore_from_backup` removes `.old` instead of renaming it when it is still the same inode as the target, meaning the swap never happened. Windows is unchanged. What I ran: - `podup update --force` against the published v5.9.8 release, binary in a scratch dir at `0700`, with a loop checking the path the whole time. Released 5.9.8: mode `700 -> 755`, 362 samples with no binary at the path. This branch: `700 -> 700`, 0 samples, no `.old` left behind. - The same run with a `0111` binary (stays `0111`) and a `0755` binary behind a symlink (symlink intact, `0755`). - Reverting to the rename turns 4 of the install tests red, reverting to the copy turns 3 red, and dropping the same-inode check turns 1 red. - `cargo fmt --check`, `cargo clippy --all-targets --all-features -D warnings`, and the full `cargo test --all-features` including `engine_integration` against local Podman. One run hit `layer not known` in `cli_up_with_build_flag`, a Podman storage error unrelated to this change; it passed three times on its own and in the other full run. Not covered: the rename fallback is not exercised by a test, since forcing `link(2)` to fail needs fault injection. --------- Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Bumps `Cargo.toml`, `Cargo.lock` and `debian/changelog` to 5.9.9 for the self-update fix in #1899. Signed-off-by: Jaro-c <75870284+Jaro-c@users.noreply.github.com>
Jaro-c
added
prio:P2
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release 5.9.9: the Unix self-update keeps the binary's mode and path during the swap (#1899).