oci/*: major split ups #90
Merged
Conversation
This file contains 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
|
On Sat, Feb 18, 2017 at 01:01:00PM -0800, Aleksa Sarai wrote:
* oci: oci/generate -> oci/spec/generate
I know runtime-tools uses ‘Spec’ for the config Go type, but I'd
recommend using ‘config’ instead of ‘spec’. Lots of things are
spec'ed, but what you're generating here is an image-spec
configuration. For more on this, see
opencontainers/runtime-tools#266.
|
|
On Sat, Feb 18, 2017 at 01:01:00PM -0800, Aleksa Sarai wrote:
* .: remove LICENSE
From the commit message for e764c54:
It confuses GitHub's automated license detection.
Because the content you have there is the boilerplate, not the license
itself. More on this exact issue in licensee/licensee#103. What you
should have there is the Apache license itself (for an example, see
opencontainers/project-template).
The header in every file describing the license is sufficient.
I don't think that's true. From the Apache license itself [1]:
4. Redistribution. You may reproduce and distribute copies of the
Work or Derivative Works thereof in any medium, with or without
modifications, and in Source or Object form, provided that You
meet the following conditions:
a. You must give any other recipients of the Work or Derivative
Works a copy of this License; and
|
cyphar
force-pushed
the
oci-split-up
branch
2 times, most recently
from
341fa9e to
22c6b39
Compare
|
@wking I already have a full copy of the license in COPYING. Not to mention that the header itself tells you where you can get a copy of the license. This is really not the point of this PR. It'd be great if you didn't derail a PR like this with license discussion. |
cyphar
force-pushed
the
oci-split-up
branch
3 times, most recently
from
5aa61e1 to
7beb7d9
Compare
|
On Sun, Feb 19, 2017 at 01:04:55PM -0800, Aleksa Sarai wrote:
@wking I already have a full copy of the license in COPYING.
That should be all you need then. No need to have a separate LICENSE
with just the boilerplate. Removing it (like you're doing in this PR)
should un-confuse Licensee by avoiding licensee/licensee#103.
This is _really_ not the point of this PR.
I'd missed the presense of a separate COPYING (sorry about that).
Maybe pull e764c54 into its own PR? I think the change it's making
is good, although I'd prefer the commit message to be closer to:
LICENSE: Remove boilerplate file (full license still in COPYING)
It confuses GitHub's automated license detection
(licensee/licensee#103). And with the full license in COPYING,
there's no reason to have a separate file with just the boilerplate.
|
cyphar
force-pushed
the
oci-split-up
branch
2 times, most recently
from
317368c to
e9a0db2
Compare
This is something that will almost certainly bite us in the future. Ultimately while it doesn't _really_ matter at the moment (and especially not for casual uses of umoci) it could cause issues for reproducible builds of images (something that will certainly become more important in the future). Signed-off-by: Aleksa Sarai <[email protected]>
Another cleanliness improvement, to get oci/ more usable as a series of composable libraries. Generation of specifications and mutation of an image will at some point both live within oci/spec [or there will be some splitting of the two parts]. Signed-off-by: Aleksa Sarai <[email protected]>
The conversion code in oci/generate really belonged in a separate package because it does not use any aspect of the image configuration _generation_. Not to mention that conversion is actually going to be defined in the spec, so it should be as self-contained as possible. Signed-off-by: Aleksa Sarai <[email protected]>
GC implements a lot of really cool recursive descent functionality, but it wasn't really exposed. Split out the core parts into Walk() -- which is meant to mirror "path/filepath".Walk. GC will be reimplemented to use the new functionality. In addition, implement Reachable and Paths which return information about the set of descriptors and digests which can be reached. This change also includes replacing gc.mark with Reachable. Signed-off-by: Aleksa Sarai <[email protected]>
Garbage collection, blob auto-detection and walking are all built on top of the CAS abstraction and don't belong in the same place as the implementation or Engine interface. In addition, Instead of making cas.Engine an argument to every extension function, use the embedded struct method of adding extensions to an interface. This also provides a template for how other extensions (third-party or otherwise) could be written. Signed-off-by: Aleksa Sarai <[email protected]>
cyphar
force-pushed
the
oci-split-up
branch
7 times, most recently
from
bc6519f to
4d3eb0c
Compare
This is a much nicer model for enabling alternative CAS implementations, using a URI-based system for determining whether a particular CAS driver implementation supports the URI. Signed-off-by: Aleksa Sarai <[email protected]>
Signed-off-by: Aleksa Sarai <[email protected]>
1 task
|
LGTM |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
In preparation for migrating most of
oci/*toimage-toolsor some suchrepo, we need to split up several packages so that they are less of a
conglomeration and more of a useful set of packages that implement the minimal
functionality so other packages can build on top of them.
This changeset also includes quite a few refactorings to make certain
interfaces more generic, in anticipation for the requirements that
image-toolsneeds to fulfill using these libraries.Things that need to be done:
image->runtime) from generation (image).Walkutility.gcandwalkfromoci/cas.Possibilities:
Instead of implementing all of the wrappers around
cas.Engineas functions that take it as an argument, implement some wrapping structure that provides extra methods (but still provides the old ones) -- like a derived type.Split
oci/cas/dir.gointo some sort of "drivers"-style code so that multiple drivers can more easily be added later. To be honest, this isn't something we need right now -- and the code is already separate enough that you could split it by just moving the file (even the tests would continue to work).Signed-off-by: Aleksa Sarai [email protected]