Some checks failed
CI Code / Code Coverage (push) Failing after 10m34s
CI Code / Check spelling (push) Failing after 10m47s
CI Code / Check coding style (push) Failing after 11m4s
CI Code / Linux (ubuntu) (push) Failing after 11m19s
CI Code / Linux (debian) (push) Failing after 11m28s
CI Code / Linux (arch) (push) Failing after 11m38s
security(CWE-134): fix format string injections + add CI check fix(ui): subwindow lifecycle, newwin/newpad guards, fallback timestamps fix(db): sqlite cleanup on failures, sqlite3_close_v2 fix(xmpp): queued_messages loop, barejid leak perf(core): g_hash_table_iter_init instead of g_hash_table_get_keys refactor(ui): CLAMP macro in _check_subwin_width test: XEP-0012 and XEP-0045 functional tests Author: jabber.developer2 Closes #58, #85
220 lines
9.1 KiB
Markdown
220 lines
9.1 KiB
Markdown
# Contributing to Profanity
|
|
|
|
## Build
|
|
|
|
Please follow the [build section](https://profanity-im.github.io/guide/latest/build.html) in our user guide.
|
|
You might also take a look at the `Dockerfile.*` in the root directory.
|
|
|
|
## Submitting patches
|
|
We recommend for people to always work on a dedicated git branch for each fix or feature.
|
|
Don't work on master.
|
|
So that they can easily pull master and rebase their work if needed.
|
|
|
|
For fixing (reported) bugs we usually use `git switch -c fix/issuenumber-somedescription`.
|
|
When working on a new feature we usually use `git switch -c feature/optionalissuenumber-somedescription`.
|
|
|
|
However this is not a rule just a recommendation to keep an overview of things.
|
|
If your change isn't a bugfix or new feature you can also just use any branch name.
|
|
|
|
### Commit messages
|
|
Write commit messages that make sense. Explain what and *why* you change.
|
|
Write in present tense.
|
|
Please give [this guideline](https://gist.github.com/robertpainsi/b632364184e70900af4ab688decf6f53) a read.
|
|
|
|
### GitHub
|
|
We would like to encourage people to use GitHub to create pull requests.
|
|
It makes it easy for us to review the patches, track WIP branches, organize branches with labels and milestones,
|
|
and help others to see what's being worked on.
|
|
|
|
Also see the blogpost [Contributing a Patch via GitHub](https://profanity-im.github.io/blog/post/contributing-a-patch-via-github/).
|
|
|
|
### E-Mail
|
|
In case GitHub is down or you can't use it for any other reason, you can send a patch to our [mailing list](https://lists.posteo.de/listinfo/profanity).
|
|
|
|
We recommend that you follow the workflow mentioned above.
|
|
And create your patch using the [`git-format-patch`](https://git-scm.com/docs/git-format-patch) tool: `git format-patch master --stdout > feature.patch`
|
|
|
|
### Another git service
|
|
We prefer if you create a pull request on GitHub.
|
|
Then our team can easily request reviews. And we have the history of the review saved in one place.
|
|
|
|
If using GitHub is out of the question but you are okay using another service (i.e.: GitLab, codeberg) then please message us in the MUC or send us an email.
|
|
We will then pull from your repository and merge manually.
|
|
|
|
### Rules
|
|
|
|
* When fixing a bug, describe it and how your patch fixes it.
|
|
* When fixing a reported issue add an `Fixes https://github.com/profanity-im/profanity/issues/23` in the commit body.
|
|
* When adding a new feature add a description of the feature and how it should be used (workflow).
|
|
* If your patch adds a new configuration option add this to the `profrc.example` file.
|
|
* If your patch adds a new theming option add this to the `theme_template` file.
|
|
* Each patch or pull request should only contain related modifications.
|
|
* Run the tests and code formatters before submitting (c.f. Chapter 'Check everything' of this README).
|
|
* When changing the UI it would be appreciated if you could add a before and after screenshot for comparison.
|
|
* Squash fixup commits into one
|
|
* If applicable, document how to test the functionality
|
|
|
|
## Hints and Pitfalls
|
|
|
|
* When adding a new hotkey/shortcut make sure it's not registered in Profanity already. And also that it's not a default shortcut of readline.
|
|
* We ship a `.git-blame-ignore-revs` file containing banal commits which you will most likely want to ignore when using `git blame`. In case you are using vim and [fugitive](https://github.com/tpope/vim-fugitive) `command Gblame Git blame --ignore-revs-file=.git-blame-ignore-revs` might be helpful in your vimrc. You can also set the `blame.ignoreRevsFile` option in your git config to have `git blame` generally ignore the listed commits.
|
|
|
|
## Coding style
|
|
Follow the style already present ;-)
|
|
|
|
To make this easier for you we created a `.clang-format` file.
|
|
You'll need to have `clang-format` installed.
|
|
|
|
Then just run `make format` before you do any commit.
|
|
|
|
It might be a good idea to add a git pre-commit hook.
|
|
So git automatically runs clang-format before doing a commit.
|
|
|
|
You can add the following snippet to `.git/hooks/pre-commit`:
|
|
```shell
|
|
for f in $(git diff --cached --name-only)
|
|
do
|
|
if [[ "$f" =~ \.(c|h)$ ]]; then
|
|
clang-format -i $f
|
|
fi
|
|
done
|
|
```
|
|
|
|
If you feel embarrassed every time the CI fails you can add the following
|
|
snippet to `.git/hooks/pre-push`:
|
|
|
|
```shell
|
|
#!/bin/sh
|
|
set -e
|
|
./ci-build.sh
|
|
```
|
|
|
|
This will run the same tests that the CI runs and refuse the push if it fails.
|
|
The CI script runs 4 parallel builds with different configurations:
|
|
- **Full** — all features enabled (+ coverage in `--coverage-only` mode)
|
|
- **Minimal** — all optional features disabled
|
|
- **NoEncrypt** — no encryption (OTR, PGP, OMEMO disabled)
|
|
- **Default** — default ./configure options
|
|
|
|
Each build runs Valgrind and functional tests on Linux.
|
|
Use `./ci-build.sh --coverage-only` to run only the Full build with coverage collection.
|
|
|
|
Output shows test results per build:
|
|
```
|
|
✓ Full PASSED
|
|
Unit tests: 437 passed, 0 failed
|
|
Functional tests: 69 passed, 0 failed
|
|
Coverage: Lines: 27.5% | Functions: 36.2% | Branches: 18.1%
|
|
Duration: 5m39s
|
|
```
|
|
|
|
Note that it will run on the actual content of the repository directory and not
|
|
what may have been staged/committed.
|
|
|
|
If you're in a hurry you can add the `--no-verify` flag when issuing `git push`
|
|
and the `pre-push` hook will be skipped.
|
|
|
|
*Note:* We provide a config file that describes our coding style for clang. But due to a mistake on their side it might happen that you can get a different result that what we expect. See [here](https://github.com/profanity-im/profanity/pull/1774) and [here](https://github.com/profanity-im/profanity/pull/1828) for details. We will try to always run latest clang-format.
|
|
|
|
## Finding mistakes
|
|
Test your changes with the following tools to find mistakes.
|
|
|
|
### unit tests
|
|
|
|
Run `make check` to run the unit tests with your current configuration or `./ci-build.sh` to check with different switches passed to configure.
|
|
|
|
### valgrind
|
|
We provide a suppressions file `prof.supp`. It is a combination of the suppressions for shipped with glib2, python and custom rules.
|
|
|
|
`G_DEBUG=gc-friendly G_SLICE=always-malloc valgrind --tool=memcheck --track-origins=yes --leak-check=full --leak-resolution=high --num-callers=30 --show-leak-kinds=definite --log-file=profval --suppressions=prof.supp ./profanity`
|
|
|
|
There's also the option to create a "personalized" suppression file with the up-to-date glib2 and python suppressions.
|
|
|
|
`make my-prof.supp`
|
|
|
|
After executing this, you can replace the `--suppressions=prof.supp` argument in the above call, by `--suppressions=my-prof.supp`.
|
|
|
|
### clang
|
|
|
|
Running the clang static code analyzer helps improving the quality too.
|
|
|
|
```
|
|
make clean
|
|
scan-build make
|
|
scan-view ...
|
|
```
|
|
|
|
### Security checks
|
|
|
|
We have a static analyzer `check-cwe134.sh` that detects CWE-134 format string vulnerabilities. It runs automatically in CI but you can also run it locally:
|
|
|
|
```bash
|
|
./check-cwe134.sh
|
|
```
|
|
|
|
This checks for unsafe patterns where data could be passed directly as a format string to functions like `printf`, `cons_show`, etc. Never pass a raw string for formatting; use `"%s"` format specifier instead.
|
|
|
|
### Finding typos
|
|
|
|
We include a `.codespellrc` configuration file for `codespell` in the root directory.
|
|
Before committing it might make sense to run `codespell` to see if you made any typos.
|
|
|
|
You can run the `make spell` command for this.
|
|
|
|
### Check everything
|
|
|
|
`make doublecheck` will run the code formatter, spell checker and unit tests.
|
|
|
|
|
|
### Functional tests
|
|
|
|
The functional test suite uses [stabber](https://git.jabber.space/devs/stabber) as a mock XMPP server. Tests are located in `tests/functionaltests/`.
|
|
|
|
#### Running functional tests
|
|
|
|
Functional tests require stabber to be installed. Once installed, tests run as part of `make check`:
|
|
|
|
```bash
|
|
make check # Run all tests (unit + functional)
|
|
make check-functional-parallel # Run functional tests in parallel (~3x faster)
|
|
./tests/functionaltests/functionaltests # Run all functional tests sequentially
|
|
./tests/functionaltests/functionaltests 1 # Run specific group (1-4)
|
|
```
|
|
|
|
#### Test groups
|
|
|
|
Tests are organized into 4 groups for parallel execution:
|
|
|
|
| Group | Description |
|
|
|-------|-------------|
|
|
| 1 | Connect, Ping, Rooms, Software |
|
|
| 2 | Message, Receipts, Roster, Chat Session |
|
|
| 3 | Presence, Disconnect |
|
|
| 4 | MUC, Carbons |
|
|
|
|
To add a new group:
|
|
1. Define the test array in `functionaltests.c`
|
|
2. Add entry to `groups[]` array
|
|
3. Update `FUNC_TEST_GROUPS` in `Makefile.am`
|
|
|
|
#### Writing functional tests
|
|
|
|
Use content-based stubbing with stabber:
|
|
|
|
```c
|
|
// Use stbbr_for_query for IQ queries (roster, disco, etc.)
|
|
stbbr_for_query("jabber:iq:roster", "<iq type='result'>...</iq>");
|
|
|
|
// Use stbbr_send for presence, message, and push-style stanzas
|
|
stbbr_send("<presence from='buddy@localhost'>...</presence>");
|
|
```
|
|
|
|
Guidelines:
|
|
|
|
1. Use `stbbr_for_query(namespace, xml)` for IQ queries where the namespace is stable.
|
|
2. Use `stbbr_send(xml)` for presence, message, and other push-style stanzas.
|
|
3. Keep assertions tolerant of ordering when possible; use `prof_output_regex()` for flexible matching.
|
|
4. If timing issues appear, use `prof_timeout()` around critical expectations and reset afterwards.
|
|
5. When adding new tests, place them in the appropriate group based on functionality.
|
|
|