7 ms·
This is just how you work with large code bases. Updating 15,000 call sites is far from trivial. These changes happen in multiple phases, and in each phase, you
by klodolph 5y ago
This is just how you work with large code bases. Updating 15,000 call sites is far from trivial. These changes happen in multiple phases, and in each phase, you still want to have a working kernel.
The unsafe version can be deprecated and eventually removed, but that is down the road.
- xiphias2 5y agoI’m not sure how it’s done with the Linux Kernel (and git definately makes it hard), but at Google (using Perfoce model) there are tools for mass renaming that try to guarantee that there are no regressions. Of course in Linux the amount of C macros make it much harder to work on the syntax trees, but comparision after the preprocessor step should be possible.
- klodolph 5y agoIIRC Google works the same way. The automated refactoring tools (Rosie?) may make it go faster, but you generally don't fix up 15,000 call sites in a single change... you break it up into smaller batches spread across the tree.
- acidbaseextract 5y agoIt's been a long time since I was there, but I thought there were plenty of 15,000 call site refactorings done in a single final CL. Not that the Linux kernel should do the same!
- skybrian 5y agoWhen I was there (several years ago) it was rare to do that across many projects. You don't want to have to roll back everyone due to a problem that affects one project. Also, there's a risk that changes that are happening in the meantime might mean the patch doesn't apply.
- Filligree 5y agoFor a global change? These days that 'risk' is more of a certainty.
- ithkuil 5y agoIt may take a lot of time to get all the approvals from all the code owners. One of the big advantages of a tool like Rosie is precisely that it splits up the update into smaller CLs and has automation that nags all necessary reviewers and reruns the transformation once head moves.
- deckard1 5y agoWhat does git or Perforce have to do with refactoring? Why would whatever version control you're using matter? > tools for mass renaming that try to guarantee that there are no regressions beyond sed or whatever a fancy IDE already does? What kind of voodoo magic are we talking about here? Did Google solve the halting problem and I just haven't noticed yet...
- bri3d 5y agoDisclaimer: I haven't worked at Google, but you can easily read their papers like https://dl.acm.org/doi/pdf/10.1145/2854146 https://dl.acm.org/doi/pdf/10.1145/2854146 and understand what's going on. Refactoring tooling is integrated with the revision control system because in a giant codebase with thousands of contributors, you need to split a refactoring patch into smaller chunks and ideally use semantic rather than text-based diff tooling, or the changes will never get reviewed or merged before they conflict with more changes. As for the actual renaming, yes, definitely beyond what sed does but probably around what the fanciest IDE does. Imagine a big global symbol dependency graph produced by the entire build toolchain all at once and cached, I think? Also, this isn't the halting problem unless your codebase allows for fully dynamic invocation :)
- worthless-trash 5y agoAren't all function pointers dynamic invocation ? Or is "fully" one of those words that i'm not grasping in context.
- DaiPlusPlus 5y ago> Aren't all function pointers dynamic invocation? Given that compilers can be smart-enough to detect "non-dynamic" function-pointer invocation (e.g. when an execution-trace proves that a function-pointer parameter always points to the same function address), it's not safe to say that "all" function pointer invocations are dynamic. Another case to consider is when one implements (Smalltalk-style) OOP with message-passing: in many cases it's possible to build that without needing to use function-pointers at all.
- arka2147483647 5y agoThe point is that the change is not a simple rename: struct foo *iterator; list_for_each_entry(iterator, &foo_list, list) { do_something_with(iterator); } While the new version would be something like: list_for_each_entry_v2(iterator, &foo_list, list) { do_something_with(iterator); } So the ’iterator’ variable may be removed, but as this is c, might also be reused multiple times, or come from some struct member, union, or whatever. So i presume something like this would have to be fixed case by case.
- klodolph 5y agoYou can do these kinds of changes with clang-tidy, if necessary. Clang-tidy is extensible and you can write your own rules for transforming source code. Mind you, there are not a ton of people around who have the time to learn how to write custom clang-tidy rules. It might be done for 15,000 changes that potentially fix kernel vulnerabilities, though.
- pitaj 5y agoUnless something has changed in the last month, clang-tidy is not extensible. You can't write your own rules without forking the whole project.
- klodolph 5y agoI’ve worked at a couple companies that had custom clang-tidy checks. This is well-documented enough at this point, but it is still a bit arcane. http://bbannier.github.io/blog/2015/05/02/Writing-a-basic-clang-static-analysis-check.html http://bbannier.github.io/blog/2015/05/02/Writing-a-basic-cl... Yes, it involves forking. Forking isn’t that big a deal. We even had checks that were specific to internal libraries.
- andi999 5y agoAt the start forking is like no deal at all. But how do you get new clang tidy features from the main branch later into your fork; I think this is the only instance which I would really call technical debt.
- staticassertion 5y agoLinux doesn't even have consistent testing let alone the kind of tooling or will that Google does.
- colin_mccabe 5y agoThe Linux kernel developers have pretty good tooling for refactoring. For example, they have Coccinelle and Sparse. 1. https://en.wikipedia.org/wiki/Coccinelle_(software) https://en.wikipedia.org/wiki/Coccinelle_(software) 2. https://en.wikipedia.org/wiki/Sparse https://en.wikipedia.org/wiki/Sparse
- marcan_42 5y agoLinux definitely has testing. Some helpful person added our repo base to the kernel-test-robot list, and now I regularly get e-mails when something I'm working on in some random WIP branch broke the build on some other architecture (often in COMPILE_TEST mode), or some other random driver. Didn't even have to do anything myself. It's great for avoiding "this broke the build on $obscure_config" issues.
- eru 5y agoI don't see how Perforce makes this easier than Git?
- interactivecode 5y agoI thought this was one of the main advantages of typed languages refactoring or renaming is safe and easy. What gives?
- joshuamorton 5y agoAtomically flipping a version is still difficult, as anything that needs to rely on the other version has to be fixed forward. If you instead make 5000 (or 500) small changes that affect 10-15 uses, you can solve the unique cases or roll-back and forward.
- jayd16 5y agoYou can still have plenty of issues crop up from code that's not currently in master yet, or forked projects that take patches sparingly, etc. Typed languages let you know whats broken at compile time and aides in refactoring code you can see. It doesn't help you refactor code you can't see.
- kibwen 5y agoWhile calling C a typed language is technically correct, I struggle to think of any typed language that's more weakly-typed than C. Arguably, Python has a stronger type system than C.
- tyingq 5y agoThe C preprocessor is arguably a not type safe language unto itself.
- mamcx 5y agoYou can done it, without complex tooling: 1. Have 3 versions of the thing to change: select_nospec, select_nospec_safe, select_nospec_unsafe. select_nospec_unsafe is identical to select_nospec. Do the mass replace. All stay same. 2. Delete select_nospec 3. Start migrating to select_nospec_safe. You can still do this in small steps, because you old thing stay. 4. Where select_nospec_unsafe must be, keep it and maybe mark with a comment or similar to not delete it later 5. You finish and all is changed. Maybe replace select_nospec_safe to select_nospec if wanna.
- slimsag 5y agoWhen there are hundreds of branches, not just feature branches but real long-lived development branches, and pull requests are managed the way they are in the Linux kernel (chain of trust all the way up to Linus).. "Do the mass replace" becomes quite non-trivial I imagine.
- kragen 5y agoThis strategy handles that fine as long as none of those branches lasts from before step 2 until the optional step 6. You pull from a long-lived branch, get a compile error because it calls select_nospec, do the replacement at that callsite, and commit. A headache but a manageable one. If you want to backport new features to old stable kernel versions, you can include a patch supplying select_nospec_unsafe as a synonym for select_nospec.
- aliswe 5y agoThe branches wont compile until you fix every instance though, so its not that risky. just a bit messy.
- mamcx 5y agoAnd if the original version is not deleted in step 2 but after the final, then nobody break!
- eru 5y agoDepends on who you want to put the burden on? It's relatively easy to do the mass replace in master. You leave the burden of the replacement in the other branches to people who own those branches. Set up an automated script, to keep the 'unreplaced' version out of master (and people can also use that script to check their own branches). Not sure if that counts as non-trivial, yet? There are probably some other trade-offs involved. The people running kernel development ain't idiots.
- hyperpallium2 5y agoFirst, do not break.
- Koshkin 5y agoSecond, if it ain't broke, don't fix it.
- rfoo 5y agoThird, accept that when someone found your code is broken, they don't have to tell you.
- Go0the0gophers 5y ago[dead]
- kache_ 5y agolots of ways to do this. codemod, ratchets. It's a solved problem
- ithkuil 5y agoI wish languages would make it easier to decouple the "name" from the "code". The unison language offers an interesting take on the problem of code evolution backward compatibility. https://www.unisonweb.org/ https://www.unisonweb.org/