[script] install nodes in ~/.local/share/otns so that OTNS can run from anywhere#773
[script] install nodes in ~/.local/share/otns so that OTNS can run from anywhere#773EskoDijk wants to merge 1 commit into
Conversation
…om anywhere The installation is updated to copy node executables to a .local/share directory. This directory is added to the node search path, so that standard OT node executables can be found from any directory location where OTNS is run from. The build and copying of node executables is added to the './script/install' script. Close openthread#587
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the portability and ease of use for OTNS by standardizing the installation location for OpenThread node executables. By moving these binaries to a user-specific shared directory and updating the search paths, users can now run OTNS from any directory without encountering issues related to missing node executables. This change streamlines the setup process and improves the overall developer experience. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors the installation scripts to separate the installation of OTNS from the OT nodes. It also updates the installation process to copy node executables to a user-local directory (~/.local/share/otns), allowing OTNS to be run from any location. This is a valuable improvement for user experience. My review includes a couple of suggestions to enhance the robustness and standards-compliance of the implementation in both the shell script and Go code.
| mkdir -p ~/.local/share/otns/bin | ||
| cp ./ot-rfsim/ot-versions/* ~/.local/share/otns/bin/ |
There was a problem hiding this comment.
For better compliance with the XDG Base Directory Specification and to improve robustness, consider using the $XDG_DATA_HOME environment variable, which defaults to $HOME/.local/share. This avoids hardcoding the path and respects user configurations. Using a variable also makes the code cleaner by avoiding repetition.
| mkdir -p ~/.local/share/otns/bin | |
| cp ./ot-rfsim/ot-versions/* ~/.local/share/otns/bin/ | |
| otns_data_dir="${XDG_DATA_HOME:-$HOME/.local/share}/otns/bin" | |
| mkdir -p "$otns_data_dir" && cp ./ot-rfsim/ot-versions/* "$otns_data_dir/" |
| func filePathInUserHomeDir(relPath string) string { | ||
| home, err := os.UserHomeDir() | ||
| if err == nil { | ||
| return filepath.Join(home, relPath) | ||
| } | ||
| return "/tmp/UserHomeDirNotFound" | ||
| } |
There was a problem hiding this comment.
The error handling for os.UserHomeDir() can be made clearer. Returning a magic string like /tmp/UserHomeDirNotFound without context can be confusing for future maintenance. It's good practice to add a comment explaining why a fallback path is returned. Also, using a guard clause for the error check would improve readability.
func filePathInUserHomeDir(relPath string) string {
home, err := os.UserHomeDir()
if err != nil {
// os.UserHomeDir() can fail in some environments (e.g. when cross-compiling).
// Return a path that is unlikely to exist, so the executable search will
// gracefully fail for this path and continue with other search paths.
return "/tmp/otns-home-dir-not-found"
}
return filepath.Join(home, relPath)
}1 participant
The installation is updated to copy node executables to a .local/share directory. This directory is added to the node search path, so that standard OT node executables can be found from any directory location where OTNS is run from. The build and copying of node executables is added to the './script/install' script.
Close #587