[PATCH] fix: config support with vms

[PATCH] fix: config support with vms

[Ville Ilvonen]

with config.nix, the build of vms fails with
error: undefined variable 'pkgs'
..
  49|
  50|   kernel = pkgs.linux_latest.override {
    |            ^

This fixes the issue for all default vms

Signed-off-by: Ville Ilvonen <ville.ilvonen@unikie.com>
---
 vm/app/catgirl/default.nix | 1 +
 vm/app/lynx/default.nix    | 1 +
 vm/sys/net/default.nix     | 1 +
 3 files changed, 3 insertions(+)

diff --git a/vm/app/catgirl/default.nix b/vm/app/catgirl/default.nix
index 61f1462..c33f512 100644
--- a/vm/app/catgirl/default.nix
+++ b/vm/app/catgirl/default.nix
@@ -13,6 +13,7 @@ config.pkgs.pkgsStatic.callPackage (
 }:
 
 let
+  inherit (config) pkgs;
   inherit (lib) cleanSource cleanSourceWith concatMapStringsSep hasSuffix;
 
   packages = [
diff --git a/vm/app/lynx/default.nix b/vm/app/lynx/default.nix
index ba715ec..f06ef1f 100644
--- a/vm/app/lynx/default.nix
+++ b/vm/app/lynx/default.nix
@@ -13,6 +13,7 @@ config.pkgs.pkgsStatic.callPackage (
 }:
 
 let
+  inherit (config) pkgs;
   inherit (lib) cleanSource cleanSourceWith concatMapStringsSep hasSuffix;
 
   packages = [
diff --git a/vm/sys/net/default.nix b/vm/sys/net/default.nix
index dfc7c35..b3dac9a 100644
--- a/vm/sys/net/default.nix
+++ b/vm/sys/net/default.nix
@@ -14,6 +14,7 @@ config.pkgs.pkgsStatic.callPackage (
 }:
 
 let
+  inherit (config) pkgs;
   inherit (lib) cleanSource cleanSourceWith concatMapStringsSep hasSuffix;
 
   connman = connmanMinimal;
-- 
2.37.2

Re: [PATCH] fix: config support with vms

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 1109 bytes --]

Ville Ilvonen <ville.ilvonen@unikie.com> writes:

> with config.nix, the build of vms fails with
> error: undefined variable 'pkgs'
> ..
>   49|
>   50|   kernel = pkgs.linux_latest.override {
>     |            ^
>
> This fixes the issue for all default vms
>
> Signed-off-by: Ville Ilvonen <ville.ilvonen@unikie.com>
> ---

Hi Ville, I was wondering how I'd have missed something like the default
VMs not evaluating, but indeed I'm not able to reproduce this.  And when
I look at the default.nix files for the default VMs, I see:

50	  kernel = buildPackages.linux.override {

So unless I've missed something, I don't think this issue exists in the
current upstream.  (I checked commit 3aa2f8a.)

Although this does make me wonder /why/ buildPackages is used here —
I think that would be incorrect in the case of cross compilation.
Possibly it was just a hack I used to work around some pkgsStatic issue
with the kernel at the time, and then forgot to change back.  The right
thing to do here would be for linux_latest to be an argument to the
function passed to callPackage.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] fix: config support with vms

[Ville Ilvonen]

On 9/20/22 14:31, Alyssa Ross wrote:
> Ville Ilvonen <ville.ilvonen@unikie.com> writes:
> 
>> with config.nix, the build of vms fails with
>> error: undefined variable 'pkgs'
>> ..
>>    49|
>>    50|   kernel = pkgs.linux_latest.override {
>>      |            ^
>>
>> This fixes the issue for all default vms
>>
>> Signed-off-by: Ville Ilvonen <ville.ilvonen@unikie.com>
>> ---
> 
> Hi Ville, I was wondering how I'd have missed something like the default
> VMs not evaluating, but indeed I'm not able to reproduce this.  And when
> I look at the default.nix files for the default VMs, I see:
> 
> 50	  kernel = buildPackages.linux.override {
> 
> So unless I've missed something, I don't think this issue exists in the
> current upstream.  (I checked commit 3aa2f8a.)

I'm not testing with default but with new config.nix support.
Have you tested with any config.nix?

-Ville

> Although this does make me wonder /why/ buildPackages is used here —
> I think that would be incorrect in the case of cross compilation.
> Possibly it was just a hack I used to work around some pkgsStatic issue
> with the kernel at the time, and then forgot to change back.  The right
> thing to do here would be for linux_latest to be an argument to the
> function passed to callPackage.

Re: [PATCH] fix: config support with vms

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 1369 bytes --]

Ville Ilvonen <ville.ilvonen@unikie.com> writes:

> On 9/20/22 14:31, Alyssa Ross wrote:
>> Ville Ilvonen <ville.ilvonen@unikie.com> writes:
>> 
>>> with config.nix, the build of vms fails with
>>> error: undefined variable 'pkgs'
>>> ..
>>>    49|
>>>    50|   kernel = pkgs.linux_latest.override {
>>>      |            ^
>>>
>>> This fixes the issue for all default vms
>>>
>>> Signed-off-by: Ville Ilvonen <ville.ilvonen@unikie.com>
>>> ---
>> 
>> Hi Ville, I was wondering how I'd have missed something like the default
>> VMs not evaluating, but indeed I'm not able to reproduce this.  And when
>> I look at the default.nix files for the default VMs, I see:
>> 
>> 50	  kernel = buildPackages.linux.override {
>> 
>> So unless I've missed something, I don't think this issue exists in the
>> current upstream.  (I checked commit 3aa2f8a.)
>
> I'm not testing with default but with new config.nix support.
> Have you tested with any config.nix?

I'm not sure I understand what you mean.  config.nix support is already
present in the commit I tested.

I created a simple config.nix file that just set "pkgs" to "import <nixpkgs> {}"
in case the presence of the config file made a difference, and ran:

	nix-instantiate vm/*/* img/live

To test evaluating VMs both directly and as part of the whole system,
and it worked fine.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] fix: config support with vms

[Ville Ilvonen]

On 9/20/22 14:42, Alyssa Ross wrote:
> Ville Ilvonen <ville.ilvonen@unikie.com> writes:
> 
>> On 9/20/22 14:31, Alyssa Ross wrote:
>>> Ville Ilvonen <ville.ilvonen@unikie.com> writes:
>>>
>>>> with config.nix, the build of vms fails with
>>>> error: undefined variable 'pkgs'
>>>> ..
>>>>     49|
>>>>     50|   kernel = pkgs.linux_latest.override {
>>>>       |            ^
>>>>
>>>> This fixes the issue for all default vms
>>>>
>>>> Signed-off-by: Ville Ilvonen <ville.ilvonen@unikie.com>
>>>> ---
>>>
>>> Hi Ville, I was wondering how I'd have missed something like the default
>>> VMs not evaluating, but indeed I'm not able to reproduce this.  And when
>>> I look at the default.nix files for the default VMs, I see:
>>>
>>> 50	  kernel = buildPackages.linux.override {
>>>
>>> So unless I've missed something, I don't think this issue exists in the
>>> current upstream.  (I checked commit 3aa2f8a.)
>>
>> I'm not testing with default but with new config.nix support.
>> Have you tested with any config.nix?
> 
> I'm not sure I understand what you mean.  config.nix support is already
> present in the commit I tested.
> 
> I created a simple config.nix file that just set "pkgs" to "import <nixpkgs> {}"
> in case the presence of the config file made a difference, and ran:
> 
> 	nix-instantiate vm/*/* img/live

Interesting. I created a simple config.nix in the root of Spectrum 
source tree - https://spectrum-os.org/doc/build-configuration.html
It only overrides the kernel - to imx8 - and sets crossSystem for 
aarch64. Build of vms fails without this patch.

-Ville

> To test evaluating VMs both directly and as part of the whole system,
> and it worked fine.

Re: [PATCH] fix: config support with vms

[Alyssa Ross]

[-- Attachment #1: Type: text/plain, Size: 2077 bytes --]

Ville Ilvonen <ville.ilvonen@unikie.com> writes:

> On 9/20/22 14:42, Alyssa Ross wrote:
>> Ville Ilvonen <ville.ilvonen@unikie.com> writes:
>> 
>>> On 9/20/22 14:31, Alyssa Ross wrote:
>>>> Ville Ilvonen <ville.ilvonen@unikie.com> writes:
>>>>
>>>>> with config.nix, the build of vms fails with
>>>>> error: undefined variable 'pkgs'
>>>>> ..
>>>>>     49|
>>>>>     50|   kernel = pkgs.linux_latest.override {
>>>>>       |            ^
>>>>>
>>>>> This fixes the issue for all default vms
>>>>>
>>>>> Signed-off-by: Ville Ilvonen <ville.ilvonen@unikie.com>
>>>>> ---
>>>>
>>>> Hi Ville, I was wondering how I'd have missed something like the default
>>>> VMs not evaluating, but indeed I'm not able to reproduce this.  And when
>>>> I look at the default.nix files for the default VMs, I see:
>>>>
>>>> 50	  kernel = buildPackages.linux.override {
>>>>
>>>> So unless I've missed something, I don't think this issue exists in the
>>>> current upstream.  (I checked commit 3aa2f8a.)
>>>
>>> I'm not testing with default but with new config.nix support.
>>> Have you tested with any config.nix?
>> 
>> I'm not sure I understand what you mean.  config.nix support is already
>> present in the commit I tested.
>> 
>> I created a simple config.nix file that just set "pkgs" to "import <nixpkgs> {}"
>> in case the presence of the config file made a difference, and ran:
>> 
>> 	nix-instantiate vm/*/* img/live
>
> Interesting. I created a simple config.nix in the root of Spectrum 
> source tree - https://spectrum-os.org/doc/build-configuration.html
> It only overrides the kernel - to imx8 - and sets crossSystem for 
> aarch64. Build of vms fails without this patch.

Are you testing with an upstream Spectrum commit, or with one from the
TII branch with cross compilation support?  It's likely that making
cross compilation work would have required changing line 50 you posted
above, because as I explained I don't think it's correct.  So if you're
using that branch, that could explain why you're seeing this and I'm
not.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH] fix: config support with vms

[Ville Ilvonen]

On 9/20/22 14:59, Alyssa Ross wrote:
> Ville Ilvonen <ville.ilvonen@unikie.com> writes:
> 
>> On 9/20/22 14:42, Alyssa Ross wrote:
>>> Ville Ilvonen <ville.ilvonen@unikie.com> writes:
>>>
>>>> On 9/20/22 14:31, Alyssa Ross wrote:
>>>>> Ville Ilvonen <ville.ilvonen@unikie.com> writes:
>>>>>
>>>>>> with config.nix, the build of vms fails with
>>>>>> error: undefined variable 'pkgs'
>>>>>> ..
>>>>>>      49|
>>>>>>      50|   kernel = pkgs.linux_latest.override {
>>>>>>        |            ^
>>>>>>
>>>>>> This fixes the issue for all default vms
>>>>>>
>>>>>> Signed-off-by: Ville Ilvonen <ville.ilvonen@unikie.com>
>>>>>> ---
>>>>>
>>>>> Hi Ville, I was wondering how I'd have missed something like the default
>>>>> VMs not evaluating, but indeed I'm not able to reproduce this.  And when
>>>>> I look at the default.nix files for the default VMs, I see:
>>>>>
>>>>> 50	  kernel = buildPackages.linux.override {
>>>>>
>>>>> So unless I've missed something, I don't think this issue exists in the
>>>>> current upstream.  (I checked commit 3aa2f8a.)
>>>>
>>>> I'm not testing with default but with new config.nix support.
>>>> Have you tested with any config.nix?
>>>
>>> I'm not sure I understand what you mean.  config.nix support is already
>>> present in the commit I tested.
>>>
>>> I created a simple config.nix file that just set "pkgs" to "import <nixpkgs> {}"
>>> in case the presence of the config file made a difference, and ran:
>>>
>>> 	nix-instantiate vm/*/* img/live
>>
>> Interesting. I created a simple config.nix in the root of Spectrum
>> source tree - https://spectrum-os.org/doc/build-configuration.html
>> It only overrides the kernel - to imx8 - and sets crossSystem for
>> aarch64. Build of vms fails without this patch.
> 
> Are you testing with an upstream Spectrum commit, or with one from the

Testing with an upstream Spectrum commit - using a branch rebased with 
spectrum/main and from which I dropped the cross-compilation 
configuration hacks and moved them to config.nix. It has got 3aa2f8a
(which is a copyright update).

> TII branch with cross compilation support?  It's likely that making
> cross compilation work would have required changing line 50 you posted
> above, because as I explained I don't think it's correct.  So if you're
> using that branch, that could explain why you're seeing this and I'm
> not.