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: Pre dx devmode #2036

Open
wants to merge 5 commits into
base: testing
Choose a base branch
from
Open

Conversation

mowglixx
Copy link

See #2035

@dosubot dosubot bot added the size:M This PR changes 30-99 lines, ignoring generated files. label Dec 31, 2024
@mowglixx mowglixx changed the title Pre dx devmode feat: Pre dx devmode Dec 31, 2024
@mowglixx
Copy link
Author

could do with patching with the whole remote dev extension pack but cba

@mowglixx mowglixx requested a review from wolfyreload December 31, 2024 22:06
@KyleGospo KyleGospo changed the base branch from main to testing December 31, 2024 22:43
@KyleGospo KyleGospo enabled auto-merge December 31, 2024 22:43
@KyleGospo
Copy link
Member

It might be worth visiting a vscode distrobox instead of layering it for the future.

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Dec 31, 2024
@mowglixx
Copy link
Author

It might be worth visiting a vscode distrobox instead of layering it for the future.

Agreed, the problem is viewing the host docker socket so it'd mean a few other steps, obviously in the -dx release it can be a regular package or maybe a bluefin dev could do the flatpak magic?

@KyleGospo KyleGospo disabled auto-merge January 1, 2025 01:36
@wolfyreload
Copy link
Contributor

wolfyreload commented Jan 1, 2025

@mowglixx I think I found one more issue. The .just files are imported manually in the Containerfile. So your custom ujust commands won't work until you add an entry there too

@mowglixx
Copy link
Author

mowglixx commented Jan 1, 2025

thanks for pointing that out, would have missed that, I've been testing with just --justfile ./thisfile.just --choose to make sure it's all working in just, the commands run with no errors (appart from this is working already), if someone has a vm env to test this with a fresh install I'd really appreciate it.

Pushing the change now.

@wolfyreload
Copy link
Contributor

I have a VM all set up, I'll test as soon as I get a chance. Probably tomorrow sometime.

Copy link
Contributor

@wolfyreload wolfyreload left a comment

Choose a reason for hiding this comment

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

I'll need to test in a VM but everything looks good to me

@mowglixx
Copy link
Author

mowglixx commented Jan 1, 2025

Thanks, I've been meaning to do the just script for virt-manager today so I might end up doing it myself, I'll drop a message if i do and I'm going to take a look at the image building process for my own image, I've been admiring @HikariKnight 's personal image for a starting point, I've done passthrough before so was good to see some familiar commands in the place it should be. =D

@KyleGospo
Copy link
Member

No need for anything virt-manager, it's a flatpak and already done the optimal way here.

@KyleGospo
Copy link
Member

I'm going to give this a containerization pass later today, we want to avoid layers at all costs. After that I'll be happy to merge into testing.

@mowglixx
Copy link
Author

mowglixx commented Jan 1, 2025

no no no, I meant I'm going to run the just script (I fresh installed the other day from cachyOS) to get virt manager all set up so might run the VM to test ^this further myself

@mowglixx
Copy link
Author

mowglixx commented Jan 1, 2025

ah so this might end up being an early -dx build instead of the layers?

@mowglixx
Copy link
Author

mowglixx commented Jan 1, 2025

echo was printing null vars from shell

@KyleGospo
Copy link
Member

1ac8f49

^ Here's docker as a container. I'm going to leave this PR open as a workshop for a potential best-solution for VSCode as well, may also end up being distrobox until DX.

Comment on lines +30 to +31
if ! grep -q "^docker:" /etc/group; then
grep '^docker:' /usr/lib/group | sudo tee -a /etc/group > /dev/null
Copy link
Contributor

Choose a reason for hiding this comment

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

are regexp patterns enabled by default?

Suggested change
if ! grep -q "^docker:" /etc/group; then
grep '^docker:' /usr/lib/group | sudo tee -a /etc/group > /dev/null
if ! grep -qE "^docker:" /etc/group; then
grep -E '^docker:' /usr/lib/group | sudo tee -a /etc/group > /dev/null

@mowglixx
Copy link
Author

Had some time away and started using the flatpak with a remote Ubuntu VM and docker installed not ideal but it's working, I'd like to continue working on this but I'm starting to realise that the DX mode needs to be a rebase like in bluefin due to docker being installed in the image and not requiring extra layers.

I'll try to put in some time this weekend learning how make the images work, tbh tho if DX mode is gunna be a rebase it may as well include the vscode package with the extensions installed for Dev containers.

@mowglixx
Copy link
Author

Think flatpak would be overkill in the instance of having purpose installed packages in the base image, maybe there is a licensing issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer size:M This PR changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants