Skip to content
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

Added install pkgs extension #295

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

agyoungs
Copy link
Contributor

I've been using this extension for a while and figured it was generic enough to be added to the main repo. If it's redundant in some way please let me know as I find this extension handy to add additional developer packages

@agyoungs agyoungs requested a review from tfoote as a code owner October 30, 2024 22:32
Copy link
Collaborator

@tfoote tfoote left a comment

Choose a reason for hiding this comment

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

In general I'm not super enthusiastic about enabling arbitrary package strings here. In particular that's very platform specific and potentially version specific. Installing a large number of packages at startup time can be very costly as well. And this content isn't cached effectively.

At the first level I'd like to see something with an interface that is more generic and uses something like the rosdep keys to be cross platform. You mention in your comments about adding standard groupings. I think those standard groupings will be much more powerful for individuals, but they're going to be pretty user specific. I'd like to find a way to enable people to make those groupings and share them such that they can be batched up . An example is the --dev-helpers extension where those are my personal simple tools that I used as a test case. But that approach doesn't really scale well. If there's a group that wants to share and distribute an extension with that batching that gets to a level of management that might work. But also if you're looking to add a bunch of packages it's likely a better solution to build a new base image before using rocker. Raw packages are not what rocker is designed for, they don't require being responsive to the local environment.

def precondition_environment(self, cli_args):
pass

def validate_environment(self, cli_args):
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should at least validate the environment here is debian if this will only work for debian packages.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does validate_environment get called anywhere? It seems like it's not used unless I'm missing something

Copy link
Collaborator

Choose a reason for hiding this comment

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

Well that's a regression. It should be being validated at the build stage. It looks like that was lost along the way.

@@ -0,0 +1,7 @@
# User specified additional packages
RUN export DEBIAN_FRONTEND=noninteractive; \
Copy link
Collaborator

Choose a reason for hiding this comment

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

This template only works for debian like systems. It should be documented.

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.

2 participants