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

Add sabayon support and new features #114

Merged
merged 4 commits into from Dec 28, 2018

Conversation

3 participants
@geaaru
Copy link
Contributor

geaaru commented Dec 9, 2018

Features added:

  • environment variables configuration from image files
  • repositories handler
  • docker http sourcer
@stgraber

This comment has been minimized.

Copy link
Member

stgraber commented Dec 10, 2018

Hi, you'll need to make sure all your commits include the needed Signed-off-by line in their message, making that DCO check happy. Until then, we can't really look at those changes.

@geaaru geaaru force-pushed the geaaru:master branch 3 times, most recently from a5c545e to 6cb3710 Dec 10, 2018

@geaaru

This comment has been minimized.

Copy link
Contributor

geaaru commented Dec 16, 2018

In my fork i added also Gopkg.toml/lock for handle dependencies through dep tool. I think that it's useful for trace versions of vendors used. WDYT ?

@stgraber

This comment has been minimized.

Copy link
Member

stgraber commented Dec 17, 2018

We usually try to stay away from dependency managers both in LXD and distrobuilder, instead preferring to rely on external packages that have a good track record of not breaking their API and switching away from any which breaks it.

@stgraber

This comment has been minimized.

Copy link
Member

stgraber commented Dec 17, 2018

@monstermunchkin can you spend a bit of time to review this please?

@geaaru

This comment has been minimized.

Copy link
Contributor

geaaru commented Dec 18, 2018

I'm not agree with this because in general there is always a tools and/or api that could be change in time and have a tool that identify what is right version to use for compilation (also for gentoo stuff) could be helpful.
However, np. Thanks for clarification.

@monstermunchkin
Copy link
Member

monstermunchkin left a comment

I don't understand the necessity of the repository actions. Can't all of that just be handled with the existing actions?

Show resolved Hide resolved shared/chroot.go Outdated
Show resolved Hide resolved shared/definition.go Outdated
Show resolved Hide resolved doc/examples/sabayon Outdated
Show resolved Hide resolved sources/sabayon.go Outdated
Show resolved Hide resolved sources/docker.go
Show resolved Hide resolved sources/docker.go Outdated
Show resolved Hide resolved sources/docker.go Outdated
Show resolved Hide resolved sources/docker.go Outdated
@geaaru

This comment has been minimized.

Copy link
Contributor

geaaru commented Dec 18, 2018

About repositories attributes.... i think that it's more clear define custom repositories to a specific section instead of create pre/post actions, so inside specific distro package manager tool could be create a specific implementation related to distro. More clear image.yaml.

We could also remove AptSources from
https://github.com/lxc/distrobuilder/blob/master/shared/definition.go#L52
if it is implements to apt manager.

imho

@geaaru geaaru force-pushed the geaaru:master branch from 6cb3710 to e40daf7 Dec 18, 2018

@monstermunchkin

This comment has been minimized.

Copy link
Member

monstermunchkin commented Dec 19, 2018

I still don't see the point of having the repo handler. You could just simply run enman add community or equo enable sabayonlinux.org in post-unpack.

@geaaru

This comment has been minimized.

Copy link
Contributor

geaaru commented Dec 19, 2018

About repository I think that reason could be the same of why exists a packages section.
For apt, emerge and any others module for manage packages inside a specific distro could be handle with post-unpack action.

It's better have a clear image files that a poor image files with a list of list of actions for any type of operations.

I simply follow what is been shared with stgraber on irc channel.

@monstermunchkin

This comment has been minimized.

Copy link
Member

monstermunchkin commented Dec 21, 2018

This branch needs to be rebased. Also, could you please implement the repo thing as discussed on IRC? (https://paste.ubuntu.com/p/7PT2BTDYjQ/)

@geaaru

This comment has been minimized.

Copy link
Contributor

geaaru commented Dec 21, 2018

yeah, of course. I will modify code as discussed.
Let me only some days because i'm in holiday.

Thanks

@geaaru geaaru force-pushed the geaaru:master branch 3 times, most recently from 131eb6f to f0993bc Dec 21, 2018

@monstermunchkin
Copy link
Member

monstermunchkin left a comment

Please run make check and fix the issues. Golint is complaining about some things. Other than that the code looks good to me.

Show resolved Hide resolved distrobuilder/chroot.go Outdated
Permit definition of environment vars on image file
Signed-off-by: Daniele Rondina <geaaru@sabayonlinux.org>

@geaaru geaaru force-pushed the geaaru:master branch from f0993bc to a20a674 Dec 22, 2018

geaaru added some commits Dec 16, 2018

Add docker downloader
Signed-off-by: Daniele Rondina <geaaru@sabayonlinux.org>
Integrate repositories handler function
Signed-off-by: Daniele Rondina <geaaru@sabayonlinux.org>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>
Added sabayon amd64 support
Signed-off-by: Daniele Rondina <geaaru@sabayonlinux.org>
Signed-off-by: Thomas Hipp <thomas.hipp@canonical.com>

@monstermunchkin monstermunchkin force-pushed the geaaru:master branch from a20a674 to 13aadef Dec 28, 2018

@monstermunchkin monstermunchkin merged commit c225472 into lxc:master Dec 28, 2018

3 checks passed

Branch target Branch target is correct
Details
DCO All commits signed-off
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@monstermunchkin

This comment has been minimized.

Copy link
Member

monstermunchkin commented Dec 29, 2018

Thanks for your contributions!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment