summaryrefslogtreecommitdiff
path: root/HACKING.md
blob: f22752ea4c0f6b6d09e9c0c8e99c28e166ab66df (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
This document is a little all over the place--I've been working on it
for a while, but I keep refactoring it as I move pieces of it into man
pages and whatnot.  It's mostly a brain dump, sorry.

There are three parts to this; procedures, "content" guidelines and
"style" guidelines.  The style guidelines are less strict; as long as
things are consistent at the file-level, I'm pretty happy.

Contributing
============

I'd love to have your patches!  Code should be hackable; if you want
to modify something, but can't figure out how: 1) ping me for help, 2)
it probably means the code was too complicated in the first place.

Patches should be sent to <dev@lists.parabolagnulinux.org>; please put
"[PATCH]" and "libretools" in the subject line.  If you have commit
access, but want me to look over it first, feel free to create a new
branch in git, and I will notice it.  Try to avoid pushing to the
"master" branch unless it's a trivial change; it makes it easier to
review things; though I *will* look over every commit before I do a
release, so don't think you can sneak something in :)

I'd love to discuss possible changes on IRC (I'm lukeshu), either on
irc.freenode.net#parabola or in personal messages.  My account may be
online even if I'm not; I will eventually see your it, I do a search
for mentions of "luke" on #parabola every time I get on.

Testing
=======

Please write unit tests for new things.  Tests can be run with `make
check`, which just runs `./testenv roundup` in the `test/` directory.
Relatedly, you need the `roundup` tool to run the tests.  `./testenv`
can be given `--no-network` and/or `--no-sudo` to dissable tests that
require those things.  Make can be made to pass those things in by
setting `TESTENVFLAGS`.  If you don't dissable either, I *strongly*
recommend setting TMPDIR to somewhere on a btrfs partition before
running the tests; otherwise the chroot tests will take forever.  I
mean, they already take long on btrfs, but without it... _dang_.

Code content
============

Be aware of the `librelib(7)` library suite, which lives `src/lib`.
It is a suite of Bash libraries that will help you out.  Most of the
people looking at the libretools code are familiar with the `messages`
part of it, which actually contains a much of utility routines, not
just message printing.  There is also a library for dealing with
`blacklist.txt`, and one for loading configuration files and
PKGBUILDs.  These are common tasks, but are tricky to handle
consistently--the libraries are there to make things easier.  Take a
look at the man pages.

Message printing: All of the message printing routines, except for
`term_title` and `flag`,  take printf-type arguments.  Take advantage
of that; don't use string interpolation (don't do `"foo ${var}
bar"`).  The reason for this is that if you don't do string
interpolation, messages can be automatically internationalized.
(Internationalization is incomplete at the momement)

Message printing: The in `--help`/`-h` text, use `print` to print
lines that should not wrap, `echo` to print blank lines, `prose` to
print paragraphs, `bullet` to print bullet points, and `flag` to print
option flags.  The text should follow this general format:

    print "Usage: %s [OPTIONS] VARS_ARE_UNDERSCORE_AND_CAPITAL" "${program_name}"
	print "One line description of program, no period"
	echo
	prose "More details.  This is a paragraph."
	echo
	print "Options:"
	flag  "-h"         "Show this message"

In the "Usage:" line, use printf `%s` and the value `"${0##*/}"` to
determine the program name at runtime.

There used to be guidelines for how to align the option flags and
descriptions, but now the `flag` command exists takes care of it for
you.  Yay for things being easier!

When using `set -u`, `set -e`, or `trap`, you should also use `set -E`
to have the error handling be passed down to subshells.

Feel free to use `set -e` (fail on error), but be careful of the
caveats (there are a bunch of them); don't assume all errors are
checked because of it.

Use `set -u` if you can; it makes using an unset variable an error.
 - If a variable not being set is valid (perhaps a configuration
   option), use `${var:-}` when accessing it to suppress the error.
 - An empty array counts as unset, so if you have an array that may be
   empty, use `set +u` before accessing it.
   - The reason for this is that a normal string variable is basically
     an array with length=1; an unset variable looks like an array
     with length=0.  Weird stuff.

In the shebang, use `#!/usr/bin/env bash`.  This allows us to not
hardcode the location of bash (I'm not sure why this is useful for
something distro-dependent like libretools, but fauno seems to have a
use-case for it).

In the shebang, don't pass flags to bash, besides breaking `env`
(above), it means people will make mistakes when debugging, and
running things with `bash FILENAME`.  Instead, use `set` to adjust the
flags inside of the program.

Obey `$TMPDIR`.  It's usually as easy as passing `--tmpdir` to
`mktemp`.

Use `trap` to clean up your temporary files.  This way, even if your
program terminates early, things will be cleaned up.

Bash best practices
===================

Basically, know what you are doing, and be safe with it.  The problem
is that most people don't know about safe bash scripting.

A lot of people look at the "Advanced Bash Scripting" ebook--DO NOT do
that, it is trash... though it contains a "reference card" page that
may be useful and isn't trash.

Take a look at Gentoo's Bash guidelines
<http://devmanual.gentoo.org/tools-reference/bash/index.html>.
They're pretty good, and cover most of the "gotcha's" about Bash
syntax.  It mentions but discourages the use of Bash 3
features... why?  Who still uses Bash 2?  Feel free to use Bash 4
features!

I wrote an article on Bash arrays
<https://lukeshu.com/blog/bash-arrays.html>.  A lot of people think
they're tricky, but they're simple once you know how they work.  It's
short enough that you should read the whole thing.  Know the
difference between `"${array[@]}"` and `"${array[*]}"`.  And I'll say
it again here, ALWAYS wrap those in double quotes; there is no reason
I can think of that the unquoted behavior would ever be the correct
thing.

My brief rules of thumb:

 - Quote every variable.
   - That includes arrays: `"${array[@]}"` and `"${array[*]}"`.
   - In most (but not all!) cases inside of `[[ ... ]]` conditions,
     variables don't need to be quoted.  When in doubt, quote them.
   - When assigning one variable to another, you don't need quotes;
     you don't need quotes for `foo=$bar`
 - Try to avoid global variables; declare all variables in functions
   with `local`.
   - Or `declare`; inside of a function, unless you pass the `-g`
     flag, `declare` makes the variable local.
 - Use `local VAR` before a `for VAR in LIST` loop--the variable is created in the
   current scope, not the scope of the loop.
 - Feeding input to `while` loops is weird because of how subshells
   work:

	# Input from a file
	# BAD
	cat file | while read line; do
		...
	done
	# GOOD
	while read line; do
		...
	done <file

	# Input from a program
	# BAD
	prog | while read line; do
		...
	done
	# GOOD
	while read line; do
		...
	done < <(prog)


Style guidelines
================

Unless you have a good reason, use `[[ ... ]]` instead of `[ ... ]`;
they work similarly, but `[[ ... ]]` is sometimes more readable (fine,
rarely, but never less), and is harder to make mistakes with quoting,
because it is syntactic magic, as opposed to `[ ... ]` which is an
executable which just happens to be implemented as a builtin.

Use a litteral tab for indent.  When indenting line-wrapped text, such
as that for `prose`, do it like this:  (» indicates tab, · indicates
space)

	func() {
	»   prose "This is the first line. This paragraph is going to be
	»   ·······wrapped."
	}

The `; then` and `; do` should go on the same line as
`if`/`elif`/`for`/`while`.  Also, there is no space before the `;`.

Prefer the `for VAR in LIST` syntax over the `for ((init; cond; inc))`
syntax, when possible.  For example (heh, `for` example):

	local i
	for (( i = 1 ; i <= 10 ; i++ )); do

should be

	local i
	for i in {1..10}; do

Of course, if the upper bound is a variable, the C-like syntax is
the better option, as otherwise you would have to use `seq` (calling
an external), or `eval` (gross, easy to mess up royally).

Indent comments like you would code; don't leave them at the beginning
of the line.  Example:

	for item in "${list[@]}"; do
		if [[ $item == foo ]]; then
	# BAD
			foobar
		fi
		if [[ $item == bar ]]; then
			# GOOD
			barbaz
		fi
	done

Fauno, I'm sorry.  But I don't know how you can read your own code :P.

Some people argue in favor of the useless use of cat, because data
should flow from left to right.  However, the input redirection
doesn't have to go on the right side of a command:

	cat file | program  # useless use of cat
	program < file      # data flows right to left
	< file program      # just right

Copyright statements should look like

	# Copyright (C) YEARS NAME <EMAIL>

for most code, for 3rd-party code that has been imported, indent it a
bit:

	#   Copyright (C) YEARS NAME <EMAIL>

Always put a line with `# License:` specifying the license of that
file.