Skip to content
This repository has been archived by the owner on Mar 4, 2024. It is now read-only.

Add Makefile, configuration files and scripts #19

Closed

Conversation

StefanJum
Copy link
Member

Update the README with instructions on setting up, configuring, building and running the helloworld app with Unikraft. As part of it, add Makefile files, configuration files and scripts; they are referenced in the README.md file.

Current instructions cover the use of QEMU/KVM platform (both on x86_64 and on AArch64) and GCC. Firecraker/KVM, Xen, Linux platforms and Clang are not yet documented.

Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

Thanks, @StefanJum. See my comments.

run-qemu-aarch64 Outdated Show resolved Hide resolved
run-qemu-x86_64 Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@StefanJum
Copy link
Member Author

Done @razvand

Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

Add a .gitignore file. Probably as a new commit.

Makefile Show resolved Hide resolved
@StefanJum
Copy link
Member Author

@razvand added the .gitignore, see the comment about the Makefile.

@StefanJum StefanJum requested a review from razvand July 3, 2023 10:25
Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Razvan Deaconescu razvand@unikraft.io
Approved-by: Razvan Deaconescu razvand@unikraft.io

Copy link
Member

@nderjung nderjung left a comment

Choose a reason for hiding this comment

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

In my opinion, we should not be adding .config* files to the repository additional scripts on the top-level (at least, in the state that they are in: they should have relative data (e.g. user paths) and unset attributes removed.) Additionally, the tutorial contains the same information as the scripts, these should be removed. Finally, and most importantly, kraft was entirely removed from the README. In my opinion this is a mistake. We are aiming to make KraftKit the first-class and go-to-choice for using and building Unikraft. The README should start with this, and progress with "for advanced, or first-principle usage"

@StefanJum
Copy link
Member Author

In my opinion, we should not be adding .config* files to the repository additional scripts on the top-level (at least, in the state that they are in: they should have metadata and unset attributes removed. Additionally, the tutorial contains the same information as the scripts, these should be removed.

What scripts?

Finally, and most importantly, kraft was entirely removed from the README. In my opinion this is a mistake. We are aiming to make KraftKit the first-class and go-to-choice for using and building Unikraft. The README should start with this, and progress with "for advanced, or first-principle usage"

Yes, we can do that after kraftkit documentation is in order. The main (short term) goal for the changes is to get very new people to quickly setup the application, mostly as part of USoC (which will happen tomorrow, 04 July, not in time for kraftkit to be properly tested and documented).

If people will see kraftkit as the top choice they will try using it, and in the current state they will very likely fail, which will get them frustrated and cause them to give up, so we want the instructions to be 100% working first, then when we are ready we can have the instructions for using kraftkit as very strongly recommended (we can even remove all the running scripts / config files at that point).

@nderjung
Copy link
Member

nderjung commented Jul 3, 2023

Hi @StefanJum,

Which scripts?

The new scripts run-qemu-aarch64 and run-qemu-x86_64.

Yes, we can do that after kraftkit documentation is in order. The main (short term) goal for the changes is to get very new people to quickly setup the application, mostly as part of USoC (which will happen tomorrow, 04 July, not in time for kraftkit to be properly tested and documented).

If people will see kraftkit as the top choice they will try using it, and in the current state they will very likely fail, which will get them frustrated and cause them to give up, so we want the instructions to be 100% working first, then when we are ready we can have the instructions for using kraftkit as very strongly recommended (we can even remove all the running scripts / config files at that point).

No. We must direct people to KraftKit regardless of the state of the documentation (which I am working on full-time now). Thi README is part of the same documentation and waiting for one piece elsewhere is not the right approach. We should update all relevant documents to usage with KraftKit. Finally, there are admittedly bugs in KraftKit, like there are with Unikraft itself. This document only serves one persona and does not help us in getting valuable bug reports and user experience stories with KraftKit. Redirecting people away will not benefit us, short-, mid- or long-term.

Finally, the relevant USoC channels should contain information about how to get started, in the USoC website and on Discord, outside of this README. This README should not be tailored for USoC.

@StefanJum
Copy link
Member Author

The new scripts run-qemu-aarch64 and run-qemu-x86_64.

Then what do you mean by the tutorial contains the same information as the scripts, the scripts have just the commands used for running, nothing more, which is not explained in the tutorial, just referenced as in run this script to start the app.

No. We must direct people to KraftKit regardless of the state of the documentation (which I am working on full-time now). Thi README is part of the same documentation and waiting for one piece elsewhere is not the right approach. We should update all relevant documents to usage with KraftKit. Finally, there are admittedly bugs in KraftKit, like there are with Unikraft itself. This document only serves one persona and does not help us in getting valuable bug reports and user experience stories with KraftKit. Redirecting people away will not benefit us, short-, mid- or long-term.

I highly doubt that newcomers will start using a new project / tool / whatever (being that Unikraft or Kraftkit), bump into issues and then report them. The very big majority of people will quit trying after 10 minutes of running into issues, the reporting happens after they see the project working, then run into issues while exploring. That is why the first instructions they see should always be working.

direct people to KraftKit should mean sending people somewhere where they can find detailed instructions on how to use kraftkit, or at least giving actual instructions on how to run the app using kraftkit, which right now did not work out of the box for me. I would really not put something that does not work outside the box for everyone in the README of app-helloworld.

@razvand
Copy link
Contributor

razvand commented Jul 3, 2023

@nderjung, this is easier for everyone. Once KraftKit comes about, we'll just update the README.

We can also make plenty of updates such as moving the documentation for each app in centralized repository, and provide extensive instructions and improve things. But that's not immediate. We simply make it work now and update it later. Good now is better than perfect sometime later. We can make it perfect at a later time. As long as nothing breaks (and it doesn't) there is no problem.

@razvand razvand added documentation Improvements or additions to documentation enhancement New feature or request labels Jul 3, 2023
Copy link
Member

@nderjung nderjung left a comment

Choose a reason for hiding this comment

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

Please include the preamble (see below) at the start of the README and:

  • Remove the run-qemu-aarch64 and run-qemu-x86_64 scripts, adjust the README to include what they include. No need to dirty the repo with scripts that contain 1-liners;
  • Remove all # XXX is not set config options;
  • Remove the attributes CONFIG_UK_BASE and CONFIG_UK_APP as they contain your personal directories and they are overwritten by Unikraft's build system;
  • Remove CONFIG_UK_FULLVERSION and CONFIG_UK_CODENAME options, these are also overwritten;
  • Rename config-qemu-aarch64 to .config.helloworld_qemu-arm64
  • Rename config-qemu-x86_64 to .config.helloworld_qemu-x86_64

README.md Outdated Show resolved Hide resolved
Ignore files and folders that are output from the set up, configure,
build or run steps.

Signed-off-by: Stefan Jumarea <stefanjumarea02@gmail.com>
@StefanJum StefanJum force-pushed the StefanJum/app-docs-update branch 3 times, most recently from 656e39d to 3983089 Compare July 3, 2023 17:05
@StefanJum StefanJum requested a review from nderjung July 3, 2023 17:07
Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

@StefanJum, see my comments.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@StefanJum
Copy link
Member Author

Check again @razvand

Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

I made a suggestion.

README.md Outdated Show resolved Hide resolved
@StefanJum
Copy link
Member Author

Done @razvand

Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

Some other comments.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@StefanJum
Copy link
Member Author

Done

README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

Thanks, @StefanJum.

Reviewed-by: Razvan Deaconescu razvand@unikraft.io
Approved-by: Razvan Deaconescu razvand@unikraft.io

@nderjung
Copy link
Member

nderjung commented Jul 3, 2023

Thank you for incorporating the changes @StefanJum :)

Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

Some minor comments, @StefanJum

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

Some other minor comments.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Update the README with instructions on setting up, configuring, building
and running the helloworld app with Unikraft. As part of it, add Makefile
files, configuration files and scripts; they are referenced in the
`README.md` file.

Current instructions cover the use of QEMU/KVM platform (both on x86_64 and
on AArch64) and GCC. Firecraker/KVM, Xen, Linux platforms and Clang are
not yet documented.

Signed-off-by: Stefan Jumarea <stefanjumarea02@gmail.com>
Copy link
Contributor

@razvand razvand left a comment

Choose a reason for hiding this comment

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

Reviewed-by: Razvan Deaconescu razvand@unikraft.io
Approved-by: Razvan Deaconescu razvand@unikraft.io

unikraft-bot pushed a commit that referenced this pull request Jul 4, 2023
Update the README with instructions on setting up, configuring, building
and running the helloworld app with Unikraft. As part of it, add Makefile
files, configuration files and scripts; they are referenced in the
`README.md` file.

Current instructions cover the use of QEMU/KVM platform (both on x86_64 and
on AArch64) and GCC. Firecraker/KVM, Xen, Linux platforms and Clang are
not yet documented.

Signed-off-by: Stefan Jumarea <stefanjumarea02@gmail.com>
Reviewed-by: Razvan Deaconescu <razvand@unikraft.io>
Approved-by: Razvan Deaconescu <razvand@unikraft.io>
Tested-by: Unikraft CI <monkey@unikraft.io>
GitHub-Closes: #19
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ci/merged documentation Improvements or additions to documentation enhancement New feature or request
Projects
Status: Done!
Development

Successfully merging this pull request may close these issues.

4 participants