Skip to content

Add overlay/ directory and empty out manifest#66

Merged
jlebon merged 1 commit intocoreos:masterfrom
jlebon:pr/integration-rpm
Mar 21, 2019
Merged

Add overlay/ directory and empty out manifest#66
jlebon merged 1 commit intocoreos:masterfrom
jlebon:pr/integration-rpm

Conversation

@jlebon
Copy link
Member

@jlebon jlebon commented Mar 15, 2019

Right now the manifest's postprocess is a combination of temporary hacks
and glue code. Split those out in a separate overlay/ directory
instead. This will be picked up by coreos-assembler and automatically
transformed into an RPM.

Uses: coreos/coreos-assembler#414

Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this looks nice but I worry that it's easier to forget about some of the things in the overlay since they will be "out of sight, out of mind".

@jlebon
Copy link
Member Author

jlebon commented Mar 19, 2019

I think this looks nice but I worry that it's easier to forget about some of the things in the overlay since they will be "out of sight, out of mind".

Yeah, that's a concern. I think the right thing to do there too is to have tracker issues to make sure it doesn't fall off the radar?

@cgwalters
Copy link
Member

cgwalters commented Mar 19, 2019

but I worry that it's easier to forget about some of the things in the overlay since they will be "out of sight, out of mind".

Isn't it the opposite? My eyes glaze over a bit reading shell-in-yaml (edit: in some places shell-generating-shell-in-YAML!) but I think a directory overlay with the files directly is very clear and hard to miss.

@dustymabe
Copy link
Member

Isn't it the opposite? My eyes glaze over a bit reading shell-in-yaml (edit: in some places shell-generating-shell-in-YAML!)

It's definitely harder to read, but I don't think harder to miss.

but I think a directory overlay with the files directly is very clear and hard to miss.

yeah. I think it depends. The issues can mostly be helped by good use of grep, but it was worth pointing out

@dustymabe dustymabe added the WIP PR still being worked on label Mar 20, 2019
@jlebon jlebon force-pushed the pr/integration-rpm branch from 4f90086 to 16ab320 Compare March 20, 2019 21:42
@jlebon jlebon changed the title WIP/RFC: Add overlay/ directory and empty manifest Add overlay/ directory and empty manifest Mar 20, 2019
@jlebon jlebon changed the title Add overlay/ directory and empty manifest Add overlay/ directory and empty out manifest Mar 20, 2019
@jlebon jlebon force-pushed the pr/integration-rpm branch from 16ab320 to 312be0a Compare March 20, 2019 21:43
@jlebon jlebon removed the WIP PR still being worked on label Mar 20, 2019
@jlebon
Copy link
Member Author

jlebon commented Mar 20, 2019

OK, this is ready to go now! I've added tracking links for some of the hacks.

@dustymabe
Copy link
Member

/me will test this out today

Copy link
Member

@dustymabe dustymabe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since various files are used to contribute to the same "fix" for certain things (i.e. a systemd unit and a executable file work together to achieve the same goal) I think it would be nice if we supported a way to group things together. One way to do this would be to support sub directories under overlay. Each sub directory could contain a readme that gets ignored but provides some more context for the group of files.

We could also still support a standalone or general directory as well for single file standalone goals.

WDYT?

@jlebon
Copy link
Member Author

jlebon commented Mar 21, 2019

One way to do this would be to support sub directories under overlay.

Hmm, I like the idea of surfacing the fact that some of the files are related somehow, but having multiple rootfs'es feels like overkill. How about just # see also ... comments? Most of the files I think are stand-alone, so it wouldn't be that many.

@jlebon jlebon force-pushed the pr/integration-rpm branch from 312be0a to 7052201 Compare March 21, 2019 18:40
Right now the manifest's postprocess is a combination of temporary hacks
and glue code. Split those out in a separate `overlay/` directory
instead. This will be picked up by coreos-assembler and automatically
transformed into an RPM.
@jlebon jlebon force-pushed the pr/integration-rpm branch from 7052201 to d5644c6 Compare March 21, 2019 18:45
@dustymabe
Copy link
Member

Hmm, I like the idea of surfacing the fact that some of the files are related somehow, but having multiple rootfs'es feels like overkill. How about just # see also ... comments? Most of the files I think are stand-alone, so it wouldn't be that many.

Yeah. It's not something i would block you on, but I do think that it's easier to set up a structure where people do the right thing vs relying on them to do the right thing. I'll approve the PR and let you make the judgement call.

@jlebon
Copy link
Member Author

jlebon commented Mar 21, 2019

I'll approve the PR and let you make the judgement call.

OK cool, let's go with this for now, and if this we have more instances, we'll reconsider!

@jlebon jlebon merged commit d4ce30f into coreos:master Mar 21, 2019
cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request May 14, 2019
This is strongly related to coreos/fedora-coreos-config#66

For development iteration, we want the ability to directly overlay
binaries.  This depends on
coreos/rpm-ostree#1830
which adds rpm-ostree ability to inject ostree refs into the build.

We could implement this in cosa by building an RPM but I think that's
silly - we end up compressing the data only to immediately decompress it.
Plus, while there are advantages to having entries for files in the RPM
database, in practice we're lying about having these files built by RPM.
So let's just directly generate an OSTree commit from them and inject
that.

(In practice for *this* use case rpm-ostree could actually learn
 a way to directly consume a directory, but indirecting via OSTree
 means it's equally easy to manage pre-generated ostree branches too
 in an efficient way rather than re-committing each time)
cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request Jun 10, 2019
This is strongly related to coreos/fedora-coreos-config#66

For development iteration, we want the ability to directly overlay
binaries.  This depends on
coreos/rpm-ostree#1830
which adds to rpm-ostree the ability to inject ostree refs into the build.

We could implement this in cosa by building an RPM (like we do for the overlay mentioned above)
but I think that's silly - we end up compressing the data only to immediately decompress it.
Plus, while there are advantages to having entries for files in the RPM
database, in practice we're lying about having these files built by RPM.
So let's just directly generate an OSTree commit from them and inject
that.

In practice for *this* use case rpm-ostree could actually learn
a way to directly consume a directory, but indirecting via OSTree
means it's equally easy to manage pre-generated ostree branches too
in an efficient way rather than re-committing each time.
cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request Jun 10, 2019
This is strongly related to coreos/fedora-coreos-config#66

For development iteration, we want the ability to directly overlay
binaries.  This depends on
coreos/rpm-ostree#1830
which adds to rpm-ostree the ability to inject ostree refs into the build.

We could implement this in cosa by building an RPM (like we do for the overlay mentioned above)
but I think that's silly - we end up compressing the data only to immediately decompress it.
Plus, while there are advantages to having entries for files in the RPM
database, in practice we're lying about having these files built by RPM.
So let's just directly generate an OSTree commit from them and inject
that.

In practice for *this* use case rpm-ostree could actually learn
a way to directly consume a directory, but indirecting via OSTree
means it's equally easy to manage pre-generated ostree branches too
in an efficient way rather than re-committing each time.
cgwalters added a commit to cgwalters/coreos-assembler that referenced this pull request Jun 11, 2019
This is strongly related to coreos/fedora-coreos-config#66

For development iteration, we want the ability to directly overlay
binaries.  This depends on
coreos/rpm-ostree#1830
which adds to rpm-ostree the ability to inject ostree refs into the build.

We could implement this in cosa by building an RPM (like we do for the overlay mentioned above)
but I think that's silly - we end up compressing the data only to immediately decompress it.
Plus, while there are advantages to having entries for files in the RPM
database, in practice we're lying about having these files built by RPM.
So let's just directly generate an OSTree commit from them and inject
that.

In practice for *this* use case rpm-ostree could actually learn
a way to directly consume a directory, but indirecting via OSTree
means it's equally easy to manage pre-generated ostree branches too
in an efficient way rather than re-committing each time.
jlebon pushed a commit to coreos/coreos-assembler that referenced this pull request Jun 11, 2019
This is strongly related to coreos/fedora-coreos-config#66

For development iteration, we want the ability to directly overlay
binaries.  This depends on
coreos/rpm-ostree#1830
which adds to rpm-ostree the ability to inject ostree refs into the build.

We could implement this in cosa by building an RPM (like we do for the overlay mentioned above)
but I think that's silly - we end up compressing the data only to immediately decompress it.
Plus, while there are advantages to having entries for files in the RPM
database, in practice we're lying about having these files built by RPM.
So let's just directly generate an OSTree commit from them and inject
that.

In practice for *this* use case rpm-ostree could actually learn
a way to directly consume a directory, but indirecting via OSTree
means it's equally easy to manage pre-generated ostree branches too
in an efficient way rather than re-committing each time.
@jlebon jlebon deleted the pr/integration-rpm branch July 4, 2019 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants