1# OpenBMC Anti-patterns 2 3From [Wikipedia](https://en.wikipedia.org/wiki/Anti-pattern): 4 5 6"An anti-pattern is a common response to a recurring problem that is usually 7ineffective and risks being highly counterproductive." 8 9 10The developers of OpenBMC do not get 100% of decisions right 100% of the time. 11That, combined with the fact that software development is often an exercise in 12copying and pasting, results in mistakes happening over and over again. 13 14 15This page aims to document some of the anti-patterns that exist in OpenBMC to 16ease the job of those reviewing code. If an anti-pattern is spotted, rather 17that repeating the same explanations over and over, a link to this document can 18be provided. 19 20 21<!-- begin copy/paste on next line --> 22 23## Anti-pattern template [one line description] 24 25### Identification 26(1 paragraph) Describe how to spot the anti-pattern. 27 28### Description 29(1 paragraph) Describe the negative effects of the anti-pattern. 30 31### Background 32(1 paragraph) Describe why the anti-pattern exists. If you don't know, try 33running git blame and look at who wrote the code originally, and ask them on the 34mailing list or in IRC what their original intent was, so it can be documented 35here (and you may possibly discover it isn't as much of an anti-pattern as you 36thought). If you are unable to determine why the anti-pattern exists, put: 37"Unknown" here. 38 39### Resolution 40(1 paragraph) Describe the preferred way to solve the problem solved by the 41anti-pattern and the positive effects of solving it in the manner described. 42 43<!-- end copy/paste on previous line --> 44 45## Explicit listing of shared library packages in RDEPENDS in bitbake metadata 46 47### Identification 48``` 49RDEPENDS_${PN} = "libsystemd" 50``` 51 52### Description 53Out of the box bitbake examines built applications, automatically adds runtime 54dependencies and thus ensures any library packages dependencies are 55automatically added to images, sdks, etc. There is no need to list them 56explicitly in a recipe. 57 58Dependencies change over time, and listing them explicitly is likely prone to 59errors - the net effect being unnecessary shared library packages being 60installed into images. 61 62Consult 63https://www.yoctoproject.org/docs/latest/mega-manual/mega-manual.html#var-RDEPENDS 64for information on when to use explicit runtime dependencies. 65 66### Background 67The initial bitbake metadata author for OpenBMC was not aware that bitbake 68added these dependencies automatically. Initial bitbake metadata therefore 69listed shared library dependencies explicitly, and was subsequently copy pasted. 70 71### Resolution 72Do not list shared library packages in RDEPENDS. This eliminates the 73possibility of installing unnecessary shared library packages due to 74unmaintained library dependency lists in bitbake metadata. 75 76## Use of /usr/bin/env in systemd service files 77 78### Identification 79In systemd unit files: 80``` 81[Service] 82 83ExecStart=/usr/bin/env some-application 84``` 85 86### Description 87Outside of OpenBMC, most applications that provide systemd unit files don't 88launch applications in this way. So if nothing else, this just looks strange 89and violates the [princple of least 90astonishment](https://en.wikipedia.org/wiki/Principle_of_least_astonishment). 91 92### Background 93This anti-pattern exists because a requirement exists to enable live patching of 94applications on read-only filesystems. Launching applications in this way was 95part of the implementation that satisfied the live patch requirement. For 96example: 97 98``` 99/usr/bin/phosphor-hwmon 100``` 101 102on a read-only filesystem becomes: 103 104``` 105/usr/local/bin/phosphor-hwmon` 106``` 107 108on a writeable /usr/local filesystem. 109 110### Resolution 111The /usr/bin/env method only enables live patching of applications. A method 112that supports live patching of any file in the read-only filesystem has emerged. 113Assuming a writeable filesystem exists _somewhere_ on the bmc, something like: 114 115``` 116mkdir -p /var/persist/usr 117mkdir -p /var/persist/work/usr 118mount -t overlay -o lowerdir=/usr,upperdir=/var/persist/usr,workdir=/var/persist/work/usr overlay /usr 119``` 120can enable live system patching without any additional requirements on how 121applications are launched from systemd service files. This is the preferred 122method for enabling live system patching as it allows OpenBMC developers to 123write systemd service files in the same way as most other projects. 124 125To undo existing instances of this anti-pattern remove /usr/bin/env from systemd 126service files and replace with the fully qualified path to the application being 127launched. For example, given an application in /usr/bin: 128 129``` 130sed -i s,/usr/bin/env ,/usr/bin/, foo.service 131``` 132