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

RFC: Add --no-docker option #11

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

apljungquist
Copy link

This is helpful if we want to build apps in Jenkins where it is convenient to install cargo-acap in a docker image and inconvenient to set up docker-in-docker.

Does that make sense?

If so I will clean up the PR.

This is helpful if we want to build apps in jenkins where it is
convenient to install cargo-acap in a docker image and inconvenient to
set up docker-in-docker.
@willglynn
Copy link
Contributor

🤔 If you're intending to run cargo-acap inside a trunnion/cargo-acap image, we could:

  1. Build and ship cargo-acap inside the image with the -axis- Rust toolchain and SDKs, and
  2. Set an environment variable for the image which cargo-acap can detect

The net result being that the user can either install cargo-acap on the host and let it invoke docker, or the user can use that image with the container runtime of their choosing and run cargo-acap inside. I think this might address your situation but I'm not sure.

If you're intending to use cargo-acap with a self-installed Axis SDK and cross-compilers and such, without the trunnion/cargo-acap image, I'd like to know more about your setup. cargo-acap would lose the conditional compilation feature but I guess it could still handle packaging 🤷‍♂️

@apljungquist
Copy link
Author

apljungquist commented May 22, 2024

Sorry for the late reply, I should have predicted that opening a PR right before going on vacation is a bad idea.

That does indeed sound like it would address my situation.

Re 1; I had not thought of this and it is not critical for my use case since I will always have a Dockerfile. But it's neat and unlikely to add much overhead, so I like it.

Re 2; It is also possible to detect docker automatically but I think your suggestion is better since it let's the user choose explicitly what to do. Compared to using command line options I also like that commands will be the same inside and outside the container, (but I also recognize this could be due to an excessive desire for tidiness 😁).

I'll update the PR to incorporate your suggestions.

I tried to make clap parse this but by default clap will treat any
value, including "", "0", and "false" as true. Every way I tried to
change this would introduce some other problem. For instance changing
the action to set would make it show up as a positional argument.

The values accepted are limited to "0" and "1" because this is
sufficient to communicate all options, and adding more accepted values
is easier than removing them.
This makes it easier for downstream users to use docker in CI without
setting up docker-in-docker.

These changes can be tested locally by doing something like

```sh
cargo install \
  --path . \
  --root cargo-acap-build
pushd cargo-acap-build
docker build \
  --build-arg RUST_VERSION=1.74.0 \
  --tag cargo-acap:1.74.0 \
  .
popd
docker run \
  --env CARGO_ACAP_NO_DOCKER=1 \
  --interactive \
  --rm \
  --tty \
  --user $(id -u):$(id -g) \
  --volume <PATH_TO_SAMPLE_APP>:<PATH_TO_SAMPLE_APP> \
  --volume ~/.cargo:/.cargo \
  --workdir <PATH_TO_SAMPLE_APP> \
  cargo-acap:1.74.0 \
  cargo-acap build
```
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.

2 participants