8 ms·
Some good advice here, and some more...controversial advice here. After inheriting quite a few giant C++ projects over the years, there are a few obvious big w
by eschneider 3y ago
Some good advice here, and some more...controversial advice here.
After inheriting quite a few giant C++ projects over the years, there are a few obvious big wins to start with:
* Reproducible builds. The sanity you save will be your own. Pro-tip: wrap your build environment with docker (or your favorite packager) so that your tooling and dependencies become both explicit and reproducable. The sanity you save will be your own.
* Get the code to build clean with -Wall. This is for a couple of reasons. a) You'll turn up some amount of bad code/undefined behavior/bugs this way. Fix them and make the warning go away. It's ok to #pragma away some warnings once you've determined you understand what's happening and it's "ok" in your situation. But that should be rare. b) Once the build is clean, you'll get obvious warnings when YOU do something sketchy and you can fix that shit immediately. Again, the sanity you save will be your own.
* Do some early testing with something like valgrind and investigate any read/write errors it turns up. This is an easy win from a bugfix/stability point of view.
* At least initially, keep refactorings localized. If you work on a section and learn what it's doing, it's fine to clean it up and make it better, but rearchitecting the world before you have a good grasp on what's going on globally is just asking for pain and agony.
- SleepyMyroslav 3y agoI would not call that 'controversial'. In the internet days people call this behavior trolling for a reason. The punchline about rewriting code in different language gives an easy hint at where this all going. PS. I have been in the shoes of inheriting old projects before. And I hope i left them in better state than they were before.
- dataflow 3y agoStep 0: reproducible builds (like you said) Step 1: run all tests, mark all the flaky ones. Step 2: run all tests under sanitizers, mark all the ones that fail. Step 3: fix all the sanitizer failures. Step 4: (the other stuff you wrote)
- daemin 3y agoProbably insert another Step 1: implement tests Be they simple acceptance tests, integration tests, or even unit tests for some things.
- eschneider 3y agoJust a note on legacy tests: Step 0.5: understand the tests. They need to be examined to see if they've rotted or not. Tests passing/failing doesn't really mean code under test works or not. The tests might have been abandoned under previous management and don't accurately reflect how the code is _supposed_ to be working.
- dataflow 3y agoI'd put that under 2.5 or 3.5, if not later. You only really need to do it before you start modifying code, and it's a massive effort to understand a new codebase. Better pick the lower-hanging fruit (like corruption bugs) so you can at least stay sane when you run the tests and try to understand them.
- fransje26 3y agoLook at mister fancy here, having tests in his legacy code base.
- mst 3y agoThe same applies to comments. I have absolutely inherited codebases where one of the early steps was to make a commit excising every single comment in the code, because so many of them were old, lies, or old lies, that it wasn't worth the risk of a junior developer accidentally thinking they could be relied upon. (and of course they remained available in history to be potentially buffed up and resurrected, but still, argh)
- dataflow 3y agoThat's brilliant, but also... sounds like hell? Wouldn't that easily add several months to the timeline?
- roland35 3y agoI find it helpful to try and intentionally break the code under test. Sometimes the test still passes, and that is a good sign that something is very wrong!
- 3y ago
- hyperman1 3y agoIf we're going to visit the circles of hell, let's do it properly: Step -1: Get it under source control and backed up. Step -2: Find out if the source code corresponds to the executable. Which of the 7 variants of the source code (if any). Step -3: Do dark rituals over a weekend with cdparanioa to scrape the source code from the bunch of scratched cd's found in someone's bottom drawer. Bonus point if said person died last week, and other eldritch horrors lurk in that bottom drawer. Build a VM clone of the one machine still capable of compiling it. Yes, I have scars, why do you ask?
- galangalalgol 3y agoI was assuming it already had unit and system tests with decent coverage. I forgot how bad stuff gets. Maybe VM clones of various users too, and recordings of their work flows?
- Cerium 3y agoI'm always careful to dump the bash history as soon as I get access to a machine involved in a legacy project.
- eschneider 3y agoOooh, smart!
- cqqxo4zV46cp 3y agoYep! Learned this one the easy way! I don’t get to say that often, so I’m taking full advantage.
- eschneider 3y agoThere was that time when I had to dump the roms off a 'test' MRI machine because that's the only version of the code they had, then decompiled it, and rewrite it from that. I think about that a lot now that I'm older and spend a fair bit of time in MRI machines...
- groby_b 3y agoStep 0 sounds so easy. Until you realize __time__ exists. Then you take that away, and you find out that some compiler heuristics might not be deterministic. Then you discover -frandom-seed - and go ballistic when you read "but it should be a different seed for each sourcefile" Then you figure out your linker likes emitting a timestamp in object files. Then you discover /Brepro (if you're lucky enough to use lld-link. Then you used to discover that Win7's app compat db expected a "real" timestamp, and a hash just won't do. (Thank God, that's dead now). This is usually the part where you start questioning your life choices. Then somebody comes to your desk and asks if you can also make partial rebuilds deterministic. On the upside, step 1 is usually quick, there will be no tests.
- dataflow 3y agoI think (well, assumed) what they meant by deterministic builds was merely hermetic builds, which are easier. True determinism is overkill for step 0.
- groby_b 3y agoOften yes. Sometimes, no. You haven't enjoyed C++ until you get reports of the app intermittently crashing, and your build at the same version just won't. But yes, if the goal is "slap it all in a container", that's probably good and at least somewhat reproducible. We aren't Python here! ;)
- boolemancer 3y ago> Often yes. Sometimes, no. You haven't enjoyed C++ until you get reports of the app intermittently crashing, and your build at the same version just won't. That's okay, it's probably just some bank in a random country that requires some software package to be installed, presumably in the interest of security, which injects a dll into every process on the machine and unsurprisingly has a bug which causes your process to crash at random in only that part of the world.
- 3y ago
- microtherion 3y ago> Get the code to build clean with -Wall. This is fine, but I would strongly recommend against putting something like -Wall -Werror into production builds. Some of the warnings produced by compilers are opinion based, and new compiler versions may add new warnings, and suddenly the previous "clean" code is no longer accepted. If you must use -Werror, use it in debug builds.
- ladberg 3y agoThis is fixed by the suggestion right before it: > * Reproducible builds. The sanity you save will be your own. Pro-tip: wrap your build environment with docker (or your favorite packager) so that your tooling and dependencies become both explicit and reproducable. The sanity you save will be your own. Upgrading compiler versions shouldn't be done out-of-band with your normal PR process.
- jchw 3y agoI agree that this helps, although I still think that in general, the default build should never do -Werror, since people may use other toolchains and it shouldn't surprise-break downstream (I'm pretty sure this is a problem Linux distros struggle with all the time..) If it does it only in your fully reproducible CI, then it should be totally fine, of course.
- eschneider 3y agoThe scripted, packaged docker with toolchain dependencies and _is_ the build. If someone decides to use a different toolchain, the problems are on them.
- jchw 3y agoYeah that works if you are not dealing with open source. If you are dealing with open source, though, it really won't save you that much trouble, if anything it will just lead to unnecessarily hostile interactions. You're not really obligated to fix any specific issues that people report, but shrugging and saying "Your problem." is just non-productive and harms valuable downstreams like Linux distributions. Especially when a lot of new failures actually do indicate bugs and portability issues.
- golergka 3y agoGreat advice. Almost all of it applies to any programming language.
- dheera 3y ago> wrap your build environment with docker For microservices it is fine, but you can't always deploy everything else with docker, especially for people who want to use your app inside a docker. Docker-in-docker is a situation that should never happen. Containers are nice but they're a horrible way to pretend the problem doesn't exist. Bundle all the dependencies and make sure it doesn't depend on 5 billion things being in /usr/lib and having the correct versions.
- cshokie 3y agoNot the OP, but I don’t think they meant that the build output is in a container. They meant that the thing you use to compile the code is in docker (and you just copy out the result). That would help ensure consistency of builds without having any effect on downstream users.
- eschneider 3y agoExactly. The compiler and what ever dependencies you need _to build_ are bundled into a docker so that you don't need to worry about whatever random tools/libraries your coworkers have installed in their local environment.
- hobofan 3y ago> wrap your build environment with docker (or your favorite packager) so that your tooling and dependencies become both explicit and reproducable If you want explicitness and reproducibility please don't reach for Docker. Unless you take a lot of care, you will only get the most watered down version of reproducibility with Docker probably luring you into a false sense of security. E.g. pointing to mutable image tags without integrity hashes and invoking apt-get are things you'll find in most Dockerfiles out there and both leave open a huge surface area for things to go wrong and end up in slightly different states. And while they are not that easy to pick up, solutions like Bazel and Nix will give you a lot better foundation to stand on.
- basicallybones 3y agoA nice middle ground is using a tool like Google's Skaffold, which provides "Bazel-like" capabilities for composing Docker images and tagging them based on a number of strategies, including file manifests. In my case, I also use build args to explicitly set versions of external dependencies. I also pull external images and build base images with upgraded versions once, then re-tag them in my private repository, which is an easy-to-implement mechanism for reproducibility. While I am in a Typescript environment with this setup at the moment, my personal experience that Skaffold with Docker has a lighter implementation and maintenance overhead than Bazel. (You also get the added benefit of easy deployment and automatic rebuilds.) I quite liked using Bazel in a small Golang monorepo, but I ran into pain when trying to do things like include third-party pre-compiled binaries in the Docker builds, because of the unusual build rules convention. The advantage of Skaffold is it provides a thin build/tag/deploy/verify layer over Docker and other container types. Might be worth a look! Kudos to the Google team building it! https://skaffold.dev https://skaffold.dev
- hawk_ 3y ago> If you want explicitness and reproducibility please don't reach for Docker Is it common in C++ builds to rely on the current O/S libraries instead of say making most dependencies explicit, close to full cross-compile? Do dependencies need to be pulled in using apt-get and not something like maven?
- vlovich123 3y agoStep 0: CI on every commit to build & run all tests / enforce that you never regress once you've finished a step.