Closed DaanDeMeyer closed 1 year ago
Thanks for the report, I'll work on adding support for these today
@DaanDeMeyer this should all be addressed now. Could you please take a look and let me know if you need any other changes? If not, please feel free to close the issue.
cargo
is still trying to update the index when I run it after downloading the dependencies. I think there's no way to check if the "download dependencies" cargo step has been executed so make will always try to run it?
Hmmm, ok. We could always add a special build-specific make target that doesn't take the cargo fetch
target step as a dependency. Would this do the trick?
diff --git a/tools/sched_ext/Makefile b/tools/sched_ext/Makefile
index fc2dca246d72..df93d3e098c0 100644
--- a/tools/sched_ext/Makefile
+++ b/tools/sched_ext/Makefile
@@ -203,9 +203,17 @@ scx_rusty_deps: $(SCX_COMMON_DEPS)
scx_rusty: export RUSTFLAGS = -C link-args=-lzstd -C link-args=-lz -C link-args=-lelf -L $(BPFOBJ_DIR)
scx_rusty: export SCX_RUSTY_CLANG = $(CLANG)
scx_rusty: export SCX_RUSTY_BPF_CFLAGS = $(BPF_CFLAGS)
+
+define ccrusty
+ cargo build --manifest-path=$1/Cargo.toml --offline $(CARGOFLAGS)
+ $(Q)cp $(OUTPUT_DIR)/release/$1 $(BINDIR)/$1
+endef
+
scx_rusty: $(INCLUDE_DIR)/vmlinux.h scx_rusty_deps
- cargo build --manifest-path=$@/Cargo.toml --offline $(CARGOFLAGS)
- $(Q)cp $(OUTPUT_DIR)/release/$@ $(BINDIR)/$@
+ $(call ccrusty,$@)
+
+scx_rusty_build: $(INCLUDE_DIR)/vmlinux.h $(SCX_COMMON_DEPS)
+ $(call ccrusty,scx_rusty)
install: all
$(Q)mkdir -p $(DESTDIR)/usr/bin/
What about allowing me to pass --offline to cargo via a make variable and not having scx_rusty depend on scx_rusty_deps? Then I can just do scx_rusty_deps first and after that set CARGO_OFFLINE=1
to make sure cargo doesn't contact the network.
Yes, we could do that, or just add another Make target so there's still a convenient way to build for others doing it locally. When I tried the above patch though I'm seeing this:
...
Compiling clap_derive v4.4.2
Compiling scroll_derive v0.11.1
Compiling num_enum_derive v0.5.11
Compiling strum_macros v0.24.3
Compiling libbpf-sys v1.2.1+v1.2.0
Compiling clap v4.4.6
Compiling num_enum v0.5.11
Compiling libbpf-rs v0.21.2
error: could not find native static library `bpf`, perhaps an -L flag is missing?
error: could not compile `libbpf-sys` (lib) due to previous error
warning: build failed, waiting for other jobs to finish...
Not yet sure why that's only happening if we don't take a dependency on scx_rusty_deps
, and instead manually run it.
Ah, this should do it:
diff --git a/tools/sched_ext/Makefile b/tools/sched_ext/Makefile
index fc2dca246d72..5251cd8fd635 100644
--- a/tools/sched_ext/Makefile
+++ b/tools/sched_ext/Makefile
@@ -199,13 +199,25 @@ scx_userland: scx_userland.c scx_userland.skel.h scx_userland.h $(SCX_COMMON_DEP
$(call ccsched,$<,$@)
scx_rusty_deps: $(SCX_COMMON_DEPS)
+ cargo update --manifest-path=scx_rusty/Cargo.toml
cargo fetch --manifest-path=scx_rusty/Cargo.toml
scx_rusty: export RUSTFLAGS = -C link-args=-lzstd -C link-args=-lz -C link-args=-lelf -L $(BPFOBJ_DIR)
scx_rusty: export SCX_RUSTY_CLANG = $(CLANG)
scx_rusty: export SCX_RUSTY_BPF_CFLAGS = $(BPF_CFLAGS)
+
+define ccrusty
+ cargo build --manifest-path=scx_rusty/Cargo.toml --offline $(CARGOFLAGS)
+ $(Q)cp $(OUTPUT_DIR)/release/scx_rusty $(BINDIR)/scx_rusty
+endef
+
scx_rusty: $(INCLUDE_DIR)/vmlinux.h scx_rusty_deps
- cargo build --manifest-path=$@/Cargo.toml --offline $(CARGOFLAGS)
- $(Q)cp $(OUTPUT_DIR)/release/$@ $(BINDIR)/$@
+ $(call ccrusty)
+
+scx_rusty_build: export RUSTFLAGS = -C link-args=-lzstd -C link-args=-lz -C link-args=-lelf -L $(BPFOBJ_DIR)
+scx_rusty_build: export SCX_RUSTY_CLANG = $(CLANG)
+scx_rusty_build: export SCX_RUSTY_BPF_CFLAGS = $(BPF_CFLAGS)
+scx_rusty_build: $(INCLUDE_DIR)/vmlinux.h $(SCX_COMMON_DEPS)
+ $(call ccrusty)
install: all
$(Q)mkdir -p $(DESTDIR)/usr/bin/
Yes, we could do that, or just add another Make target so there's still a convenient way to build for others doing it locally. When I tried the above patch though I'm seeing this:
What I was thinking was that if CARGO_OFFLINE
is not set, then --offline
won't be passed to cargo which means it will download the dependencies as usual, so for existing users nothing would change, but I'd be able to tell cargo to not go to the network. And this also means I don't have to invoke a special target to build scx_rusty, I can just do a regular make all
with CARGO_OFFLINE
enabled and it'll do the right thing.
Ahhh, I see. Good idea, how about this?
diff --git a/tools/sched_ext/Makefile b/tools/sched_ext/Makefile
index fc2dca246d72..00bf2f292b5b 100644
--- a/tools/sched_ext/Makefile
+++ b/tools/sched_ext/Makefile
@@ -91,6 +91,9 @@ CFLAGS += -g -O2 -rdynamic -pthread -Wall -Werror $(GENFLAGS) \
-I$(INCLUDE_DIR) -I$(GENDIR) -I$(LIBDIR) \
-I$(TOOLSINCDIR) -I$(APIDIR)
+ifneq ($(CARGO_OFFLINE),)
+C_OFFLINE := --offline
+endif
CARGOFLAGS := --release --target-dir $(OUTPUT_DIR)
# Silence some warnings when compiled with clang
@@ -199,12 +202,13 @@ scx_userland: scx_userland.c scx_userland.skel.h scx_userland.h $(SCX_COMMON_DEP
$(call ccsched,$<,$@)
scx_rusty_deps: $(SCX_COMMON_DEPS)
- cargo fetch --manifest-path=scx_rusty/Cargo.toml
+ cargo fetch --manifest-path=scx_rusty/Cargo.toml $(C_OFFLINE)
+
scx_rusty: export RUSTFLAGS = -C link-args=-lzstd -C link-args=-lz -C link-args=-lelf -L $(BPFOBJ_DIR)
scx_rusty: export SCX_RUSTY_CLANG = $(CLANG)
scx_rusty: export SCX_RUSTY_BPF_CFLAGS = $(BPF_CFLAGS)
scx_rusty: $(INCLUDE_DIR)/vmlinux.h scx_rusty_deps
- cargo build --manifest-path=$@/Cargo.toml --offline $(CARGOFLAGS)
+ cargo build --manifest-path=$@/Cargo.toml $(CARGOFLAGS) $(C_OFFLINE)
$(Q)cp $(OUTPUT_DIR)/release/$@ $(BINDIR)/$@
install: all
Yeah that should do the trick I think, I'm not sure if the dependency on scx_rusty_deps needs to be removed if we do it like this, but if cargo is sensible it should be fine to keep it. Otherwise if it insists on contacting the network when doing cargo fetch
even if --offline
is passed then we'd need to remove the scx_rusty_deps
dependency (which shouldn't hurt because cargo build
will do the fetching as well.
Ah right. Let's just remove it as a dependency then. That seems simplest and least error prone.
Ah right. Let's just remove it as a dependency then. That seems simplest and least error prone.
Updated https://github.com/sched-ext/sched_ext/pull/62 accordingly
Grab bag of issues I discovered while integrating this into mkosi-kernel:
O=
,KBUILD_OUTPUT=
).cargo
is invoked as part of the build and will download dependencies, meaning the build needs network access. It should be possible to do all things that require network access up-front in a separate stepmake install
instead.