doc: New developer getting started guide #19123
Conversation
|
All checks are passing now. Review history of this comment for details about previous failed status. |
|
Take a look at the GSG page in the build artifacts for this PR to see the generated HTML, much easier than staring at the reST files :) |
dbkinder
added
DNM
There was a problem hiding this comment.
I think this is great work which obviously reflects a lot of serious research into how our documentation plays out in the real world. Thanks a lot for what I'm sure has been a dedicated and involved (and hopefully not entirely thankless) effort behind the scenes. I do have some initial feedback on the particulars, but I agree wholeheartedly with the general direction.
doc/getting_started/index.rst
Outdated
|
|
||
| .. toctree:: | ||
| :maxdepth: 2 | ||
| #. Set environment variables: |
There was a problem hiding this comment.
Do you think that getting rid of this step would be useful for users? I have some patches I've been experimenting with to set up the toolchain via west configuration. I can resurrect them if you agree that'd be useful for 2.1. I personally have seen users stumble on not realizing that environment variable settings do not persist across shell sessions.
There was a problem hiding this comment.
exporting in the current shell is not enough. Either these need to be made persistent, or @mbolivar need to add more west magic as he just offered.
A link to https://docs.zephyrproject.org/latest/guides/env_vars.html wouldn't hurt.
Usual caveat https://superuser.com/questions/183870/difference-between-bashrc-and-bash-profile/1452523#1452523
There was a problem hiding this comment.
We've been trying to address issues via documentation, but it would also be great to address them with tool changes or additions too.
doc/getting_started/index.rst
Outdated
|
|
||
| You can run ``./build/zephyr/zephyr.exe --help`` to get a list of available | ||
| options. | ||
| sudo -E PATH=$PATH sh -c "west flash" |
There was a problem hiding this comment.
I think the goal here is to avoid having to teach the user about udev rules in case their user does not have permissions to write to the relevant device nodes involved in the flash process. But there are unintended consequences here which in my opinion make this advice a mistake.
In particular, west flash will regenerate the build artifacts if any of them are out of date before flashing, for consistency with the build system flash target. This has been the expected behavior in Zephyr since ancient (1.9 at least, IIRC) days.
But if you add sudo, the newly generated files can be owned by root. This makes future "west build" invocations used to do incremental builds afterwards fail due to permissions errors.
I think we cannot paper over the inherent complexity of device permissions on linux here, and ought to add information about necessary udev rules on supported platforms to the documentation as a troubleshooting step if this doesn't work.
There was a problem hiding this comment.
But if you add sudo, the newly generated files can be owned by root.
Building as root is a big no-no. Especially not after pip has downloaded a bunch of random packages from the Internet, half of them users don't need (I understand different users need the other half)
I think we cannot paper over the inherent complexity of device permissions on linux here, and ought to add information about necessary udev rules on supported platforms to the documentation as a troubleshooting step if this doesn't work.
Unfortunately agreed 100%. There's a lot that good automation and good documentation can achieve, but they can solve neither world hunger nor the complicated and fragmented state of udev security across Linux distributions. The best that can be done here is:
- Make sure Ubuntu 12.34 works
- For the 1000s of other Linux distributions, defer. Provide generic test instructions or even better: generic test code that prints: "You don't seem to have permission to access to /dev/whatever. Please check the udev part of the documentation of your Linux distribution"
BTW it would really be nice for chocolatey to run without the Adminstrator account. I digress.
There was a problem hiding this comment.
I stand by the discussion from earlier. For whatever it's worth, flashing as root in the GSG is not OK with me.
If your flash tool installer doesn't set up udev rules properly, its installer is broken.
(Nordics' tools installed via .deb do take care of this properly, e.g.)
There was a problem hiding this comment.
I'm really, really not OK with flashing as root. I don't like that this was marked resolved without any changes.
There was a problem hiding this comment.
@mbolivar Do you have a counter proposal that would generically work for some set of supported boards? I'm trying to find a simple path for a new developer to get a first sample app running on a board. I tried saying using sudo is not the proper way, but we'll use it for this quick start. Let's talk offline and figure this out.
There was a problem hiding this comment.
Ignoring the security disaster for a moment, a build directory that can't be cleaned because it has a random number of files owned by root is everything but new user friendly!
Do you have a counter proposal that would generically work for some set of supported boards?
A generic solution that works across a wide range of boards and Linux distros is probably impossible. On the other hand, a single udev rule that works reliably on one specific Ubuntu version for a reasonable number of boards shouldn't be rocket science.
Breaking news: security is rarely user-friendly.
There was a problem hiding this comment.
Talking with the bug triage folks, they recommend removing the sudo and just do west flash and add a troubleshooting note about udev rules. As you mention, security isn't free.
There was a problem hiding this comment.
Talking with the bug triage folks, they recommend removing the sudo and just do west flash and add a troubleshooting note about udev rules. As you mention, security isn't free.
That's reasonable. Thanks.
There was a problem hiding this comment.
a build directory that can't be cleaned because it has a random number of files owned by root is everything but new user friendly!
^^ also, it's not just a security thing. This is also an issue.
doc/getting_started/index.rst
Outdated
| .. tip:: | ||
|
|
||
| Need help with something? See :ref:`help`. | ||
| #. **Set up a command-line development environment** for Linux*, macOS*, or |
There was a problem hiding this comment.
Why the "*" at the end of each OS?
There was a problem hiding this comment.
We're not consistent about marking company trademarks in our documentation... :(
There was a problem hiding this comment.
It just looks strange or broken right now, so I would either remove it or fix it
doc/getting_started/index.rst
Outdated
|
|
||
| You can run ``./build/zephyr/zephyr.exe --help`` to get a list of available | ||
| options. | ||
| sudo -E PATH=$PATH sh -c "west flash" |
There was a problem hiding this comment.
But if you add sudo, the newly generated files can be owned by root.
Building as root is a big no-no. Especially not after pip has downloaded a bunch of random packages from the Internet, half of them users don't need (I understand different users need the other half)
I think we cannot paper over the inherent complexity of device permissions on linux here, and ought to add information about necessary udev rules on supported platforms to the documentation as a troubleshooting step if this doesn't work.
Unfortunately agreed 100%. There's a lot that good automation and good documentation can achieve, but they can solve neither world hunger nor the complicated and fragmented state of udev security across Linux distributions. The best that can be done here is:
- Make sure Ubuntu 12.34 works
- For the 1000s of other Linux distributions, defer. Provide generic test instructions or even better: generic test code that prints: "You don't seem to have permission to access to /dev/whatever. Please check the udev part of the documentation of your Linux distribution"
BTW it would really be nice for chocolatey to run without the Adminstrator account. I digress.
doc/getting_started/index.rst
Outdated
|
|
||
| .. toctree:: | ||
| :maxdepth: 2 | ||
| #. Set environment variables: |
There was a problem hiding this comment.
exporting in the current shell is not enough. Either these need to be made persistent, or @mbolivar need to add more west magic as he just offered.
A link to https://docs.zephyrproject.org/latest/guides/env_vars.html wouldn't hurt.
Usual caveat https://superuser.com/questions/183870/difference-between-bashrc-and-bash-profile/1452523#1452523
|
|
||
| .. code-block:: console | ||
|
|
||
| pip3 install --user cmake | ||
| cd ~ | ||
| wget https://github.com/Kitware/CMake/releases/download/v3.15.3/cmake-3.15.3-Linux-x86_64.sh |
There was a problem hiding this comment.
There should really be a checksum here.... Is there a SDK issue filed for this already?
There was a problem hiding this comment.
can we just add cmake to scripts/requirements.txt and get rid of this???
There was a problem hiding this comment.
Please don't make everyone else install an entirely separate cmake into ~/.local just because Ubuntu 18.04 and 18.10 have outdated versions.
doc/getting_started/index.rst
Outdated
| cd ~ | ||
| west init zephyrproject | ||
| cd zephyrproject | ||
| west update |
There was a problem hiding this comment.
See comment about the pip3 sequence above
|
Thanks for the comments all. I'm waiting for some additional feedback from a usability test using this new GSG and will be sending an update. |
doc/getting_started/index.rst
Outdated
| .. tip:: | ||
|
|
||
| Need help with something? See :ref:`help`. | ||
| #. **Set up a command-line development environment** for Linux*, macOS*, or |
There was a problem hiding this comment.
It just looks strange or broken right now, so I would either remove it or fix it
doc/getting_started/index.rst
Outdated
| ==================== | ||
|
|
||
| Zephyr development depends on up-to-date releases of common build system | ||
| tools so first, make sure you're development system OS is updated: |
| Advanced Setup and tool chain alternatives | ||
| ****************************************** | ||
|
|
||
| Here are some alternative instructions for more advanced platform setup |
There was a problem hiding this comment.
Need to decide what we want to have here - repetition of the GSG, alternative setups, or troubleshooting. Currently, it's a bit of a mix.
|
|
||
| .. _gs_toolchain: | ||
|
|
||
| Set Up a Toolchain |
There was a problem hiding this comment.
We should have a link to this section from the GSG, because you need to install one (unless you're on Linux and installed the SDK).
| ../getting_started/toolchain_host.rst | ||
| ../getting_started/toolchain_custom_cmake.rst | ||
|
|
||
| Cloning the Zephyr Repositories |
There was a problem hiding this comment.
I like the idea of giving more detail in this guide than in the GSG, but it must be made clearer what is repetition and what is additional content. Also, we should have links from each section in the GSG to the "more information if you need it" section here.
dbkinder
requested review from
mbolivar,
carlescufi,
ru-fu,
marc-hb and
aescolar
|
I've incorporated changes based on your comments and additional DX usability testing. Overall, this updated and simplified GSG approach has significantly improved the initial setup experience of Zephyr. When the SDK improvements are released we can submit a new PR to update these instructions with some further simplifications and time improvements, followed by more DX improvements over time. |
dbkinder
removed
DNM
dbkinder
changed the title
|
And you can view the build artifacts for this PR to see the generated HTML, much easier than staring at the reST files :) |
dbkinder
dismissed
mbolivar’s stale review
we can remove any extra packages being installed after investigating this further.
|
@mbolivar @nashif @andrewboie @ru-fu @carlescufi @aescolar @marc-hb I'm hoping this PR is close enough to get merged as an improvement to the existing GSG and we can continue with other DX improvements (and improvements to this documentation) in subsequent PRs... |
|
@dbkinder , What if we consider a reference link to a new page on udev needs? We can then add the appropriate udev rules as they are identified for the various target boards. That would than remove the desire to have the sudo command for west flash |
|
Worth pointing out: if you clone the openocd-git repo: There's a file contrib/60-openocd.rules which at least for me has allowed me to flash every Zephyr supported board I have tried so far. There might not be support for every board, but at least it gets you pretty far: Ideally though, the documentation for every board contains information on the necessary udev string to get it going. |
Alas, a quick look shows that only 4 boards (out of over 200) mention udev rules in their documentation. Not sure I'm ready to submit PR(s) to touch all the board docs, so maybe indeed the notion of a udev troubleshooting section in the linux-specific doc is the answer. (And have |
For some strange reason, everyone on the internet and their dog recommends |
|
Flashing with sudo is out now, use of the openOCD udev rules file is in. @marc-hb sorry about the forced pushes hiding what changes were made. :( |
doc/getting_started/index.rst
Outdated
| Add the kitware repo corresponding to your Ubuntu release (use | ||
| ``cat /etc/os-release`` to check): | ||
|
|
||
| - For Ubuntu Bionic Beaver (18.04) use: |
There was a problem hiding this comment.
@dbkinder Ubuntu 18.10 ships CMake 3.12, so these instructions won't work for that version of Ubuntu, and the user will still have an out of date CMake.
Ubuntu 19.04 (and thus all subsequent versions) ship a recent enough version.
To fix these instructions for 18.10, you could just add a generic reference to their download page:
There was a problem hiding this comment.
Maybe I'm missing something here, but since CMake is available with pip, couldn't we just add cmake to scripts/requirements.txt and just standardize on that for every host OS?
doc/getting_started/index.rst
Outdated
|
|
||
| sudo apt-add-repository 'deb https://apt.kitware.com/ubuntu/ bionic main' | ||
|
|
||
| - For Ubuntu Xeniel Xerus (16.04) use: |
There was a problem hiding this comment.
| - For Ubuntu Xeniel Xerus (16.04) use: | |
| - For Ubuntu Xenial Xerus (16.04) use: |
There's an openocd rules file in the SDK, FWIW. Installing that file into /etc/udev/rules.d only works if your user is in the plugdev group, of course. And then you have to reset the udev state machines or unplug/replug in your device. Fiddling with udev rules is a very annoying rite of passage doing embedded development on Linux in my experience, unfortunately. I blame greg KH :). |
At the high risk of stating the obvious: this is a github limitation, nothing about you (or any other user) github's preferred review model is based on "fixup" commits and squash. So it's compatible with neither force pushes nor the review of a series of multiple commits. Off-topic sorry, in the unlikely urge to discuss further use #14444 or the #github-usage channel on Slack |
As presented to the TSC, Zephyr's out-of-box experience for new developers is, well, complicated. A number of suggestions were presented including simplifying the getting started material to present a straight-forward path through the setup and installation steps through to getting a sample application built, flashed, and running. This PR is a work-in-progress towards addressing this OOB experience with a minimal-distractions version of the GSG. Alternatives, warnings, and material that could lead the developer astray were moved to alternative/advanced instruction documents (based on the previous separate Linux/macOS/Windows setup guides) and a new "Beyond the GSG" document. We do take advantage of a sphinx-tabs extension for synchronized tabs to present OS-specific instructions: clicking on one tab will display all same-named tabs throughout the doc. We hope (and will continue evaluating) that this new GSG gets developers set up quickly and then we can send them along to other documents to continue learning about Zephyr and trying other sample apps. Thanks for all your previous feedback that I've worked into this new version. Signed-off-by: David B. Kinder <[email protected]>
|
@dbkinder thanks for rounding up some of the discussion on udev rules into the force-push you just made. That wasn't necessary IMO, but I appreciate it! This comment could eventually be added too, if you want to improve this further:
According to https://wiki.ubuntu.com/Security/Privileges, only the first user added to an Ubuntu desktop system is a member of this group. So multi-user systems (admittedly very rare) could run into issues here. |
|
Thanks @mbolivar I think I'll save that for another update. Time to get this one merged :) |
Yep yep, not asking for a change, just making a note. |
|
something I think is worth pointing out: you can install a suitable cmake on Ubuntu using snap. This seems a lot simpler than the current instructions. |
|
@andrewboie It is listed as an alternative in the linux-specific detailed doc (https://builds.zephyrproject.org/zephyrproject-rtos/zephyr/19123/docs/getting_started/installation_linux.html#cmake) along with using pip :) I can submit an update after this big and long-delayed PR gets merged... |
I gave it a try and needed to configure the snap proxy and use the Snap puts its binaries into |
As presented to the TSC, Zephyr's out-of-box experience for new
developers is, well, complicated. A number of suggestions were
presented including simplifying the getting started material to present
a straight-forward path through the setup and installation steps through
to getting a sample application built, flashed, and running.
This PR is a work-in-progress towards addressing this OOB experience
with a minimal-distractions version of the GSG. Alternatives, warnings,
and material that could lead the developer astray were moved to
alternative/advanced instruction documents (based on the previous
separate Linux/macOS/Windows setup guides) and a new "Beyond the GSG"
document.
We do take advantage of a sphinx-tabs extension for synchronized tabs to
present OS-specific instructions: clicking on one tab will display all
same-named tabs throughout the doc.
We hope (and will continue evaluating) that this new GSG gets developers
set up quickly and then we can send them along to other documents to
continue learning about Zephyr and trying other sample apps.
Appreciate your feedback.
Signed-off-by: David B. Kinder [email protected]