* [PATCH] fix: config support with vms
@ 2022-09-20 11:09 Ville Ilvonen
2022-09-20 11:31 ` Alyssa Ross
0 siblings, 1 reply; 7+ messages in thread
From: Ville Ilvonen @ 2022-09-20 11:09 UTC (permalink / raw)
To: devel; +Cc: 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] fix: config support with vms
2022-09-20 11:09 [PATCH] fix: config support with vms Ville Ilvonen
@ 2022-09-20 11:31 ` Alyssa Ross
2022-09-20 11:34 ` Ville Ilvonen
0 siblings, 1 reply; 7+ messages in thread
From: Alyssa Ross @ 2022-09-20 11:31 UTC (permalink / raw)
To: Ville Ilvonen; +Cc: devel
[-- 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 --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] fix: config support with vms
2022-09-20 11:31 ` Alyssa Ross
@ 2022-09-20 11:34 ` Ville Ilvonen
2022-09-20 11:42 ` Alyssa Ross
0 siblings, 1 reply; 7+ messages in thread
From: Ville Ilvonen @ 2022-09-20 11:34 UTC (permalink / raw)
To: Alyssa Ross; +Cc: devel
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] fix: config support with vms
2022-09-20 11:34 ` Ville Ilvonen
@ 2022-09-20 11:42 ` Alyssa Ross
2022-09-20 11:46 ` Ville Ilvonen
0 siblings, 1 reply; 7+ messages in thread
From: Alyssa Ross @ 2022-09-20 11:42 UTC (permalink / raw)
To: Ville Ilvonen; +Cc: devel
[-- 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 --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] fix: config support with vms
2022-09-20 11:42 ` Alyssa Ross
@ 2022-09-20 11:46 ` Ville Ilvonen
2022-09-20 11:59 ` Alyssa Ross
0 siblings, 1 reply; 7+ messages in thread
From: Ville Ilvonen @ 2022-09-20 11:46 UTC (permalink / raw)
To: Alyssa Ross; +Cc: devel
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] fix: config support with vms
2022-09-20 11:46 ` Ville Ilvonen
@ 2022-09-20 11:59 ` Alyssa Ross
2022-09-20 12:17 ` Ville Ilvonen
0 siblings, 1 reply; 7+ messages in thread
From: Alyssa Ross @ 2022-09-20 11:59 UTC (permalink / raw)
To: Ville Ilvonen; +Cc: devel
[-- 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 --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] fix: config support with vms
2022-09-20 11:59 ` Alyssa Ross
@ 2022-09-20 12:17 ` Ville Ilvonen
0 siblings, 0 replies; 7+ messages in thread
From: Ville Ilvonen @ 2022-09-20 12:17 UTC (permalink / raw)
To: Alyssa Ross; +Cc: devel
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-09-20 12:17 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-20 11:09 [PATCH] fix: config support with vms Ville Ilvonen
2022-09-20 11:31 ` Alyssa Ross
2022-09-20 11:34 ` Ville Ilvonen
2022-09-20 11:42 ` Alyssa Ross
2022-09-20 11:46 ` Ville Ilvonen
2022-09-20 11:59 ` Alyssa Ross
2022-09-20 12:17 ` Ville Ilvonen
Code repositories for project(s) associated with this public inbox
https://spectrum-os.org/git/crosvm
https://spectrum-os.org/git/doc
https://spectrum-os.org/git/mktuntap
https://spectrum-os.org/git/nixpkgs
https://spectrum-os.org/git/spectrum
https://spectrum-os.org/git/ucspi-vsock
https://spectrum-os.org/git/www
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).