Skip to contentSkip to conversation

RFC - trailing whitespace cleanup script

KJ Jim Cromie 6 participants 10 messages Jul 17, 2005 – Jul 22, 2005 Kernel janitors

lore
Share & tools
lore

Jim Cromie

author
#0 lore raw

Domen, all,

Ive considered running following 1-lineer and submitting per-subsystem cleanup patches.

cp -al $tree $tree-clean; perl -pi.bak -e 's/\s+\n//' find $tree-clean -print > mega-patch

the problem with doing so is that lots of whitespace has already been cleaned,
and rc3-kj has presumably been superseded by more whitespace fixes, which would result in needless rejects.

So, what Id propose is:

a. just prior to releasing rc4-kj, do an rc4-kj-pre b. run script against pre tree.
c. diff -rup pre pre-clean > mega-patch
d. manually edit/chop up mega-patch on subsystem boundaries e. apply each to pre, release rc4-kj

f. repeat periodically
g. get janitors to recognize that they should do harder fixes, and leave whitespace
for the robo-janitor / whitespace-roomba

BTW

when were kj patches last merged into mainline ?
how often (on average) does this happen ?

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#1 lore raw

Quoting Jim Cromie <[email protected]>
Sent on Sat, 16 Jul 2005 20:38:13 -0600

Quoted 2 lines

g. get janitors to recognize that they should do harder fixes, and leave whitespace for the robo-janitor / whitespace-roomba

sounds great to me :)

Signature · 5 lines
Ricardo Nabinger Sanchez
GNU/Linux #140696 [http://counter.li.org]
Slackware Linux + FreeBSD

  Left to themselves, things tend to go from bad to worse.

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#2 lore raw
In reply to Jim Cromie

that includes an 'indent -kr -i8' ?

re,
walter

Quoted 10 lines · Ricardo Nabinger Sanchez

Quoting Jim Cromie <[email protected]>
Sent on Sat, 16 Jul 2005 20:38:13 -0600

Quoted 2 lines

g. get janitors to recognize that they should do harder fixes, and leave whitespace for the robo-janitor / whitespace-roomba

sounds great to me :)

Quoted 6 lines

------------------------------------------------------------------------

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

author
#4 lore raw
In reply to Jim Cromie
Quoted 10 lines · Alexey Dobriyan
Quoted 3 lines · walter harms

that includes an 'indent -kr -i8' ?

Of the whole tree? Ick.

can you refine that ick ?

its easy to cut a megapatch into subsystem patches, or even individual files.
does that address your esthetics, or is it something else ?

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#5 lore raw
In reply to Jim Cromie
Quoted 1 line · Jim Cromie

Domen, all,

Hi.

Quoted 21 lines

Ive considered running following 1-lineer and submitting per-subsystem cleanup patches.

cp -al $tree $tree-clean; perl -pi.bak -e 's/\s+\n//' find $tree-clean -print > mega-patch

the problem with doing so is that lots of whitespace has already been cleaned,
and rc3-kj has presumably been superseded by more whitespace fixes, which would result in needless rejects.

So, what Id propose is:

a. just prior to releasing rc4-kj, do an rc4-kj-pre b. run script against pre tree.
c. diff -rup pre pre-clean > mega-patch
d. manually edit/chop up mega-patch on subsystem boundaries e. apply each to pre, release rc4-kj

Quoted 4 lines

f. repeat periodically
g. get janitors to recognize that they should do harder fixes, and leave whitespace
for the robo-janitor / whitespace-roomba

No, no, no, and again no. And I do hate trailing whitespace, I have it coloured red in vim, so there's really no other way than hating.

So... why not?
- Maintainers will want to kill us, because no patch will apply anymore.
- It would generate _lots_ of traffic.
- It would make it harder to trace bugs ("try to unapply that patch")
- I could probably think of some more, but first one makes it obsolete anyways. ;-)

When could whitespace fixes be good?
Maybe on a driver that is in bug-fix-only state and is expected to stay in kernel for a while (IOW, is not broken :-) ).

Quoted 4 lines

BTW

when were kj patches last merged into mainline ?

They don't get merged as a whole, just parts, through appropriate maintainers.
I will need to send some very old ones to akpm, because it looks like they won't get merged any other way.

Quoted 1 line

how often (on average) does this happen ?

Hmm... patch per day? It really depends.

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

author
#6 lore raw
In reply to Jim Cromie
Quoted 8 lines · Jim Cromie

Domen, all,

Ive considered running following 1-lineer and submitting per-subsystem cleanup patches.

cp -al $tree $tree-clean; perl -pi.bak -e 's/\s+\n//' find $tree-clean -print > mega-patch

I just ran this in a linux-kj directory

     egrep -rl '\s$' * | xargs perl -pi -e 's/\s+\n/\n/'

and omygod, it produces 21 MB of unified diffs.

some observations:
60K lines of arch/ diffs
40K lines of Documentation diffs ( not relevant, tb scrapped) lots of trailing whitespace in comments
hard to programmatically avoid patching.
99% avoided if script tolerates one trailing whitespace. ( ie shrinks diff-file by 75% )

Quoted 8 lines

So, what Id propose is:

a. just prior to releasing rc4-kj, do an rc4-kj-pre b. run script against pre tree.
c. diff -rup pre pre-clean > mega-patch
d. manually edit/chop up mega-patch on subsystem boundaries e. apply each to pre, release rc4-kj

given the diff size, this is probably inadequate.
Ive written a script to carve a megapatch into individual chunk-files, and plan to modify it to carve on a specified directory depth, (ala -pN).

Im familiar with LKML patch submission rules
1. no gzips
2. no tars
3. no mega-patches
4. no patch-frags ( ie reams of tiny patches )
5. use your judgement wrt patch-size, content.

For this list, are the rules the same, slightly different ? Particularly wrt 5, we're dealing with less/little semantic content, so can be bigger, esp to avoid 4.
and gzips seems useful, esp for patches with 100k of whitespace.

As an extreme example, I could send to you, privately,
a tarball with thousands of patch frags, and a script to apply them serially,
and to delete the ones that fail to apply. Youre left with the ones that did.

Quoted 10 lines

f. repeat periodically
g. get janitors to recognize that they should do harder fixes, and leave whitespace
for the robo-janitor / whitespace-roomba

BTW

when were kj patches last merged into mainline ? how often (on average) does this happen ?

also, are you using git ?
_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

author
#7 lore raw
In reply to Jim Cromie
Quoted 28 lines · Domen Puncer
Quoted 3 lines · Jim Cromie

Domen, all,

Hi.

Quoted 5 lines

Ive considered running following 1-lineer and submitting per-subsystem cleanup patches.

No, no, no, and again no. And I do hate trailing whitespace, I have it coloured red in vim, so there's really no other way than hating.

So... why not?
- Maintainers will want to kill us, because no patch will apply anymore.
- It would generate _lots_ of traffic.
- It would make it harder to trace bugs ("try to unapply that patch")
- I could probably think of some more, but first one makes it obsolete anyways. ;-)

Quoted 5 lines

When could whitespace fixes be good?
Maybe on a driver that is in bug-fix-only state and is expected to stay in kernel for a while (IOW, is not broken :-) ).

OK, nevermind.

and pls disregard msg I just sent, b4 seeing this one.
_______________________________________________
Kernel-janitors mailing list [email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#8 lore raw
In reply to Jim Cromie
Quoted 5 lines · Jim Cromie
Quoted 3 lines · Alexey Dobriyan

On Mon, Jul 18, 2005 at 03:09:31PM +0200, walter harms wrote:

Quoted 8 lines
Quoted 7 lines
Quoted 3 lines

that includes an 'indent -kr -i8' ?

Of the whole tree? Ick.

can you refine that ick ?

Current scripts/Lindent isn't right quite often:
1) misplaces labels

-no_symlink:
+ no_symlink:
remove_proc_entry("tty", example_dir);

2) Screws carefully indented C99 initializers

- .name = "Ghost Device",
- .bus_id = "ghost0",
+ .name = "Ghost Device",
+ .bus_id = "ghost0",

3) Screws carefully indented generic initializers

-	{&master_mtu,     "SIOCGIFMTU",     SIOCGIFMTU},
-	{&master_flags,   "SIOCGIFFLAGS",   SIOCGIFFLAGS},
-	{&master_hwaddr,  "SIOCGIFHWADDR",  SIOCGIFHWADDR},

+ {&master_mtu, "SIOCGIFMTU", SIOCGIFMTU},
+ {&master_flags, "SIOCGIFFLAGS", SIOCGIFFLAGS},
+ {&master_hwaddr, "SIOCGIFHWADDR", SIOCGIFHWADDR},

4) Imagine what will it do with firmware images

5) Some changes one can categorize as "wtf?"
----------------------------------------------------------------------------
static inline
-struct class_device_attribute *to_class_dev_attr(struct attribute *_attr)
+ struct class_device_attribute *to_class_dev_attr(struct attribute *_attr)
----------------------------------------------------------------------------
- .attr = {.name = "data", .mode = 0644},
+ .attr = {.name = "data",.mode = 0644},
----------------------------------------------------------------------------
- ifr.ifr_data = (caddr_t)&info;
+ ifr.ifr_data = (caddr_t) & info;
----------------------------------------------------------------------------
irongate_read_config(struct pci_bus *bus, unsigned int devfn, int where,
- int size, u32 *value)
+ int size, u32 * value)
----------------------------------------------------------------------------
6-?) Unknown amount of other decreasing readability changes

Though it can be very useful:

1)

	class_dev->class = &firmware_class,	<===

- class_set_devdata(class_dev, fw_priv);
+ class_set_devdata(class_dev, fw_priv);

2-?) Unknown amount of increasing readability changes

Conclusion: blind Lindent'ing of huge code blocks is not right thing to do.

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors

#9 lore raw
In reply to Jim Cromie

On Mon, 18 Jul 2005, Jim Cromie wrote:
<snip>

Quoted 1 line

also, are you using git ?

i ran the akpm patch scripts, afair randy too:
-> http://www.zip.com.au/~akpm/linux/patches/patch-scripts-0.20/
domen switched to quilt.
-> http://savannah.nongnu.org/projects/quilt

they are easier if you push lots of patches
and have to pull random ones out.
--
maks

_______________________________________________
Kernel-janitors mailing list
[email protected]
https://lists.osdl.org/mailman/listinfo/kernel-janitors