From 7a3320f4bb4af5a6d1844fdefb7f2d3178917ccf Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Thu, 17 Sep 2020 11:54:49 -0700 Subject: [PATCH 01/10] build(make): Defensive make settings https://tech.davis-hansson.com/p/make/ Prevents things like silent shell pipeline errors in recipes, unexpected behavior from undefined variables, etc. --- Makefile | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/Makefile b/Makefile index 6dd87409e..e85f29b48 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,16 @@ # NOTE: this Makefile requires GNU make +### Defensive settings for make: +# https://tech.davis-hansson.com/p/make/ +SHELL := bash +.ONESHELL: +.SHELLFLAGS := -xeu -o pipefail -c +.SILENT: +.DELETE_ON_ERROR: +MAKEFLAGS += --warn-undefined-variables +MAKEFLAGS += --no-builtin-rules + default: @echo "no default target" From da24739c59255b6b2217fcdf16cfd3c7d78801cf Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Thu, 17 Sep 2020 12:19:35 -0700 Subject: [PATCH 02/10] build(make): Cleanup unused target AFAIK, prefixing a target with a `.` (aside from the special targets make itself treats differently) is just to exclude it from being the default target, but the default target is already defined above this one. Also, it just runs another target which doesn't exist AFAIKT. --- Makefile | 3 --- 1 file changed, 3 deletions(-) diff --git a/Makefile b/Makefile index e85f29b48..5b0defe32 100644 --- a/Makefile +++ b/Makefile @@ -28,9 +28,6 @@ APPNAME=tahoe-lafs make-version: $(PYTHON) ./setup.py update_version -.built: - $(MAKE) build - src/allmydata/_version.py: $(MAKE) make-version From 735ff1e709acd31d05e3cb610194ebf79feedfce Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Thu, 17 Sep 2020 12:25:34 -0700 Subject: [PATCH 03/10] build(make): Improve and clarify Makefile org This is opinionated, so I understand if community members disagree and I'm happy to back this out. It's conventional (and I prefer) to group variables toward the top of the `./Makefile` so I've done that. I also prefer separating task-oriented "phony" targets above any "real" targets (whose recipes create/update actual build artifacts on the filesystem). The task-oriented targets tend to be a better starting point for any developers approaching the `./Makefile` to understand where to get started. --- Makefile | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/Makefile b/Makefile index 5b0defe32..07762f925 100644 --- a/Makefile +++ b/Makefile @@ -1,4 +1,5 @@ - +# Tahoe LFS Development and maintenance tasks +# # NOTE: this Makefile requires GNU make ### Defensive settings for make: @@ -11,26 +12,26 @@ SHELL := bash MAKEFLAGS += --warn-undefined-variables MAKEFLAGS += --no-builtin-rules -default: - @echo "no default target" - +# Local target variables PYTHON=python export PYTHON PYFLAKES=flake8 export PYFLAKES - SOURCES=src/allmydata static misc setup.py APPNAME=tahoe-lafs + +# Top-level targets + +default: + @echo "no default target" + # This is necessary only if you want to automatically produce a new # _version.py file from the current git history (without doing a build). .PHONY: make-version make-version: $(PYTHON) ./setup.py update_version -src/allmydata/_version.py: - $(MAKE) make-version - # Build OS X pkg packages. .PHONY: build-osx-pkg test-osx-pkg upload-osx-pkg build-osx-pkg: @@ -66,7 +67,6 @@ upload-osx-pkg: ## # --include appeared in coverage-3.4 ## COVERAGE_OMIT=--include '$(CURDIR)/src/allmydata/*' --omit '$(CURDIR)/src/allmydata/test/*' - .PHONY: code-checks #code-checks: build version-and-path check-interfaces check-miscaptures -find-trailing-spaces -check-umids pyflakes code-checks: check-interfaces check-debugging check-miscaptures -find-trailing-spaces -check-umids pyflakes @@ -227,3 +227,9 @@ tarballs: # delegated to tox, so setup.py can update setuptools if needed .PHONY: upload-tarballs upload-tarballs: @if [ "X${BB_BRANCH}" = "Xmaster" ] || [ "X${BB_BRANCH}" = "X" ]; then for f in dist/*; do flappclient --furlfile ~/.tahoe-tarball-upload.furl upload-file $$f; done ; else echo not uploading tarballs because this is not trunk but is branch \"${BB_BRANCH}\" ; fi + + +# Real targets + +src/allmydata/_version.py: + $(MAKE) make-version From 775a3dba33bc5345c34083d20f576bbf94ef1033 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Thu, 17 Sep 2020 12:33:13 -0700 Subject: [PATCH 04/10] build(make): Fix missing target name From context, I'm assuming this is just an omission. --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 07762f925..6ab6db754 100644 --- a/Makefile +++ b/Makefile @@ -72,6 +72,7 @@ upload-osx-pkg: code-checks: check-interfaces check-debugging check-miscaptures -find-trailing-spaces -check-umids pyflakes .PHONY: check-interfaces +check-interfaces: $(PYTHON) misc/coding_tools/check-interfaces.py 2>&1 |tee violations.txt @echo From d8df630729c6632cb02986fe36203531e9219d5b Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Thu, 17 Sep 2020 12:36:55 -0700 Subject: [PATCH 05/10] build(make): Add missing phony target declarations I also prefer having a `.PHONY: ...` declaration for each phony target, as is done in the rest of the `./Makefile`, because it makes is easier when removing or refactoring targets not to forget to also remove or adjust the `.PHONY: ...` declaration. --- Makefile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 6ab6db754..780d27c26 100644 --- a/Makefile +++ b/Makefile @@ -21,8 +21,9 @@ SOURCES=src/allmydata static misc setup.py APPNAME=tahoe-lafs -# Top-level targets +# Top-level, phony targets +.PHONY: default default: @echo "no default target" @@ -33,13 +34,15 @@ make-version: $(PYTHON) ./setup.py update_version # Build OS X pkg packages. -.PHONY: build-osx-pkg test-osx-pkg upload-osx-pkg +.PHONY: build-osx-pkg build-osx-pkg: misc/build_helpers/build-osx-pkg.sh $(APPNAME) +.PHONY: test-osx-pkg test-osx-pkg: $(PYTHON) misc/build_helpers/test-osx-pkg.py +.PHONY: upload-osx-pkg upload-osx-pkg: # [Failure instance: Traceback: : [('SSL routines', 'ssl3_read_bytes', 'tlsv1 alert unknown ca'), ('SSL routines', 'ssl3_write_bytes', 'ssl handshake failure')] # From 51338bd87429bd283a5adc7ecb274dd80b345f59 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Thu, 17 Sep 2020 14:09:08 -0700 Subject: [PATCH 06/10] build(make): Add targets for running tests Fully parallelize the build of the environments since they tend to be network I/O bound. Parallelize the run of tests to use all CPU cores. --- .gitignore | 1 + Makefile | 9 +++++++++ 2 files changed, 10 insertions(+) diff --git a/.gitignore b/.gitignore index 93802b22b..0bc7dc0b9 100644 --- a/.gitignore +++ b/.gitignore @@ -37,6 +37,7 @@ zope.interface-*.egg /tahoe-deps/ /tahoe-deps.tar.gz /.coverage +/.coverage.* /.coverage.el /coverage-html/ /miscaptures.txt diff --git a/Makefile b/Makefile index 780d27c26..2b176c4be 100644 --- a/Makefile +++ b/Makefile @@ -27,6 +27,11 @@ APPNAME=tahoe-lafs default: @echo "no default target" +.PHONY: test +## Run all tests and code reports +test: .tox + tox -p auto + # This is necessary only if you want to automatically produce a new # _version.py file from the current git history (without doing a build). .PHONY: make-version @@ -199,6 +204,7 @@ distclean: clean rm -rf src/*.egg-info rm -f src/allmydata/_version.py rm -f src/allmydata/_appname.py + rm -rf ./.tox/ .PHONY: find-trailing-spaces @@ -237,3 +243,6 @@ upload-tarballs: src/allmydata/_version.py: $(MAKE) make-version + +.tox: tox.ini setup.py + tox --notest -p all From b4b996c3e7cd3f0d53aacba6cbb0f71ede88c079 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Thu, 17 Sep 2020 16:09:39 -0700 Subject: [PATCH 07/10] build(make): Cleanup remnant coverage bits I don't see anything in here that I can find references to elsewhere and we're certainly running test coverage reports in tox and on CI now. --- Makefile | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/Makefile b/Makefile index 2b176c4be..c1b8e7579 100644 --- a/Makefile +++ b/Makefile @@ -58,23 +58,6 @@ upload-osx-pkg: # echo not uploading tahoe-lafs-osx-pkg because this is not trunk but is branch \"${BB_BRANCH}\" ; \ # fi -# code coverage-based testing is disabled temporarily, as we switch to tox. -# This will eventually be added to a tox environment. The following comments -# and variable settings are retained as notes for that future effort. - -## # code coverage: install the "coverage" package from PyPI, do "make -## # test-coverage" to do a unit test run with coverage-gathering enabled, then -## # use "make coverage-output" to generate an HTML report. Also see "make -## # .coverage.el" and misc/coding_tools/coverage.el for Emacs integration. -## -## # This might need to be python-coverage on Debian-based distros. -## COVERAGE=coverage -## -## COVERAGEARGS=--branch --source=src/allmydata -## -## # --include appeared in coverage-3.4 -## COVERAGE_OMIT=--include '$(CURDIR)/src/allmydata/*' --omit '$(CURDIR)/src/allmydata/test/*' - .PHONY: code-checks #code-checks: build version-and-path check-interfaces check-miscaptures -find-trailing-spaces -check-umids pyflakes code-checks: check-interfaces check-debugging check-miscaptures -find-trailing-spaces -check-umids pyflakes From d0d11a5444d52b4ead4165ad2b418943da196b91 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Fri, 18 Sep 2020 11:48:49 -0700 Subject: [PATCH 08/10] fix(style): Wrong Python version for codechecks On systems where the default Python is Python 3 (such as on recent Debian/Ubuntu versions), then `$ tox -e codechecks` has a ton of failures related to Python 3 compatibility. This explicitly forces it to use Python 2.7 until we have Python 3 compatibility. --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index 6bc24273c..c9ec2bb1d 100644 --- a/tox.ini +++ b/tox.ini @@ -80,6 +80,7 @@ commands = [testenv:codechecks] +basepython = python2.7 # On macOS, git inside of towncrier needs $HOME. passenv = HOME whitelist_externals = From 52015df7e4598a0a2372712a26c2fe3fbd793139 Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Fri, 18 Sep 2020 11:41:53 -0700 Subject: [PATCH 09/10] build(make): Add changelog entry for PR --- newsfragments/3421.other | 1 + 1 file changed, 1 insertion(+) create mode 100644 newsfragments/3421.other diff --git a/newsfragments/3421.other b/newsfragments/3421.other new file mode 100644 index 000000000..d6f70f6d9 --- /dev/null +++ b/newsfragments/3421.other @@ -0,0 +1 @@ +Various, minor development `./Makefile` cleanup and improvement. From 2645675649d3ba6b7e7b5381b4e1493fa11b4fee Mon Sep 17 00:00:00 2001 From: Ross Patterson Date: Wed, 23 Sep 2020 10:31:42 -0700 Subject: [PATCH 10/10] build(make): Exclude changelog entry from NEWS This includes only developer-oriented changes. --- newsfragments/{3421.other => 3421.minor} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename newsfragments/{3421.other => 3421.minor} (100%) diff --git a/newsfragments/3421.other b/newsfragments/3421.minor similarity index 100% rename from newsfragments/3421.other rename to newsfragments/3421.minor