| From 1f205c0aec5ea9e983d61a64e7ce871ae416bebd Mon Sep 17 00:00:00 2001 |
| From: "W. Trevor King" <wking@tremily.us> |
| Date: Tue, 18 Oct 2016 02:16:46 -0700 |
| Subject: [PATCH 1/2] image/manifest: Recursively remove pre-existing entries |
| when unpacking |
| |
| Implementing the logic that is in-flight with [1], but using recursive |
| removal [2]. GNU tar has a --recursive-unlink option that's not |
| enabled by default, with the motivation being something like "folks |
| would be mad if we blew away a full tree and replaced it with a broken |
| symlink" [3]. That makes sense for working filesystems, but we're |
| building the rootfs from scratch here so losing information is not a |
| concern. This commit always uses recursive removal to get that old |
| thing off the filesystem (whatever it takes ;). |
| |
| The exception to the removal is if both the tar entry and existing |
| path occupant are directories. In this case we want to use GNU tar's |
| default --overwrite-dir behavior, but unpackLayer's metadata handling |
| is currently very weak so I've left it at "don't delete the old |
| directory". |
| |
| The reworked directory case also fixes a minor bug from 44210d05 |
| (cmd/oci-image-tool: fix unpacking..., 2016-07-22, #177) where the: |
| |
| if fi, err := os.Lstat(path); !(err == nil && fi.IsDir()) { |
| |
| block would not error out if the Lstat failed for a reason besides the |
| acceptable IsNotExist. Instead, it would attempt to call MkdirAll, |
| which would probably fail for the same reason that Lstat failed |
| (e.g. ENOTDIR). But it's better to handle the Lstat errors directly. |
| |
| [1]: https://github.com/opencontainers/image-spec/pull/317 |
| [2]: https://github.com/opencontainers/image-spec/pull/317/files#r79214718 |
| [3]: https://www.gnu.org/software/tar/manual/html_node/Dealing-with-Old-Files.html |
| |
| Signed-off-by: W. Trevor King <wking@tremily.us> |
| --- |
| image/manifest.go | 22 +++++++++++++++++++--- |
| 1 file changed, 19 insertions(+), 3 deletions(-) |
| |
| diff --git a/image/manifest.go b/image/manifest.go |
| index 8834c1e5f2f0..144bd4f62219 100644 |
| --- a/src/import/image/manifest.go |
| +++ b/src/import/image/manifest.go |
| @@ -253,11 +253,27 @@ loop: |
| continue loop |
| } |
| |
| + if hdr.Typeflag != tar.TypeDir { |
| + err = os.RemoveAll(path) |
| + if err != nil && !os.IsNotExist(err) { |
| + return err |
| + } |
| + } |
| + |
| switch hdr.Typeflag { |
| case tar.TypeDir: |
| - if fi, err := os.Lstat(path); !(err == nil && fi.IsDir()) { |
| - if err2 := os.MkdirAll(path, info.Mode()); err2 != nil { |
| - return errors.Wrap(err2, "error creating directory") |
| + fi, err := os.Lstat(path) |
| + if err != nil && !os.IsNotExist(err) { |
| + return err |
| + } |
| + if os.IsNotExist(err) || !fi.IsDir() { |
| + err = os.RemoveAll(path) |
| + if err != nil && !os.IsNotExist(err) { |
| + return err |
| + } |
| + err = os.MkdirAll(path, info.Mode()) |
| + if err != nil { |
| + return err |
| } |
| } |
| |
| -- |
| 2.4.0.53.g8440f74 |
| |