# Contributing balenaCLI is an open source project and your contribution is welcome! * Install the dependencies listed in the [NPM Installation section](./INSTALL-ADVANCED.md#npm-installation) section of the installation instructions. Check the section [Additional Dependencies](./INSTALL-ADVANCED.md#additional-dependencies) too. * Clone the `balena-cli` repository (or a [forked repo](https://docs.github.com/en/free-pro-team@latest/github/getting-started-with-github/fork-a-repo), if you are not in the balena team), `cd` to it and run `npm install`. * Build the CLI with `npm run build` or `npm test`, and execute it with `./bin/balena` (on a Windows command prompt, you may need to run `node .\bin\balena`). In order to ease development: * `npm run build:fast` skips some of the build steps for interactive testing, or * `npm run test:source` skips testing the standalone zip packages (which is rather slow) * `./bin/balena-dev` uses `ts-node/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! ## Semantic versioning, commit messages and the ChangeLog The CLI version numbering adheres to [Semantic Versioning](http://semver.org/). 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. If `package.json` is updated for dependencies listed in the `repo.yml` file (like `balena-sdk`), the commit message body should include a line in the following format: ``` Update balena-sdk from 12.0.0 to 12.1.0 ``` This allows the CI to produce nested change logs (with expandable arrows), pulling in commit messages from the upstream repositories. The following npm script can be used to automatically produce a commit with a suitable commit message: ``` npm run update balena-sdk ^12.1.0 ``` The script will create a new branch (only if `master` is currently checked out), run `npm update` with the given target version and commit the `package.json` and `npm-shrinkwrap.json` files. The script by default will set the `Change-type` to `patch` or `minor`, depending on the semver change of the updated dependency. For a `major` change type, it can specified as an extra argument: ``` npm run update balena-sdk ^12.14.0 patch npm run update balena-sdk ^13.0.0 major ``` ## Editing documentation files (README, INSTALL, Reference 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](https://github.com/balena-io/docs/) GitHub repo for publishing at the [CLI Documentation page](https://www.balena.io/docs/reference/cli/). The content sources for the auto generation of `doc/cli.markdown` are: * [Selected sections](https://github.com/balena-io/balena-cli/blob/v12.23.0/automation/capitanodoc/capitanodoc.ts#L199-L204) of the README file. * The CLI's command documentation in source code (`lib/commands/` folder), for example: * `lib/commands/push.ts` * `lib/commands/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`](https://github.com/balena-io/balena-cli/blob/master/automation/capitanodoc/capitanodoc.ts). The `INSTALL*.md` and `TROUBLESHOOTING.md` files are also manually edited. ## Windows The `npm run build:installer` script (which generates the `.exe` executable installer on Windows) specifically requires [MSYS2](https://www.msys2.org/) 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 changes are made to npm scripts in `package.json`, check that 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. Changes to `npm-shrinkwrap.json` can be automatically merged by git during operations like `rebase`, `pull` and `cherry-pick`, but in some cases this results in suboptimal dependency resolution (the `node_modules` folder may end up larger than necessary, with consequences to CLI load time too). For this reason, the recommended way to update `npm-shrinkwrap.json` is to run `npm install`, possibly alongside `npm dedupe` as well. The following commands can be used to fix shrinkwrap issues and optimize the dependencies: ```sh git checkout master -- npm-shrinkwrap.json rm -rf node_modules npm install # update npm-shrinkwrap.json to satisfy changes to package.json npm dedupe # deduplicate dependencies from npm-shrinkwrap.json npm install # re-add optional dependencies removed by dedupe git add npm-shrinkwrap.json # add it for committing (solve merge errors) ``` Note that `npm dedupe` should always be followed by `npm install`, as shown above, even if `npm install` had already been executed before `npm dedupe`. Optionally, these steps may be automated by installing the [npm-merge-driver](https://www.npmjs.com/package/npm-merge-driver): ```sh npx npm-merge-driver install -g ``` ## TypeScript and oclif The CLI currently contains a mix of plain JavaScript and [TypeScript](https://www.typescriptlang.org/) code. The goal is to have all code written in Typescript, in order to take advantage of static typing and formal programming interfaces. The migration towards Typescript is taking place gradually, as part of maintenance work or the implementation of new features. Of historical interest, the CLI was originally written in [CoffeeScript](https://coffeescript.org) and used the [Capitano](https://github.com/balena-io/capitano) framework. All CoffeeScript code was migrated to either Javascript or Typescript, and Capitano was replaced with oclif. A few file or variable names still refer to this legacy, for example `automation/capitanodoc/capitanodoc.ts`. ## Programming style `npm run build` also runs [balena-lint](https://www.npmjs.com/package/@balena/lint), which automatically reformats the code. Beyond that, we have a preference for Javascript promises over callbacks, and for `async/await` over `.then()`. ## 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](https://nodejs.org/docs/latest-v12.x/api/path.html#path_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](https://nodejs.org/docs/latest-v12.x/api/path.html#path_path_join_paths) 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 if `mypath` contains forward slashes. The [path.parse](https://nodejs.org/docs/latest-v12.x/api/path.html#path_path_parse_path) function understands both forward slashes and backslashes on Windows, and the [path.normalize](https://nodejs.org/docs/latest-v12.x/api/path.html#path_path_normalize_path) function will _replace_ forward slashes with backslashes. * In [tar](https://en.wikipedia.org/wiki/Tar_(computing)#File_format) 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` and `path.join` should never be used when handling paths in tar streams! `path.posix.join` may be used instead of `path.join`. * 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](https://www.npmjs.com/package/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), because `npm 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 because `npm install` added an additional copy of the `balena-errors` package to satisfy a minor `balena-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 of `TypedError`, a string comparison may be used instead: `error.name === 'BalenaApplicationNotFound'`