-
Notifications
You must be signed in to change notification settings - Fork 686
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
layer: revamp for readability #255
Conversation
Fixes #196 @opencontainers/image-spec-maintainers PTAL For convenience, the view or the document is https://github.com/vbatts/oci-image-spec/blob/clarify_tar_entry_order/layer.md |
b7e2fec
to
6c8bf26
Compare
NOTE: a copy-on-write or union filesystem can make this very efficient: | ||
### Populate a Comparison Filesystem | ||
|
||
Create a new directory and initialize it with an identical copy or snapshot of the prior root filesystem. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we make an identical copy
more clear here or add a example ? if we just cp -r rootfs-c9d-v1 rootfs-c9d-v1.s1
, this is not an identical copy ,the timestamp is changed and I think the timestamp is important to generate diff. To make a copy identical use cp
, we should along with -p
flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wondered about that. Something like cp -pav <src> <dst>
or rsync -avPHS <src> <dst>
or so on. These are all very Linux centric, which may be fine for starters, but this line is more of the rule, rather than the example.
Perhaps, having a statement of the files, their contents, and all available attributes (timestamps, mode, owner, group, xattrs, etc)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, identical should be more thoroughly defined
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonboulle so, then provide the reader with example commands to accomplish this?
I reckon "identical" could get into inode discussions and otherwise. Though just a couple lines down the attributes are mentioned. Perhaps this ought to itemize the important attributes when making a copy?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we are defining tar as the distributable format, then can't we lean on that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonboulle fair, though most tar usages are through the golang library and up to an implementer.
Something like a tar command would be tar --xattrs --selinux --acls -c .
where selinux is supported and step back from there?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated @coolljt0725 @jonboulle how it that?
6c8bf26
to
c375847
Compare
### Populate a Comparison Filesystem | ||
|
||
Create a new directory and initialize it with an identical copy or snapshot of the prior root filesystem. | ||
Any changes to the snapshot MUST not change or affect the directory it was copied from. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MUST NOT
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yeah. got it
c375847
to
3cae2d8
Compare
|
||
The resulting Tar archive for `f60c56784b83` has the following entries: | ||
- Added and modified files and directories in their entirety | ||
- Deleted files or directory marked with a [whiteout file](#whiteouts) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
directories
5d71bad
to
728e772
Compare
|
||
An example of creating an Image Filesystem Changeset follows. | ||
A layer changeset is the distributable difference between filesystems. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like this:
A layer encodes a distributable changeset between filesystems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
got it.
|
||
### File Attributes | ||
|
||
File attributes that are provided for Additions and Modifications include: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should replace “are provided” with something testable. I expect at least:
Implementations MUST support the following file attributes for Additions and Modifications:
Although I'd still rather get out of the file-attribute-requiring business or cite a tar-like spec such as pax.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wking your chain of links and footnotes kill me.
The options your proposing are getting in the weeds. The requirement are known and have been worked with up to this point. If there are features needed to be added to tools, I'm certain folks will do what they need to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Wed, Sep 07, 2016 at 06:32:48PM -0700, Vincent Batts wrote:
+File attributes that are provided for Additions and Modifications include:
@wking your chain of links and footnotes kill me.
Sorry, it's hard to preserve the continuity of a sub-thread when
GitHub keeps hiding it on us :p. Links and footnotes are the best I
can do to make the older conversation discoverable without rehashing
all of it.
The options your proposing are getting in the weeds. The requirement
are known and have been worked with up to this point. If there are
features needed to be added to tools, I'm certain folks will do what
they need to do.
That sounds like (a) (Punt the specifics to users) from 1, and I'm
ok with that. However, in that case, I see no particular use to call
out specific file types or attributes. If a user wants to put some
oddball filetype/attribute into the tarball, more power to them. If
an implementer wants to only support regular files, directories, and
mode (ignoring or erroring on symlinks, owner, mtime, etc.), more
power to them as well. Sitting in the middle with a half-done spec
(e.g. requiring ‘xattrs’ but not defining it) doesn't seem helpful.
4b40467
to
62565b3
Compare
@philips broken link? what was missed in the rebase? |
See [Change Types](#change-types) for more details on changes. | ||
|
||
For example, add a directory at `/etc/my-app.d` containing a default config file, removing the existing config file. | ||
Also a change (be it change in attribute or file content) to `./bin/my-app-tools` binary to handle the config layout change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
be it change
? typo here? I don't quite understand this from my english
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may be ok as it stands, but I think it would read more clearly if we dropped "be it change".
When adding verbiage for restriction on duplicate entries in the tar archive, it was not immediately clear where this information would be organized. This revamp seeks to make the document on the whole more suitable for implementors and be interpretable for compliance. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>
62565b3
to
f5a91ff
Compare
LGTM @vbatts the link was to a comment that is now hidden because you fixed the issue. Thanks! |
@opencontainers/image-spec-maintainers please take a look and LGTM |
|
||
### File Types | ||
|
||
Throughout this document section, the use of word "files" includes: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"or entries" maybe?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just submitted a PR to your branch using the edit feature... give that a try?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah yeah. Now that this is merge'able, can I carry that to a new PR?
This didn't make it into 17c1f00 (*: rename serialization.md -> config.md, 2016-09-13, opencontainers#305), but I think we still want it [1,2,3]. While I was touching headers, I shifted the informative example section down under the normative property definitions, and move the normative whitespace rule up out of the example section. [1]: opencontainers#255 (comment) [2]: opencontainers#305 (comment) [3]: opencontainers#305 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
This didn't make it into 17c1f000 (*: rename serialization.md -> config.md, 2016-09-13, #305), but I think we still want it [1,2,3]. While I was touching headers, I shifted the informative example section down under the normative property definitions, and move the normative whitespace rule up out of the example section. [1]: opencontainers/image-spec#255 (comment) [2]: opencontainers/image-spec#305 (comment) [3]: opencontainers/image-spec#305 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
This didn't make it into 17c1f000 (*: rename serialization.md -> config.md, 2016-09-13, #305), but I think we still want it [1,2,3]. While I was touching headers, I shifted the informative example section down under the normative property definitions, and move the normative whitespace rule up out of the example section. [1]: opencontainers/image-spec#255 (comment) [2]: opencontainers/image-spec#305 (comment) [3]: opencontainers/image-spec#305 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
This didn't make it into 17c1f000 (*: rename serialization.md -> config.md, 2016-09-13, #305), but I think we still want it [1,2,3]. While I was touching headers, I shifted the informative example section down under the normative property definitions, and move the normative whitespace rule up out of the example section. [1]: opencontainers/image-spec#255 (comment) [2]: opencontainers/image-spec#305 (comment) [3]: opencontainers/image-spec#305 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
This didn't make it into 17c1f000 (*: rename serialization.md -> config.md, 2016-09-13, #305), but I think we still want it [1,2,3]. While I was touching headers, I shifted the informative example section down under the normative property definitions, and move the normative whitespace rule up out of the example section. [1]: opencontainers/image-spec#255 (comment) [2]: opencontainers/image-spec#305 (comment) [3]: opencontainers/image-spec#305 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
This didn't make it into 17c1f000 (*: rename serialization.md -> config.md, 2016-09-13, #305), but I think we still want it [1,2,3]. While I was touching headers, I shifted the informative example section down under the normative property definitions, and move the normative whitespace rule up out of the example section. [1]: opencontainers/image-spec#255 (comment) [2]: opencontainers/image-spec#305 (comment) [3]: opencontainers/image-spec#305 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>
When adding verbiage for restriction on duplicate entries in the tar
archive, it was not immediately clear where this information would be
organized.
This revamp seeks to make the document on the whole more suitable for
implementors and be interpretable for compliance.
Signed-off-by: Vincent Batts vbatts@hashbangbash.com