deps: add support for vendor and use local vendor for deps

[flouthoc]

While releasing netavark binary lot of platform does not allows build
process to pull deps from upstream repo.

Vendor and ship dependency locally in a vendor directory and point
cargo to use local vendored deps.

Automatically uses vendor while doing make build or make

See: #115

[flouthoc]

@lsm5 @cevich @mheon @baude @Luap99 PTAL

[flouthoc]

This should also allow us to build with --offline flag.

[cevich]

Oof, github nearly crashes my browser on this PR, lol. Taking a look from the command-line...

[cevich]

@flouthoc I'm showing the new vendor directory at 158M, is there any way to get that smaller? (podman's vendor directory is only 53M). Not sure if it will help the size, but I see some duplication - for example, three different versions of nix.

I think what you have here will work fine in CI (I checked, the "build" task runs a make all). Main concern is users and git churning through that much data. Maybe it will be okay past the first download, but sheesh it's a lot.

Assuming there's no cargo squeeze vendor command (ha!) I tried compressing it. It does squash down quite small, I got 19M with zip. But that would mean more Makefile complications and shipping a binary blob in SCM ☹️

Unless you have any easy ideas, this LGTM. I would suggest rebasing though to verify it works with the new dedicated VM images (merged a few hours ago).

[cevich]

Ahh ha! The major culprets appear to be:

7.6M    winapi
52M     winapi-i686-pc-windows-gnu
60K     winapi-util
54M     winapi-x86_64-pc-windows-gnu

[cevich]

Yeah, there's a TON of binary archive files, e.g. vendor/winapi-x86_64-pc-windows-gnu/lib/libwinapi_windowsapp_downlevel-xmllite.a. I'm guessing we don't need those in the source SCM?

[cevich]

This should also allow us to build with --offline flag.

I'm in favor for actually doing this in CI, and/or have it the default in Makefile unless overridden. Simply to limit surprises. Also note: We do a cargo install mandown (with CARGO_HOME=/var/cache/cargo) when building the VM image. I could not find a F35 RPM for this, and I don't see that in the vendor directory. When you get a chance, let me know if there's anything I should change WRT this.

[flouthoc]

This should also allow us to build with --offline flag.

I'm in favor for actually doing this in CI, and/or have it the default in Makefile unless overridden. Simply to limit surprises. Also note: We do a cargo install mandown (with CARGO_HOME=/var/cache/cargo) when building the VM image. I could not find a F35 RPM for this, and I don't see that in the vendor directory. When you get a chance, let me know if there's anything I should change WRT this.

Sure I'll try checking if we have a crate for mandown or somthing equivalent. I removed docs from all rule since its a NOOP as of now.

Yeah, there's a TON of binary archive files, e.g. vendor/winapi-x86_64-pc-windows-gnu/lib/libwinapi_windowsapp_downlevel-xmllite.a. I'm guessing we don't need those in the source SCM?

They are sub-dependency of another crate it seems we don't have lot of control over other crates dependency except to carry them afaik. :\

[cevich]

They are sub-dependency of another crate it seems we don't have lot of control over other crates dependency except to carry them afaik. :\

This strikes me as both impractical (distributing/managing this repo. code) and dangerous (who knows what's in those binaries). Is there a way we can carry the source for them instead?

There's a message on https://lib.rs/crates/winapi-x86_64-pc-windows-gnu which says "Please don’t use this crate directly, depend on winapi instead." (in case that means something to you).

[Luap99]

Why do we depend on windows code? Netavark is and should only be compiled on linux. I think we should clean our dependencies first before we vendor unnecessary stuff.

[Luap99]

Why do we depend on windows code? Netavark is and should only be compiled on linux. I think we should clean our dependencies first before we vendor unnecessary stuff.

It looks like there is no solution for this problem yet, cargo will always pull in dependencies for all platforms: rust-lang/cargo#7058
Maybe we can add winapi to gitignore?

[flouthoc]

@cevich As @Luap99 suggested cargo we still pull everything and its not possible to remove whole directory structure but we could safely remove all the libraries inside it.

We still need some files otherwise cargo will try to pull them again so cant add everything to .gitignore but all the heavy content could be removed safely.

Made this change in a latest commit.

[cevich]

Ahh this is much better now, thanks.

$ du -shxc ./* | egrep '[[:digit:]]+M'
1.2M    ./futures-util
2.3M    ./idna
3.6M    ./libc
1.1M    ./nix
1.2M    ./nix-0.17.0
1.3M    ./nix-0.20.0
1.4M    ./nix-0.22.0
1.1M    ./regex
1.5M    ./regex-syntax
2.1M    ./syn
3.6M    ./tokio
7.6M    ./winapi
53M     total

LGTM

[Luap99]

Please do this in one commit, by removing the files in an extra commit you now include these huge binary files in the git history forever.

[Luap99]

Well it is still over 50 MB but I guess we cannot get it much smaller. Does dependabot work with the vendor dir?
I think we also need a CI check so that the the vendor dir is always up to date and never changed manually.

[flouthoc]

Well it is still over 50 MB but I guess we cannot get it much smaller. Does dependabot work with the vendor dir? I think we also need a CI check so that the the vendor dir is always up to date and never changed manually.

make always syncs vendor from Cargo.toml. But i think dependabot will also do it.

[lsm5]

@cevich the winapi is a dependency of the faccess crate. The faccess crate does have it listed as a windows only dependency, but I'm not sure if we can specify something like skip windows deps in the build process.

The other option would be to get rid of faccess dep and replace it with stdlib calls.

[cevich]

I'm seeing some old things in .cirrus.yml, please rebase. Also, the new task should alias the cache attributes, not re-anchor them. It's also important to reference all tasks as dependencies for the success task. So, something like this (I didn't not check it):

diff --git a/.cirrus.yml b/.cirrus.yml
index 9bb4b76..277ee0b 100644
--- a/.cirrus.yml
+++ b/.cirrus.yml
@@ -97,21 +97,9 @@ verify_vendor_task:
   alias: "verify_vendor"
   depends_on:
     - "build"
-  # From this point forward, all cache's become read-only - meaning
-  # any changes made in this task aren't re-uploaded to the cache.
-  # This avoids some flapping between tasks, along with the upload time.
-  pkg_cache: &ro_pkg_cache # TODO: Remove when packages included in static VM images
-    <<: *pkg_cache
-    reupload_on_changes: false
-  cargo_cache: &ro_cargo_cache
-    <<: *cargo_cache
-    reupload_on_changes: false
-  targets_cache: &ro_targets_cache
-    <<: *targets_cache
-    reupload_on_changes: false
-  bin_cache: &ro_bin_cache
-    <<: *bin_cache
-    reupload_on_changes: false
+  cargo_cache: *ro_cargo_cache
+  targets_cache: *ro_targets_cache
+  bin_cache: *ro_bin_cache
   setup_script: *setup
   main_script: *main

@@ -165,6 +153,7 @@ success_task:
   depends_on:
     - "build"
     - "validate"
+    - "verify_vendor"
     - "unit"
     - "integration"
     - "meta"

[flouthoc]

@cevich @Luap99 Well I don't think we need a verify_vendor check. cargo fmt fails if there is a diff between what's in vendor and what's in Cargo.toml. See it in action: /~https://github.com/containers/netavark/pull/123/checks?check_run_id=4537357195

I think validate already covers that 😄

[cevich]

I think validate already covers that smile

Oh, great news!

[cevich]

LGTM

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cevich, flouthoc

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [flouthoc]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Luap99]

LGTM but can you rebase to pick up the lates dep update from #133

[Luap99]

@mheon @baude PTAL

[flouthoc]

LGTM but can you rebase to pick up the lates dep update from #133

Done. @mheon @baude PTAL

[mheon]

/lgtm

[cevich]

Thanks for taking the initiative @flouthoc and for seeing this through to merge. I appreciate it.