2011-08-01 19:04:03

by compat

[permalink] [raw]
Subject: Compat-wireless release for 2011-08-01 is baked

>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


2011-08-01 20:38:59

by Pavel Roskin

[permalink] [raw]
Subject: Re: Compat-wireless release for 2011-08-01 is (NOT) baked

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

2011-08-02 06:31:50

by Julia Lawall

[permalink] [raw]
Subject: Re: Compat-wireless release for 2011-08-01 is (NOT) baked

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

> On Mon, Aug 1, 2011 at 1:38 PM, Pavel Roskin <[email protected]> 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

2011-08-01 23:34:43

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: Compat-wireless release for 2011-08-01 is (NOT) baked

On Mon, Aug 1, 2011 at 1:38 PM, Pavel Roskin <[email protected]> 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

2011-08-02 01:17:59

by Pavel Roskin

[permalink] [raw]
Subject: Re: Compat-wireless release for 2011-08-01 is (NOT) baked

Quoting "Luis R. Rodriguez" <[email protected]>:

> 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