From c83d2b58e1776982a7ce45009fb373ec5702c521 Mon Sep 17 00:00:00 2001 From: Luke Shumaker Date: Tue, 1 Mar 2016 19:15:29 -0500 Subject: improve HACKING documentation --- COPYING | 2 +- HACKING.md | 253 ------------------------------------------------ HACKING/build-system.md | 24 +++++ HACKING/code-quality.md | 134 +++++++++++++++++++++++++ HACKING/code-style.md | 62 ++++++++++++ HACKING/contributing.md | 26 +++++ HACKING/licensing.md | 19 ++++ HACKING/testing.md | 18 ++++ automake.head.mk | 2 +- automake.txt | 131 ++++++++++++++----------- 10 files changed, 359 insertions(+), 312 deletions(-) delete mode 100644 HACKING.md create mode 100644 HACKING/build-system.md create mode 100644 HACKING/code-quality.md create mode 100644 HACKING/code-style.md create mode 100644 HACKING/contributing.md create mode 100644 HACKING/licensing.md create mode 100644 HACKING/testing.md diff --git a/COPYING b/COPYING index def2dff..0cca3f1 100644 --- a/COPYING +++ b/COPYING @@ -6,4 +6,4 @@ and license statement, each file contains a "License:" line to make searching easy. Full copies of the GNU General Public License versions 2 and 3 are -included as `COPYING-GPLv2` and `COPYING-GPLv3`, respectively. \ No newline at end of file +included as `COPYING-GPLv2` and `COPYING-GPLv3`, respectively. diff --git a/HACKING.md b/HACKING.md deleted file mode 100644 index ee5cd65..0000000 --- a/HACKING.md +++ /dev/null @@ -1,253 +0,0 @@ -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 ; 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_. - -I also recommend having the `haveged` daemon running. That's good -general advice, but also: some of the tests make GPG keys, this -"should" take on the order of 1 second, but can take several minutes -if you don't have `haveged` running. - -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 -. -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 -. 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 - -for most code, for 3rd-party code that has been imported, indent it a -bit: - - # Copyright (C) YEARS NAME - -Always put a line with `# License:` specifying the license of that -file. diff --git a/HACKING/build-system.md b/HACKING/build-system.md new file mode 100644 index 0000000..93770cc --- /dev/null +++ b/HACKING/build-system.md @@ -0,0 +1,24 @@ +The build system is built around an automake-like system for GNU Make +that I wrote. It is documented in `automake.txt`. It provides all of +the standard targets and such; you tell it what to do by setting a +series of `am_whatever` variables. I'm just going to call it +"automake" here. + +I also hook into non-exposed parts of automake with a couple of +`_am_whatever` variables. Hopefully I will come up with good ways to +expose the needed functionality in the future. + +There are a couple of variables that get automatically set from git. +This happens by `include`ing a hidden makefile that sets them; if +`$(topsrcdir)/.git` exists, it knows to use git to generate it; +otherwise it expects the file to exist: + +| variable | file | +|--------------------+----------------------------------------| +| srcfiles | $(srcdir)/.srcfiles.mk | +| LIBRETOOLS_VERSION | $(topsrcdir)/.srcversion-libretools.mk | +| LIBRETOOLS_COMMIT | $(topsrcdir)/.srcversion-libretools.mk | +| DEVTOOLS_VERSION | $(topsrcdir)/.srcversion-devtools.mk | +| DEVTOOLS_COMMIT | $(topsrcdir)/.srcversion-devtools.mk | + +`srcfiles` basically becomes `am_src_files`. diff --git a/HACKING/code-quality.md b/HACKING/code-quality.md new file mode 100644 index 0000000..a520ce5 --- /dev/null +++ b/HACKING/code-quality.md @@ -0,0 +1,134 @@ +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 bunch 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 +. +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 +. 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 ; 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 :) + +Be sure to make sure to follow the licensing requirements in +`HACKING/licensing.md` + +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 message, I do a +search for mentions of "luke" on #parabola every time I get on. + +Please write unit tests for new functionality. Or old functionality. +Please write unit tests! See `HACKING/testing.md` for details on +testing. diff --git a/HACKING/licensing.md b/HACKING/licensing.md new file mode 100644 index 0000000..6d17e31 --- /dev/null +++ b/HACKING/licensing.md @@ -0,0 +1,19 @@ +We don't require copyright assignment or any fancy paperwork! Just +make sure you specify the license and include a copyright statement +with your name and the current year. + +New code should (please) be licensed GPLv2+. I say v2 instead of v3 +because some code from Arch is GPLv2 (no "or any later version"), and +having to worry about which programs can be combined is a huge pain. + +Copyright statements should look like + + # Copyright (C) YEARS NAME + +for most code, for 3rd-party code that has been imported, indent it a +bit: + + # Copyright (C) YEARS NAME + +Always put a line with `# License:` specifying the license of that +file. diff --git a/HACKING/testing.md b/HACKING/testing.md new file mode 100644 index 0000000..8dee485 --- /dev/null +++ b/HACKING/testing.md @@ -0,0 +1,18 @@ +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` (the `sh-roundup` package on +Parabola) 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_. + +I also recommend having the `haveged` daemon running. That's good +general advice, but also: some of the tests make GPG keys, this +"should" take on the order of 1 second, but can take several minutes +if you don't have `haveged` running. diff --git a/automake.head.mk b/automake.head.mk index a3c90fd..ad7154c 100644 --- a/automake.head.mk +++ b/automake.head.mk @@ -39,7 +39,7 @@ endif _am_included_makefiles := $(_am_included_makefiles) $(call _am_path,$(outdir)/Makefile) -## Empty variables for use by the module +## Empty variables for use by each Makefile $(_am)subdirs = $(_am)depdirs = diff --git a/automake.txt b/automake.txt index 307b321..22a0b84 100644 --- a/automake.txt +++ b/automake.txt @@ -7,14 +7,13 @@ automake.{head,tail}.mk Makefiles and how to use them, kinda. I wrote a "clone" of automake. I say clone, because it works differently. Yeah, I need a new name for it. -Anyway, how to use it: +High-level overview +------------------- In each source directory, you write a `Makefile`, very similarly to if you were writing for plain GNU Make, with -_am_phony = build install uninstall mostlyclean clean distclean maintainer-clean check - - + # adjust the number of `../` segments as appropriate include $(dir $(lastword $(MAKEFILE_LIST)))/../../config.mk include $(topsrcdir)/automake.head.mk @@ -22,63 +21,81 @@ _am_phony = build install uninstall mostlyclean clean distclean maintainer-clean include $(topsrcdir)/automake.tail.mk -Write your own `common.each.mk` that gets included after the body of -your Makefile for each Makefile. - -Write your own `common.once.mk` that gets included once after -everything else has been parsed. - -There are several commands that generate files; simply record what -they the list of files in their output to the following variables: - -| Variable | Command | Description | Relative to | -|-----------+--------------+-----------------------------------+-------------| -| src_files | emacs | Files that the developer writes | srcdir | -| gen_files | ??? | Files the developer compiles | srcdir | -| cfg_files | ./configure | Users' compile-time configuration | outdir | -| out_files | make all | Files the user compiles | outdir | -| sys_files | make install | Files the user installs | DESTDIR | - -In addition, there are - - subdirs : A list of other directories containing Makefiles that - contain or generate files that are dependencies of - targets in this directory. They are not necesarily - actually subdirectories of this directory in the - filesystem. - - clean_files : A list of things to `rm` in addition to - `$(out_files)` when you run `make clean`. (Example: - `*.o`). - - slow_files : A list of things in `$(out_files)` that (as an - exception) should _not_ be deleted when you run `make - mostlyclean`. - -Each directory containing a Makefile is a "module". The module name -is one of 4 things (with / replaced with _ in all cases): - - `all` - - $(realpath --relative-to=. $dir_name) - - dep-top - - dep-$(realpath --relative-to=$(topoutdir) $dir_name) - -The dep-* options are only used if that directory is not a child of -the current directory. +Write your own `common.{each,once}.{head,tail}.mk` files that get +included: + - `common.once.head.mk`: before parsing any of your Makefiles + - `common.each.head.mk`: before parsing each of your Makefiles + - `common.each.tail.mk`: after parsing each of your Makefiles + - `common.each.tail.mk`: after parsing all of your Makefiles Here is a table of all of the .PHONY targets that automake takes care of for you: -| this | and this | are aliases for this | which is just a case of this | -|------+------------------+----------------------+------------------------------| -| all | build | build-all | build-$(module) | -| | install | install-all | install-$(module) | -| | uninstall | uninstall-all | uninstall-$(module) | -| | mostlyclean | mostlyclean-all | mostlyclean-$(module) | -| | clean | clean-all | clean-$(module) | -| | distclean | distclean-all | distclean-$(module) | -| | maintainer-clean | maintainer-clean-all | maintainer-clean-$(module) | -| | check | check-all | check-$(module) | -| | | | dist | +| this | and this | are aliases for this | +|------+------------------+--------------------------------------------------------| +| all | build | $(outdir)/build | +| | install | $(outdir)/install | +| | uninstall | $(outdir)/uninstall | +| | mostlyclean | $(outdir)/mostlyclean | +| | clean | $(outdir)/clean | +| | distclean | $(outdir)/distclean | +| | maintainer-clean | $(outdir)/maintainer-clean | +| | check | $(outdir)/check (not implemented for you) | +| | dist | $(topoutdir)/$(PACKAGE)-$(VERSION).tar.gz (not .PHONY) | + +You are responsible for implementing the `$(outdir)/check` target in +each of your Makefiles. + +Telling automake about your program +----------------------------------- + +You tell automake what to do for you by setting some variables. They +are all prefixed with `am_`; this prefix may be changed by editing the +`_am` variable at the top of `automake.head.mk`. + +There are several commands that generate files; simply record the list +of files that each command generates as the following variable +variables: + +| Variable | Create Command | Delete Command | Description | Relative to | +|--------------+----------------+-----------------------------+-----------------------------------+-------------| +| am_src_files | emacs | rm -rf . | Files that the developer writes | srcdir | +| am_gen_files | ??? | make maintainer-clean | Files the developer compiles | srcdir | +| am_cfg_files | ./configure | make distclean | Users' compile-time configuration | outdir | +| am_out_files | make all | make mostlyclean/make clean | Files the user compiles | outdir | +| am_sys_files | make install | make uninstall | Files the user installs | DESTDIR | + +In addition, there are two more variables that control not how files +are created, but how they are deleted: + +| Variable | Affected command | Description | Relative to | +|----------------+------------------+------------------------------------------------+-------------| +| am_clean_files | make clean | A list of things to `rm` in addition to the | outdir | +| | | files in `$(am_out_files)`. (Example: `*.o`) | | +|----------------+------------------+------------------------------------------------+-------------| +| am_slow_files | make mostlyclean | A list of things that (as an exception) should | outdir | +| | | _not_ be deleted. (otherwise, `mostlyclean` | | +| | | is the same as `clean`) | | + +Finally, there are two variables that express the relationships +between directories: + +| Variable | Description | +|------------+---------------------------------------------------------| +| am_subdirs | A list of other directories (containing Makefiles) that | +| | may be considered "children" of this | +| | directory/Makefile; building a phony target in this | +| | directory should also build it in the subdirectory. | +| | They are not necesarily actually subdirectories of this | +| | directory in the filesystem. | +|------------+---------------------------------------------------------| +| am_depdirs | A list of other directories (containing Makefiles) that | +| | contain or generate files that are dependencies of | +| | targets in this directory. They are not necesarily | +| | actually subdirectories of this directory in the | +| | filesystem. Except for files that are dependencies of | +| | files in this directory, things in the dependency | +| | directory will not be built. | ---- Copyright (C) 2016 Luke Shumaker -- cgit v1.2.2