Change-type: patch
12 KiB
Contributing
The balena CLI is an open source project and your contribution is welcome!
- Install the dependencies listed in the NPM Installation
section of the
INSTALL.md
file. Check the section Additional Dependencies too. - Clone the
balena-cli
repository,cd
to it and runnpm install
. - Build the CLI with
npm run build
ornpm test
, and execute it with./bin/balena
(on a Windows command prompt, you may need to runnode .\bin\balena
).
In order to ease development:
npm run build:fast
skips some of the build steps for interactive testing, or./bin/balena-dev
usests-node/register
andcoffeescript/register
to transpile on the fly.
Before opening a PR, test your changes with npm test
. Keep compatibility in mind, as the CLI is
meant to run on Linux, macOS and Windows. balena CI will run test code on all three platforms, but
this will only help if you add some test cases for your new code!
./bin/balena-dev and oclif
When using ./bin/balena-dev
with oclif-converted commands, it is currently necessary to manually
edit the oclif
section of package.json
to replace ./build
with ./lib
as follows:
Change from:
"oclif": {
"commands": "./build/actions-oclif",
"hooks": {
"prerun": "./build/hooks/prerun/track"
To:
"oclif": {
"commands": "./lib/actions-oclif",
"hooks": {
"prerun": "./lib/hooks/prerun/track"
And then remember to change it back before pushing the pull request. This is obviously error prone
and inconvenient, and improvement suggestions are welcome: is there a better solution than
automatically editing package.json
? It is doable, if it is what needs to be done.
Semantic versioning and commit messages
The CLI version numbering adheres to Semantic Versioning. The following header/row is required in the body of a commit message, and will cause the CI build to fail if absent:
Change-type: patch|minor|major
Version numbers and commit messages are automatically added to the CHANGELOG.md
file by the CI
build flow, after a pull request is merged. It should not be manually edited.
Editing documentation files (CHANGELOG, README, website...)
The doc/cli.markdown
file is automatically generated by running npm run build:doc
(which also
runs as part of npm run build
). That file is then pulled by scripts in the
balena-io/docs GitHub repo for publishing at the CLI
Documentation page.
The content sources for the auto generation of doc/cli.markdown
are:
- Selected sections of the README file.
- The CLI's command documentation in source code (both Capitano and oclif commands), for example:
lib/actions/build.coffee
lib/actions-oclif/env/add.ts
The README file is manually edited, but subsections are automatically extracted for inclusion in
doc/cli.markdown
by the getCapitanoDoc()
function in
automation/capitanodoc/capitanodoc.ts
.
The INSTALL.md
and TROUBLESHOOTING.md
files are also manually edited.
Windows
Please note that npm run build:installer
(which generates the .exe
executable installer on
Windows) specifically requires MSYS2 to be installed. Other than that,
the standard Command Prompt or PowerShell can be used (though MSYS2 is still handy, as it provides
'git' and a number of common unix utilities). If you make changes to package.json
scripts, check
they also run on a standard Windows Command Prompt.
Updating the 'npm-shrinkwrap.json' file
The npm-shrinkwrap.json
file is used to control package dependencies, as documented at
https://docs.npmjs.com/files/shrinkwrap.json.
While developing, the package.json
file is often modified by, or before, running npm install
in order to add, remove or modify dependencies. When npm install
is executed, it automatically
updates the npm-shrinkwrap.json
file as well, taking into account not only the package.json
file but also the current state of the node_modules
folder in your computer.
Meanwhile, as a text (JSON) file, git
is capable of merging the npm-shrinkwrap.json
file during
operations like rebase
, cherry-pick
and pull
. But git's automated merge is not the
recommended way of updating the npm-shrinkwrap.json
file, because it does not take into account
duplicates or conflicts in the dependency tree, or indeed the state of the package.json
file
(which may have just been merged). In extreme cases, the automated merge may actually result in a
broken installation. For these reasons, automatic merging of the npm-shrinkwrap.json
was disabled
through the .gitattributes
file (the "binary merge driver" allows diff'ing but prevents automatic
merging). Operations like git rebase
may then result in an error like:
$ git rebase master
warning: Cannot merge binary files: npm-shrinkwrap.json (HEAD vs. c34942b9... test)
Auto-merging npm-shrinkwrap.json
CONFLICT (content): Merge conflict in npm-shrinkwrap.json
error: Failed to merge in the changes.
Whether or not there is a merge error, the following commands are the recommended way of updating
and committing the npm-shrinkwrap.json
file:
$ rm -rf node_modules # Linux / Mac
$ rmdir /s node_modules # Windows Command Prompt
$ npm checkout master -- npm-shrinkwrap.json # revert it to the master branch state
$ npm install # "cleanly" update the npm-shrinkwrap.json file
$ git add npm-shrinkwrap.json # add it for committing (solve merge errors)
TypeScript vs CoffeeScript, and Capitano vs oclif
The CLI was originally written in CoffeeScript, but we decided to migrate to TypeScript in order to take advantage of static typing and formal programming interfaces. The migration is taking place gradually, as part of maintenance work or the implementation of new features. The recommended way of making the conversion is to first generate plain Javascript, for example using the command:
npx decaffeinate --use-js-modules file.coffee
Then manually convert plain Javascript to Typescript. There is also a "Coffeescript Preview" Visual Studio Code extension that you may find handy.
Similarly, Capitano was originally adopted as the CLI's
framework, but later we decided to take advantage of oclif's features such
as native installers for Windows, macOS and Linux, and support for custom flag parsing (for
example, we're still battling with Capitano's behavior of dropping leading zeros of arguments that
look like integers, such as some abbreviated UUIDs). Again, the migration is taking place
gradually, with some CLI commands parsed by oclif and others by Capitano. A simple command line
pre-parsing takes place in preparser.ts
, to decide whether to route full parsing to Capitano or
to oclif.
Programming style
npm run build
also runs balena-lint, which automatically
reformats the code. Beyond that, we have a preference for Javascript promises over callbacks, and for
async/await
over .then()
.
Updating upstream dependencies
In order to get proper nested changelogs, when updating upstream modules that are in the repo.yml (like the balena-sdk), the commit body has to contain a line with the following format:
Update balena-sdk from 12.0.0 to 12.1.0
Since this is error prone, it's suggested to use the following npm script:
npm run update balena-sdk ^12.1.0
This will create a new branch (only if you are currently on master), run npm update
with the
version you provided as a target and commit the package.json & npm-shrinkwrap.json. The script by
default will set the Change-type
to patch
or minor
, depending on the semver change of the
updated dependency, but if you need to use a different one (eg major
) you can specify it as an
extra argument:
npm run update balena-sdk ^12.14.0 patch
npm run update balena-sdk ^13.0.0 major
Common gotchas
One thing that most CLI bugs have in common is the absence of test cases exercising the broken code, so writing some test code is a great idea. Having said that, there are also some common gotchas to bear in mind:
-
Forward slashes ('/') vs. backslashes ('') in file paths. The Node.js path.sep variable stores a platform-specific path separator character: the backslash on Windows and the forward slash on Linux and macOS. The path.join function builds paths using such platform-specific path separator. However:
- Note that Windows (kernel, cmd.exe, PowerShell, many applications) accepts both forward
slashes and backslashes as path separators (including mixing them in a path string), so code
like
mypath.split(path.sep)
may fail on Windows ifmypath
contains forward slashes. The path.parse function understands both forward slashes and backslashes on Windows, and the path.normalize function will replace forward slashes with backslashes. - In tar streams sent to the Docker
daemon and to balenaCloud, the forward slash is the only acceptable path separator, regardless
of the OS where the CLI is running. Therefore,
path.sep
andpath.join
should never be used when handling paths in tar streams!path.posix.join
may be used instead ofpath.join
.
- Note that Windows (kernel, cmd.exe, PowerShell, many applications) accepts both forward
slashes and backslashes as path separators (including mixing them in a path string), so code
like
-
Avoid using the system shell to execute external commands, for example:
child_process.exec('ssh "arg1" "arg2"');
child_process.spawn('ssh "arg1" "arg2"', { shell: true });
Besides the usual security concerns of unsanitized strings, another problem is to get argument escaping right because of the differences between the Windows 'cmd.exe' shell and the Unix '/bin/sh'. For example, 'cmd.exe' doesn't recognize single quotes like '/bin/sh', and uses the caret (^) instead of the backslash as the escape character. Bug territory! Most of the time, it is possible to avoid relying on the shell altogether by providing a Javascript array of arguments:
spawn('ssh', ['arg1', 'arg2'], { shell: false});
To allow for logging and debugging, the which package may be used to get the full path of a command before executing it, without relying on any shell:
const fullPath = await which('ssh');
console.log(fullPath); # 'C:\WINDOWS\System32\OpenSSH\ssh.EXE'
spawn(fullPath, ['arg1', 'arg2'], { shell: false });
-
Avoid the
instanceof
operator when testing against classes/types from external packages (including base classes), becausenpm install
may result in multiple versions of the same package being installed (to satisfy declared dependencies) and a false negative may result when comparing an object instance from one package version with a class of another package version (even if the implementations are identical in both packages). For example, once we fixed a bug where the test:
error instanceof BalenaApplicationNotFound
changed from true to false becausenpm install
added an additional copy of thebalena-errors
package to satisfy a minorbalena-sdk
version update:
$ find node_modules -name balena-errors
node_modules/balena-errors
node_modules/balena-sdk/node_modules/balena-errors
In the case of subclasses ofTypedError
, a string comparison may be used instead:
error.name === 'BalenaApplicationNotFound'