-
Notifications
You must be signed in to change notification settings - Fork 142
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
actions: Support for local packages installation #165
Conversation
0f8fb38
to
278c836
Compare
bikeshedding: I think "source" may be a bit unfortunate name/keyword for this as it's not obvious what it would mean without a description. eg. it could (incorrectly) be interpretted as source code, source packages, etc. Is there any other more obvious keyword that fits better perhaps? Just thinks it's useful to consider the question now, because once it's in changing it would break recipies which would be bad.... |
yes, you're right, what about "local-packages-dir"? |
278c836
to
cc795a7
Compare
actions/apt_action.go
Outdated
@@ -49,6 +56,49 @@ func (apt *AptAction) Run(context *debos.DebosContext) error { | |||
aptOptions = append(aptOptions, "install") | |||
aptOptions = append(aptOptions, apt.Packages...) | |||
|
|||
if apt.LocalPackagesDir != "" { | |||
localpackagesdir := path.Join(context.RecipeDir, apt.LocalPackagesDir) | |||
destination, err := debos.RestrictedPath(context.Rootdir, "/usr/local/debs") |
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.
is /usr/local/debs
the best place for this ? would a temp directory be better?
i think it is worth allowing the user to optionally override this path from the recipe; for instance the rootfs may be too small.
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.
apt
command can only access packages from network or rootfs
the idea is to hide this from user, so I don't think allowing user to set the temporary destination
directory is useful
if err != nil { | ||
return err | ||
} | ||
defer os.RemoveAll(destination) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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.
Defer statement runs at the end of the function.
So, the destination
directory is only available during this apt
action, and subsequent apt
actions will not have access to it.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
|
||
// apt-ftparchive tries to read /etc/apt/apt.conf.d, add this path temporarily to prevent warnings | ||
os.MkdirAll("/etc/apt/apt.conf.d", 0755) | ||
defer os.RemoveAll("/etc/apt/apt.conf.d/") |
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 this is only a warning i wonder if this should be a separate patch since debootstrap action also moans about this
also, this is in the middle of a chdir
, for clarity move it above
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 do not see this warning with debootstrap
action
As it is related to following apt-ftparchive
commands I think doing this here is right place
@@ -54,6 +54,7 @@ LABEL org.label-schema.docker.cmd 'docker run \ | |||
RUN apt-get update && \ | |||
apt-get install -y --no-install-recommends \ | |||
apt-transport-https \ | |||
apt-utils \ |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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, you're right but this is not managed in this project
actions/apt_action.go
Outdated
|
||
os.Chdir(currentDir) | ||
|
||
locallist, err := debos.RestrictedPath(context.Rootdir, "/etc/apt/sources.list.d/local.list") |
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.
local.list
is quite generic
actions/apt_action.go
Outdated
return err | ||
} | ||
defer os.RemoveAll(locallist) | ||
cmd := fmt.Sprintf("echo 'deb [trusted=yes] file:///usr/local/debs/ ./' > %s", locallist) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
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.
Defer statement runs at the end of the function.
actions/apt_action.go
Outdated
} | ||
defer os.RemoveAll(locallist) | ||
cmd := fmt.Sprintf("echo 'deb [trusted=yes] file:///usr/local/debs/ ./' > %s", locallist) | ||
err = debos.Command{}.Run("apt", "sh", "-c", cmd) |
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.
can this be done as a direct go file write rather than a command?
actions/apt_action.go
Outdated
@@ -20,10 +21,15 @@ Optional properties: | |||
- recommends -- boolean indicating if suggested packages will be installed | |||
|
|||
- unauthenticated -- boolean indicating if unauthenticated packages can be installed | |||
- local-packages-dir -- directory containing local packages to be installed |
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's not entirely clear without thinking about it where this directory lives - could make it clear that it is under $RECIPEDIR
This add a new 'local-packages-dir' option to 'apt' action, allowing to install local packages from the 'local-packages-dir' directory in addition to the repository. Fix go-debos#157 Signed-off-by: Frédéric Danis <[email protected]>
cc795a7
to
cd4da4a
Compare
if err != nil { | ||
return err | ||
} | ||
defer os.RemoveAll(destination) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
What feels incredibly awkward with this action for me is the amount of hakcs it has to do to the root filesystem we're building to "add" the local packages directory; And then you have to specify the pakcage to install in your normal apt run; however i don't think you generate a policy file to ensure the local file:// archive gets priority so if the os you build against has a newer version there is a chance it install the non-local package.. Would probably be better to simplify this and rather then extending the that should also support the typical |
afaik |
The apt-file action allows local .deb packages to be installed using the `apt` command, much like the apt action but for local packages rather than those from apt repositories. Resolves: go-debos#157 Closes: go-debos#165 Signed-off-by: Christopher Obbard <[email protected]>
…debian packages The apt-file action allows local .deb packages to be installed using the `apt` command, much like the apt action but for local packages rather than those from apt repositories. Resolves: go-debos#157 Closes: go-debos#165 Signed-off-by: Christopher Obbard <[email protected]>
…debian packages The apt-file action allows local .deb packages to be installed using the `apt` command, much like the apt action but for local packages rather than those from apt repositories. Resolves: go-debos#157 Closes: go-debos#165 Signed-off-by: Christopher Obbard <[email protected]>
…debian packages The apt-file action allows local .deb packages to be installed using the `apt` command, much like the apt action but for local packages rather than those from apt repositories. Resolves: go-debos#157 Closes: go-debos#165 Signed-off-by: Christopher Obbard <[email protected]>
The apt-file action allows local .deb packages to be installed using the `apt` command, much like the apt action but for local packages rather than those from apt repositories. Resolves: go-debos#157 Closes: go-debos#165 Signed-off-by: Christopher Obbard <[email protected]>
The install-dpkg action allows local .deb packages to be installed using the apt command, much like the apt action but for local packages rather than for packages retrieved from remote apt repositories. Resolves: go-debos#157 Closes: go-debos#165 Signed-off-by: Christopher Obbard <[email protected]>
Continuing the alternative implementation under #220 so closing this one. |
The install-dpkg action allows local .deb packages to be installed using the apt command, much like the apt action but for local packages rather than for packages retrieved from remote apt repositories. Resolves: go-debos#157 Closes: go-debos#165 Signed-off-by: Christopher Obbard <[email protected]>
The install-dpkg action allows local .deb packages to be installed using the apt command, much like the apt action but for local packages rather than for packages retrieved from remote apt repositories. Resolves: go-debos#157 Closes: go-debos#165 Signed-off-by: Christopher Obbard <[email protected]>
The install-dpkg action allows local .deb packages to be installed using the apt command, much like the apt action but for local packages rather than for packages retrieved from remote apt repositories. Resolves: go-debos#157 Closes: go-debos#165 Signed-off-by: Christopher Obbard <[email protected]>
This add a new 'source' option to 'apt' action, allowing to install local
packages from the 'source' directory in addition to the repository.
Fix #157
Signed-off-by: Frédéric Danis [email protected]