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

Move userland to libtock-c #987

Merged
merged 6 commits into from Jun 20, 2018

Conversation

Projects
None yet
4 participants
@bradjc
Contributor

bradjc commented Jun 12, 2018

Pull Request Overview

This pull request removes the userland folder from the tock kernel repository and updates various documentation to point to libtock-c.

My major concern with this was that it makes it harder for new users to get started. To remedy this, tockloader 1.0 adds a very basic app store, so installing an app is as easy as tockloader install blink.

Testing Strategy

n/a

TODO or Help Wanted

  • Some documentation in /docs needs to be moved to libtock-c. I don't want to do that until this is merged so that the commit history in libtock-c continues to match this repository. But I promise to do it.
  • What should we do with the tutorials? I could see them moving to libtock-c as they are all userland specific (and use c).
  • The courses are probably the main issue. The apps won't compile because userland isn't there.

Documentation Updated

  • Kernel: Updated the relevant files in /docs, or no updates are required.
  • Userland: Added/updated the application README, if needed. ha

Formatting

  • Ran make formatall.

bradjc added some commits Jun 12, 2018

@bradjc bradjc added the blocked label Jun 12, 2018

@ppannuto

This comment has been minimized.

Member

ppannuto commented Jun 12, 2018

@brghena

This comment has been minimized.

Contributor

brghena commented Jun 14, 2018

What is the benefit of moving userland to its own repo?

On the con list, it makes starting out with Tock and loading applications more difficult and more confusing (you have to find and clone an extra repository).

What are the pros? I don't think it will reduce the size of the repo when cloning since it's still in the history, right? I guess it also ensures that there are no dependencies between userland and the kernel, but I don't think we've really had that problem in over a year (barring Makefile-app which was an intentional kludge).

@bradjc

This comment has been minimized.

Contributor

bradjc commented Jun 14, 2018

Well starting out is now tockloader install blink (and we can add any app) to get an app running. That is hopefully easier, and if someone is coming from the rust world they don't need to worry about compiling c.

Also,

  • it puts c on par with our rust userland, which isn't in the repo
  • code reviews and travis can be done separately
  • it helps emphasize the interface and abstraction between the kernel and userland

These are pretty compelling to me, but yeah I don't have a clear technical reason. It seems like it is time to graduate userland to its own thing, as any os would likely do.

@bradjc bradjc removed the blocked label Jun 14, 2018

@alevy alevy added the P-Significant label Jun 16, 2018

@alevy

alevy approved these changes Jun 16, 2018

@alevy

This comment has been minimized.

Member

alevy commented Jun 16, 2018

I think we can work on tockloader to be a more general-purpose "app store" fairly simply. For example, if it was able to take, as an argument for the TAB file, an file path or a URL, it would be fairly trivial to have:

tockloader install https://apps.tockos.org/examples/[some app currently in userland]

and we could fairly trivially setup travis to always publish all compiled example apps there. Then we wouldn't have to special-case the blink app.

Anyone could publish their app anywhere on the web.

But.... that's an issue for a different repository :)

@bradjc

This comment has been minimized.

Contributor

bradjc commented Jun 17, 2018

Then we wouldn't have to special-case the blink app.

The blink app isn't special cased, but the URL is. Any app here can be installed that way: https://github.com/tock/tock-www/tree/master/assets/tabs

But yeah I agree specifying a full URL should be supported.

@alevy

This comment has been minimized.

Member

alevy commented Jun 19, 2018

When did the clock start on this? I believe there has been a fair amount of consensus on extracting userland to a separate repo. It's been up for a week, and I don't think there are any outstanding concerns.

On the other hand, I only marked it P-Significant three days ago.

Shall we give this another day ot two to gather a few more approvals? Or just merge?

@bradjc

This comment has been minimized.

Contributor

bradjc commented Jun 19, 2018

Let's do last call, but not super urgently as there is fixup that has to be done after this is merged.

@bradjc bradjc added the last-call label Jun 19, 2018

@brghena

I still have concerns that this is going to make Tock harder to use rather than easier, but I won't stand in the way of this change if other people prefer it.

You're right that other non-c userlands are already their own repos and this would then parallel them.

@bradjc bradjc merged commit ae2157a into master Jun 20, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details

@bradjc bradjc deleted the graduate-userland branch Jun 20, 2018

@bradjc bradjc referenced this pull request Jun 22, 2018

Merged

Courses: Remove from repo and specify commit to view them #1020

2 of 2 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment