Skip to content
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 25 commits into from
Jul 18, 2024
Merged

Add acap-ssh-utils bin and lib crate #19

merged 25 commits into from
Jul 18, 2024

Conversation

apljungquist
Copy link
Contributor

@apljungquist apljungquist commented May 9, 2024

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 and cargo 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:

  • Add sshpass because it is needed by acap-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:

  • Add /.env because defining the AXIS_DEVICE_* variables in a file makes it reduces the friction of rebuilding the dev container and to run tests.

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).
  • Always set RUST_LOG and RUST_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 for cargo-acap-build and acap-ssh-utils at the same time.

@apljungquist apljungquist requested a review from a team as a code owner May 9, 2024 13:42
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 and others added 22 commits June 10, 2024 20:07
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 apljungquist merged commit 4400657 into main Jul 18, 2024
2 checks passed
@apljungquist apljungquist deleted the add_ssh_utils branch July 18, 2024 20:21
apljungquist added a commit that referenced this pull request Jul 19, 2024
…ample

* upstream/main:
  Generate app `LICENSE` files (#44)
  Add acap-ssh-utils bin and lib crate (#19)
apljungquist added a commit that referenced this pull request Jul 19, 2024
…ample

* upstream/main:
  Generate app `LICENSE` files (#44)
  Add acap-ssh-utils bin and lib crate (#19)
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
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant