6 ms·

Admittedly I didn't actually read the mailing list discussion. It's entirely possible that The Register made up a big drama where there was none.

echlebek 6y ago
There wasn't much drama in the mailing list discussion as I read it. Mostly comments asking "can you make this easier for us to review?".
mehrdadn 6y ago
Not sure if this counts as drama per Linux kernel mailing list standards: https://lore.kernel.org/linux-fsdevel/2911ac5cd20b46e397be506268718d74@paragon-software.com/t/#m34fc84d93a7009caf533ff400f7c763da4cab366 https://lore.kernel.org/linux-fsdevel/2911ac5cd20b46e397be50... > So how exactly do you expect someone to review this monstrosity ?
- isatty 6y ago
  It’s a legitimate concern - I would not assume malice. How exactly would someone review a 25k loc .patch file?
  - mehrdadn 6y ago
    I didn't mean to imply otherwise. Drama is often not malice and rather due to legitimate concerns on one or both sides.
  - michaelt 6y ago
    It would be tough, no doubt. But it's not like splitting the feature into 100 patches of 250 lines each would make it any quicker to review. Or merging code that was known not to work, as it was only a fraction of what was needed for the functionality.
    - wtallis 6y ago
      > But it's not like splitting the feature into 100 patches of 250 lines each would make it any quicker to review. Or merging code that was known not to work, as it was only a fraction of what was needed for the functionality. That would also be rejected, because the kernel maintainers aren't idiots and their standards aren't the stupid arbitrary rules you construe them to be. They generally want big changes to be broken up into logical, sensible chunks that each leave the tree in a usable state, so that git-bisect still works.
      - skissane 6y ago
        How do people merge big new filesystems in practice though? Especially one with years of pre-existing out-of-tree development? I guess one could start by merging a skeleton of the filesystem which supports mount/unmount but then returns an IO error on every operation? And then a patch to add directory traversal (you can view the files but not their contents), and then a patch to add file reading, and then a patch to add file writing, and then a patch to add mkdir/rmdir, and then a patch to add rename/delete of regular files. Breaking down an existing filesystem into a sequence of patches like that, no doubt it is doable, but it is going to be a lot of work.
        wtallis 6y ago
        My guess is that given the history of this filesystem implementation, most of the review effort will be focused on the interface between this FS and the rest of the kernel. It's typical for all the changes touching communal files or introducing generic helper functions or data structures to be broken out into separate commits. If any of those helpers are a reinvention of stuff that's already in the kernel, there will need to be a justification for why NTFS needs its own special versions. It's not typical for a large patch series adding genuinely new stuff to be broken up into absurdly tiny commits. For the stuff that's truly internal to the filesystem implementation, it looks like one patch per file will be an acceptable granularity.
  - rat9988 6y ago
    I assume malice because of the tone. The concern is legitimate, the tone is offputting.
    - phs318u 6y ago
      Welcome to Linux kernel development. By lkml history, this tone is very mild. There are many examples of far worse commentary and personal attacks on devs. I’m not justifying this by the way. Linus can be a very smart jerk, and as a leader (THE leader) he sets the tone for what’s acceptable in the community. https://www.zdnet.com/article/linux-developer-who-took-on-linus-torvalds-over-abuse-quits-toxic-kernel-community/ https://www.zdnet.com/article/linux-developer-who-took-on-li...
  - deleted 6y ago
    [deleted]
- tripletao 6y ago
  It seems from the link that the kernel developers would rather have one patch per new file plus a patch that does the integration, instead of one big patch with everything. That's a bit unconventional, perhaps because they tend to use git alone instead of higher-level software that would help them break down a big single commit; but whatever they're doing clearly works for them. The entire dispute seems to be that minor question of style, nothing substantive. I don't think anyone's specially unhappy on either side. The controversy seems manufactured, perhaps by a reporter who noticed the gruff language but lacked the technical knowledge to understand what's actually going on. Most people developing free software (probably including both the submitters and recipients of this patch) could make a lot more money elsewhere, but have chosen to instead to do work with considerable public benefit. That's thankless enough already without some reporter inventing drama for clicks.
  - u801e 6y ago
    > perhaps because they tend to use git alone instead of higher-level software that would help them break down a big single commit git is capable of breaking down a large diff into manageable pieces (e.g., limiting a diff to a single file), but reviewing code in a mailing list means replying to the message that contains a patch and replying inline to certain parts to comment on it. As for higher level software that could break down a large commit, what specifically do you have in mind? I can't think of any feature that other review tools like Git??b, gerrit, reviewboard, phabricator, etc. that would make something like this easy to review.
    - tripletao 6y ago
      I meant like GitHub and competitors, which let you attach comments to specific lines and files and such, and perhaps follow references into the full code faster than you could flipping between your mail client and your editor (and save you the effort of applying the patch to a local tree for that review). Since the kernel developers prefer to discuss on a plain mailing list and not use such tools, it makes sense that they prefer smaller chunks. 27 kLOC will be a big project to review no matter what, but I'd probably rather take them in a single commit--the files presumably depend on each other, and there's probably no order in which the files could be reviewed in isolation without reference to files not yet reviewed. (Obviously we try for hierarchical structure that would make that possible, but not usually with perfect success.) That's a matter of personal preference, though, and people who want a project to merge their contributions should adhere to the maintainer's preferences. In any case, it seems Paragon intends to do exactly that. I doubt Paragon expected their reward for their contribution would be an article read by thousands of people that called it "half-baked" over this minor point, and I can't imagine such publicity encourages others to make similar contributions in future.
chx 6y ago
I read the discussion. There's no drama at all. Paragon did an unreviewable code dump with intent to maintain and they are warmly welcome in general. David laid out the path to review and probable acceptance https://lore.kernel.org/linux-fsdevel/20200815190642.GZ2026@twin.jikos.cz/ https://lore.kernel.org/linux-fsdevel/20200815190642.GZ2026@... If there was any fuss it's because of the unreviewable nature of the patch but especially by kernel standards the discussion was cordial. In fact, aside from Nikolay's outburst by any standard it was a cordial discussion. (Someone should've gently told him this is no way to welcome newcomers especially newcomers carrying such a gift.) Others noted it needs to pass the existing test suite and that it is close.

echlebek 6y ago

There wasn't much drama in the mailing list discussion as I read it. Mostly comments asking "can you make this easier for us to review?".

mehrdadn 6y ago

Not sure if this counts as drama per Linux kernel mailing list standards: https://lore.kernel.org/linux-fsdevel/2911ac5cd20b46e397be506268718d74@paragon-software.com/t/#m34fc84d93a7009caf533ff400f7c763da4cab366 https://lore.kernel.org/linux-fsdevel/2911ac5cd20b46e397be50... > So how exactly do you expect someone to review this monstrosity ?

isatty 6y ago
It’s a legitimate concern - I would not assume malice. How exactly would someone review a 25k loc .patch file?
- mehrdadn 6y ago
  I didn't mean to imply otherwise. Drama is often not malice and rather due to legitimate concerns on one or both sides.
- michaelt 6y ago
  It would be tough, no doubt. But it's not like splitting the feature into 100 patches of 250 lines each would make it any quicker to review. Or merging code that was known not to work, as it was only a fraction of what was needed for the functionality.
  - wtallis 6y ago
    > But it's not like splitting the feature into 100 patches of 250 lines each would make it any quicker to review. Or merging code that was known not to work, as it was only a fraction of what was needed for the functionality. That would also be rejected, because the kernel maintainers aren't idiots and their standards aren't the stupid arbitrary rules you construe them to be. They generally want big changes to be broken up into logical, sensible chunks that each leave the tree in a usable state, so that git-bisect still works.
    - skissane 6y ago
      How do people merge big new filesystems in practice though? Especially one with years of pre-existing out-of-tree development? I guess one could start by merging a skeleton of the filesystem which supports mount/unmount but then returns an IO error on every operation? And then a patch to add directory traversal (you can view the files but not their contents), and then a patch to add file reading, and then a patch to add file writing, and then a patch to add mkdir/rmdir, and then a patch to add rename/delete of regular files. Breaking down an existing filesystem into a sequence of patches like that, no doubt it is doable, but it is going to be a lot of work.
      - wtallis 6y ago
        My guess is that given the history of this filesystem implementation, most of the review effort will be focused on the interface between this FS and the rest of the kernel. It's typical for all the changes touching communal files or introducing generic helper functions or data structures to be broken out into separate commits. If any of those helpers are a reinvention of stuff that's already in the kernel, there will need to be a justification for why NTFS needs its own special versions. It's not typical for a large patch series adding genuinely new stuff to be broken up into absurdly tiny commits. For the stuff that's truly internal to the filesystem implementation, it looks like one patch per file will be an acceptable granularity.
- rat9988 6y ago
  I assume malice because of the tone. The concern is legitimate, the tone is offputting.
  - phs318u 6y ago
    Welcome to Linux kernel development. By lkml history, this tone is very mild. There are many examples of far worse commentary and personal attacks on devs. I’m not justifying this by the way. Linus can be a very smart jerk, and as a leader (THE leader) he sets the tone for what’s acceptable in the community. https://www.zdnet.com/article/linux-developer-who-took-on-linus-torvalds-over-abuse-quits-toxic-kernel-community/ https://www.zdnet.com/article/linux-developer-who-took-on-li...
- deleted 6y ago
  [deleted]
tripletao 6y ago
It seems from the link that the kernel developers would rather have one patch per new file plus a patch that does the integration, instead of one big patch with everything. That's a bit unconventional, perhaps because they tend to use git alone instead of higher-level software that would help them break down a big single commit; but whatever they're doing clearly works for them. The entire dispute seems to be that minor question of style, nothing substantive. I don't think anyone's specially unhappy on either side. The controversy seems manufactured, perhaps by a reporter who noticed the gruff language but lacked the technical knowledge to understand what's actually going on. Most people developing free software (probably including both the submitters and recipients of this patch) could make a lot more money elsewhere, but have chosen to instead to do work with considerable public benefit. That's thankless enough already without some reporter inventing drama for clicks.
- u801e 6y ago
  > perhaps because they tend to use git alone instead of higher-level software that would help them break down a big single commit git is capable of breaking down a large diff into manageable pieces (e.g., limiting a diff to a single file), but reviewing code in a mailing list means replying to the message that contains a patch and replying inline to certain parts to comment on it. As for higher level software that could break down a large commit, what specifically do you have in mind? I can't think of any feature that other review tools like Git??b, gerrit, reviewboard, phabricator, etc. that would make something like this easy to review.
  - tripletao 6y ago
    I meant like GitHub and competitors, which let you attach comments to specific lines and files and such, and perhaps follow references into the full code faster than you could flipping between your mail client and your editor (and save you the effort of applying the patch to a local tree for that review). Since the kernel developers prefer to discuss on a plain mailing list and not use such tools, it makes sense that they prefer smaller chunks. 27 kLOC will be a big project to review no matter what, but I'd probably rather take them in a single commit--the files presumably depend on each other, and there's probably no order in which the files could be reviewed in isolation without reference to files not yet reviewed. (Obviously we try for hierarchical structure that would make that possible, but not usually with perfect success.) That's a matter of personal preference, though, and people who want a project to merge their contributions should adhere to the maintainer's preferences. In any case, it seems Paragon intends to do exactly that. I doubt Paragon expected their reward for their contribution would be an article read by thousands of people that called it "half-baked" over this minor point, and I can't imagine such publicity encourages others to make similar contributions in future.

chx 6y ago

I read the discussion. There's no drama at all. Paragon did an unreviewable code dump with intent to maintain and they are warmly welcome in general. David laid out the path to review and probable acceptance https://lore.kernel.org/linux-fsdevel/20200815190642.GZ2026@twin.jikos.cz/ https://lore.kernel.org/linux-fsdevel/20200815190642.GZ2026@... If there was any fuss it's because of the unreviewable nature of the patch but especially by kernel standards the discussion was cordial. In fact, aside from Nikolay's outburst by any standard it was a cordial discussion. (Someone should've gently told him this is no way to welcome newcomers especially newcomers carrying such a gift.) Others noted it needs to pass the existing test suite and that it is close.