develop.md 19.6 KB
Newer Older
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19
Develop VTK with Git
====================

This page documents how to develop VTK through [Git][].
See the [README](README.md) for more information.

[Git]: http://git-scm.com

Git is an extremely powerful version control tool that supports many
different "workflows" for individual development and collaboration.
Here we document procedures used by the VTK development community.
In the interest of simplicity and brevity we do *not* provide an
explanation of why we use this approach.

Setup
-----

Before you begin, perform initial setup:

20
1.  Register [GitLab Access] to create an account and select a user name.
21

22 23 24 25
2.  [Fork VTK][] into your user's namespace on GitLab.

3.  Follow the [download instructions](download.md#clone) to create a
    local clone of the main VTK repository:
26 27 28

        $ git clone https://gitlab.kitware.com/vtk/vtk.git VTK
        $ cd VTK
29
        $ git submodule update --init
30
    The main repository will be configured as your `origin` remote.
31

32
4.  Run the [developer setup script][] to prepare your VTK work tree and
33 34 35
    create Git command aliases used below:

        $ ./Utilities/SetupForDevelopment.sh
36 37
    This will prompt for your GitLab user name and configure a remote
    called `gitlab` to refer to it.
38

Ben Boeckel's avatar
Ben Boeckel committed
39
5.  (Optional, but highly recommended.)
40 41 42 43 44 45 46
    [Register](https://open.cdash.org/register.php) with the VTK project
    on Kitware's CDash instance to better know how your code performs in
    regression tests.  After registering and signing in, click on
    "All Dashboards" link in the upper left corner, scroll down and click
    "Subscribe to this project" on the right of VTK.

[GitLab Access]: https://gitlab.kitware.com/users/sign_in
47
[Fork VTK]: https://gitlab.kitware.com/vtk/vtk/-/forks/new
48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79
[developer setup script]: /Utilities/SetupForDevelopment.sh

Workflow
--------

VTK development uses a [branchy workflow][] based on topic branches.
Our collaboration workflow consists of three main steps:

1.  Local Development:
    * [Update](#update)
    * [Create a Topic](#create-a-topic)

2.  Code Review (requires [GitLab Access][]):
    * [Share a Topic](#share-a-topic)
    * [Create a Merge Request](#create-a-merge-request)
    * [Review a Merge Request](#review-a-merge-request)
    * [Revise a Topic](#revise-a-topic)

3.  Integrate Changes:
    * [Merge a Topic](#merge-a-topic) (requires permission in GitLab)
    * [Delete a Topic](#delete-a-topic)

[branchy workflow]: http://public.kitware.com/Wiki/Git/Workflow/Topic

Update
------

1.  Update your local `master` branch:

        $ git checkout master
        $ git pull

80 81 82 83 84 85
2.  Optionally push `master` to your fork in GitLab:

        $ git push gitlab master
    to keep it in sync.  The `git gitlab-push` script used to
    [Share a Topic](#share-a-topic) below will also do this.

86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105
Create a Topic
--------------

All new work must be committed on topic branches.
Name topics like you might name functions: concise but precise.
A reader should have a general idea of the feature or fix to be developed given just the branch name.

1.  To start a new topic branch:

        $ git fetch origin

2.  For new development, start the topic from `origin/master`:

        $ git checkout -b my-topic origin/master

    For release branch fixes, start the topic from `origin/release`, and
    by convention use a topic name starting in `release-`:

        $ git checkout -b release-my-topic origin/release

106 107 108 109 110 111 112 113 114 115 116 117 118 119
    If backporting a change, you may rebase the branch back onto
    `origin/release`:

        $ git checkout -b release-my-topic my-topic
        $ git rebase --onto origin/release origin/master

    Alternatively, for more targeted or aggregate backports, use the `-x` flag
    when performing `git cherry-pick` so that a reference to the original
    commit is added to the commit message:

        $ git checkout -b release-my-topic origin/release
        $ git cherry-pick -x $hash_a $hash_b $hash_c
        $ git cherry-pick -x $hash_d $hash_e $hash_f

120 121 122 123 124 125 126 127
3.  Edit files and create commits (repeat as needed):

        $ edit file1 file2 file3
        $ git add file1 file2 file3
        $ git commit

    Caveats:
    * To add data follow [these instructions](data.md).
128 129
    * If your change modifies third party code, see [its
      documentation](../../../ThirdParty/UPDATING.md).
130
    * To deprecate APIs, follow [these instructions](deprecation.md).
131

Utkarsh Ayachit's avatar
Utkarsh Ayachit committed
132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156
Guidelines for Commit logs
--------------------------

Remember to *motivate & summarize*. When writing commit logs, make sure
that there is enough information there for any developer to read and glean
relevant information such as:

1.  Is this change important and why?
2.  If addressing an issue, which issue(s)?
3.  If a new feature, why is it useful and/or necessary?
4.  Are there background references or documentation?

A short description of what the issue being addressed and how will go a long way
towards making the log more readable and the software more maintainable.

Style guidelines for commit logs are as follows:

1. Separate subject from body with a blank line
2. Limit the subject line to 60 characters
3. Capitalize the subject line
4. Use the imperative mood in the subject line e.g. "Refactor foo" or "Fix Issue #12322",
   instead of "Refactoring foo", or "Fixing issue #12322".
5. Wrap the body at 80 characters
6. Use the body to explain `what` and `why` and if applicable a brief `how`.

157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182
Share a Topic
-------------

When a topic is ready for review and possible inclusion, share it by pushing
to a fork of your repository in GitLab.  Be sure you have registered and
signed in for [GitLab Access][] and created your fork by visiting the main
[VTK GitLab][] repository page and using the "Fork" button in the upper right.

[VTK GitLab]: https://gitlab.kitware.com/vtk/vtk

1.  Checkout the topic if it is not your current branch:

        $ git checkout my-topic

2.  Check what commits will be pushed to your fork in GitLab:

        $ git prepush

3.  Push commits in your topic branch to your fork in GitLab:

        $ git gitlab-push

    Notes:
    * If you are revising a previously pushed topic and have rewritten the
      topic history, add `-f` or `--force` to overwrite the destination.
    * If the topic adds data see [this note](data.md#push).
183 184
    * The `gitlab-push` script also pushes the `master` branch to your
      fork in GitLab to keep it in sync with the upstream `master`.
185 186 187 188 189 190 191 192 193 194 195 196 197 198 199

    The output will include a link to the topic branch in your fork in GitLab
    and a link to a page for creating a Merge Request.

Create a Merge Request
----------------------

(If you already created a merge request for a given topic and have reached
this step after revising it, skip to the [next step](#review-a-merge-request).)

Visit your fork in GitLab, browse to the "**Merge Requests**" link on the
left, and use the "**New Merge Request**" button in the upper right to
reach the URL printed at the end of the [previous step](#share-a-topic).
It should be of the form:

200
    https://gitlab.kitware.com/<username>/vtk/-/merge_requests/new
201 202 203 204 205 206 207 208 209 210 211 212 213

Follow these steps:

1.  In the "**Source branch**" box select the `<username>/vtk` repository
    and the `my-topic` branch.

2.  In the "**Target branch**" box select the `vtk/vtk` repository and
    the `master` branch.  It should be the default.

    If your change is a fix for the `release` branch, you should still
    select the `master` branch as the target because the change needs
    to end up there too.

214 215 216 217
    For other `release` branches (e.g., `release-6.3`), merge requests should
    go directly to the branch (they are not tied with `master` in our
    workflow).

218 219
3.  Use the "**Compare branches**" button to proceed to the next page
    and fill out the merge request creation form.
220

221 222 223 224 225 226
4.  In the "**Title**" field provide a one-line summary of the entire
    topic.  This will become the title of the Merge Request.

    Example Merge Request Title:

        Wrapping: Add Java 1.x support
227 228 229

5.  In the "**Description**" field provide a high-level description
    of the change the topic makes and any relevant information about
230 231 232 233 234 235 236 237 238 239 240 241 242 243 244
    how to try it.
    *   Use `@username` syntax to draw attention of specific developers.
        This syntax may be used anywhere outside literal text and code
        blocks.  Or, wait until the [next step](#review-a-merge-request)
        and add comments to draw attention of developers.
    *   If your change is a fix for the `release` branch, indicate this
        so that a maintainer knows it should be merged to `release`.
    *   Optionally use a fenced code block with type `message` to specify
        text to be included in the generated merge commit message when the
        topic is [merged](#merge-a-topic).

    Example Merge Request Description:

        This branch requires Java 1.x which is not generally available yet.
        Get Java 1.x from ... in order to try these changes.
245 246

        ```message
247
        Add support for Java 1.x to the wrapping infrastructure.
248 249
        ```

250 251 252 253 254 255 256
        Cc: @user1 @user2

6.  The "**Assign to**", "**Milestone**", and "**Labels**" fields
    may be left blank.

7.  Use the "**Submit merge request**" button to create the merge request
    and visit its page.
257

258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277
Guidelines for Merge Requests
-----------------------------

Remember to *motivate & summarize*. When creating a merge request, consider the
reviewers and future perusers of the software. Provide enough information to motivate
the merge request such as:

1.  Is this merge request important and why?
2.  If addressing an issue, which issue(s)?
3.  If a new feature, why is it useful and/or necessary?
4.  Are there background references or documentation?

Also provide a summary statement expressing what you did and if there is a choice
in implementation or design pattern, the rationale for choosing a certain path.
Notable software or data features should be mentioned as well.

A well written merge request will motivate your reviewers, and bring them up
to speed faster. Future software developers will be able to understand the
reasons why something was done, and possibly avoid chasing down dead ends,
Although it may take you a little more time to write a good merge request,
278
you'll likely see payback in faster reviews and better understood and
279 280
maintainable software.

281 282 283 284
Review a Merge Request
----------------------

Add comments mentioning specific developers using `@username` syntax to
285 286 287
draw their attention and have the topic reviewed.  After typing `@` and
some text, GitLab will offer completions for developers whose real names
or user names match.
288

289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325
Here is a list of developpers usernames and their specific area of
expertise. A merge request without a developer tagged has very low chance
to be merged in a reasonable timeframe.

 * @mwestphal: Qt, filters, data Model, widgets, parallel, anything else.
 * @charles.gueunet: filters, data model, SMP, events, pipeline.
 * @kmorel: General VTK Expertise, VTK-m accelerators.
 * @demarle: Ray tracing.
 * @will.schroeder: algorithms, computational geometry, filters, SPH, SMP, widgets,  point cloud, spatial locators.
 * @sujin.philip: VTK-m Accelerators, SMP, DIY.
 * @robertmaynard: build-system, VTK-m accelerators, filters, data model, IO.
 * @yohann.bearzi: filters, data model, HTG, computational geometry, algorithms.
 * @ken-martin: OpenGL, polygonal and volume rendering, OpenVR, Vulkan, native windows, WebAssembly.
 * @sebastien.jourdain: web, WebAssembly, Python, Java
 * @allisonvacanti: VTK-m, vtkDataArray, vtkArrayDispatch, vtk::Range, data model, text rendering.
 * @sankhesh: volume rendering, Qt, OpenGL, widgets, vtkImageData, DICOM, VR.
 * @ben.boeckel: CMake, module system, third-parties.
 * @cory.quammen: readers, filters, data modeling, general usage, documentation.

If you would like to be included in this list, juste create a merge request.

### Human Reviews ###

Reviewers may add comments providing feedback or to acknowledge their
approval. When a human reviewers suggest a change, please take it into
account or discuss your choices with the reviewers until an agreement
is reached. At this point, please `resolve` the discussion by clicking
on the dedicated button.

When all discussion have been adressed, the reviewers will either do
another pass of comment or acknowledge their approval in some form.

Please be swift to adress or discuss comments, it will increase
the speed at which your changes will be merged.

### Comments Formatting ###

326 327 328 329 330 331 332
Comments use [GitLab Flavored Markdown][] for formatting.  See GitLab
documentation on [Special GitLab References][] to add links to things
like merge requests and commits in other repositories.

[GitLab Flavored Markdown]: https://gitlab.kitware.com/help/markdown/markdown
[Special GitLab References]: https://gitlab.kitware.com/help/markdown/markdown#special-gitlab-references

333

334
Lines of specific forms will be extracted during
335
[merging](#merge-a-topic) and included as trailing lines of the
336 337 338 339 340 341 342 343
generated merge commit message.

A commit message consists of up to three parts which must be specified
in the following order: the [leading line](#leading-line), then
[middle lines](#middle-lines), then [trailing lines](#trailing-lines).
Each part is optional, but they must be specified in this order.

#### Leading Line ####
344 345 346 347 348 349

The *leading* line of a comment may optionally be exactly one of the
following votes followed by nothing but whitespace before the end
of the line:

*   `-1` or :-1: (`:-1:`) means "The change is not ready for integration."
350 351
*   `+1` or :+1: (`:+1:`) means "The change is ready for integration."
*   `+2` means "I have tested the change and verified it works."
352

353 354
#### Middle Lines ####

355 356
The middle lines of a comment may be free-form [GitLab Flavored Markdown][].

357 358 359 360 361
#### Trailing Lines ####

Zero or more *trailing* lines in the last section of a comment may
each contain exactly one of the following votes followed by nothing
but whitespace before the end of the line:
362 363 364 365 366 367 368 369 370 371 372

*   `Rejected-by: me` means "The change is not ready for integration."
*   `Acked-by: me` means "I like the change but defer to others."
*   `Reviewed-by: me` means "The change is ready for integration."
*   `Tested-by: me` means "I have tested the change and verified it works."

Each `me` reference may instead be an `@username` reference or a full
`Real Name <user@domain>` reference to credit someone else for performing
the review.  References to `me` and `@username` will automatically be
transformed into a real name and email address according to the user's
GitLab account profile.
373

374 375 376 377 378 379 380 381 382 383 384 385 386 387 388 389 390 391 392 393 394 395 396
#### Fetching Changes ####

One may fetch the changes associated with a merge request by using
the `git fetch` command line shown at the top of the Merge Request
page.  It is of the form:

    $ git fetch https://gitlab.kitware.com/$username/vtk.git $branch

This updates the local `FETCH_HEAD` to refer to the branch.

There are a few options for checking out the changes in a work tree:

*   One may checkout the branch:

        $ git checkout FETCH_HEAD -b $branch
    or checkout the commit without creating a local branch:

        $ git checkout FETCH_HEAD

*   Or, one may cherry-pick the commits to minimize rebuild time:

        $ git cherry-pick ..FETCH_HEAD

397 398 399 400 401
### Robot Reviews ###

The "Kitware Robot" automatically performs basic checks on the commits
and adds a comment acknowledging or rejecting the topic.  This will be
repeated automatically whenever the topic is pushed to your fork again.
402
A re-check may be explicitly requested by adding a comment with a single
403
[*trailing* line](#trailing-lines):
404 405 406 407 408 409

    Do: check

A topic cannot be [merged](#merge-a-topic) until the automatic review
succeeds.

Ben Boeckel's avatar
Ben Boeckel committed
410 411 412
### Testing ###

VTK has a [buildbot](http://buildbot.net) instance watching for merge requests
Ben Boeckel's avatar
Ben Boeckel committed
413
to test.  A developer must issue a command to buildbot to enable builds:
Ben Boeckel's avatar
Ben Boeckel committed
414

Ben Boeckel's avatar
Ben Boeckel committed
415
    Do: test
Ben Boeckel's avatar
Ben Boeckel committed
416 417 418 419

The buildbot user (@buildbot) will respond with a comment linking to the CDash
results when it schedules builds.

Ben Boeckel's avatar
Ben Boeckel committed
420 421 422 423 424 425 426 427
The `Do: test` command accepts the following arguments:

  * `--stop`
        clear the list of commands for the merge request
  * `--superbuild`
        build the superbuilds related to the project
  * `--clear`
        clear previous commands before adding this command
428
  * `--regex-include <arg>` or `-i <arg>`
Ben Boeckel's avatar
Ben Boeckel committed
429
        only build on builders matching `<arg>` (a Python regular expression)
430
  * `--regex-exclude <arg>` or `-e <arg>`
Ben Boeckel's avatar
Ben Boeckel committed
431 432 433
        excludes builds on builders matching `<arg>` (a Python regular
        expression)

434 435 436 437
Multiple `Do: test` commands may be given in separate comments. A new `Do: test`
command must be explicitly issued for each branch update for which testing is
desired. Buildbot may skip tests for older branch updates that have not started
before a test for a new update is requested.
Ben Boeckel's avatar
Ben Boeckel committed
438 439 440 441 442 443 444 445 446 447 448 449

Builder names always follow this pattern:

        project-host-os-libtype-buildtype+feature1+feature2

  * project: always `vtk` for vtk
  * host: the buildbot host
  * os: one of `windows`, `osx`, or `linux`
  * libtype: `shared` or `static`
  * buildtype: `release` or `debug`
  * feature: alphabetical list of features enabled for the build

450 451 452 453 454
For a list of all builders, visit the
[VTK project on open.cdash.org](https://open.cdash.org/index.php?project=VTK).

Otherwise, `Expected`, `Superbuild`, or `Experimental` builds can be
directly accesssed from within Kitware at the following sites:
Ben Boeckel's avatar
Ben Boeckel committed
455 456 457 458 459

  * [vtk-expected](https://buildbot.kitware.com/builders?category=vtk-expected)
  * [vtk-superbuild](https://buildbot.kitware.com/builders?category=vtk-superbuild)
  * [vtk-experimental](https://buildbot.kitware.com/builders?category=vtk-experimental)

460 461 462 463 464 465 466 467 468 469 470 471 472 473 474 475 476 477 478 479 480 481 482 483
Revise a Topic
--------------

If a topic is approved during GitLab review, skip to the
[next step](#merge-a-topic).  Otherwise, revise the topic
and push it back to GitLab for another review as follows:

1.  Checkout the topic if it is not your current branch:

        $ git checkout my-topic

2.  To revise the `3`rd commit back on the topic:

        $ git rebase -i HEAD~3

    (Substitute the correct number of commits back, as low as `1`.)
    Follow Git's interactive instructions.

3.  Return to the [above step](#share-a-topic) to share the revised topic.

Merge a Topic
-------------

After a topic has been reviewed and approved in a GitLab Merge Request,
484 485
authorized developers may add a comment with a single
[*trailing* line](#trailing-lines):
486 487 488

    Do: merge

489 490 491 492 493 494 495
in order for your change to be merged into the upstream repository.

If your merge request has been already approved by developpers
but not merged yet, do not hesitate to tag an authorized developer
and ask for a merge.

By convention, do not request a merge if any `-1` or `Rejected-by:`
496 497
review comments have not been resolved and superseded by at least
`+1` or `Acked-by:` review comments from the same user.
498 499 500 501 502 503 504 505 506 507 508 509 510 511 512 513 514 515 516 517 518 519 520 521 522 523 524 525 526 527 528 529 530 531 532 533 534 535 536 537 538 539

### Merge Success ###

If the merge succeeds the topic will appear in the upstream repository
`master` branch and the Merge Request will be closed automatically.

### Merge Failure ###

If the merge fails (likely due to a conflict), a comment will be added
describing the failure.  In the case of a conflict, fetch the latest
upstream history and rebase on it:

    $ git fetch origin
    $ git rebase origin/master

(If you are fixing a bug in the latest release then substitute
`origin/release` for `origin/master`.)

Return to the [above step](#share-a-topic) to share the revised topic.

Delete a Topic
--------------

After a topic has been merged upstream the Merge Request will be closed.
Now you may delete your copies of the branch.

1.  In the GitLab Merge Request page a "**Remove Source Branch**"
    button will appear.  Use it to delete the `my-topic` branch
    from your fork in GitLab.

2.  In your work tree checkout and update the `master` branch:

        $ git checkout master
        $ git pull

3.  Delete the local topic branch:

        $ git branch -d my-topic

    The `branch -d` command works only when the topic branch has been
    correctly merged.  Use `-D` instead of `-d` to force the deletion
    of an unmerged topic branch (warning - you could lose commits).