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

Support custom run-stage targets #44

Closed
jschlatow opened this issue Jun 12, 2023 · 12 comments
Closed

Support custom run-stage targets #44

jschlatow opened this issue Jun 12, 2023 · 12 comments
Assignees
Labels
feature fixed Issue is resolved

Comments

@jschlatow
Copy link
Member

Currently, Goa uses base-linux for its run stage. In principle, the tool is able to support other targets like qemu or real hardware. Since run/linux.tcl serves as a blueprint for any future run-target implementation, it should undergo some restructuring. Moreover, we should add a bit of documentation about the limitations of the linux target and how custom targets can be added.

@jschlatow jschlatow self-assigned this Jun 12, 2023
jschlatow added a commit to jschlatow/goa that referenced this issue Jun 12, 2023
jschlatow added a commit to jschlatow/goa that referenced this issue Jun 12, 2023
- for future support of different run targets
- move generate_runtime_config from linux.tcl to common.tcl
- implement bind_ procedures for individual services in linux.tcl
- move a few procedures from util.tcl to common.tcl

genodelabs#44
jschlatow added a commit to jschlatow/goa that referenced this issue Jun 12, 2023
jschlatow added a commit to jschlatow/goa that referenced this issue Jun 12, 2023
jschlatow added a commit to jschlatow/goa that referenced this issue Jun 12, 2023
jschlatow added a commit to jschlatow/goa that referenced this issue Jun 12, 2023
By providing the `goa run --target <target-name>` option, one can switch
to a different run-stage target. Currently, the only implemented target
is `linux`.

genodelabs#44
@jschlatow
Copy link
Member Author

I just pushed a commit series to my target_support branch. By restructuring the linux.tcl, I addressed aspects from #31, #34. The simplification of the file should clear the way for #32.

@nfeske @ssumpf I'd appreciate any feedback on my changes. The main commit is 41591b6. Note that the new goa help targets provides the corresponding documentation.

@ssumpf
Copy link
Member

ssumpf commented Jun 12, 2023

@jschlatow: I like the only using config variables as globals approach and the way linux.tcl evolves. Looks good to me, I will try it out tomorrow and have a more profound look.

@nfeske
Copy link
Member

nfeske commented Jun 13, 2023

@jschlatow I very much appreciate where you are taking Goa. I had not spent much thought on the linux.tcl part in the first place, and all subsequent changes where feature additions following my initial ad-hoc patterns. Now you replaced plenty of duct tape and a bunch of bolts by proper screws. Thank you!

BTW, I have to steal this reference-syntax convention for upvar arguments. I have never seen it before but it makes so much sense.

jschlatow added a commit to jschlatow/goa that referenced this issue Jun 13, 2023
jschlatow added a commit to jschlatow/goa that referenced this issue Jun 13, 2023
@jschlatow
Copy link
Member Author

@ssumpf I just pushed commit cbb80e1 that fixes a few issues I discovered today while testing the new implementation with a wider range of scenarios.

@ssumpf
Copy link
Member

ssumpf commented Jun 13, 2023

@ssumpf I just pushed commit cbb80e1 that fixes a few issues I discovered today while testing the new implementation with a wider range of scenarios.

@jschlatow: Tested with the examples. Works as advertised. If I have some spare time, I will test dhewm3 since it uses networking, gui, gpu, etc. I also like the upvar approach, had to do some research, but it looks way better than the global everywhere.

jschlatow added a commit that referenced this issue Jun 13, 2023
- for future support of different run targets
- move generate_runtime_config from linux.tcl to common.tcl
- implement bind_ procedures for individual services in linux.tcl
- move a few procedures from util.tcl to common.tcl

#44
jschlatow added a commit that referenced this issue Jun 13, 2023
jschlatow added a commit that referenced this issue Jun 13, 2023
By providing the `goa run --target <target-name>` option, one can switch
to a different run-stage target. Currently, the only implemented target
is `linux`.

#44
@jschlatow
Copy link
Member Author

Thanks for your feedback. Im glad both of you like it. I just pushed the commits to the official staging branch.

@jschlatow
Copy link
Member Author

@nfeske I just gave your Goa unix tutorial a spin with the recent changes. The first part fails because it's runtime file lacks <requires> <timer/> </requires>. The other parts state this requirement on the other hand.

I am aware that Sculpt always adds a route to parent for the Timer service. In Goa, I decided to only instantiate the timer component and a route if it is explicitly mentioned in the runtime file. My intuition was to make the runtime files more consistent. Most of them state the timer requirement but some don't, yet they still magically work, which I find confusing. Do you support this decision or would you rather prefer we mimic Sculpt's behaviour?

jschlatow added a commit to jschlatow/genode that referenced this issue Jun 14, 2023
Goa now makes use of the black-hole component and has been enabled to provide
a fonts_fs.

genodelabs#4928
genodelabs/goa#44
@nfeske
Copy link
Member

nfeske commented Jun 14, 2023

It's perfectly fine to reinforce stricter <requires> requirements in Goa. My decision of allowing the timer by default was just pragmatism. In the future, it might actually be nice to drop this implicit permission, e.g. to deliberately disallow the notion of "timing" from a subsystem. (as a measure to raise the bar for covert timing channels, or to foster the deliberate determinism of a subsystem)

Regarding the Goa-Unix tutorial, it would be nice to take the error message (due to the missing timer) as opportunity to introduce the <requires> node in the first place (I guess somewhere in 2019-12-13-goa-unix-bash.txt). I'll have to check where it would fit in. Alternatively, I'm of course open for a suggestion from you.

@jschlatow
Copy link
Member Author

I think the best spot to introduce the <requires> node in 2019-12-13-goa-unix-bash.txt is right after adding the <config> because goa run will complain about the services that are mentioned in <parent-provides> but not in <requires>.

@nfeske
Copy link
Member

nfeske commented Jun 15, 2023

I just noticed that you also added the detection for <requires> <rm/> </requires>. I would leave this out because I think that the RM service should eventually be consolidated into the PD session. The hard-to-explain purpose of the RM session is just a technical detail that we should better not put in front of each Goa user.

nfeske added a commit to nfeske/genodian that referenced this issue Jun 15, 2023
Related to the change discussed in issue genodelabs/goa#44.
@nfeske
Copy link
Member

nfeske commented Jun 15, 2023

I have adjusted the article in nfeske/genodian@b12c3d4 now.

@jschlatow
Copy link
Member Author

I just noticed that you also added the detection for <requires> <rm/> </requires>. I would leave this out because I think that the RM service should eventually be consolidated into the PD session. The hard-to-explain purpose of the RM session is just a technical detail that we should better not put in front of each Goa user.

Fixed by ef26628

jschlatow added a commit that referenced this issue Jul 5, 2023
- for future support of different run targets
- move generate_runtime_config from linux.tcl to common.tcl
- implement bind_ procedures for individual services in linux.tcl
- move a few procedures from util.tcl to common.tcl

#44
jschlatow added a commit that referenced this issue Jul 5, 2023
jschlatow added a commit that referenced this issue Jul 5, 2023
By providing the `goa run --target <target-name>` option, one can switch
to a different run-stage target. Currently, the only implemented target
is `linux`.

#44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature fixed Issue is resolved
Projects
None yet
Development

No branches or pull requests

3 participants