-
Notifications
You must be signed in to change notification settings - Fork 34
Pull out path constants from imager into yml file #44
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
base: main
Are you sure you want to change the base?
Conversation
|
Approach taken is to expose the same constants as currently exist, though the build.rs file, which parses the yaml. Introduces new dependency: rust-yaml The other option is loading the file at runtime, performance isn't a concern here, but my opinion is that build time is a better option here because it is more aligned with the fact that the lists are constants, and should not be manipulated by logic. |
|
Thanks a lot for the PR! I was actually imagining processing a config file in the main.rs, not in build.rs. Why do you think processing the config file in build.rs is better vs at runtime? The benefits of having a "runtime" config (processed in main.rs) is that a single imager binary can build different images, based on different config files. So in our current setup, imager will be invoked (via Makefile) three times, once with full.yml, once with web.yml, and once with motor-fs.yml configs (so the config file describes input binaries and the file system to use). In the future I can imagine the same imager binary used to package a "test" image that will run tests "on boot". |
|
Thanks for the comment - makes sense to me to do it at runtime since you are going for one build capable of generating multiple images. I had originally done it at build time as I thought of it more as easily configurable constants that won't change. I've pulled it into runtime, and added more complex "syntax" for the config file. My goal here was to allow for defining Let me know if this is not what you had in mind: In this, an arbitrary number of images can be added and configured. Some names should probably be changed so let me know what changes you'd have in mind. Maybe config structure as well if you don't like the format. For one, would you prefer I have a couple questions. (I am very much a novice when it comes to OS topics)
Switching to a draft because I definetely have some cleanup to do. For example, I want to remove the types I defined, and think that it could be good to make the config have a bit more redundancy in exchange for clarity. For this, it could be good to make it such that every image requires relisting all binaries. Wanted to grab an opinion on this approach first though. |
|
Thanks for the update! I didn't look much into all the details - will do it later, when the changes suggested below are in place. For now I have two high-level comments:
To answer your questions:
Partitions 1 and 2 are basically chained bootloaders, so that Qemu can load Motor OS from a bootable disk image. This is a technical detail related to booting an OS from a bootable disk, not really relevant to operating systems per se. You can read up on "chain loading", but this is somewhat tangential.
As I mentioned above, get rid of Thanks again for working on this! |
f6eafa6 to
13e91af
Compare
Paths to binaries were hardcoded in imager. They have been pulled out into a yml file. At build time, the file is parsed and imported as constants into the imager code.
7d5614e to
4c969cb
Compare
In order to use ./run-qemu.sh to launch a specific image file, one needed to modify the script to use a different hardcoded file name. This commit adds an optional --img=<image_file> argument to the script.
d7788b3 to
beac2b1
Compare
|
Thanks @lasiotus for your answers! I updated the configs to hold one image per file, and have modified the Makefile to generate all three existing configs. In this PR I included a commit which adds an argument to |
|
This looks close to what I had in mind, thank you! I'll test the PR later today. I want to refactor it a bit, but it will be much easier/faster for me to do it myself rather than explain the refactor and go through another one or more rounds of reviews, so I think I'll prepare a refactor commit on top of this PR and then merge the PR and the commit immediately after. (Assuming the PR tests OK). So maybe another day or two. Will this work for you? If so, maybe mark this PR as ready for review? It is currently a draft. Thanks! |
Paths to binaries were hardcoded in imager. They have been pulled out into a yml file. At build time, the file is parsed and imported as constants into the imager code.
Furthermore, the folder name for the output of the images is included in the config file, as well as the path to static files to be included.
Issue: #24