-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add acap-ssh-utils bin and lib crate #19
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
apljungquist
force-pushed
the
add_ssh_utils
branch
from
May 9, 2024 14:33
b18c252
to
8f4cd91
Compare
apljungquist
force-pushed
the
add_ssh_utils
branch
from
May 9, 2024 14:39
8f4cd91
to
2d3bb03
Compare
Benefits of having this include * documents how to quickly deploy and test incremental changes. * makes it easier for other tools to do so by taking care of the many details while presenting a friendly interface. This will come in handy for implementing analogs of `cargo run` and `cargo test`, both of which will cross-compile a binary and execute it on target. Exclude the new crate when checking other targets because supporting other targets is not a priority, especially not for this crate that will depend on `sshpass`, `ssh`, and `scp` for the foreseeable future which makes it less portable. Change the behavior of `make run` to always set the `RUST_LOG*` environment variables to decouple the log level of the tool that runs on host from the log level of the app that is running on target. Always require a password because it is not possible to set up public key authentication for users other than root, so we have to support password authentication and supporting only one method makes both the implementation and the user experience more straightforward. Print information about environment from `hello_world` example because that way it becomes a useful tool and it doesn't make the app noticeably more difficult to understand.
apljungquist
force-pushed
the
add_ssh_utils
branch
from
May 9, 2024 14:41
2d3bb03
to
de7a820
Compare
apljungquist
force-pushed
the
add_ssh_utils
branch
from
May 26, 2024 17:12
6aaceb4
to
851ca74
Compare
Coming back to the code and having to fix the make target I found the other way unintuitive, even though it makes sense once the mapping is encoded in a hash map (because each destination must be created at most once but multiple destinations could be created from the same source). I think there are two reasons why this order makes sense: * It is chronological; we start with a source and then we create the destination. * Other tools I'm familiar with use this order. For instance when mapping volumes in docker the host (local) path is on the left hand side and the guest (remote) is on the right hand side.
The crate is focused around the package as a unit that is patched and run. Since every ACAP developer have systems set up to produce `.eap` files I expect this to be an accessible interface. I previously considered an interface where files individual files could be replaced. That was driven by - having worked with an app for which incremental compilation worked poorly and for fast iterations it was necessary to build and patch only part of the app. I know think this would be better solved by improving that build system, and it is not something that this tool should attempt to accomodate. - having a build system for apps written in rust that does not produce `.eap` files when compiling tests. I have since rewritten the build tool so be more `.eap` centric and I think this is the easier interface to work with when combining tools.
I think these names are clear given the context and they are less work typing out. It would also be consistent with the `Makefile` where the password is labeled `PASS`.
* upstream/main: Rename environment variables controlling the Makefile (#26) Adopt dev container as primary environment (#39) Make the `build` verb build for only one target (#31) chore(deps): bump ghcr.io/devcontainers/features/common-utils (#38) chore(deps): bump log from 0.4.21 to 0.4.22 in the default group (#37) Add dev container (#36) Add link to acap-rs-app-template in `README.md` (#34) Document `python3-venv` dependency (#35) fix: Repair `Makefile` (#32) fix(embedded_web_page): Put web page in `html/` (#30) Organize dependencies consistently (#29) chore(deps): bump glib-sys from 0.19.5 to 0.19.8 in the default group (#28) chore: Explain more make targets (#27) Add embedded web page example (#14)
`.env` is added to `.gitignore` so that I can use this to easily restore environment variables when rebuilding the dev container.
In `Makefile`: - Export the `AXIS_*` variables to make sure they are available for scripts when the default values are used. This is particularly useful for the password argument since it allows programs such as `acap-ssh-utils` to pick this up from the environment, which is better from a security perspective (that tool still passes it as an argument to other commands though).
While the tool remains relatively untested it seems wise to not advertise it.
It was lost when fixing merge conflicts.
apljungquist
added a commit
that referenced
this pull request
Jul 19, 2024
* upstream/main: Generate app `LICENSE` files (#44) Add acap-ssh-utils bin and lib crate (#19) Add cargo-acap-build tool (#25) Remove partially containerized workflow (#42) Improve acap-logging docs (#41) chore: Prepare logging library crate for publishing (#40) feat: Add metadata broker wrapper and example (#22) Rename environment variables controlling the Makefile (#26) Adopt dev container as primary environment (#39) Make the `build` verb build for only one target (#31) chore(deps): bump ghcr.io/devcontainers/features/common-utils (#38) chore(deps): bump log from 0.4.21 to 0.4.22 in the default group (#37) Add dev container (#36) Add link to acap-rs-app-template in `README.md` (#34) Document `python3-venv` dependency (#35) fix: Repair `Makefile` (#32)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is primarily intended as a building block for a more tightly integrated development tool, where it will come in handy for implementing analogs of
cargo run
andcargo test
, both of which will cross-compile a binary and execute it on target. But, since it solves a common problem among ACAP developers using any language, it is factored out into its own lib crate with a thin binary crate wrapper.The crate is focused around the package as a unit that is patched and run. Since every ACAP developer have systems set up to produce
.eap
files I expect this to be an accessible interface. An earlier version allowed individual files to be replaced. This was driven by working with build systems, including my own, that did not package tests as.eap
files or were slow to produce.eap
files. I now think that these issues are better addressed in the build system since the action of creating the tar archive should in theory be cheap.In
install-system-packages.sh
:sshpass
because it is needed byacap-ssh-utils
which in turn needs it because I have not found a way to add authorized keys for users other than root.In
.gitignore
:/.env
because defining theAXIS_DEVICE_*
variables in a file makes it reduces the friction of rebuilding the dev container and to run tests.In
Makefile
:AXIS_*
variables to make sure they are available for scripts when the default values are used. This is particularly useful for the password argument since it allows programs such asacap-ssh-utils
to pick this up from the environment, which is better from a security perspective (that tool still passes it as an argument to other commands though).RUST_LOG
andRUST_LOG_STYLE
when running on target because I found that most of the time this is what I wanted and oftentimes I did not want to set it forcargo-acap-build
andacap-ssh-utils
at the same time.