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

feat(just): Add RHEL Toolboxes and UBI Images #185

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

noelmiller
Copy link
Member

This is to fix: #106

I decided to go with the official toolbox images for RHEL which will require users to log in. I did add a check to confirm the user is logged in to registry.redhat.io before attempting to create the distrobox.

For the UBI images, I went with the URL that does not require authentication to pull them down.

Copy link
Contributor

@bsherman bsherman left a comment

Choose a reason for hiding this comment

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

I made a recent comment on #106 which may provide a bit of context for this change request:

I think this effort of adding the RHEL distroboxes is excellent, but unless I was mistaken, I thought we were hoping to use toolbox for it.

So, maybe the compromise is have both?

So I have 2 possible suggestions:

  1. in this PR, simply duplicate the distrobox-rhel* recipes, but as toolbox-rhel* using toolbox instead.
    It would obviously work fine and be good for uses wanting either.

  2. we could merge this as is, but maybe add an argument to all our distrobox recipes which allows toggling toolbox, and perhaps allows setting an env var which defaults to toolbox. The idea people that some users definitely do use it exclusively.

@noelmiller
Copy link
Member Author

Giving users a choice definitely is better in my opinion (but I think we should default to distrobox). I'm not as familiar with Toolbox as I am with distrobox.

That being said, it might make sense to have a separate just file specifically for toolbox since it is a different tool. Is it possible to import variable files into justfiles? That way, we could define all of our containers we want to use and make them available to both toolbox and distrobox.

@bsherman
Copy link
Contributor

bsherman commented Jan 9, 2024

Giving users a choice definitely is better in my opinion (but I think we should default to distrobox).

I agree, mostly from the position that our just recipes are all, already distrobox-*

That being said, it might make sense to have a separate just file specifically for toolbox since it is a different tool.

Per the newer restructured just recipe files, yes, I think that could make sense, but, also that seems like a lot of duplication instead of DRY.

Is it possible to import variable files into justfiles? That way, we could define all of our containers we want to use and make them available to both toolbox and distrobox.

Yes. This latter thought is basically what I was suggesting in my second option...

I'd be happy to merge this as is, but file a follow-up issue to be more "toolbox-friendly" and either coach you through the env var stuff or provide the PR myself.

@noelmiller
Copy link
Member Author

Follow up issue would be great! I'm happy to work together on this to get more familiar with just. I was just reading that you can use environment variables and export them. There are also parameters as well which we could explore!

Copy link
Contributor

@bsherman bsherman left a comment

Choose a reason for hiding this comment

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

Per our discussion, I've created #186 and I approve these changes.

@KyleGospo
Copy link
Member

This looks great to me, don't even need to create the distrobox-* images now that distrobox overrides xdg-open. +1

@KyleGospo KyleGospo added this pull request to the merge queue Jan 9, 2024
Merged via the queue into ublue-os:main with commit 116d52f Jan 9, 2024
2 checks passed
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.

just: automate setting up RHEL containers
3 participants