Problem: Prior to this change, it was necessary to first create and
attach to a screen session and then to run `make deploy` within it. This
meant extra steps for the user and was generally error-prone.
Solution: Usage of screen has been refined such that a screen session
named 'localnet' is created on the users behalf without any need to
attach to it. Individual node deployment targets such as `make
bitcoind`, `make alice`, et al. are issued to new windows within the
localnet screen session, and the user is free to attach or not whenever
they choose. The result is that a new user can clone the repository and
type nothing more than `make deploy` to get up and running with their
localnet.
This also reverts the changes in commit 97dd342e5 ("Make build target
phony") for the following reasons:
- As mentioned in that commit message, Gradle was not deleting the its
'build' directory when running `gradle clean`, meaning that the
'build' target was always up-to-date, even after running `make
clean`. This made it impossible to get a correct rebuild workflow. On
analysis, howewer, this situation was because of a badly behaving
Kotlin plugin not cleaning up after itself, leaving a subdirectory at
build/kotlin and preventing the build directory itself from being
deleted altogether. To address this, the `make clean` target has been
updated to `rm -rf build` instead of calling `build gradle`. While
it's a workaround until we back out the Kotlin changes that caused
this, it does have the added benefit of being faster than invoking
`gradle clean`.
- By making the 'build' target PHONY, this meant that `./gradlew build`
was getting invoked every time a dependent target was called. For
example, `make alice` depends on the 'setup' target, which in turn
depends on the 'build' target. When calling such targets in
isolation, this arrangement works out fine, because the phony 'build'
target always runs, invoking `./gradle build`, and the Gradle build
completes quickly assuming everything is up-to-date. The problem
arises when calling a number of these targets in rapid succession, as
we do when calling `make deploy` and running each individual node
target in its own screen window. This causes contention in two ways.
The first is that these multiple, simultaneous Gradle processes
compete for access to an available Gradle daemon, and because each
process needs its own, it ends up that as many Gradle daemons get
created as Bisq nodes we need to deploy (5 in total). This is a big
waste of time and resources. The second way it causes not only
contention but outright failure is that each of these builds are
operating in the same directory, and while most aspects of the build
are in fact up-to-date and therefore not modified in any way, there
are exceptions to this rule. The result is that build artifacts, e.g.
jars are getting deleted and rebuilt from underneath competing Gradle
processes, and all manner of chaos ensues, such as NoClassDefFound
errors and much more. This change (reverting 'build' back to a
normal, non-phony target) avoids these problems entirely. When
running `make deploy`, we run the 'build' target once as a function
of the 'deploy' target depending on it. At this point, the 'build'
directory exists, and all subsequent node deployment targets, e.g.
'alice', 'bob', etc do not re-run the build target because it is
up-to-date. For workflows where the user definitely wants to rebuild
prior to redeploying a given node, they can either run `make
clean-build`, or drop down to issuing Gradle build commands directly,
e.g. `./gradlew :desktop:build` followed by `make desktop`.
Problem: we use soft 4-space tabs throughout the Bisq codebase, and the
new makefile is a break to this rule due to make's default requirement
for hard tabs in recipes.
Solution: This commit updates our Editorconfig settings to reflect this
exception.
For vim users, it is also recommended that you add the following entry
to your .vimrc:
au FileType make set tw=72 noet cc=72
It will ensure that you wrap (documentation) lines at 72 chars. It also
sets noexpandtab explicitly. Even though .editorconfig should already be
doing this for you when working in Bisq, this more general vim
configuration will ensure you use tabs correctly in any makefile. The
`cc=72` setting adds a visual right margin at 72 characters.
This commit also updates the existing makefile, wrapping lines of
documentation that had exceeded the 72-char margin.
This change follows up on commit 650c5894d, which:
1. Renamed the 'localdir' directory to '.localdir' to better follow
convention with how local data directories are often managed, e.g.
.git and .gradle.
2. Introduced the STATE_DIR variable to avoid duplication of the
'.localdir' string throughout the Makefile, and at least in concept to
allow this value to be customized via setting an environment variable.
The changes in (1) are preserved, while the changes in (2) have been
backed out. Rationale:
- The STATE_DIR name introduces a new concept to the reader. They must
reason about its meaning, and this works against the intention of the
Makefile, which is to maximize understandability for the uninitiated.
- The name, if we were to preserve the variable, probably should have
been something like DATA_DIR_ROOT. 'STATE_DIR' is not conceptually
incorrect, but industry convention is to refer to such directories as
"data directories", e.g. Bitcoin Core's `datadir` option, LND's
`datadir` option and Bisq's `userDataDir` and `appDataDir` options.
- The variable, whatever its name, introduces a layer of indirection,
which while convenient to the makefile maintainer, is a barrier to
comprehension for the reader / contributor. For example, if a user
wished to copy and paste the recipe for a target, say 'bob' from the
makefile, with the varible in place, the user would have to figure
out its correct value and replace it before they could paste and use
the copied command. Like in the first note above, the idea with the
makefile is to maximize understanding for the uninitiated, i.e.
working code as executable documentation. It is reasonable given this
goal to increase the burden on a few maintainers in order to ease the
potentially many contributors.
Finally, this change follows up on the renaming of the 'localnet'
directory to '.localnet' by reflecting this change in the name of the
associated target as well. This is order to avoid dependent targets e.g.
'bitcoind', 'alice' or 'bob' constantly re-running the localnet target.
In turn it also adds an 'alias' target named 'localnet' (without the
leading dot) because targets with a leading dot are (I believe) treated
as "implicit targets". In any case, they do not show up in a tab
completion context, so introducing the normally-named alias fixes that.
This is a follow-up to cbeams/bisq#3.
This partially reverts commit e3a3fb5, removing the dependency from
the 'localnet' target to the 'clean-localnet' target. The reason for
this is that a number of higher level targets that deploy nodes, e.g.
the 'alice' and 'bob' targets depend on 'localnet' and, prior to this
reversion, therefore also depended on 'clean-localnet'. The effect was
that every time a node is deployed, the .localnet directory was removed
and re-created, destroying the state of any and all nodes that had been
deployed and modified thus far.
The change in the original commit that removes the temporary 'dao-setup'
directory in case of partial failures has been preserved.
This is a follow-up to cbeams/bisq#3.
Sometimes when running setup something goes wrong and the ./dao-state
dir is still hanging around, requiring manual cleanup nad preventing from simply
re-running the command.
Problem: contributors old and new must read and follow many manual steps
spread across three documents (docs/{build,dev-setup,dao-setup}.md) in
order to get up and running with a local regtest Bisq network deployment
suitable for isolated development and end-to-end testing. This process
is not only manual, but requires considerable trial and error for most
contributors, and can amount to hours of effort. Perhaps most
detrimental is that this friction makes it much less likely that we get
"all hands on deck" to cover test scenarios at release time. Getting up
and running with what this change refers to as a "localnet" should be
among the very first things a new contributor does. It should be fast
and easy, maximizing the contributor's ability to get productive right
away.
Solution: this commit introduces a simple and well-documented makefile
to the root of the source tree. It instructs the user to issue a series
of simple `make` commands, at the end of which they'll have a fully
functional localnet deployment.
Caveats:
- No support for Windows unless the user is running Git Bash, Cygwin or
similar. In any case, the makefile serves as clear documentation
about what a Windows user would need to do manually, i.e. without the
benefit of `make` automating it all.
- The aforementioned setup documents should be updated to point to this
makefile instead of explaining everything in prose. The dev-setup.md
and dao-setup.md documents may actually be candidates for deletion if
this new approach proves successful.
- These changes do not include passing the new -peerbloomfilters=1
option to bitcoin versions 0.19 and above. Those who have already
upgraded should take care to add that option.
Notes:
- The introduction of this makefile has no impact on Bisq's use of
Gradle as a build system. Everything there is as it has been. This
makefile is a completely optional convenience being added into the
mix. It has the added benefit of being a "friendly face" to those not
familiar with the Java / JVM ecosystem. Developers from many
different backgrounds are familiar with make and makefiles, and they
may find this one a pleasant and inviting surprise.
