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.

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.

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

If this is frequently a problem you're doing something wrong, or using such a crappy external library/toolchain that breaks frequently on the same version.

Docker is a way to ensure that the software builds with "the most recent minor version" of some OS/toolchain/libraries.

The reason why you want the most recent version is because of security fixes and bugs.

I agree that you should check integrtiy hashes where appropriate, if you really want to fix versions.

I've had issues with Dockerfiles not building anymore due to changes in the package registry, but it was like 1-2 times out of 1000 of times I used docker.

Though we use docker for a different reason: we are building on linux, targeting linux. It is really easy to use the host system headers or library instead of the target versions of the same - and 99% of the time you will get away with it as they are the same. Then several years later you upgrade your host linux machine and can't figure out why a build that used to work isn't (and since 99% of the stuff is the same this can be months before you test that one exception and then it crashes). Docker ensures we don't install any host headers, or libraries except what is needed for the compiler.

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?

From what I've seen the "minor version" is fixed. e.g. FROM ubuntu:22.04 and not FROM ubuntu:laatest

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)

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?

Answer: Our office was in the WTC and was destroyed on 9/11. Luckily everyone got out alive, but then we discovered we had no off-site backups of the source code. In order to continue development, we had to retrieve the released binaries from our customers and decompile them to get back source code.

I don't know what the practice was at that time, but some years later, people weren't allowed to have the source on their laptops, they had to SSH/RDP/etc in to a hosted development system to work on it, which might explain how losing the office resulted in losing the source code even with people doing WFH

The first time I looked at it, I saw that the make file set the warning level to 0 and redirected the output to NUL. Removing that, I ran nmake and prayed. About 45 minutes later it finished building, evidently successfully. It turned out that having the warnings actually print added 10 minutes to the build.

It was averaging more than one error per line of code at /w3. And it was around 40k lines of code. Not large by the standards of today but huge back then for MSDOS. Peeking, it used K&R c and included no header files. So step 1 for me was to hook up some scaffolding using various tools from mks to make sure as I edited things that the error count didn’t increase.

The biggest thing I learned from this project was to not combine other coding with the warning removal or other cleanup. Makes it much easier to spot when you introduce bugs.

I think about that a lot now that I'm older and spend a fair bit of time in MRI machines...

Maybe VM clones of various users too, and recordings of their work flows?

/s

Step -4: Get the version of windows and the compiler it was last known to compile with.

Been there. Done that.

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)

Sometimes the test still passes, and that is a good sign that something is very wrong!

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.

Though in the last 6 years I've seen at least one case where truly deterministic builds mattered:

A performance bug only happens when a malloc() during init was not aligned to 32 bytes, glibc on x86_64 only guaranteed 16 bytes, but depending on what alloc / dealloc happened before it may just land on 32 bytes boundary.

The alloc / dealloc sequence before that point was pretty deterministic, however there were a few strings containing __FILE__. And gitlab runner checked-out codes to a path with random number (or an index? I don't remember) without -ffile-prefix-map or $PWD trick so its length varies.

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! ;)

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.

You don't even have to get that far. Shell extensions (for file open or save dialogs) and printer drivers also introduce arbitrary DLLs to your processes. And some of them are compiled in an old version of IIRC Delphi or Turbo Pascal, which on the DLL startup code unconditionally changes the floating point control word to something which causes unexpected behavior in some framework you're using.

(We ended up wrapping all calls to file open or save or print dialogs with code to save and restore the floating point control word, just in case they had loaded one of these annoying DLLs.)

But that sort of report isn't a deep mystery, it's just a specific class of bug. Given the description, you've got a pretty good idea of what you're looking for.

For a long-ago example: I worked on a project that had an optimizer that used time-bounded simulated annealing to optimize. No two builds ever the same. It was "great".

> The string can either be a number (decimal, octal or hex) or an arbitrary string (in which case it’s converted to a number by computing CRC32). > The string should be different for every file you compile.

So basically just pass in the project relative path to the file into random-seed and you'll be fine. It's a shame the guidance doesn't explain why the string should be different because that feels like it could be advice that's not rooted in any technical reality.

__time__ isn't actually that bad as it's an anti-pattern and much better for the build system to inject the build time explicitly as an input macro (if your software needs it for UX purposes).

__FILE__ is the more annoying one but can be solved through fmacro-prefix-map.

This is a good guy. Knows what they need, knows you are smart enough to potentially finally slay the dragon, will fight the bureaucracy on your behalf. Asking a hard ask is rarely beneficial for the asker on the failure side. Don't burn yourself out for it though and don't be afraid to ask hard favors from the asker.

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.

New warnings added to new compiler versions can identify problems that weren't previously detected. You _want_ those to -Werror when they happen, so you can fix them if they need it.

Changing a compiler version is a task that has to be resourced appropriately. Part of that is dealing with any fallout like this. Randomly updating your compiler is just asking for trouble.

> * 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.

Docker is too fraught with issues for that, and as anyone can attest, there are few things more frustrating in computing than having to follow down a chain of chasing issues in things only superficially related to what you actually want to do.

The least that can be done is for the project to do its best to not be dependent on specific versions, and explicitly document, in a visible place, the minimum and maximum versions known to work, along with a last changed date.

For some things the tradeoffs are much less clear, open-source or not e.g. a complex multi-platform GUI application. If you're going to ship a Flatpak to Linux users for example, then the utility of allowing any random build environments is not so clear; users will be running your produced binaries anyway. These are the minority of cases, though. (No, maybe not every user wants a Flatpak, but the developers also need to make decisions that balance many needs, and not everything will be perfect.)

Half of the problem, of course, is C and C++'s lack of consistent build environments/build systems/build tooling, but that's a conversation for another day.

That said, I generally agree with you that if you want to be a Good Citizen in the general realm of open-source C and C++ code, you should not use -Werror by default, and you should try (to whatever reasonable extent) to allow and support dependencies your users have. And try to support sanitizers, custom CFLAGS/CXXFLAGS, allow PREFIX and DESTDIR installation options, obey the FHS, etc etc. A lot of things have consolidated in the Linux world over the years, so this isn't as bad as it used to be -- and sometimes really does find legitimate issues in your code, or even issues in other projects.

The "utility" of allowing "any random build environment" is that those random build environments are the ones that exist on your user's computers, and absent a particularly good reason why it shouldn't work (like, your compiler is too old, or literally broken,) for the most part it should, and usually, it's not even that hard to make it work. Adopting practices like defaulting -Werror -Wall on and closing bugs as WONTFIX INVALID because it's not any of the blessed toolchains gains you... not sure. I guess piece of mind from having less open issues and one less flag in your CI? But it is sure to be very annoying to users who have fairly standard setups and are trying to build your software; it's pretty standard behavior to report your build failures upstream, because again, usually it does actually signal something wrong somewhere.

Developers are free to do whatever they want when releasing open source code. That doesn't mean that what they are doing is good or makes any sense. There are plenty of perfectly legal things that are utterly stupid to do, like that utterly bizarre spat between Home Assistant and NixOS.

- Who are the consumers of the source code, i.e. who will ever check it out and build it? Sometimes, it's just one person. Sometimes, it's a team of engineers. In that case, -W -Werror is fine.

- How does a warning being reported make the engineers on the team feel? If the answer is, "Hold my beer for five minutes while I commit a fix", then -W -Werror might be the right call. I've been on projects like that and some of them had nontrivial source code consumers.

- How easy is it to hack the build system? Some projects have wonderfully laid out build systems. If that's the case and -W -Werror is the default, then it's not hard to go in there and change the default, if the -Werror creates problems.

- Does the project have a facility (in the build system) and policy (as a matter of process) to just simply add -Wno-blah-blah as the immediate fix for any new warning that arises? I've seen that, too.

(I'm using -Werror in some parts of a personal project. If you're a solo maintainer of a codebase that can be built that way, then it's worth it - IMO much lower cognitive load to never have non-error warnings. The choice of what to do when the compiler complains is a more straightforward choice.)

Most libraries not written by Google have some kind of backwards compatibility policy. This is for good reasons. For example, if Debian updates libpng because there's a new RCE, it's ideal if they can update every package to the same new version of libpng all at once. If we go to the extreme of "exact dependencies for every package", then this would actually mean that you have to update every dependent package to a new release that has the new version of libpng, all at the same time, across all supported versions of the distribution. Not to mention, imagine the number of duplicate libraries. Many Linux distros, including Debian, have adopted a policy of only having one version of any given library across the whole repo. As far as I understand, that even includes banning statically linked copies, requiring potentially invasive patching to make sure that downstream packages use the dynamically linked system version. And trust me, if they want to do this, they *will* do this. If they can do it for Chromium, they sure as hell can do it for literally any package.

There's a balance, of course. If a distro does invasive patching and it is problematic, I think most people will be reasonable about it and accept that they need to report the issue to their distribution instead. Distros generally do accept bugs for the packages that they manage, and honestly for most packages, by the time a bug gets to you, there is a pretty reasonable chance that it's actually a valid issue, so throwing away the issue simply because it came from someone running an "unofficial" build seems really counterproductive and definitely not in the spirit of open source.

Reproducibility is good for many reasons. I do not feel it is a good excuse to just throw away potentially valid bug reports though. It's not that maintainers are under any obligation to actually act on bug reports, or for that matter, even accept them at all in the first place, but if you do accept bugs, I think that "this is broken in new version of Clang" is a very good and useful bug report that likely signals a problem.

It Debian is upgrading a dependency instead of a developer, then Debian should be ready to fix any bugs they introduce.

>then this would actually mean that you have to update every dependent package to a new release that has the new version of libpng, all at the same time, across all supported versions of the distribution

This is already how it works. All vulnerable programs make an update and try to hold off in releasing it until near an embargo date. You don't have to literally update them all at the same time. It's okay of some are updated at different times than others.

>Not to mention, imagine the number of duplicate libraries.

Duplicate libraries are not an issue.

>Many Linux distros, including Debian, have adopted a policy of only having one version of any given library across the whole repo.

This is a ridiculous policy to me as you are forcing programs to use dependencies they were not designed for. This is something that should be avoided as much as possible.

>by the time a bug gets to you, there is a pretty reasonable chance that it's actually a valid issue

That doesn't mean there isn't damage done. There are many people who consider kdenlive an unstable program that constantly crashes because of distros shipping it with the incorrect dependencies. This creates reputational damage.

Software should be extensively tested and code review should be done before it gets shipped to users. Most users don't know about the Debian Bug Tracking system, but they do know about upstream.

>Honestly, this whole tangent is pointless. Distributions like Debian have been operating like this for like 20+ years. It's dramatically too late to argue about it now, but if you're going to, this is not exactly the strongest argument.

It's not too late as evidence by the growth of solutions like appimage and flatpak which allows developers to avoid this.

>So let's say I want to depend on two libraries, and both of them eventually depend on two different but compatible versions of a library, and only one of them can be loaded into the process space. Is this a made-up problem? No, this exact thing happens constantly, for example with libwayland.

Multiple versions of a library can be loaded into the same address space. Developers can choose to have their libraries support a range of versions.

>that's why we have shared libraries and semver to begin with

Hyrum's Law. Semver doesn't prevent breakages on minor bumps.

That's why distributions have multiple branches. Debian Unstable packages get promoted to Debian Testing, which get promoted to a stable Debian release. Distributions do bug tracking and testing.

> Most users don't know about the Debian Bug Tracking system, but they do know about upstream.

There are over 80,000 bugs in the Debian bug tracker. There are over 144,000 bugs in the Ubuntu bug tracker. It would suffice to say that a lot of users indeed know about upstream bug trackers.

I am not blaming anyone who did not know this. It's fully understandable. (And if you ask your users to please go report bugs to their distribution, I think most distributions will absolutely not blame you or get mad at you. I've seen it happen plenty of times.) But just FYI, this is literally one of the main reasons distributions exist in the first place. Most people do not want to be in charge of release engineering for an entire system's worth of packages. All distributions, Debian, Ubuntu, Arch, NixOS, etc. wind up needing THOUSANDS of at least temporarily downstream patches to make a system usable, because the programs and libraries in isolation are not designed for any specific distribution. Like, many of them don't have an exact build environment or runtime environment.

Flatpak solves this, right? Well yes, but actually no. When you target Flatpak, you pick a runtime. You don't get to decide the version of every library in the runtime unless you actually build your own runtime from scratch, which is actually ill-advised in most cases, since it's essentially just making a Linux distribution. And yeah. That's the thing about those Flatpak runtimes. They're effectively, Linux distributions!

So it's nice that Flatpak provides reproducibility, but it's absolutely the same concept as just testing your program on a distro's stable branch. Stable branches pretty much only apply security updates, so while it's not bit-for-bit reproducible, it's not very different in practice; Ubuntu Pro will flat out just default to automatically applying security updates for you, because the risk is essentially nil.

> It's not too late as evidence by the growth of solutions like appimage and flatpak which allows developers to avoid this.

That's not what AppImage is for, AppImage is just meant to bring portable binaries to Linux. It is about developers being able to package their application into a single file, and then users being able to use that on whatever distribution they want. Flatpak is the same.

AppImage and Flatpak don't replace Linux distribution packaging, mainly because they literally can not. For one thing, apps still have interdependencies even if you containerize them. For another, neither AppImage nor Flatpak solve the problem of providing the base operating system for which they run under, both are pretty squarely aimed at providing a distribution platform specifically for applications the user would install. The distribution inevitably still has to do a lot of packaging of C and C++ projects no matter what happens.

I do not find AppImage or Flatpak to be bad developments, but they are not in the business of replacing distribution packaging. What it's doing instead is introducing multiple tiers of packaging. However, for now, both distribution methods are limited and not going to be desirable in all cases. A good example is something like OBS plugins. I'm sure Flatpak either has or will provide solutions for plugins, but today, plugins are very awkward for containerized applications.

> Multiple versions of a library can be loaded into the same address space. Developers can choose to have their libraries support a range of versions.

Sorry, but this is not necessarily correct. Some libraries can be loaded into the address space multiple times, however, this is not often the case for libraries that are not reentrant. For example, if your library has internal state that maintains a global connection pool, passing handles from one instance of the library to the other will not work. I use libwayland as an example because this is exactly what you do when you want to initialize a graphics context on a Wayland surface!

With static linking, this is complicated too. Your program only has one symbol table. If you try to statically link e.g. multiple versions of SDL, you will quickly find that the two versions will in fact conflict.

Dynamic linking makes it better, right? Well, not easily. We're talking about Linux, so we're talking about ELF platforms. The funny thing about ELF platforms is that the way the linker works, there is a global symbol table and the default behavior you get is that symbols are resolved globally and libraries load in a certain order. This behavior is good in some cases as it is how libpthreads replaces libc functionality to be thread-safe, in addition to implementing the pthreads APIs. However it's bad if you want multiple versions, as instead you will get mostly one version of a library. In some catastrophic cases, like having both GTK+2 and GTK3 in the same address space, it will just crash as you call a GTK+2 symbol that tries to access other symbols and winds up hitting a GTK3 symbol instead of what it expected. You CAN resolve this, but that's the most hilarious part: The only obvious way to fix this, to my knowledge, is to compile your dependencies with different flags, namely -Bsymbolic (iirc), and it may or may not even compile with these settings; they're likely to be unsupported by your dependencies, ironically. (Though maybe they would accept bug reports about it.) The only other way to do this that I am aware of is to replace the shared library calls with dlopen with RTLD_LOCAL. Neither of these options are ideal though, because they require invasive changes: in the former, in your dependencies, in the latter, in your own program. I could be missing something obvious, but this is my understanding!

> Hyrum's Law. Semver doesn't prevent breakages on minor bumps.

Hyrum's law describes buggy code that either accidentally or intentionally violates contracts to depend on implementation details. Thankfully, people will, for free, report these bugs to you. It's legitimately a service, because chances are you will have to deal with these problems eventually, and "as soon as possible" is a great time.

Just leaving your dependencies out of date and not testing against newer versions ever will lead to ossification, especially if you continue to build more code on top of other flawed code.

Hyrum's law does not state that it is good that people depend on implementation details. It just states that people will. Also, it's not really true in practice, in the sense that not all implementation details will actually wind up being depended on. It's true in spherical cow land, but taking it to its "theoretical" extreme implies infinite time and infinite users. In the real world, libraries like SDL2 make potentially breaking changes all the time that never break anything. But even when people do experience breakages as a result of a change, sometimes it's good. Sometimes these breakages reveal actual bugs that were causing silent problems before they turned into loud problems. This is especially true for memory issues, but it's even true for other issues. For example, a change to the Go programming language recently fixed a lot of accidental bugs and broke, as far as anyone can tell, absolutely nobody. But it did lead to "breakages" anyways, in the form of code that used to be accidentally not actually doing the work it was intended to do, and now it is, and it turns out that code was broken the whole time. (The specific change is the semantics change to for loop binding, and I believe the example was a test suite that accidentally wasn't running most of the tests.)

Hyrum's law also describes a phenomena that happens to libraries being depended on. For you as a user, you should want to avoid invoking Hyrum's law because it makes your life harder. And of course, statistically, even if you treat it as fact, the odds that your software will break due to an unintended API breakage is relatively low; it's just higher that across an entire distribution's worth of software something will go wrong. But for your libraries, they actually know that this problem exists and do their best to make it hard to rely on things outside the contract. Good C libraries use opaque pointers and carefully constrain the input domain on each of their APIs to try to expose as little unintended API surface area as humanly possible. This is a good thing, because again, Hyrum's law is an undesirable consequence!

If some warnings are flaky then disable them specifically. In my experience most warnings in -Wall are OK and you can suppress the rare false positives in code. Don't suppress without a comment.

edit:

Having said that there are entirely valid reasons to not have -Werror outside of CI. It should be absolutely disabled by default if you distribute source.

As you fix the inherited ones you can regenerate the file, hopefully smaller each time.

"If I don't have time to fix all of this -now-, I can at least make sure it can't get any -worse- in the mean time" is a very useful approach when automating the 'make sure' is something quick enough that you can find time for that.

If you are delivering source code to someone else (or getting source code from someone else that you build but do not otherwise work on then you should ensure warnings are disabled for those builds. However all developers and your CI system still needs to run with all warnings possible.

The days when compilers put in warnings that were of questionable value are mostly gone. Today if you see a warning from your compiler it is almost always right.

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.

Things that get flagged by a static analysis tool (today) will often be areas where you can tear out entire functions and maybe even classes and files because they'll be a re-creation of STL concepts. Like homegrown iterator libraries (with subtle problems) that can be replaced with the STL algorithms library, or homegrown smart pointers that can just be replaced with actual smart pointers, or replacing the C string functions with C++'s on string class (and related classes) and functions/methods.

But you won't see that as easily until you start scanning the code. And you won't be able to evaluate the consequences until you push towards more rapid test builds (at least) if not deployment.

That said, sure, skip them if you're worried about the history getting messed up or use them more selectively.

Not sure if I agree here or not - whilst yes, the history isn't there, if it's a small enough team you'll have a good guess at who wrote it.

Definitely found I've learnt the style of colleages so know who to ask just from the code outline.

Then again, I think most of the tells for that for me are around the sort of structure that would survive reformatting anyway.

(and, y'know, legacy stuff, everything's a bloody trade-off)

edit:typo

I think after a year or so I realised that even bothering to -try- to use SourceSafe was largely silly, got permission to stop, and installed a CVS server on a dev box for my own use.

(yes I know the VCS server shouldn't really be on the dev box I could potentially trash, I didn't have another machine handy and it was still a vast improvement)

It's all about the team or organization and their laziness or non-laziness.

I was very worried when we transitioned to git that history will not be preserved and tried to preserve it, but it proved too much hassle so I dropped it.

In fact that proved not to be a problem. Well, not a problem for me, since I remember all the history of the code and all the half forgotten half baked features and why they are there. But if I'm gone then yes, it's going to be a problem. It's in a dire need for a rewrite, but this has been postponed again and again.

This is assuming Git of course, which is not a given at all for the average legacy c++ codebase.

See "git blame ignore revs file".

Intended use is exactly to ignore bulk changes like auto formatting.

  man git-blame
  git help blame

Not saying that fixes decades of cruft because you shouldn't change files without good reason and non-white space formatting is not a good reason, but I'm mentioning it because I've seen people naively belief bullshit like "code is self explanatory" and "the reason is in the commit message"

Just comment your code folks, this becomes less of a problem

I guess if it splits or combines lines that could cause some noise if you really want the history of a single line... But that happens all the time, and I don't see how it would really prevent understanding the history. You can always do a blame on a range of lines.

Maybe I'm missing something though, genuinely curious for a concrete example where reformatting makes it hard to understand history!

Adding it will remove white space nitpicking from code review, even if it isn't perfect.

1. You're going to reformat it eventually anyway. You're just delaying things. The best time to plant a tree, etc.

2. If it's an old codebase and you're trying to understand some bit of code you're almost always going to have to walk through about 5 commits to get to the original one anyway. One extra formatting commit doesn't really make any difference.

And you're jumping around - and you'll have to! - odds are you'll have a bunch of things changed locally, and might accidentally create a commit that doesn't separate out one concern from another. CI will be a godsend at that point.

If going the route, I'd recommend commenting out the lines rather than removing them outright to simplify the diffs at least until you're ready to squash and merge the branch.

I used to use a tool called Source Navigator (written in Tcl/tk!) that was great at indexing code bases. You could then check the Call Hierarchy of the current method, for example, then use that to make UML Sequence Diagrams. A similar one called Source Insight shown below [1].

And oh, notes. Writing as if you're teaching someone is key.

Over the years, I got quite good at comprehending code, even code written by an entire team over years. For a brief period, I was the only person actively supporting and developing an algorithmic trading code base in Java that traded ~$200m per day on 4 or 5 exchanges. I had 35 MB of documentation on that, lol. Loved the responsibility (ignoring the key man risk :|). Honestly, there's a lot of overengineering and redundancy in most large code bases.

[1] References in "Source Insight" https://d4.alternativeto.net/6S4rr6_0rutCUWnpHNhVq7HMs8GTBs6...

I can't believe I'm finding someone in the wild that also has used Source Navigator.

My university forced this artifact on me in the computer architecture course because it has some arcane feature set + support for an ARM emulator that isn't found elsewhere. We used it for bare metal ARM assembly programming

I'm pretty sure my stomach did somersaults on that.

But as for the advice:

>Get out the chainsaw and rip out everything that’s not absolutely required to provide the features your company/open source project is advertising and selling

I hear you, but this is incredibly dangerous. Might as well take that chainsaw to yourself if you want to try this.

It's dangerous for multiple reasons. Mainly it's a case of Chesterton's fence. Unless you fully understand why X was in the software and fully understand how the current version of the software is used, you cannot remove it. A worst case scenario would be that maybe a month or so later you make a release and the users find out an important feature is subtly broken. You'll spend days trying to track down exactly how it broke.

>Make the project enter the 21st century by adding CI, linters, fuzzing, auto-formatting, etc

It's a nice idea, but it's hard to do. One person is using VIM, another is using emacs, another is using QTCreator, another primarily edits in VSCode.. Trying to get everyone on the same page about all this is very, very hard.

If it's an optional step that requires that they install something new (like commit hook) it's just not going to happen.

Linters also won't do you any good when you open the project and 2000+ warnings appear.

The things the author listed there are commonly not IDE integrated. I've never seen a C++ development environment where cpplint/clang-tidy and fuzzers are IDE integrated, they're too slow to run automatically on keystrokes. Auto-formatting is the only one that is sometimes integrated. All of this stuff you can do from the command line without caring about each user's chosen development environment. You should definitely at least try rather than giving up before you start just because you have two different text editors in use. This is C++; if your team won't install any tools, you're gonna have a bad time. Consider containerizing the tools so it's easier.

CLion from JetBrains has clang-tidy integrated (real-time).

The project size is probably a lot smaller than what most people are working on though, and I have a fast CPU and NVME disk, but it's definitely possible to do!

I'm not sure about the fuzzer part though.

On the editor/IDE of your choose.

I wouldn't call it fast, but it is quite usable.

This is what's wrong with our industry, and it's no longer an acceptable answer. We're supposed to be fucking professional, and if a job needs to build a tool chain from the IDE up we need to learn to use it and live with it.

Built on my machine, with my IDE, the way I like it and it works is not software. It's arts and fucking crafts.

First guy: hand saw and impact driver... cut and screw Second guy: Power Saw, and hammer. Cut and nail. Third guy: Safety glasses and a Nail gun. Forth guy shows up: compressor, asks where the power is (none) and if he can use some tools.

It would not work. You dont build a CNC production line for parts with every CNC being unique. We dont let devs pick what their production server OS is, we dont let them choose random languages. Tooling matters.

The other saw was hand forged in a basement by the user, breaks every other day, and has a totally different blade width, and can only be used by the owner.

The GP was suggesting the effort to add CI, linters, fuzzing, auto-formatting, etc was too hard. If that can be abandoned entirely, perhaps the legacy codebase isn't providing enough value, and the effort to maintain it would be better spent replacing it. But the implication is that the value outweighs the costs.

Put all the linters, fuzzing, and format checking in an automated build toolchain. Allow individuals to work how they want, except they can't break the build. Usually this will reign in the edge cases using inadequate tools. The "built on my machine, with my IDE, the way I like it and it works" is no longer the arbiter of correct, but neither does the organization have to deal with the endless yak shaving over brace style and tool choice.

I hear you, but an organization that fears this, instead of Just Pick Something And Deal With It, is an organization that probably doesn't have the right people in it to succeed at any task more arduous than that.

I use vim, IntelliJ, Visual Studio, and VSCode at least once every two weeks apiece, and it's no skin off my back to switch. Do thou likewise.

For those that dont know, Arts and Crafts movement in the states is known for some pretty interesting pottery that was produced at scale.

https://excellentjourney.net/2015/03/04/art-fear-the-ceramic...

I must have missed the memo where I could just say no to basic things my boss requires of me. You know, the guy that pays my salary.

It's ok if you use your own tooling you are comfortable with, but you should adapt to their workflow, and the employer has no obligation to tweak their workflow to integrate your own, it's yours to adapt.

As far as linters causing thousands of warnings to appear on opening the project, the developer adding the linter should make sure that the linter returns no warnings before they merge that change. This can be accomplished by disabling the linter for some warnings, some files, making some fixes, or some combination of the above.

Bullshit, all of these (and additionally C lion) are fairly easy to configure these for, with the possible exception of QTCreator (not a ton of experience on my end).

Just make it a CI requirement, and let everyone figure it out for their own tools. If you can't figure that out, you get to run it as a shell script before you do your PRs. If you can't figure that out, you probably shouldn't be on a legacy C++ project.

I don't get your point. You know you can autoformat outside editors right? Just configure pre-commit and run it in CI. It's trivial.

> If it's an optional step that requires that they install something new (like commit hook) it's just not going to happen.

It will because if they don't then their PRs will fail CI.

This is really basic stuff, but knowledge of how to do CI and infra right does seem to vary massively.

If this is a function that no one links to, and your project does not mess with manual dynamic linking (or the function is not exposed), then it's pretty safe to remove it. If it's internal utility which does not get packaged into final release package, it is likely be safe to remove too. If it's a program which does not compile because it requires Solaris STREAMS and your targets are Linux + MacOS - kill it with fire.

(Of course removing function calls, or removing functionality that in-use code depends on, is dangerous. But there is plenty of stuff which has no connection to main code)

  0. You reach out to the previous maintainers, visit them, buy them tea/beer and chat (eventually) about the codebase. Learned Wizards will teach you much.

But I'd def be going for that sweeet, sweet low-hangin' fruit of just talking to the ol' folks who came that way before. Haha :)

Try to work on it a little bit first, and once you get stuck in various places, now you can talk to the previous maintainers, it will be much more productive. They will also appreciate the effort.

I did encounter a bunch of bugs and problems underway, and wanted to reach out to the devs that wrote it - as it was customer made for my employer.

Welp, turns out it was written by one guy/contractor, and that he had passed away a couple of years earlier.

At least in the defense industry you'll find these sort of things all the time. Lots of custom/one-off stuff, made for very specific systems. Especially on the hardware side it is not uncommon that the engineers that made the equipment are either long gone or retired.

Interesting point about the "author expiry" window. I was thinking about that today regarding something else:

Let's say in 20 years time, no database code has been updated for the last 20 years. And all the people who worked on it, can't remember anything about it. Yet, it still works.

That means that everyone who uses that database everyday, doesn't know how it works. They believe that it works, this belief is widespread. And the database providing results to queries, is a real thing. And it does work -- but nobody knows how.

This is common. I don't know in any detail how the MacBook I use works. But it does. I don't know how many things I use actually work. But they do work.

It seems the only difference, in the world of things that "work", and which most people who use them do not understand how they work, is that there are two classes of things: those things for which there is a widespread belief that they do work; and those things for which the belief that they work, is not widespread. But in either case, they work.

Lots of old early-gen PLCs from the 70s/80s still ticking, with no documentation and the techs/engineers/companies long gone.

We worked on one such PLC, around 3 decades old at the point, and it came down to probing I/O, reverse engineering the functionality.

But at some point, if there hasn't been enough legacy support, there comes a time where people just have to bite the bullet and re-build a system from the ground up - and integrate it in parallel with the old system running, until it can be removed completely.

Too bad many of the old and forgotten systems are still running and integral, so they get put inside a glass cage with "DON'T TOUCH!" warning sticker.

> 0. You reach out to the previous maintainers, visit them, buy them tea/beer and chat (eventually) about the codebase. Learned Wizards will teach you much.

Have you ever tried that? This is legacy code. Even if the handover was yesterday, they cannot tell you about anything useful they did more than 6m in the past.

And that's the best-case scenario. The common-case is "That's the way I got it when it was handed over to me" answer to every question you ask.

Especially if you manage to get to just chatting about the project - at some point they'll almost certainly go "oh! while I remember," followed by the answer to a question you didn't realise you should've asked.

The value is often in the commiseration rather than the interrogation, basically.

After all, if you have inherited a codebase with no tests, with build process which fails every other time, with unknown dependency info, and which can only be built on a single server with severely outdated OS... are you sure the previous maintainer is a real wizard and all the problems are result of not enough time? Or are they a "wizard" who keep things broken for job security and/or because they don't want to learn new things?

    > You reach out to the previous maintainers, visit them

Monumentally disagree. One-off session with a guy who knows the codebase inside out can save you days of research work. Plus telling you all about the problematic/historical areas.

But the devil is in all the tiny details. What is this tiny correction factor that was added 20 years ago? Why was this value cut off to X decimals? Why didn't they just do Y here? Why do we override this default behavior?

It's tens of thousands of tiny questions like that which you can't ask until you're there.

It's not a bad thing, and it's certainly good to do, but it's not a solution to the problem.

Effectively everything becomes a "drop in the bucket".

Like ... it doesn't help that much. Mainly it saves some frustration of the build system is insane or the source code is spread out in mails, a hd and a floppy in a drawer or the current source state is broken and need to be reverted.

But if the original author is there to do a handover, the chances are the company is properly run and the work will be a breeze anyway becouse the code is good and well structured.

Ideal is if they have a life such that something morally equivalent to "how about we meet up and I'll pay for the food/beer" is a viable thing to suggest later.

Oh, and always remember - the way to a geek's schedule is often through their wife. If you get a chance to meet their partner, TAKE IT and be on your best behaviour, if $partner likes you then you have massively increased chances of making useful things happen later.

So basically you get to go from: 0 - can do nothing at all; to 1 - being able to feel confident diving in and looking around, framed with the downloaded conceptual models, knowing how to hit the ground trotting, if not running.

To me, 0.1 is more like you slugged it out for a couple hours poking around by yourself, and now you know a handful of confusing things with low confidence.

Chatting to one who has been there, done that lets you have more confidence, and know more actionable stuff, which is highly useful. I suppose it depends on your conversation, listening and comprehension skills tho! Hahaha! :)

But seriously not everybody learns well in that situation. I just happen to. I'd much rather talk it over with someone and absorb, than watch a video or read a tutorial.

Finally -- haha! :) -- I guess there are some geeks to whose schedule the way is through their husband or whatevs haha! :)

Normally, employment agreements specifically restrict contact with former staff, or discussions of sensitive matters like compensation packages.

C++ is like any other language, in that it will often take 3 times longer to understand something than re-implement the same. If you are lucky, than everything is a minimal API lib, and you get repetitive examples of the use cases... but the cooperative OSS breadcrumb model almost never happens in commercial shops...

Legacy code bases can be hell to work with, as you end up with the responsibility for 14 years of IT kludges. Also, the opinions from entrenched lamers on what productivity means will be painful at first.

Usually, with C++ it can become its own project specific language variant (STL or Boost may help wrangle the chaos).

You have my sympathy, but no checklist can help with naive design inertia. Have a wonderful day. =)

Indeed, if the legal encumbrance is not legal, than its often unenforceable.

Talking with your own lawyers before doing something silly is a good habit. ;-)

My step 0 would be: run it through an UML tool to get a class diagram and other diagrams.

This will help you a lot.

> Get the tests passing on your machine

Tests? On a C++ codebase? I like your optimism, rs

You may luck out with auto-generated documentation, but few use these tools properly (javadoc or doxygen). =)

OO requires a lot of planning to get right (again, no shop will give your team time to do this properly), and in practice it usually degenerates into spiral development rather quickly (

500k lines is not that bad if most of it is encapsulated libraries... =)

If it’s a big enough change, export whatever you need out of the legacy code (by calling an external function/introducing a network layer/pulling the exact same code out into a library/other assorted ways of separating code) and do the rest in a fresh environment.

I wouldn’t try to do any major refactor unless several people are going to work on the code in the future and the code needs to have certain assumptions and standards so it is easy for the group to work together on it.

In few lucky cases. In real life new feature is most likely change in behavior of already existing one and suddenly you have to do some heavy refactoring in numerous places.

one thing I always do is throwaway major refactors. its the fastest way for me to learn what the structure is, what depends on what, and what's really kinky. and I might just learn enough to do it for real should it become necessary.

Thank you for providing me a term for this! I indeed learned a lot from doing this because some things can only be understood by hitting them with a hammer, putting them together again and observing where that doesn't work.

its software. we should take full of advantage of its plasticity.

  * You can find out the most relevant files/functions for your upcoming works. If some functions/files have been frequently changed, then it's going to be the hot spot for your works. Focus on them to improve your quality of life. If you want to introduce unit tests? Then focus on the hot spot. Suffer from lots of merge conflicts? The same.
  * You can also figure out correlation among the project and its source files. Some seemingly distant files are frequently changed together? Those might suggest an implicit structure that not might be clear from the code itself. This kind of information from external contexts can be useful to understand the bird's eye view.
  * Real ownership models of each module can be inferred from the history. Having a clear ownership model helps, especially if you want to introduce some form of code review. If some code/data/module seems to have unclear ownership? That might be a signal for refactoring needs.
  * Specific to C/C++ contexts, build time improvements could be focused on important modules, in a data driven way. Incremental build time matters a lot. Break down frequently changed modules rather than blindly removing dependencies on random files. You can even combine this with header dependency to score the module with the real build time impact. 

I would break this down:

a) CI - Ensure not just you can build this, but it can be built elsewhere too. This should prevent compile-based regressions.

b) Compiler warnings and static analysers - They are likely both smarter than you. When it says "warning, you're doing weird things with a pointer and it scares me", it's a good indication you should go check it out.

c) Unit testing - Set up a series of tests for important parts of the code to ensure it performs precisely the task you expect it to, all the way down to the low level. There's a really good chance it doesn't, and you need to understand why. Fixing something could cause something else to blow up as it was written around this bugged code. You also end up with a series of regression tests for the most important code.

n) Auto-formatting - Not a priority. You should adopt the same style as the original maintainer.

> 5. If you can, contemplate rewrite some parts in a memory safe language

The last step of an inherited C++ codebase is to rewrite it in a memory safe language? A few reasons why this probably won't work:

1. Getting resources to do additional work on something that isn't broken can be difficult.

2. Rather than just needing knowledge in C++, you now also need knowledge in an additional language too.

3. Your testing potentially becomes more complex.

4. Your project likely won't lend itself to being written in multiple languages, due to memory/performance constraints. It must be a significantly hard problem that you didn't just write it yourself.

5. You have chosen to inherit a legacy codebase rather than write something from scratch. It's an admittance that you don't have some resource (time/money/knowledge/etc) to do so.

Simply getting rid of any actually memory unsafe C++ and enforcing guidelines will do this for you in the C++ codebase.

"Rewrite it in X" only adds complexity because it's the flavour of the month as you said in your comment.

Author is already doing the work of rewriting large chunks of the codebase in C++, they may as well follow and implement a more restrictive subset of the language, I find High integrity C++ to be good. If I can get my hands on the latest MISRA standard that is likely good as well. These may not be "required" but they specify what is enforced in . So instead of having to reskill your entire devteam on a new language which has many many sharp edges, how about just having your dev team use the language they already know and enforce guidelines to avoid known footguns.

C++ has had RAII forever, since C++11(2024 btw now) have actually good wrappers, Box = std::unique_ptr, whatever the ref counter version is = std::shared_pte.

Do the other things he already said to do, ie clang-tidy with the correct rule will warn/error on any raw pointer usage. You don't need to "not make mistakes". If you never use raw pointers, other than these specific places you tell the linter that it was fine and you have checked then by default it will be memory safe. Does that sound familiar?

"But we don't need a linter in Rust!" It's just built into the LLVM frontend called the rust compiler. If it bugs you so much build a custom executable that runs the linter then the c++ compiler and call it your own internal compiler. First line of code can be "#!/bin/sh"...

If you say you aren't talking about Rust but one of the GC languages, then I agree with you, write it in that other language but then the correct solution is to rewrite the software in the first place since it was never written in the correct language to start with. Rewriting something in Rust is likely the same amount of complexity as fixing the C++ code, if you however first need to learn Rust well there's a reason I am not exactly pro rust ans yes I bave tried to use it in a proper complex project, all deadlines were missed and we ended up writing it in modern safe C++ instead purely because the language didn't force us to make up some obscure abstraction to appease the borrow checker.

It is strange that the author complains so much about automating BOMs, package versioning, dependency sources, etc, and then proceeds to suggest git submodules as superior to package managers.

The author needs to try vcpkg before making these criticisms; almost all of these are straightforwardly satisfied with vcpkg, barring a few sharp edges (updating dependencies is a little harder than with git submodules, but that's IMO a feature and not a bug—dependencies are built in individual sandboxes which are then installed to a specified directory. vcpkg can set internal repositories as the registry instead of the official one, thus maintaining the 'vendored in' aspect. vcpkg can chainload toolchains to compile everything with a fixed set of flags, and allows users to specify per-port customisations.

These are useful abstractions and it's why package managers are so popular, rather than having everyone deal with veritable bedsheets' worth of strings containing compile flags, macros, warnings, etc.

I’d say that most C++ developers will generally have an easy time using Rust and will get equivalent performance.

But sometimes the project did not have a good reason to be in C++ in the first place and I’ve seen successful rewrites in Java for example.

Apple is rewriting some C++ code in Swift, etc. So, the language the team/company is comfortable with is a good rule of thumb.

We’ve reached the rewrite-in-rust meme stage of questioning whether the author is a nefarious crypto-Rust programmer in lieu of not being able to complain about it (since it wasn’t brought up!).

The approach I've taken is, when you do work on a function and find that it uses a global variable, try to add the GV as a function parameter (and update the calling sites). Even if it's just a pointer to the global variable, you now have another function that is more easily testable. Eventually you can get to the point where the GV can be trivially changed to a local variable somewhere appropriate.

Most of the time I spend with C++ code revolves around figuring out compile/link errors. Heaven forbid you need to deal with non-portable `make` files that for some reason work on the old box, but not yours... Oh, and I hope you have a ton of spare space because of some reason building a 500k exe takes 4GB.

Keep in mind, this advice only applies to inherited C++ code bases. If you've written your own or are working on an actively maintained project these are non-issues. Sort-of.

Be careful about that. Hyrum's Law and all.

Perhaps if it's a change requested by the organization or the users. Just don't go "fixing" things that look like bugs without knowing if it's really a bug or expected behavior.

Except every legacy C++ codebase I've worked on is decades old. Just enumerating the different "features" is a fool's errand. Because of reshuffling and process changes, even marketing doesn't have a complete list of our "features". And even it there was a complete list of features, we have too many customers that rely on spacebar heating[0] to just remove code that we think doesn't map to a feature.

That's if we can even tease apart which bits of code map to a feature. It's not like we only added brand new code for each feature. We also relied on and modified existing code. The only code that's "safe" to remove is dead code, and sometimes that's not as dead as you might think.

Even if we had a list of features and even if code mapped cleanly to features, the idea of removing all code not related to "features your company is advertising or selling" is absurd. Sometimes a feature is so widely used that you don't advertise it anymore. It's just there. Should Microsoft remove boldface text from Word because they're not actively advertising it?

The only way this makes sense is if the author and I have wildly different ideas about what "legacy" means.

[0] https://xkcd.com/1172/

> Except every legacy C++ codebase I've worked on is decades old. Just enumerating the different "features" is a fool's errand. Because of reshuffling and process changes, even marketing doesn't have a complete list of our "features".

Yeah, this struck me also, and your post should be modded up more. Anyone with significant experience in development knows what "Legacy" means.

Regardless of the language, after a specific point in a product's lifetime you cannot "know" all the features. Just not possible, no matter how well you think you documented it.

In an old product, every single line of code is there because of a reason that is not in the docs. Some examples that I've seen:

1. Using `int8_t` because at some point we integrated a precompiled library that was compiled with signed char, and we want warnings to pop up when we mix signs.

2. Wrote our own stripped-down SSL library because OpenSSL was not EMV certified at the time and did not come with the devkit. Now callers depend on a feature in our own library.

3. Client calls our DLL with Windows-specific UTF-16 strings. That's why that function has three variants that take 3 different types of strings.

4. This library can't be compiled with any gcc/glibc newer than X.Y, because the compiled library is loaded with `dlopen` in some environments.

5. We have our own 'safe' versions of string functions, because MSVC which takes the same parameter types for those functions assigns different meanings to the `size` parameter.

6. Converting fixed-precision floats to an int, performing the additions, and then converting the last division is faster and more accurate, but the test suite at the client expects the cumulative floating point errors and the test will fail.

Not to mention uncountable "marketing said this is not offered as a feature, but client depends on it" things.

removing a feature is possibly the most politically intractable thing you can try to do with a legacy codebase, almost never worth trying

We built our own find-dead-code tool, because the extant ones were imprecise, and boy oh boy did they find lots of dead stuff. And more dead stuff. And more dead stuff. Like peeling an onion, it went on for quite a while. But totally worth it in the end, made various improvements much easier.

[1] https://www.joelonsoftware.com/2000/04/06/things-you-should-...

I switched from Python to C++ because Cython, Numba, etc. just weren't cutting it for my CPU-intensive research needs (program synthesis), and I've never looked back.