* Re: Compat-wireless release for 2011-08-01 is (NOT) baked
2011-08-01 23:34 ` Luis R. Rodriguez
@ 2011-08-02 1:18 ` Pavel Roskin
2011-08-02 6:31 ` Julia Lawall
1 sibling, 0 replies; 5+ messages in thread
From: Pavel Roskin @ 2011-08-02 1:18 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: linux-wireless, Julia Lawall, Nicolas Palix, linux-kernel
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Compat-wireless release for 2011-08-01 is (NOT) baked
2011-08-01 23:34 ` Luis R. Rodriguez
2011-08-02 1:18 ` Pavel Roskin
@ 2011-08-02 6:31 ` Julia Lawall
1 sibling, 0 replies; 5+ messages in thread
From: Julia Lawall @ 2011-08-02 6:31 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Pavel Roskin, linux-wireless, Nicolas Palix, linux-kernel,
Gilles Muller
[-- 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
^ permalink raw reply [flat|nested] 5+ messages in thread