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.

Sign up for GitHub

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

Jump to bottom

External packages #455

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

External packages #455

wants to merge 7 commits into from

Conversation

gparmer
Copy link
Collaborator

@gparmer gparmer commented Jun 27, 2022

Summary of this Pull Request (PR)

Add description here.

Intent for your PR

Choose one (Mandatory):

  • This PR is for a code-review and is intended to get feedback, but not to be pulled yet.
  • This PR is mature, and ready to be integrated into the repo.

Reviewers (Mandatory):

@betahxy @WenyuanShao If you're interested. Mostly Rust and build system updates.

Can

./cos compose composition_scripts/external_hello_world.toml test
./cos run test

And it will run the component defined in https://github.com/gparmer/hello_world

The composition scripts have the repo directive:

[system]
description = "Test for the hello world external component"

[[components]]
name = "hw1"
img  = ""
repo = "github:gparmer/hello_world"
deps = [{srv = "kernel", interface = "init", variant = "kernel"}]
constructor = "kernel"

Existing scripts should run as expected.

Code Quality

As part of this pull request, I've considered the following:

Style:

  • Comments adhere to the Style Guide (SG)
  • Spacing adhere's to the SG
  • Naming adhere's to the SG
  • All other aspects of the SG are adhered to, or exceptions are justified in this pull request
  • I have run the auto formatter on my code before submitting this PR (see doc/auto_formatter.md for instructions)

Code Craftsmanship:

  • I've made an attempt to remove all redundant code
  • I've considered ways in which my changes might impact existing code, and cleaned it up
  • I've formatted the code in an effort to make it easier to read (proper error handling, function use, etc...)
  • I've commented appropriately where code is tricky
  • I agree that there is no "throw-away" code, and that code in this PR is of high quality

@gparmer
Updated Makefiles to pass along the make include path.
8c96044
@gparmer
CPU burn component, and simple composition script on top of the composer
fdc8978
@gparmer
TRANSIENT COMMIT
f041af4
@gparmer
Added logic to drive component's compilation path and use github repos.
1645b07 
DONE:

1. Add support for specifying the repo in the composition script, and supporting build-system updates, and validate the previous scripts still work.

TODO:

2. Validate that the external repo download and usage works.
3. Update `img` to `src` and enable it to not just have the form of two nested directories (i.e. "x.y"), but also a single subdir (i.e. "x"), or no directory (in the case that a repo is the single component).

FUTURE FEATURES:

I won't be able to get to these soon, but they are desirable.

- Add external library and interface repo support.
    This is more challenging as it requires more build system logic as dependencies on libraries/interfaces are specified in the build system, not in the composition scripts.
@gparmer
External components can be pulled into the system.
115b8e4 
Demonstrated with `external_hello_world.toml`, and the `github.com/gparmer/hello_world/` component.

DONE:

1. Add support for specifying the repo in the composition script, and supporting build-system updates, and validate the previous scripts still work.
2. Validate that the external repo download and usage works.

TODO:

3. Update `img` to `src` and enable it to not just have the form of two nested directories (i.e. "x.y"), but also a single subdir (i.e. "x"), or no directory (in the case that a repo ish.

FUTURE FEATURES:

I won't be able to get to these soon, but they are desirable.

- Enable the composition scripts to be referenced in github repos, rather than only in local files.
- Add external library and interface repo support.
    This is more challenging as it requires more build system logic as dependencies on libraries/interfaces are specified in the build system, not in the composition scripts.
@gparmer
merging with xinyu's code
ac3bf7a
@gparmer
VAS -> external merge
2a04983 
Merge branch 'main' into external_packages
gparmer
gparmer commented Jun 27, 2022
View reviewed changes
.gitignore
src/components/lib/libc/musl-1.2.0/include/sys/queue.h
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@betahxy Is this an error? Looks like this comes from a merge with your code. I might have done this incorrectly.

Copy link
Contributor

Choose a reason for hiding this comment

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

@gparmer This is OK. I'll add this back. Not a big problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@betahxy betahxy betahxy left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gparmer @betahxy