Out of idle curiosity, is there a reason more of the modules_closure.sh logic wasn't lifted into rust?

Out of idle curiosity, is there a reason more of the modules_closure.sh logic wasn't lifted into rust?

No reason in particular, I might do more actually.
I wanted to get something out there, to get some feedback first.

edit: I remember now. One piece of code that can not be easily ported to rust is nuke-ref..

Before I review this, it puts rustc into the kernel build closure, right? Do we already depend on rustc for that? Do we want to? I’d say it’s okay, but other people might have a different opinion.

Before I review this, it puts rustc into the kernel build closure, right? Do we already depend on rustc for that? Do we want to? I’d say it’s okay, but other people might have a different opinion.

I think it's not in the closure currently. But it should not be needed for compiling the kernel even with this. The piece of code here should only be needed when assembling an initrd. Maybe the rust program should go in all-packages, so its built by hydra? Then most people will never see rustc anyway, except for those that build everything from scratch.

By the way I just started to learn rust, so if you should review this I am very much interested in opinion on style etc. as well!

The use right here is probably fine, it's strictly a "native build input" use case, so no cross-compilation will be required for this feature.

(The following does not really apply for this instance... But this is causing a precedent, so I will say it.)

With that said, adding Rust dependencies to the basic build environment without nix-build -A pkgsCross.aarch64-multiplatform.tests.writers.bin.rust building successfully is, in my opinion, a bad idea.

Adding Rust dependencies as things are currently would be a regression against cross-compiling NixOS, which @flokli had recently managed to manage without external workarounds for a few generations. (Before a regression in an upstream dep caused new breakage.)

The use right here is probably fine, it's strictly a "native build input" use case, so no cross-compilation will be required for this feature.

(The following does not really apply for this instance... But this is causing a precedent, so I will say it.)

With that said, adding Rust dependencies to the basic build environment without nix-build -A pkgsCross.aarch64-multiplatform.tests.writers.bin.rust building successfully is, in my opinion, a bad idea.

Adding Rust dependencies as things are currently would be a regression against cross-compiling NixOS, which @flokli had recently managed to manage without external workarounds for a few generations. (Before a regression in an upstream dep caused new breakage.)

I think our rust infrastructure should be cross-compile ready. I don't know about writers but a fix should not be that complicated. Otherwise just switching to buildRustPackage would be the easy fix.

I think our rust infrastructure should be cross-compile ready. I don't know about writers but a fix should not be that complicated. Otherwise just switching to buildRustPackage would be the easy fix.

Hm, I guess I had a wrong commit up when I last tried, yesterday, as it failed to build relatively early. Trying fresh, and the writeRust writer currently doesn't work, but manix cross-compiled fine. So there's some work to do yet, but nothing too concerning as our Rust ecosystem itself proves to be able to manage.

We don't yet have a job in Hydra to look at to guesstimate whether Rust cross-compilation through pkgsCross is consistently working well or not.

Note that the concern is still present: we shouldn't add dependencies (any!) to the base system unless cross-compilation for them is figured out.

Sorry to for the noise right here.

I think our rust infrastructure should be cross-compile ready. I don't know about writers but a fix should not be that complicated. Otherwise just switching to buildRustPackage would be the easy fix.

Hm, I guess I had a wrong commit up when I last tried, yesterday, as it failed to build relatively early. Trying fresh, and the writeRust writer currently doesn't work, but manix cross-compiled fine. So there's some work to do yet, but nothing too concerning as our Rust ecosystem itself proves to be able to manage.

We don't yet have a job in Hydra to look at to guesstimate whether Rust cross-compilation through pkgsCross is consistently working well or not.

Note that the concern is still present: we shouldn't add dependencies (any!) to the base system unless cross-compilation for them is figured out.

Sorry to for the noise right here.

Yeah, this is up to people interested in cross-compilation and not in the scope of this PR. Right now it is too much of a burden to ask contributors to test cross-compilation like this one if it the majority of dependencies to cross-compile NixOS are not cached by hydra.

Yeah, this is up to people interested in cross-compilation and not in the scope of this PR.

Hm...

Right now it is too much of a burden to ask contributors to test cross-compilation like this one if it the majority of dependencies to cross-compile NixOS are not cached by hydra.

I would agree on a simple change, but this is about introducing a totally new ecosystem as a basic build dependency to NixOS. This is a design decision with broad repercussions.

Yes it is up to the people interested in cross-compilation, and here I am, stating my concerns.

I agree that it is too much of a burden right now to test it. I'm not asking that the change works as it is with cross-compilation. I'm asking that, as a distribution, we properly consider whether it is causing more breakage than it is worth.

In this instance adding Rust is okay, it looks like it does cross-compile with pkgsCross. Sure, writers fail, but at the very least the ecosystem is not completely in a bad state. If it had been in a bad state like it was during last year (not working), then it would have been a different thing.

I will state it again, in case people only skim my message: I am not against the change. I am bringing up concerns that any decisions to greatly affect the build closure of the base system should keep in mind that cross-compilation should be in a good enough state. It's fine if it doesn't work at the moment, things are still in flux, but they have to be in a place where it is known that cross-compilation was working fine recently.

[...] it is up to people interested in cross-compilation

Repeating this from your message again. What is the solution, if not to raise concerns? Should this get merged-in, then someone interested in cross-compilation comes behind, reverts?

Repeating this from your message again. What is the solution, if not to raise concerns? Should this get merged-in, then someone interested in cross-compilation comes behind, reverts?

I was talking about adding rust to pkgsCross not beeing in scope of this PR. Not the other stuff.

wrt. cross compiling, am I correct that putting the rust helper in a proper package and using buildRustPackage will fix the issue (then I will just do that)?

Or use packages from buildPackages in nativeBuildInputs. Though exposing the tool as a package isn't a bad idea either.

I'm playing around with some code changes and fixing some clippy warnings etc.
What's the best way to test this? 🙂

I'm playing around with some code changes and fixing some clippy warnings etc. What's the best way to test this? slightly_smiling_face

You want to build the config.system.build.bootStage1.modulesClosure attribute of a nixos configuration (with nixpkgs pointed to a checkout of this pr).
I then compare this with a build of the same attribute from the master branch, ideally there should be no differences apart from impurities introduced by the old approach.

concerning documentation, should I add a README in pkgs/build-support/kernel/copy-modules-closure?

There already is a short description in pkgs/build-support/kernel/modules-closure.nix. The split between the two is a bit weird, should I make the rust program call depmod as well, or is it good to separate the two steps?