WIP:aryan/build #2

Draft
chopper wants to merge 7 commits from aryan/build into main
Owner

go akemi go review this pleaseeeee

go akemi go review this pleaseeeee
akemi was assigned by chopper 2024-08-21 19:41:01 -06:00
chopper added 4 commits 2024-08-21 19:41:01 -06:00
akemi requested changes 2024-08-22 20:17:17 -06:00
akemi left a comment
Owner

Looks great! Just a few minor changes

Looks great! Just a few minor changes
build.py Outdated
@ -3,1 +6,3 @@
print("meow")
def vm_start(nographic=False):
home = Path.home()
user = os.getlogin()
Owner

Documentation suggests using getuser

Documentation suggests using [getuser](https://docs.python.org/3/library/getpass.html#getpass.getuser)
build.py Outdated
@ -4,0 +13,4 @@
initialize = [
"qemu-system-x86_64",
"-enable-kvm",
"-m", "2G", # ram allocation
Owner

Isn't this far too low? It should also be adjustable via argparse

Isn't this far too low? It should also be adjustable via argparse
build.py Outdated
@ -4,0 +14,4 @@
"qemu-system-x86_64",
"-enable-kvm",
"-m", "2G", # ram allocation
"-nic", "user,hostfwd=tcp::5555-:22", #forward port 5555 in host to port 22 in vm
Owner

This port should be configurable, otherwise we can't run several vms at the same time. Best put it in argparse

This port should be configurable, otherwise we can't run several vms at the same time. Best put it in argparse
build.py Outdated
@ -4,0 +18,4 @@
"-drive", "file=%s,media=disk,if=virtio" % image_dest,
# "-nographic",
# "-serial", "mon:stdio",
Owner

What are these doing here?

What are these doing here?
@ -4,0 +21,4 @@
# "-serial", "mon:stdio",
"-vga", "virtio", # i think this fixes resolution but idk yet
"-display", "sdl",
Owner

Say I enable nographic. It seems like this display arg will conflict. Instead, it'd be better for these to be the else branch on the conditional below

Say I enable `nographic`. It seems like this display arg will conflict. Instead, it'd be better for these to be the else branch on the conditional below
build.py Outdated
@ -4,0 +24,4 @@
"-display", "sdl",
]
if nographic==True:
Owner

==True O.O

if nographic:

`==True` O.O `if nographic:`
@ -4,0 +31,4 @@
#copy the base image to a directory with everyones virtual machines. -n should not overwrite any existing vms.
#subprocess.run(["cp", "-n", image_src, vm_storage_location])
Popen(initialize)
Owner

You'll want some sort of error handling here. What if the vm fails to start? Check the returncode and such on a .communicate()

You'll want some sort of error handling here. What if the vm fails to start? Check the returncode and such on a `.communicate()`
build.py Outdated
@ -4,0 +39,4 @@
parser.add_argument(
"--nographic",
action="store_true",
help="Run QEMU in nographic mode (runs the VM embedded in your terminal instead of opening a new window)",)
Owner

Did you use black to format this? Looks a bit odd

Did you use `black` to format this? Looks a bit odd
akemi removed their assignment 2024-08-23 14:40:08 -06:00
chopper was assigned by akemi 2024-08-23 14:40:08 -06:00
chopper added 1 commit 2024-09-04 12:31:00 -06:00
chopper added 1 commit 2024-09-04 12:46:32 -06:00
chopper added 1 commit 2024-09-04 12:46:53 -06:00
This pull request is marked as a work in progress.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin aryan/build:aryan/build
git checkout aryan/build

Merge

Merge the changes and update on Forgejo.
git checkout main
git merge --no-ff aryan/build
git checkout main
git merge --ff-only aryan/build
git checkout aryan/build
git rebase main
git checkout main
git merge --no-ff aryan/build
git checkout main
git merge --squash aryan/build
git checkout main
git merge --ff-only aryan/build
git checkout main
git merge aryan/build
git push origin main
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: chopper/emulationstation#2
No description provided.