astro / nix-openwrt-imagebuilder

Build OpenWRT images in Nix derivations
MIT License
116 stars 15 forks source link

How to install custom packages? #38

Open jfly opened 4 months ago

jfly commented 4 months ago

Is it possible to add custom packages (as documented by OpenWrt here)?

I played around with this for a while, and wasn't able to figure out a way to accomplish this without patching astro/nix-openwrt-imagebuilder. Basically, the only thing passed to make image PACKAGES=... is the packages parameter to the builder, and it appears to be assumed that those packages are the ones available on https://downloads.openwrt.org/.

I'd like to be able to point at either a specific ipk file, or a full package repository.

Happy to send in a PR implementing this if we can agree upon an approach.

astro commented 4 months ago

Ideally, we could just throw paths into packages but it doesn't work for me as-is:

       error: the string '/nix/store/76mxhprmm4hy537x6w9bgilhb56472fw-6rd_9-4_all.ipk' is not allowed to refer to a store path (such as '76mxhprmm4hy537x6w9bgilhb56472fw-6rd_9-4_all.ipk')
jfly commented 4 months ago

From code inspection, I can't immediately tell where that's falling apart. It feels kind of weird to me to use the existing packages argument for this as right now it's just a list of package names (strings possibly prefixed with a -), the sort of thing you could pass to make image PACKAGES=.... If we wanted to support paths as well, wouldn't we need to do some clever parsing of the filename to deduce the human readable package name so we could pass it into make image PACKAGES=...? Or perhaps I don't quite understand what the underlying openwrt imagebuilder Makefile supports?

pedorich-n commented 1 month ago

Ideally, we could just throw paths into packages but it doesn't work for me as-is:

       error: the string '/nix/store/76mxhprmm4hy537x6w9bgilhb56472fw-6rd_9-4_all.ipk' is not allowed to refer to a store path (such as '76mxhprmm4hy537x6w9bgilhb56472fw-6rd_9-4_all.ipk')

It won't work with packages, because packages is a list of package names (strings), like @jfly mentioned. It could be implemented with a new attr, something like customPackages which will be a list of paths. And then the configurePhase would need to be modified to symlink ipk files from these paths into packages.

I was able to achieve it with the following override:

(openwrt-imagebuilder-lib.build config).overrideAttrs (final: previous: {
  passAsFile = (previous.passAsFile or [ ]) ++ [ "customPackages" ];
  customPackages = [ ... ];
  configurePhase = ''
    for pkg in $(cat $customPackagesPath); do
      for ipk in $pkg/*.ipk; do
        ln -s $ipk packages/$(stripHash $ipk)
      done
    done 

    ${previous.configurePhase or ""}
  '';
})

It's not perfect, as it assumes customPackages is a list of folders, but it should be easy to make it work with both files and folders.

Also: the dependencies for these custom packages have to be provided manually in packages.

jfly commented 1 month ago

@pedorich-n, I'm not totally understanding the code you shared. Don't you need to somehow invoke openwrt's underlying makefile with additional strings specified in the PACKAGES argument (as I've done here: https://github.com/jfly/nix-openwrt-imagebuilder/commit/53f1a8a1287ec024eb2aa1325b38ab60e701b02d#diff-cd5a1dc9ff54d6ea98c183cce514f03e0582473dfe2f71811709c6fa30728b40R111)?

pedorich-n commented 1 month ago

@pedorich-n, I'm not totally understanding the code you shared. Don't you need to somehow invoke openwrt's underlying makefile with additional strings specified in the PACKAGES argument (as I've done here: jfly@53f1a8a#diff-cd5a1dc9ff54d6ea98c183cce514f03e0582473dfe2f71811709c6fa30728b40R111)?

You are correct. Adding anipk to the packages just makes it available to the builder. I must've installed the package manually and forgot about it, thinking the builder added it 🤦‍♂️

jfly commented 1 month ago

@pedorich-n thanks for your recent changes! It looks like I can finally drop my awful fork to accomplish this =)

pedorich-n commented 1 month ago

It's not ideal, you'll have to redefine the env variables like profile, disabled services, and packages, but at least you can do it now without a fork, with just an overrideAttrs.

astro commented 1 month ago

Could we document these new capabilities with an example?