Skip to content

Conversation

@kit-ty-kate
Copy link
Contributor

@kit-ty-kate kit-ty-kate commented Sep 13, 2022

Fixes #109
Requires kit-ty-kate/ocaml-docker-hub#1

TODO:

  • Replace the uses of Docker by a handwritten fetcher (or wait for docker to be ported to FreeBSD)
  • Test it

@kit-ty-kate kit-ty-kate marked this pull request as ready for review September 16, 2022 20:21
@kit-ty-kate kit-ty-kate changed the title [WIP] Add support for FreeBSD Add support for FreeBSD Sep 16, 2022
@patricoferris
Copy link
Contributor

When implementing the (still unmerged) macOS backend, we seemed to have agreed on deciding the sandbox at build time. See question 3 here #87 (comment) and also #87 (comment). I don't know which approach would be best?

@kit-ty-kate
Copy link
Contributor Author

sure, build-time is fine by me, I'll change it. It was just faster to implement it this way for the PoC

@MisterDA
Copy link
Contributor

When implementing the (still unmerged) macOS backend, we seemed to have agreed on deciding the sandbox at build time. See question 3 here #87 (comment) and also #87 (comment). I don't know which approach would be best?

Haha I just reimplemented the opposite: sandbox selection at runtime. I want to be able to use the Docker backend on Linux too, and now if you select the Docker store it will automatically select the docker "sandbox" too instead of runc.
171b718

@kit-ty-kate
Copy link
Contributor Author

Haha I just reimplemented the opposite: sandbox selection at runtime. I want to be able to use the Docker backend on Linux too, and now if you select the Docker store it will automatically select the docker "sandbox" too instead of runc. 171b718

That can always be implemented on top of build-time selection. You'd just have to list the available backends at build times instead of just one implementation.

I think both are not exclusive with each other, and on my side it allows us to remove a dependency on extunix

Copy link
Contributor

@patricoferris patricoferris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great! I was wondering how you're testing it @kit-ty-kate, I could try a similar setup?

| `Output -> Buffer.add_string buffer x

let healthcheck ?(timeout=30.0) t =
Os.with_pipe_from_child (fun ~r ~w ->
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should sandboxes each specify a "I have some system dependencies I need, let me check for them" function that we could call here? Or do you think the get_base + run_steps etc is sufficient?


let fetch ~log ~rootfs base =
with_container ~log base (fun cid ->
let fetch ~log:_ ~rootfs base =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have we lost the (* We might need to do a pull here, so log the output to show progress. *) by not doing anything with the log.


let with_container manifest token fn =
Lwt_io.with_temp_dir ~perm:0o700 ~prefix:"obuilder-docker-hub-" @@ fun output_file ->
Docker_hub.fetch_rootfs ~output_file:(Fpath.v output_file) manifest token >>=
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Converting to the docker-hub library seems sensible to me (presumably for FreeBSD support), the problem at the moment is that the means by which the tests mock lots of things is by hijacking the exec function that is used and catching calls to things like docker create.... Perhaps this code could be functorised over a docker_hub like thing ?

@kit-ty-kate
Copy link
Contributor Author

This looks great! I was wondering how you're testing it @kit-ty-kate, I could try a similar setup?

it's not working yet. I've only opened the PR to allow for some first-pass reviews. I've only just succeeded pushing a freebsd image to docker hub so far, i still haven't tried pulling it (i'm working on opam today)

@MisterDA MisterDA force-pushed the master branch 2 times, most recently from 89e84ac to ccd3f2f Compare February 3, 2023 12:39
@tmcgilchrist
Copy link
Member

FreeBSD support added in #174

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.

FreeBSD Support

4 participants