[gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

[gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

["Paweł Hajdan, Jr."]

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

check_extra_config requires a configured kernel
(/usr/src/linux/.config), while I think it should also be satisfied by
/proc/config.gz (i.e. just a way to verify the config, not necessarily
kernel built locally).

An example use case is www-client/chromium, which makes sure the kernel
will support its sandbox. It's generally a bad idea to run without full
sandboxing support (it can work without kernel support, just doesn't
prevent a compromised renderer from connecting to network or sending
signals to processes).

My suggestion is to replace the following code fregment:

if [[ ${config_required} == 0 ]]; then
    # In the case where we don't require a .config, we can now bail out
    # if the user has no .config as there is nothing to do. Otherwise
    # code later will cause a failure due to missing .config.
    if ! linux_config_exists; then
        ewarn "Unable to check for the following kernel config options due"
        ewarn "to absence of any configured kernel sources or compiled"
        ewarn "config:"
        for config in ${CONFIG_CHECK}; do
            local_error="ERROR_${config#\~}"
            msg="${!local_error}"
            if [[ "x${msg}" == "x" ]]; then
                local_error="WARNING_${config#\~}"
                msg="${!local_error}"
            fi
            ewarn " - ${config#\~}${msg:+ - }${msg}"
        done
        ewarn "You're on your own to make sure they are set if needed."
        export LINUX_CONFIG_EXISTS_DONE="${old_LINUX_CONFIG_EXISTS_DONE}"
        return 0
    fi
else
    require_configured_kernel
fi

With something more like this:

# In the case where we don't require a .config, we can now bail out
# if the user has no .config as there is nothing to do. Otherwise
# code later will cause a failure due to missing .config.
if ! linux_config_exists; then
    ewarn "Unable to check for the following kernel config options due"
    ewarn "to absence of any configured kernel sources or compiled"
    ewarn "config:"
    for config in ${CONFIG_CHECK}; do
        local_error="ERROR_${config#\~}"
        msg="${!local_error}"
        if [[ "x${msg}" == "x" ]]; then
            local_error="WARNING_${config#\~}"
            msg="${!local_error}"
        fi
        ewarn " - ${config#\~}${msg:+ - }${msg}"
    done
    ewarn "You're on your own to make sure they are set if needed."
    export LINUX_CONFIG_EXISTS_DONE="${old_LINUX_CONFIG_EXISTS_DONE}"
    if [[ ${config_required} == 0 ]]; then
        return 0
    else
        die "unable to check for required kernel options"
    fi
fi

Thoughts?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 203 bytes --]

Re: [gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

[Fabio Erculiani]

Anything using /proc/config.gz is broken.

For the following reasons:
1) could be not available (CONFIG not enabled)
2) doesn't reflect the kernel you're compiling against (chrooted env,
multiple kernels on the system, etc)

-- 
Fabio Erculiani

Re: [gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

[Patrick Lauer]

On 11/04/11 13:59, "Paweł Hajdan, Jr." wrote:
> check_extra_config requires a configured kernel
> (/usr/src/linux/.config), while I think it should also be satisfied by
> /proc/config.gz (i.e. just a way to verify the config, not necessarily
> kernel built locally).
The running kernel is really irrelevant for those of us that build binpkgs.
/usr/src/linux is "more correct" in the case of binpkgs and most upgrade 
scenarios where you don't reboot for a new kernel immediately.

It's hard to find a solution that is correct and works for everyone - at 
least we've mostly got people to stop dying and only warn on a mismatch 
there.

Re: [gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

[Nirbheek Chauhan]

On Fri, Nov 4, 2011 at 6:53 PM, Patrick Lauer <patrick@gentoo.org> wrote:
> The running kernel is really irrelevant for those of us that build binpkgs.
> /usr/src/linux is "more correct" in the case of binpkgs and most upgrade
> scenarios where you don't reboot for a new kernel immediately.
>

Also, for out-of-kernel modules that need the kernel source for
building, the build-time .config is much more relevant than the
runtime config.

-- 
~Nirbheek Chauhan

Gentoo GNOME+Mozilla Team

Re: [gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

[Mike Gilbert]

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

On 11/4/2011 8:59 AM, "Paweł Hajdan, Jr." wrote:
> Thoughts?
> 

Shouldn't chromium be dealing with this at run-time where it actually
matters? This would be better than doing it in the ebuild where we can
only do it at build and/or install-time.

It is good that we warn users about this when they install the package,
but I don't think the ebuild should die.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

[Fabio Erculiani]

On Fri, Nov 4, 2011 at 3:46 PM, Mike Gilbert <floppym@gentoo.org> wrote:
>
> It is good that we warn users about this when they install the package,
> but I don't think the ebuild should die.

I've always found ebuilds dying at kernel config checks really annoying.
Checking kernel features at build time (if we die) is broken and
should be banned IMO:

1) in case of packages not needing kernel headers for building (but
just checking for a feature)
a) this breaks the compilation inside chroots on build servers where
the goal is to just build the package (hint: binpkgs)
b) it is anyway unable to reliably check if the feature is actually
available or not (/proc/config.gz might be unavailable, as well as
kernel sources or headers)

2) in case of packages needing kernel headers (or sources) for building
a) doing the check in pkg_setup() breaks binpkgs support (the check
becomes part of a build time task, and not install time)

I'm sure there are other reasons as well...

-- 
Fabio Erculiani
http://lxnay.com

Re: [gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

[Ian Stakenvicius]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On 04/11/11 11:11 AM, Fabio Erculiani wrote:
> On Fri, Nov 4, 2011 at 3:46 PM, Mike Gilbert <floppym@gentoo.org> wrote:
>>
>> It is good that we warn users about this when they install the package,
>> but I don't think the ebuild should die.
> 
> I've always found ebuilds dying at kernel config checks really annoying.
> Checking kernel features at build time (if we die) is broken and
> should be banned IMO:
> 
> 1) in case of packages not needing kernel headers for building (but
> just checking for a feature)
> a) this breaks the compilation inside chroots on build servers where
> the goal is to just build the package (hint: binpkgs)
> b) it is anyway unable to reliably check if the feature is actually
> available or not (/proc/config.gz might be unavailable, as well as
> kernel sources or headers)

As i think has already been mentioned, when building binpkgs or building
for other systems, since the idea here is to ensure build-time
requirements are met, things should be OK as long as you have a
configured kernel for the target in place as well.  And you probably
should; if you're building packages for target systems you might as well
configure and/or build their kernels too.

Also as was mentioned, /proc/config.gz would never be appropriate in
these cases since the running kernel probably has nothing to do with the
target system's kernel.

> 
> 2) in case of packages needing kernel headers (or sources) for building
> a) doing the check in pkg_setup() breaks binpkgs support (the check
> becomes part of a build time task, and not install time)

.. so you're recommending the check be moved to pkg_preinst() ?  Or
perhaps duplicated into both pkg_setup/pkg_pretend and pkg_preinst ?
That would be doable, wouldn't it?

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.17 (GNU/Linux)

iF4EAREIAAYFAk60BMQACgkQAJxUfCtlWe17qwEAhh2udNzptIJZHeEyh8DJhTj3
HLbC/ZpIJUQTbpHueOEBAITcA3DasGAQIdSWGvn7E2xwYMFTS+U0UyDWJCesv5xR
=U+gT
-----END PGP SIGNATURE-----

Re: [gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

[Fabio Erculiani]

pkg_setup() is shared between binpkgs and srcpkgs, and often it ends
up containing stuff that should be rather placed into
src_{prepare,configure,whatever}.

-- 
Fabio Erculiani
http://lxnay.com

Re: [gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

[Robin H. Johnson]

On Fri, Nov 04, 2011 at 01:59:14PM +0100, "Paweł Hajdan, Jr." wrote:
> check_extra_config requires a configured kernel
> (/usr/src/linux/.config), while I think it should also be satisfied by
> /proc/config.gz (i.e. just a way to verify the config, not necessarily
> kernel built locally).
> 
> An example use case is www-client/chromium, which makes sure the kernel
> will support its sandbox. It's generally a bad idea to run without full
> sandboxing support (it can work without kernel support, just doesn't
> prevent a compromised renderer from connecting to network or sending
> signals to processes).
chromium is using CONFIG_CHECK wrongly.
It should be CONFIG_CHECK="~PID_NS ~NET_NS"

- Does the package _build_ kernel modules?
  Yes: .config is required. 
  No: .config is NOT required. All options must have '~' prefix.
- Does the package use the options specified _during_ src_configure,
  src_compile, src_test process?
  (eg there's a testsuite that uses namespaces).
  Yes: .config/config.gz may not be available, devise an actual test
  that tries to use the option.
  No: Just keep the ~ prefix on the option.

> # In the case where we don't require a .config, we can now bail out
> # if the user has no .config as there is nothing to do. Otherwise
> # code later will cause a failure due to missing .config.
> if ! linux_config_exists; then
[snip]
> fi
This will cause out-of-kernel module building to fail much later instead
of correctly failing early when there is no .config.

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

Re: [gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

[Robin H. Johnson]

On Fri, Nov 04, 2011 at 04:11:42PM +0100, Fabio Erculiani wrote:
> On Fri, Nov 4, 2011 at 3:46 PM, Mike Gilbert <floppym@gentoo.org> wrote:
> >
> > It is good that we warn users about this when they install the package,
> > but I don't think the ebuild should die.
> 
> I've always found ebuilds dying at kernel config checks really annoying.
> Checking kernel features at build time (if we die) is broken and
> should be banned IMO:
You're going off on a tangent.

The ONLY time that kernel config checks are fatal is when you're
building kernel modules, and the module will fail to compile unless
there is a .config and suitable options set.

chromium is using the fatal mode of CONFIG_CHECK wrongly.

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

Re: [gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

[Fabio Erculiani]

On Fri, Nov 4, 2011 at 10:18 PM, Robin H. Johnson <robbat2@gentoo.org> wrote:
> On Fri, Nov 04, 2011 at 04:11:42PM +0100, Fabio Erculiani wrote:
>> On Fri, Nov 4, 2011 at 3:46 PM, Mike Gilbert <floppym@gentoo.org> wrote:
>> >
>> > It is good that we warn users about this when they install the package,
>> > but I don't think the ebuild should die.
>>
>> I've always found ebuilds dying at kernel config checks really annoying.
>> Checking kernel features at build time (if we die) is broken and
>> should be banned IMO:
> You're going off on a tangent.
>
> The ONLY time that kernel config checks are fatal is when you're
> building kernel modules, and the module will fail to compile unless
> there is a .config and suitable options set.

And that is bad anyway, because it doesn't work as expected when the
package is merged from tbz2, there are no kernel sources installed and
multiple kernels are on the same system (and perhaps you are using a
package manager that properly supports multiple installed kernel
module packages).

>
> chromium is using the fatal mode of CONFIG_CHECK wrongly.
>
> --
> Robin Hugh Johnson
> Gentoo Linux: Developer, Trustee & Infrastructure Lead
> E-Mail     : robbat2@gentoo.org
> GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85
>
>



-- 
Fabio Erculiani
http://lxnay.com

Re: [gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

[Robin H. Johnson]

On Fri, Nov 04, 2011 at 11:02:18PM +0100, Fabio Erculiani wrote:
> > The ONLY time that kernel config checks are fatal is when you're
> > building kernel modules, and the module will fail to compile unless
> > there is a .config and suitable options set.
> And that is bad anyway, because it doesn't work as expected when the
> package is merged from tbz2, there are no kernel sources installed and
> multiple kernels are on the same system (and perhaps you are using a
> package manager that properly supports multiple installed kernel
> module packages).
I said when you're building. When you're merging from binpkg, you're not
building...

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85

Re: [gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

[Fabio Erculiani]

On Fri, Nov 4, 2011 at 11:12 PM, Robin H. Johnson <robbat2@gentoo.org> wrote:
> On Fri, Nov 04, 2011 at 11:02:18PM +0100, Fabio Erculiani wrote:
>> > The ONLY time that kernel config checks are fatal is when you're
>> > building kernel modules, and the module will fail to compile unless
>> > there is a .config and suitable options set.
>> And that is bad anyway, because it doesn't work as expected when the
>> package is merged from tbz2, there are no kernel sources installed and
>> multiple kernels are on the same system (and perhaps you are using a
>> package manager that properly supports multiple installed kernel
>> module packages).
> I said when you're building. When you're merging from binpkg, you're not
> building...

Correct :-)

>
> --
> Robin Hugh Johnson
> Gentoo Linux: Developer, Trustee & Infrastructure Lead
> E-Mail     : robbat2@gentoo.org
> GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85
>
>



-- 
Fabio Erculiani
http://lxnay.com

Re: [gentoo-dev] linux-info.eclass: check_extra_config requires a configured kernel

[Robin H. Johnson]

On Fri, Nov 04, 2011 at 09:16:13PM +0000, Robin H. Johnson wrote:
> - Does the package _build_ kernel modules?
>   Yes: .config is required. 
>   No: .config is NOT required. All options must have '~' prefix.
> - Does the package use the options specified _during_ src_configure,
>   src_compile, src_test process?
>   (eg there's a testsuite that uses namespaces).
>   Yes: .config/config.gz may not be available, devise an actual test
>   that tries to use the option.
>   No: Just keep the ~ prefix on the option.
I've just checked _every_ ebuild in the tree that:
(used CONFIG_CHECK with non-~ arguments)
AND
(did not inherit linux-mod).
I've fixed almost all of them.

Testcase was building on a system with no /usr/src, no /proc/config*.

I fixed one or more ebuilds in each of the following packages:
app-emulation/spice-vdagent 
gnome-extra/nautilus-dropbox 
net-misc/iodine 
net-wireless/madwimax 
sys-block/open-iscsi 

I did NOT fix:
www-client/chromium-16.0.912.15 (failed to build for unrelated reasons).

TODO:
- Look for ebuilds that call the check functions manually.
- Check all ebuilds inheriting linux-mod, with USE=-modules.

-- 
Robin Hugh Johnson
Gentoo Linux: Developer, Trustee & Infrastructure Lead
E-Mail     : robbat2@gentoo.org
GnuPG FP   : 11AC BA4F 4778 E3F6 E4ED  F38E B27B 944E 3488 4E85