Compat-wireless release for 2011-08-01 is baked

Compat-wireless release for 2011-08-01 is baked

[Compat-wireless cronjob account]

>From git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next
 + d7c6e83...c39a889 akpm-end   -> origin/akpm-end  (forced update)
   55f9c40..3da3f87  akpm-start -> origin/akpm-start
 + cf684c9...346346f master     -> origin/master  (forced update)
   55f9c40..3da3f87  stable     -> origin/stable
 * [new tag]         next-20110801 -> next-20110801
/usr/bin/sha1sum: *.tar.bz2: No such file or directory

compat-wireless code metrics

    835336 - Total upstream lines of code being pulled

Re: Compat-wireless release for 2011-08-01 is (NOT) baked

[Pavel Roskin]

On 08/01/2011 03:04 PM, Compat-wireless cronjob account wrote:
>  From git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next
>   + d7c6e83...c39a889 akpm-end   ->  origin/akpm-end  (forced update)
>     55f9c40..3da3f87  akpm-start ->  origin/akpm-start
>   + cf684c9...346346f master     ->  origin/master  (forced update)
>     55f9c40..3da3f87  stable     ->  origin/stable
>   * [new tag]         next-20110801 ->  next-20110801
> /usr/bin/sha1sum: *.tar.bz2: No such file or directory

Something must be failing.
The last tarball is compat-wireless-2011-07-28.

-- 
Regards,
Pavel Roskin

Re: Compat-wireless release for 2011-08-01 is (NOT) baked

[Luis R. Rodriguez]

On Mon, Aug 1, 2011 at 1:38 PM, Pavel Roskin <proski@gnu.org> wrote:
> On 08/01/2011 03:04 PM, Compat-wireless cronjob account wrote:
>>
>>  From git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next
>>  + d7c6e83...c39a889 akpm-end   ->  origin/akpm-end  (forced update)
>>    55f9c40..3da3f87  akpm-start ->  origin/akpm-start
>>  + cf684c9...346346f master     ->  origin/master  (forced update)
>>    55f9c40..3da3f87  stable     ->  origin/stable
>>  * [new tag]         next-20110801 ->  next-20110801
>> /usr/bin/sha1sum: *.tar.bz2: No such file or directory
>

Right, this failed to build the compat-wireless daily tarball based on
linux-next.git due to a failed hunk:

Applying backport patch: patches/01-netdev.patch
patching file drivers/net/usb/rndis_host.c
patching file drivers/net/usb/usbnet.c
patching file drivers/net/wireless/rndis_wlan.c
patching file net/mac80211/iface.c
Hunk #1 FAILED at 698.
Hunk #2 succeeded at 846 (offset 1 line).
Hunk #3 succeeded at 885 (offset 1 line).
Hunk #4 succeeded at 1136 (offset 1 line).
Hunk #5 succeeded at 1146 (offset 1 line).
1 out of 5 hunks FAILED -- saving rejects to file net/mac80211/iface.c.rej
patching file drivers/net/b44.c
patching file net/wireless/wext-core.c
patching file drivers/net/wireless/ipw2x00/ipw2100.c
patching file drivers/net/wireless/ipw2x00/ipw2200.c
patching file drivers/net/wireless/iwmc3200wifi/netdev.c
patching file drivers/net/wireless/libertas/main.c
patching file drivers/net/wireless/libertas/mesh.c
patching file drivers/net/wireless/libertas/defs.h
patching file drivers/net/wireless/mac80211_hwsim.c
patching file drivers/net/wireless/mwifiex/main.c
patching file drivers/net/wireless/orinoco/main.c
patching file net/bluetooth/bnep/netdev.c
Hunk #2 FAILED at 235.
1 out of 2 hunks FAILED -- saving rejects to file
net/bluetooth/bnep/netdev.c.rej
patching file drivers/net/atl1e/atl1e_main.c
patching file drivers/net/atl1c/atl1c_main.c
patching file drivers/net/atlx/atl1.c
patching file drivers/net/atlx/atl2.c
Patching patches/01-netdev.patch failed, update it

Whenever we have a failed hunk we run into this. I think we can do
better, we could break down 01-netdev.patch into one a coccinelle
spatch for example and then the rest to be handled separately, this
may help speed with making us lazier and making the backup even more
automatic.

For example:

mcgrof@tux ~/linux-next (git::master)$ cat netdev-attach.cocci
@@
expression ptr, ops;
@@
...
-ptr->netdev_ops = ops;
+netdev_attach_ops(ptr, ops);
...


mcgrof@tux ~/linux-next (git::master)$ spatch -sp_file
netdev-attach.cocci -in_place drivers/net/usb/rndis_host.c
init_defs_builtins: /usr/share/coccinelle/standard.h
HANDLING: drivers/net/usb/rndis_host.c
diff =
--- drivers/net/usb/rndis_host.c        2011-04-25 11:44:25.713248001 -0700
+++ /tmp/cocci-output-14779-d7a728-rndis_host.c 2011-08-01
16:30:48.342283464 -0700
@@ -355,7 +355,7 @@ generic_rndis_bind(struct usbnet *dev, s
        dev->rx_urb_size &= ~(dev->maxpacket - 1);
        u.init->max_transfer_size = cpu_to_le32(dev->rx_urb_size);

-       net->netdev_ops = &rndis_netdev_ops;
+       netdev_attach_ops(net, &rndis_netdev_ops);

        retval = rndis_command(dev, u.header, CONTROL_BUFFER_SIZE);
        if (unlikely(retval < 0)) {

Question then is -- are we willing to require spatch for building the
tarballs? I'm OK with this, anyone else?

  Luis

Re: Compat-wireless release for 2011-08-01 is (NOT) baked

[Pavel Roskin]

Quoting "Luis R. Rodriguez" <mcgrof@gmail.com>:

> Question then is -- are we willing to require spatch for building the
> tarballs? I'm OK with this, anyone else?

Good question!  I think it may be worth it, but let's consider the arguments.

Against:

coccinelle is not nearly as widespread as other required tools (gcc,  
make, patch).

coccinelle depends on ocaml.  That's a whole programming language that  
many systems (including my home system with Fedora 15) don't have  
installed by default, even if the "development" was selected at the  
system install.

I don't know the status of coccinelle and ocaml packaging on various  
distros.  Asking users to compile both to have the latest wireless  
drivers is pretty cruel.

Even Fedora 15, as I found out while writing this message, mispackages  
coccinelle.  The dependency on ocaml-findlib is missing, so coccinelle  
is not functional when installed by yum.  I'm going to report is as  
soon as possible.

There are two names, spatch and coccinelle.  That's going to be  
confusing for the users.  They won't know which package to look for.   
Some will look for both.

For:

There is no need to have any special variant of coccinelle to  
cross-compile compat-wireless.

Popularization of coccinelle and ocaml may be a good thing.  More  
users means more eyeballs looking for bugs.

Patches will become more clear, at least for those knowing the basics  
of coccinelle syntax.  Non-trivial changes won't be buried in trivial  
ones.

compat-wireless will be more robust.

It may be possible to port compat-wireless to more kernels.  Or maybe  
more subsystems will be backported.  Replacing even large amounts of  
code would become easier.

While some people can be inconvenienced by the need to install  
coccinelle, it's a solvable problem for most users.  On the other  
hand, backporting compat-wireless to older kernels or backporting more  
features is something that very few people can do, and those people  
should not be bothered with trivial stuff too much.

My experience shows that placing too much boring stuff on the  
shoulders of the developers is detrimental to the users in the long  
run.  It's one thing to require extensive testing, which can be  
automated, or to require compatibility with older tools, which can be  
easily achieved.  On the other hand, requiring to update huge patches  
manually is boring and error prone.  Make the developers busy with  
boring stuff, and they will stop making improvements that nobody else  
can do.

-- 
Regards,
Pavel Roskin

Re: Compat-wireless release for 2011-08-01 is (NOT) baked

[Julia Lawall]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 4386 bytes --]

On Mon, 1 Aug 2011, Luis R. Rodriguez wrote:

> On Mon, Aug 1, 2011 at 1:38 PM, Pavel Roskin <proski@gnu.org> wrote:
> > On 08/01/2011 03:04 PM, Compat-wireless cronjob account wrote:
> >>
> >>  From git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next
> >>  + d7c6e83...c39a889 akpm-end   ->  origin/akpm-end  (forced update)
> >>    55f9c40..3da3f87  akpm-start ->  origin/akpm-start
> >>  + cf684c9...346346f master     ->  origin/master  (forced update)
> >>    55f9c40..3da3f87  stable     ->  origin/stable
> >>  * [new tag]         next-20110801 ->  next-20110801
> >> /usr/bin/sha1sum: *.tar.bz2: No such file or directory
> >
> 
> Right, this failed to build the compat-wireless daily tarball based on
> linux-next.git due to a failed hunk:
> 
> Applying backport patch: patches/01-netdev.patch
> patching file drivers/net/usb/rndis_host.c
> patching file drivers/net/usb/usbnet.c
> patching file drivers/net/wireless/rndis_wlan.c
> patching file net/mac80211/iface.c
> Hunk #1 FAILED at 698.
> Hunk #2 succeeded at 846 (offset 1 line).
> Hunk #3 succeeded at 885 (offset 1 line).
> Hunk #4 succeeded at 1136 (offset 1 line).
> Hunk #5 succeeded at 1146 (offset 1 line).
> 1 out of 5 hunks FAILED -- saving rejects to file net/mac80211/iface.c.rej
> patching file drivers/net/b44.c
> patching file net/wireless/wext-core.c
> patching file drivers/net/wireless/ipw2x00/ipw2100.c
> patching file drivers/net/wireless/ipw2x00/ipw2200.c
> patching file drivers/net/wireless/iwmc3200wifi/netdev.c
> patching file drivers/net/wireless/libertas/main.c
> patching file drivers/net/wireless/libertas/mesh.c
> patching file drivers/net/wireless/libertas/defs.h
> patching file drivers/net/wireless/mac80211_hwsim.c
> patching file drivers/net/wireless/mwifiex/main.c
> patching file drivers/net/wireless/orinoco/main.c
> patching file net/bluetooth/bnep/netdev.c
> Hunk #2 FAILED at 235.
> 1 out of 2 hunks FAILED -- saving rejects to file
> net/bluetooth/bnep/netdev.c.rej
> patching file drivers/net/atl1e/atl1e_main.c
> patching file drivers/net/atl1c/atl1c_main.c
> patching file drivers/net/atlx/atl1.c
> patching file drivers/net/atlx/atl2.c
> Patching patches/01-netdev.patch failed, update it
> 
> Whenever we have a failed hunk we run into this. I think we can do
> better, we could break down 01-netdev.patch into one a coccinelle
> spatch for example and then the rest to be handled separately, this
> may help speed with making us lazier and making the backup even more
> automatic.
> 
> For example:
> 
> mcgrof@tux ~/linux-next (git::master)$ cat netdev-attach.cocci
> @@
> expression ptr, ops;
> @@
> ...
> -ptr->netdev_ops = ops;
> +netdev_attach_ops(ptr, ops);
> ...

The outer dots are not needed.  Even incorrect, because they imply that 
there is only one occurrence of this code in the current function.

> mcgrof@tux ~/linux-next (git::master)$ spatch -sp_file
> netdev-attach.cocci -in_place drivers/net/usb/rndis_host.c
> init_defs_builtins: /usr/share/coccinelle/standard.h
> HANDLING: drivers/net/usb/rndis_host.c
> diff =
> --- drivers/net/usb/rndis_host.c        2011-04-25 11:44:25.713248001 -0700
> +++ /tmp/cocci-output-14779-d7a728-rndis_host.c 2011-08-01
> 16:30:48.342283464 -0700
> @@ -355,7 +355,7 @@ generic_rndis_bind(struct usbnet *dev, s
>         dev->rx_urb_size &= ~(dev->maxpacket - 1);
>         u.init->max_transfer_size = cpu_to_le32(dev->rx_urb_size);
> 
> -       net->netdev_ops = &rndis_netdev_ops;
> +       netdev_attach_ops(net, &rndis_netdev_ops);
> 
>         retval = rndis_command(dev, u.header, CONTROL_BUFFER_SIZE);
>         if (unlikely(retval < 0)) {
> 
> Question then is -- are we willing to require spatch for building the
> tarballs? I'm OK with this, anyone else?

The problem with Coccinelle is that there can be false positives and false 
negatives, ie th rule applies where it should not and doesn't apply where 
it should.  But there are probably cases like this one where one knows 
that it should always be done this way.  It would be nice, though if there 
could be some kind of guarantee that the right thing has happened.  
Perhaps there could be a way to generate from a semantic patch some checks 
that it has applied everywhere possible.  Or perhaps the semantic patch 
could be only used as a last resort, if the normal application fails.  In 
that way there would be less to check by hand.

julia