Skip to content
Snippets Groups Projects
Commit 49e9e038 authored by Martyn Welch's avatar Martyn Welch Committed by Emanuele Aina
Browse files

Update and move the version control guide


The version control guide looks useful, but is a little out of date.
Update the guide to use current workflows and move to `guides/`.

Signed-off-by: default avatarMartyn Welch <martyn.welch@collabora.com>
parent daf89406
No related branches found
No related tags found
1 merge request!42First chunk of guidelines rework
......@@ -47,7 +47,7 @@ internal functions which cannot easily be called from unit tests.
Overall, coverage levels of over 90% should be aimed for; don’t just
test cases covered by project requirements, test everything.
Like [git commits]( {{< ref "/guidelines/version_control.md" >}} ), each unit
Like [git commits]( {{< ref "version_control.md" >}} ), each unit
test should be ‘as small as possible, but no smaller’, testing a single
specific API or behaviour. Each test case must be able to be run
individually, without depending on state from other test cases. This is
......
+++
date = "2018-06-06"
weight = 100
title = "Version Control"
aliases = [
"/old-wiki/Guidelines/Version_control"
]
+++
# Version control
git is used for version control for all Apertis projects. This page
assumes good working knowledge of git; some introductory material is
available [here](https://www.atlassian.com/git/tutorials/), and a [git
cheatsheet is
here](https://training.github.com/kit/downloads/github-git-cheat-sheet.pdf).
## Summary
- Make atomic, revertable commits. ([Guidelines for making
commits](#guidelines-for-making-commits))
- Include full reasoning in commit messages, plus links to relevant
bug reports or specifications. ([Guidelines for making
commits](#guidelines-for-making-commits))
- Keep large changes, such as renames, in separate commits.
([Guidelines for making
commits](#guidelines-for-making-commits))
- Merge changes from feature branches by rebasing. ([Use of
git](#use-of-git))
## Use of git
For projects where Apertis is the upstream developer, the git
repositories follow these rules:
- No forced pushes.
- Rebase commits rather than merging.
- Work on feature branches on Apertis git, then rebase on master and
fast-forward merge the changes.
- Hide [sausage
making](https://sethrobertson.github.io/GitBestPractices/#sausage)
by squashing commits before merging.
- Changes intended for the development branch of Apertis should be
based on the `master` branch, and merged to `master` when ready.
- Targeted changes for frozen or stable branches of Apertis (for
example 16.03) should use a branch named like `16.03` instead of
`master`.
- The commit representing the "upstream" release (for example
Canterbury 0.8.0) should be tagged with a tag name like `v0.8.0`.
- The commit representing a release into the Apertis distribution (for
example Canterbury 0.8.0-0co1 for the release of Canterbury 0.8.0
done by Collabora) should be tagged with a tag name like
`apertis/0.8.0-0co1`. This is often the same commit as `v0.8.0`.
- The `git-buildpackage` tool will create these tags if run as
`gbp buildpackage --git-tag-only
--git-debian-tag='apertis/%(version)s'`.
- There must be no differences between `v0.8.0` and any tag
matching `apertis/0.8.0-*`, except for changes to the `debian/`
directory or to a file that is ignored by `extend-diff-ignore`
in `debian/source/options`.
### Third-party packages
For packages where we are packaging and maybe modifying a third-party
project such as `dbus`, the git repositories follow these rules:
- If the changes are relatively small/infrequent, there is no git
repository, and all packaging is done via OBS. If the changes are
more extensive and require significant work to update to a new
upstream version, there can be a git repository in
`https://gitlab.apertis.org/packaging/${DEB_SOURCE}` where
`${DEB_SOURCE}` is the source package name, for example `dbus`.
- We follow the naming convention from
[DEP-14](http://dep.debian.net/deps/dep14/):
- The main development branch is `apertis/master`
- Updates for frozen or stable branches of Apertis are on a branch
named like `apertis/16.03`, and tags look like
`apertis/1.2.3-4co5`.
- For example, `gbp buildpackage
--git-debian-branch=apertis/master --git-tag-only
--git-debian-tag='apertis/%(version)s'` creates suitable
tags.
- Non-native packages are assumed to use the `3.0 (quilt)` source
format. The `apertis/*` branches must be in the "patches unapplied"
state: all changes outside `debian/` are represented by (unified
diff) patches in `debian/patches` and listed in
`debian/patches/series`. For example, if we patch `configure.ac`,
the version of `configure.ac` seen in git is the *upstream* version,
and our change is only visible as a patch in
`debian/patches/series`.
- Developers can use either `quilt` or `gbp pq` to manipulate the
patch series. If using `gbp pq`, the `patch-queue/*` branches
must not be pushed to the central repository.
- The versions from Ubuntu and/or Debian on which our versions are
based, should be imported onto an `ubuntu/master` or `debian/master`
branch like this:
- `gbp import-dsc --debian-branch=debian/master
--debian-tag='debian/%(version)s'` (for Debian packages)
- `gbp import-dsc --debian-branch=ubuntu/master
--debian-tag='ubuntu/%(version)s'` (for Ubuntu packages)
## Guidelines for making commits
Commits should be as small as possible, but no smaller. Each commit
should address a single issue, containing only changes related to that
issue. The message for each commit should describe the issue, explain
what causes it, and explain how it has been fixed if it is not obvious.
If the commit is associated with a bug report or Phabricator
Differential revision, the full URI for the bug report or revision
should be put on a line by itself at the bottom of the commit message.
Similarly, the ID for the git commit (e.g. from `git log --oneline`)
should be copied into the bug report once the commit has been pushed, so
it is easy to find one from the other (this is done automatically for
Phabricator Differential revisions).
The changes in each commit should be easy to read. For example, they
should not unnecessarily change whitespace or indentation. Large,
mechanical changes, such as renaming a file or function, should be put
in separate commits from modifications to code inside that file or
function, so that the latter changes do not get buried and lost in the
former.
The following principles give the reasoning for all the advice above:
- Each commit should take the repository from one working state to
another, otherwise
[bisection](http://git-scm.com/book/en/v2/Git-Tools-Debugging-with-Git#Binary-Search)
is impossible.
- Each commit should be individually revertable. If it later turns out
that the commit was a bad idea, `git revert [commit ID]` should take
the repository from a working state to another working state.
- The reasoning for each commit, and its relationship to external
resources like specifications and bug reports, should be clear, to
the extent that commits written by one developer a year in the past
should still be understandable by a second developer without having
to trace through the changes and work out what they do. See the
[Commit messages](#commit-messages) section for details.
- Each commit should be written once, and designed to be read many
times, by many reviewers and future programmers.
### Commit messages
Commit messages should focus on **why** a change has been introduced
rather than what it does, as the latter should be understandable by
looking at the commit diff.
In more detail, the messages should include as much reasoning about the
change as possible, pointing out its relationship to external resources
like specifications and bug reports: always write them such that they
can be understood by other developers a year after you commit.
There are a few guidelines that describe how to write good commits and
the style and formatting to be used:
- the very short [seven
rules](https://chris.beams.io/posts/git-commit/#seven-rules)
- the longer [Linux kernel style guide for commit
messages](https://www.kernel.org/doc/html/v4.14/process/submitting-patches.html#describe-your-changes)
A few rules are automatically checked by the Apertis infrastructure:
- Subject: less than 50 characters
- Body: each line should be less than 72 characters long
- A `Signed-off-by` tag must be present as a [Developer Certificate of
Origin](https://developercertificate.org/)
If a `Apertis:` tag is available and links to a Phabricator task, the
task will be automatically closed once the commit is shipped in a image.
Other tags can be used to [make commit messages suitable for
automatically generating
`debian/changelog`]( {{< ref "/guidelines/howtoreleasepackages.md#writing-commit-messages-suitable-for-generating-debian/changelog-with-gbp-dch" >}} ).
Here's [an
example](https://git.apertis.org/cgit/canterbury.git/commit/?id=6338e04aeefe):
CbyServiceManager: Instruct systemd to send SIGKILL after 10s
After executing the ExecStop command-line or sending SIGTERM,
by default systemd will wait up to 90 seconds for a service to exit
before it becomes impatient and sends SIGKILL. This seems far too long
for our use-case; wait 10 seconds instead.
The choice of this arbitrary timeout is a trade-off. If it is too
short, applications with a lot of state to serialize to disk might
be killed before they have done so (we'd better hope they're using
crash-safe I/O patterns like g_file_set_contents()). If it is too
long, a user uninstalling an app-bundle will be left waiting
a long time.
When Ribchester calls TerminateBundle (T2696) it will need to wait
a little longer than this; whatever timeout it uses, a broken or
compromised per-user instance of Canterbury would be able to delay
app-bundle upgrade, rollback or removal by up to that long.
Apertis: https://phabricator.apertis.org/T12345
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Frédéric Dalleau <frederic.dalleau@collabora.co.uk>
Differential Revision: https://phabricator.apertis.org/D7088
## Merging procedure
To merge a feature branch named `my-branch` into master, use the
following commands:
git checkout master
git pull
git checkout my-branch
git rebase --interactive master
# Ensure the rebase is successful; test the changes
git checkout master
git merge my-branch
git push
# my-branch can now be deleted
git push origin :my-branch
git branch -D my-branch
## External links
- [Pro Git](http://git-scm.com/book/en/v2), a comprehensive guide
- [Git best
practices](https://sethrobertson.github.io/GitBestPractices/)
- [Git FAQ](https://help.github.com/categories/using-git/)
- [Atlassian git tutorial](https://www.atlassian.com/git/tutorials/)
- [Official git tutorial](http://git-scm.com/docs/gittutorial)
- [Interactive git tutorial](https://try.github.io/)
- [git-tower tutorial](http://www.git-tower.com/learn/)
......@@ -111,8 +111,7 @@ porting or developing additional features.
Existing patches can be imported using `git am`, which will import each
patch as a separate commit. If developing additional drivers/board
support it is advisable to break down this work into well explained
[atomic
commits]( {{< ref "/guidelines/version_control.md#guidelines-for-making-commits" >}} ).
[atomic commits]( {{< ref "version_control.md#guidelines-for-making-commits" >}} ).
As a developer you will need to build and test your modifications during
development, prior to completing packaging. There is no single
......
+++
date = "2018-06-06"
weight = 100
title = "Version Control"
aliases = [
"/old-wiki/Guidelines/Version_control"
]
+++
Git is used for version control for all Apertis projects. This page assumes
good working knowledge of git; some introductory material is available
[here](https://www.atlassian.com/git/tutorials/), and a
[git cheatsheet is here](https://training.github.com/kit/downloads/github-git-cheat-sheet.pdf).
# Summary
- Make atomic, revertable commits.
([Guidelines for making commits](#guidelines-for-making-commits))
- Include full reasoning in commit messages, plus links to relevant bug reports
or specifications.
([Guidelines for making commits](#guidelines-for-making-commits))
- Keep large changes, such as renames, in separate commits.
([Guidelines for making commits](#guidelines-for-making-commits))
- Merge changes from feature branches by rebasing. ([Use of git](#use-of-git))
# Use of git
For projects where Apertis is the upstream developer, the git repositories
follow these rules:
- No forced pushes.
- Rebase commits rather than merging.
- Work on feature branches on Apertis git, then rebase on master and
fast-forward merge the changes.
- Hide
[sausage making](https://sethrobertson.github.io/GitBestPractices/#sausage)
by squashing commits before merging.
- Changes intended for the development branch of Apertis should be based on the
branch named after that release under `apertis/`, for example the v2021dev3
branch is `apertis/v2021dev3`.
- Targeted changes for stable branches of Apertis (for example `v2019`)
should use the relevant branch named like `apertis/v2019-security` or
`apertis/v2019-updates`, depending on the focus of the change.
- Changes should be proposed via a merged request to the relevant release
branch when ready.
## Third-party packages
For packages where we are packaging and maybe modifying a third-party
project such as `dbus`, the git repositories follow these rules:
- We follow the naming convention from
[DEP-14](http://dep.debian.net/deps/dep14/):
- The main development branch is `apertis/${APERTIS_RELEASE}`
- Updates for frozen or stable branches of Apertis are on a branch named like
`apertis/${APERTIS_RELEASE}-security` and
`apertis/${APERTIS_RELEASE}-updates`, depending on the focus of the change.
- Non-native packages are assumed to use the `3.0 (quilt)` source format. The
`apertis/*` branches must be in the "patches unapplied" state: all changes
outside `debian/` are represented by (unified diff) patches in
`debian/patches` and listed in `debian/patches/series`. For example, if we
patch `configure.ac`, the version of `configure.ac` seen in git is the
*upstream* version, and our change is only visible as a patch in
`debian/patches/series`.
- Developers can use either `quilt` or `gbp pq` to manipulate the
patch series. If using `gbp pq`, the `patch-queue/*` branches
must not be pushed to the central repository.
- The versions from Debian on which our versions are
based, should be imported onto `debian/${UPSTREAM_DISTRIBUTION}` (e.g.
`debian/buster`). The required process is covered in the
[ci-package-builder documentation](https://gitlab.apertis.org/infrastructure/ci-package-builder#adding-new-packages-from-debian).
# Guidelines for making commits
Commits should be as small as possible, but no smaller. Each commit
should address a single issue, containing only changes related to that
issue. The message for each commit should describe the issue, explain
what causes it, and explain how it has been fixed if it is not obvious.
The changes in each commit should be easy to read. For example, they
should not unnecessarily change whitespace or indentation. Large,
mechanical changes, such as renaming a file or function, should be put
in separate commits from modifications to code inside that file or
function, so that the latter changes do not get buried and lost in the
former.
The following principles give the reasoning for all the advice above:
- Each commit should take the repository from one working state to another,
otherwise
[bisection](http://git-scm.com/book/en/v2/Git-Tools-Debugging-with-Git#Binary-Search)
is impossible.
- Each commit should be individually revertable. If it later turns out that the
commit was a bad idea, `git revert [commit ID]` should take the repository
from a working state to another working state.
- The reasoning for each commit, and its relationship to external resources
like specifications and bug reports, should be clear, to the extent that
commits written by one developer a year in the past should still be
understandable by a second developer without having to trace through the
changes and work out what they do. See the
[Commit messages](#commit-messages) section for details.
- Each commit should be written once, and designed to be read many times, by
many reviewers and future programmers.
## Commit messages
Commit messages should focus on **why** a change has been introduced rather
than what it does, as the latter should be understandable by looking at the
commit diff.
In more detail, the messages should include as much reasoning about the change
as possible, pointing out its relationship to external resources like
specifications and bug reports: always write them such that they can be
understood by other developers a year after you commit.
There are a few guidelines that describe how to write good commits and the
style and formatting to be used:
- the very short
[seven rules](https://chris.beams.io/posts/git-commit/#seven-rules)
- the longer
[Linux kernel style guide for commit messages](https://www.kernel.org/doc/html/v4.14/process/submitting-patches.html#describe-your-changes)
A few rules are automatically checked by the Apertis infrastructure:
- Subject: less than 50 characters
- Body: each line should be less than 72 characters long
- A `Signed-off-by` tag must be present as a
[Developer Certificate of Origin](https://developercertificate.org/)
Tags can be used to
[make commit messages suitable for automatically generating `debian/changelog`]( {{< ref "/guidelines/howtoreleasepackages.md#writing-commit-messages-suitable-for-generating-debian/changelog-with-gbp-dch" >}} ).
Here's
[an example](https://git.apertis.org/cgit/canterbury.git/commit/?id=6338e04aeefe):
CbyServiceManager: Instruct systemd to send SIGKILL after 10s
After executing the ExecStop command-line or sending SIGTERM,
by default systemd will wait up to 90 seconds for a service to exit
before it becomes impatient and sends SIGKILL. This seems far too long
for our use-case; wait 10 seconds instead.
The choice of this arbitrary timeout is a trade-off. If it is too
short, applications with a lot of state to serialize to disk might
be killed before they have done so (we'd better hope they're using
crash-safe I/O patterns like g_file_set_contents()). If it is too
long, a user uninstalling an app-bundle will be left waiting
a long time.
When Ribchester calls TerminateBundle (T2696) it will need to wait
a little longer than this; whatever timeout it uses, a broken or
compromised per-user instance of Canterbury would be able to delay
app-bundle upgrade, rollback or removal by up to that long.
Apertis: https://phabricator.apertis.org/T12345
Signed-off-by: Simon McVittie <smcv@collabora.com>
Reviewed-by: Frédéric Dalleau <frederic.dalleau@collabora.co.uk>
Differential Revision: https://phabricator.apertis.org/D7088
# Merging procedure
Merges should be carried out via the gitlab UI as a merge request to enable easy review.
- Ensure the local copy of the main development branch is up-to-date
git checkout apertis/${APERTIS_RELEASE}
git pull
- Checkout the feature branch and rebase on the main development branch
git checkout my-branch
git rebase --interactive apertis/${APERTIS_RELEASE}
- Ensure the rebase is successful; test the changes.
- Push the feature branch to git as a "wip" branch for review
git push origin my-branch:wip/${USER}/my-branch
- Follow the URL provided by the above command to complete creating a
[merge request](https://docs.gitlab.com/ce/user/project/merge_requests/)
# External links
- [Pro Git](http://git-scm.com/book/en/v2), a comprehensive guide
- [Git best practices](https://sethrobertson.github.io/GitBestPractices/)
- [Git FAQ](https://help.github.com/categories/using-git/)
- [Atlassian git tutorial](https://www.atlassian.com/git/tutorials/)
- [Official git tutorial](http://git-scm.com/docs/gittutorial)
- [Interactive git tutorial](https://try.github.io/)
- [git-tower tutorial](http://www.git-tower.com/learn/)
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment