diff mbox

[GIT] kbuild fixes for 3.0

Message ID BANLkTinKMmYG7=KBHr2LfOYzcgbtszwy5g@mail.gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Arnaud Lacombe June 10, 2011, 3:45 a.m. UTC
Hi,

On Thu, Jun 9, 2011 at 11:37 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Jun 9, 2011 at 8:14 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
>>
>> any chance to share your .config ? x86-64's defconfig, plus Michal
>> branch merged on top of the tip of your tree, plus the following patch
>> removing the SUBLEVEL:
>>
>> diff --git a/Makefile b/Makefile
>> index 72c0e32..5c75864 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -1,6 +1,6 @@
>>  VERSION = 3
>>  PATCHLEVEL = 0
>> -SUBLEVEL = 0
>> +SUBLEVEL =
>>  EXTRAVERSION = -rc2
>>  NAME = Sneaky Weasel
>>
>> install is fine for me. What bugs me is that "git grep '\.temp'" in
>> that tree does not return anything relevant.
>
> "make install" just runs the distro install script, usually
> /sbin/installkernel, which at least on F-14 will then run
> /sbin/new-kernel-pkg /sbin/dracut to build the initrd etc.
>
> And that runs "depmod", which seems to end up being confused: we give
> it the new kernel version as an argument, but it seems to not like it,
> so it decides to do "uname()" to get the _current_ kernel version
> instead, and that is where the confusion comes from.
>
For the record, this should have been fixed by Michal in:

commit 3328d178247017affd90b7897393699f2f45227d
Author: Michal Marek <mmarek@suse.cz>
Date:   Mon May 30 15:58:43 2011 +0200

    depmod: Handle X.Y kernel versions

    What a stupid check.

    Signed-off-by: Michal Marek <mmarek@suse.cz>
    Signed-off-by: Jon Masters <jcm@jonmasters.org>

from the module-init-tools git tree. Change is pretty trivial:


> If it works for you, I suspect you're running a different distribution.
>
>                  Linus
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Linus Torvalds June 10, 2011, 3:57 a.m. UTC | #1
On Thu, Jun 9, 2011 at 8:45 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
>
> For the record, this should have been fixed by Michal in:

You're missing the problem: we can't assume that people have updated user land.

Furthermore, the very pull request I'm responding to actually tries to
handle this, see commit bfe5424a8b31 ("kbuild: Hack for depmod not
handling X.Y versions"), but it's just not working for me.

And it's not working, because it only handles the "modules_install"
case, not the case where the system install scripts do their own
depmod.

                       Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnaud Lacombe June 10, 2011, 5:16 a.m. UTC | #2
Hi,

On Thu, Jun 9, 2011 at 11:57 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Jun 9, 2011 at 8:45 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
>>
>> For the record, this should have been fixed by Michal in:
>
> You're missing the problem: we can't assume that people have updated user land.
>
I absolutely agree with you. But I am not the one about to decide to
break a 15 years old interface :) [I gave me a 5 year delta as I have
no idea how the version number was at that time; moreover, do not get
me wrong, I do not really care about the kernel version being 2 or 3
digits.].

> Furthermore, the very pull request I'm responding to actually tries to
> handle this, see commit bfe5424a8b31 ("kbuild: Hack for depmod not
> handling X.Y versions"), but it's just not working for me.
>
> And it's not working, because it only handles the "modules_install"
> case, not the case where the system install scripts do their own
> depmod.
>
I totally agree! But, it is a technical challenge to give a 2 digit
version number to an application expecting a 3 digit version number
without much control over its environment.

Beside that, no matter what, you are about to break
`/usr/sbin/sensors-detect' (from my Fedora 14), which rely on a 3
digits version number:

# [0] -> VERSION
# [1] -> PATCHLEVEL
# [2] -> SUBLEVEL
# [3] -> EXTRAVERSION
#
use vars qw(@kernel_version $kernel_arch);

sub initialize_kernel_version
{
        `uname -r` =~ /(\d+)\.(\d+)\.(\d+)(.*)/;
        @kernel_version = ($1, $2, $3, $4);
        chomp($kernel_arch = `uname -m`);

        # We only support kernels >= 2.6.5
        if (!kernel_version_at_least(2, 6, 5)) {
                print "Kernel version is unsupported (too old, >=
2.6.5 needed)\n";
                exit -1;
        }
}

sub kernel_version_at_least
{
        my ($vers, $plvl, $slvl) = @_;
        return 1 if ($kernel_version[0] > $vers ||
                     ($kernel_version[0] == $vers &&
                      ($kernel_version[1] > $plvl ||
                       ($kernel_version[1] == $plvl &&
                        ($kernel_version[2] >= $slvl)))));
        return 0;
}

would fail with:

Kernel version is unsupported (too old, >= 2.6.5 needed)

with "uname -r" being "3.0-rc2"

 - Arnaud
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ray Lee June 10, 2011, 5:53 a.m. UTC | #3
On Thu, Jun 9, 2011 at 10:16 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> Beside that, no matter what, you are about to break
> `/usr/sbin/sensors-detect' (from my Fedora 14), which rely on a 3
> digits version number:

Is there really any compelling technical reason to _not_ have Linus
release the kernels with a superfluous .0 on the end, with the
understanding that the -stable team gets to increment it for their
future releases? It'd make every kernel.org release exactly three
dotted decimals from here on out, which certainly simplifies things.

It seems like we're just borrowing trouble here by trying to drop it
down to two numbers.

(Not, mind you, that I give a damn at all one way or the other, but it
feels like you all are going to be spending a lot of time tripping
over poor assumptions in userspace rather than doing actual work.)

~r.
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
maximilian attems June 10, 2011, 10:03 a.m. UTC | #4
On Thu, Jun 09, 2011 at 10:53:34PM -0700, Ray Lee wrote:
> On Thu, Jun 9, 2011 at 10:16 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> > Beside that, no matter what, you are about to break
> > `/usr/sbin/sensors-detect' (from my Fedora 14), which rely on a 3
> > digits version number:
> 
> Is there really any compelling technical reason to _not_ have Linus
> release the kernels with a superfluous .0 on the end, with the
> understanding that the -stable team gets to increment it for their
> future releases? It'd make every kernel.org release exactly three
> dotted decimals from here on out, which certainly simplifies things.
> 
> It seems like we're just borrowing trouble here by trying to drop it
> down to two numbers.
> 
> (Not, mind you, that I give a damn at all one way or the other, but it
> feels like you all are going to be spending a lot of time tripping
> over poor assumptions in userspace rather than doing actual work.)

you have forgotten the rule that linus himself is allowed to break userland. :P

in Debian an three digit 3.0.0 release would also be very much appreciated as
dpkg will order wrongly 3.0.0~rcX to 3.0.
Michal Marek June 10, 2011, 12:13 p.m. UTC | #5
On 10.6.2011 05:57, Linus Torvalds wrote:
> On Thu, Jun 9, 2011 at 8:45 PM, Arnaud Lacombe<lacombar@gmail.com>  wrote:
>>
>> For the record, this should have been fixed by Michal in:
>
> You're missing the problem: we can't assume that people have updated user land.
>
> Furthermore, the very pull request I'm responding to actually tries to
> handle this, see commit bfe5424a8b31 ("kbuild: Hack for depmod not
> handling X.Y versions"), but it's just not working for me.

It handles what can be handled inside the kernel build, i.e. the depmod 
called from the Makefile during modules_install.


> And it's not working, because it only handles the "modules_install"
> case, not the case where the system install scripts do their own
> depmod.

Yes, tools creating an initrd would need a similar workaround, that's a 
pity :-(.

Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Andy Whitcroft June 10, 2011, 3:06 p.m. UTC | #6
On Fri, Jun 10, 2011 at 6:53 AM, Ray Lee <ray-lk@madrabbit.org> wrote:
> On Thu, Jun 9, 2011 at 10:16 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
>> Beside that, no matter what, you are about to break
>> `/usr/sbin/sensors-detect' (from my Fedora 14), which rely on a 3
>> digits version number:
>
> Is there really any compelling technical reason to _not_ have Linus
> release the kernels with a superfluous .0 on the end, with the
> understanding that the -stable team gets to increment it for their
> future releases? It'd make every kernel.org release exactly three
> dotted decimals from here on out, which certainly simplifies things.
>
> It seems like we're just borrowing trouble here by trying to drop it
> down to two numbers.
>
> (Not, mind you, that I give a damn at all one way or the other, but it
> feels like you all are going to be spending a lot of time tripping
> over poor assumptions in userspace rather than doing actual work.)

As an experiment I have uploaded a 3.0 reporting kernel into Ubuntu
Oneiric.  So far we have had issues with:

 - depmod -- (as above)
 - procps (ps/top) -- reporting errors on startup
 - glibc version checks are preventing upgrade of that package

I am sure there will be others, but the last one above has me cornered
at the moment.  Cirtainly we can fix all of these but we are not going
to make it easy for people with un-upgraded userspace if my experience
is anything to go by.

-apw
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Torvalds June 10, 2011, 3:22 p.m. UTC | #7
On Fri, Jun 10, 2011 at 3:03 AM, maximilian attems <max@stro.at> wrote:
>
> you have forgotten the rule that linus himself is allowed to break userland. :P

No.

But Linus _is_ allowed to try to fix problems. So what I'm trying to do is:

 - see if we can find work-arounds for known broken packages (by
giving them a three-digit version number despite the kernel actually
only having two digits)

 - if no such work-arounds can be done without excessive hackery, at
least make sure that the packages in question know that they are
broken, so that they get fixed and we can drop the third digit later
even if we don't drop it at 3.0 itself.

It does look like there are too many problems to actually make it call
itself "3.0", and that's sad. That's not an excuse for not trying to
get those problems fixed, though.

                       Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Milan Broz June 10, 2011, 3:26 p.m. UTC | #8
On 06/10/2011 05:06 PM, Andy Whitcroft wrote:
> As an experiment I have uploaded a 3.0 reporting kernel into Ubuntu
> Oneiric.  So far we have had issues with:
> 
>  - depmod -- (as above)
>  - procps (ps/top) -- reporting errors on startup
>  - glibc version checks are preventing upgrade of that package
> 
> I am sure there will be others

All tools based on libdevmapper (lvm, cryptsetup, kpartx, ...)
fails in initialization. (fixed already in devel tree)

mdadm is broken with 3.0 as well (it fails to auto assemble devices)
because of misparsing kernel version.

Fedora was quite proactive and added uts name patch "3.0-rc1",
so you can see what it caused... (unbootable system in short)

https://bugzilla.redhat.com/show_bug.cgi?id=710646

Milan
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Arnaud Lacombe June 10, 2011, 4:30 p.m. UTC | #9
Hi,

[following-up to lm-sensors@lm-sensors.org for bug-report.]

On Fri, Jun 10, 2011 at 1:16 AM, Arnaud Lacombe <lacombar@gmail.com> wrote:
> Hi,
>
> On Thu, Jun 9, 2011 at 11:57 PM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> On Thu, Jun 9, 2011 at 8:45 PM, Arnaud Lacombe <lacombar@gmail.com> wrote:
>>>
>>> For the record, this should have been fixed by Michal in:
>>
>> You're missing the problem: we can't assume that people have updated user land.
>>
> I absolutely agree with you. But I am not the one about to decide to
> break a 15 years old interface :) [I gave me a 5 year delta as I have
> no idea how the version number was at that time; moreover, do not get
> me wrong, I do not really care about the kernel version being 2 or 3
> digits.].
>
>> Furthermore, the very pull request I'm responding to actually tries to
>> handle this, see commit bfe5424a8b31 ("kbuild: Hack for depmod not
>> handling X.Y versions"), but it's just not working for me.
>>
>> And it's not working, because it only handles the "modules_install"
>> case, not the case where the system install scripts do their own
>> depmod.
>>
> I totally agree! But, it is a technical challenge to give a 2 digit
> version number to an application expecting a 3 digit version number
> without much control over its environment.
>
> Beside that, no matter what, you are about to break
> `/usr/sbin/sensors-detect' (from my Fedora 14), which rely on a 3
> digits version number:
>
> # [0] -> VERSION
> # [1] -> PATCHLEVEL
> # [2] -> SUBLEVEL
> # [3] -> EXTRAVERSION
> #
> use vars qw(@kernel_version $kernel_arch);
>
> sub initialize_kernel_version
> {
>        `uname -r` =~ /(\d+)\.(\d+)\.(\d+)(.*)/;
>        @kernel_version = ($1, $2, $3, $4);
>        chomp($kernel_arch = `uname -m`);
>
>        # We only support kernels >= 2.6.5
>        if (!kernel_version_at_least(2, 6, 5)) {
>                print "Kernel version is unsupported (too old, >=
> 2.6.5 needed)\n";
>                exit -1;
>        }
> }
>
`sensors-detect's kernel version detection regexp is unable to parse 2
digits kernel version number and dies with:

[From Fedora 14's /usr/sbin/sensors-detect]

Use of uninitialized value $kernel_version[0] in numeric gt (>) at
./sensors-detect line 2442.
Use of uninitialized value $kernel_version[0] in numeric eq (==) at
./sensors-detect line 2442.
Kernel version is unsupported (too old, >= 2.6.5 needed)

I just checked the SVN repository, the issue still seems to be
present. I am not sure if other lm-sensors part are affected. This
will becomes an issue with the upcoming 3.x kernel serie.

Regards,
 - Arnaud

> sub kernel_version_at_least
> {
>        my ($vers, $plvl, $slvl) = @_;
>        return 1 if ($kernel_version[0] > $vers ||
>                     ($kernel_version[0] == $vers &&
>                      ($kernel_version[1] > $plvl ||
>                       ($kernel_version[1] == $plvl &&
>                        ($kernel_version[2] >= $slvl)))));
>        return 0;
> }
>
> would fail with:
>
> Kernel version is unsupported (too old, >= 2.6.5 needed)
>
> with "uname -r" being "3.0-rc2"
>
>  - Arnaud
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Michal Marek June 10, 2011, 8:45 p.m. UTC | #10
Dne 10.6.2011 17:22, Linus Torvalds napsal(a):
>  - if no such work-arounds can be done without excessive hackery, at
> least make sure that the packages in question know that they are
> broken, so that they get fixed and we can drop the third digit later
> even if we don't drop it at 3.0 itself.

So can we fix the kernel package now, even if the version has to stay at
3.0.0? The depmod hack is probably superfluous, but the rest are either
cleanups or fixes to make a future NUM.NUM release:

      kbuild: Fix KERNELVERSION for empty SUBLEVEL or PATCHLEVEL
      kbuild: Fix <linux/version.h> for empty SUBLEVEL or PATCHLEVEL
      kbuild: Move depmod call to a separate script
      perf: Use make kernelversion instead of parsing the Makefile

and this one is a genuine regression fix:

      kbuild: silence Nothing to be done for 'all' message

So would pull the branch if I revert "kbuild: Hack for depmod not
handling X.Y versions"?

thanks,
Michal
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Linus Torvalds June 10, 2011, 8:56 p.m. UTC | #11
On Fri, Jun 10, 2011 at 1:45 PM, Michal Marek <mmarek@suse.cz> wrote:
>
> So would pull the branch if I revert "kbuild: Hack for depmod not
> handling X.Y versions"?

Umm. I already pulled it yesterday. The reason I then replied to the
pull request was to report that it isn't enough to actually get things
working..

                    Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/depmod.c b/depmod.c
index abfb11e..98a5efa 100644
--- a/depmod.c
+++ b/depmod.c
@@ -247,7 +247,7 @@  static int is_version_number(const char *version)
 {
        unsigned int dummy;

-       return (sscanf(version, "%u.%u.%u", &dummy, &dummy, &dummy) == 3);
+       return (sscanf(version, "%u.%u", &dummy, &dummy) == 2);
 }

 - Arnaud