summaryrefslogtreecommitdiff
path: root/lib/spack/docs/contribution_guide.rst
blob: 108b23a6d0b600904adf0c25edb3b8fdcd30e1cc (plain) (blame)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
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
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
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
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
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
.. _contribution-guide:

Contribution Guide
=====================

This guide is intended for developers or administrators who want to
contribute a new package, feature, or bugfix to Spack.
It assumes that you have at least some familiarity with Git VCS and Github.
The guide will show a few examples of contributing workflow and discuss
the granularity of pull-requests (PRs).

First, what is a PR? Quoting `Bitbucket's tutorials <https://www.atlassian.com/git/tutorials/making-a-pull-request/>`_:

  Pull requests are a mechanism for a developer to notify team members that they have **completed a feature**.
  The pull request is more than just a notification—it’s a dedicated forum for discussing the proposed feature

Important is completed feature, i.e. the changes one propose in a PR should
correspond to one feature/bugfix/extension/etc. One can create PRs with
changes relevant to different ideas, however reviewing such PRs becomes tedious
and error prone. If possible, try to follow the rule **one-PR-one-package/feature.**

Spack uses a rough approximation of the `Git Flow <http://nvie.com/posts/a-successful-git-branching-model/>`_ branching
model. The develop branch contains the latest contributions, and master is
always tagged and points to the latest stable release. Thereby when you send
your request, make ``develop`` the destination branch on the
`Spack repository <https://github.com/LLNL/spack>`_.

Let's assume that the current (patched) state of your fork of Spack is only
relevant to yourself. Now you come across a bug in a package or would like to
extend a package and contribute this fix to Spack. It is important that
whenever you change something that might be of importance upstream,
create a pull-request (PR) as soon as possible. Do not wait for weeks/months to
do this: a) you might forget why did you modified certain files; b) it could get
difficult to isolate this change into a stand-alone clean PR.

Now let us discuss several approaches one may use to submit a PR while
also keeping your local version of Spack patched.


First approach (cherry-picking):
--------------------------------

First approach is as follows.
You checkout your local develop branch, which for the purpose of this guide
will be called ``develop_modified``:

.. code-block:: console

  $ git checkout develop_modified

Let us assume that lines in files you will be modifying
are the same in `develop_modified` branch and upstream ``develop``.
Next edit files, make sure they work for you and create a commit

.. code-block:: console

  $ git add <files_to_be_commited>
  $ git commit -m <descriptive note about changes>

Normally we prefer that commits pertaining to a package ``<package-name>``` have
a message ``<package-name>: descriptive message``. It is important to add
descriptive message so that others, who might be looking at your changes later
(in a year or maybe two), would understand the rationale behind.


Next we will create a branch off upstream's ``develop`` and copy this commit.
Before doing this, while still on your modified branch, get the hash of the
last commit

.. code-block:: console

  $ git log -1

and copy-paste this ``<hash>`` to the buffer. Now switch to upstream's ``develop``,
make sure it's updated, checkout the new branch, apply the patch and push to
GitHub:

.. code-block:: console

  $ git checkout develop
  $ git pull upstream develop
  $ git checkout -b <descriptive_branch_name>
  $ git cherry-pick <hash>
  $ git push <your_origin> <descriptive_branch_name> -u

Here we assume that local ``develop`` branch tracks upstream develop branch of
Spack. This is not a requirement and you could also do the same with remote
branches. Yet to some it is more convenient to have a local branch that
tracks upstream.

Now you can create a PR from web-interface of GitHub. The net result is as
follows:

#. You patched your local version of Spack and can use it further
#. You "cherry-picked" these changes in a stand-alone branch and submitted it
   as a PR upstream.


Should you have several commits to contribute, you could follow the same
procedure by getting hashes of all of them and cherry-picking to the PR branch.
This could get tedious and therefore there is another way:


Second approach:
----------------

In the second approach we start from upstream ``develop`` (again assuming
that your local branch `develop` tracks upstream):

.. code-block:: console

  $ git checkout develop
  $ git pull upstream develop
  $ git checkout -b <descriptive_branch_name>

Next edit a few files and create a few commits by

.. code-block:: console

  $ git add <files_to_be_part_of_the_commit>
  $ git commit -m <descriptive_message_of_this_particular_commit>

Now you can push it to your fork and create a PR

.. code-block:: console

  $ git push <your_origin> <descriptive_branch_name> -u

Most likely you would want to have those changes in your (modified) local
version of Spack. To that end you need to merge this branch

.. code-block:: console

  $ git checkout develop_modified
  $ git merge <descriptive_branch_name>

The net result is similar to the first approach with a minor difference that
you would also merge upstream develop into you modified version in the last
step. Should this not be desirable, you have to follow the first approach.



How to clean-up a branch by rewriting history:
-----------------------------------------------

Sometimes you may end up on a branch that has a lot of commits, merges of
upstream branch and alike but it can't be rebased on ``develop`` due to a long
and convoluted history. If the current commits history is more of an experimental
nature and only the net result is important, you may rewrite the history.
To that end you need to first merge upstream `develop` and reset you branch to
it. So on the branch in question do:

.. code-block:: console

   $ git merge develop
   $ git reset develop

At this point you your branch will point to the same commit as develop and
thereby the two are indistinguishable. However, all the files that were
previously modified will stay as such. In other words, you do not loose the
changes you made. Changes can be reviewed by looking at diffs

.. code-block:: console

   $ git status
   $ git diff

One can also run GUI to visualize the current changes

.. code-block:: console

   $ git difftool

Next step is to rewrite the history by adding files and creating commits

.. code-block:: console

   $ git add <files_to_be_part_of_commit>
   $ git commit -m <descriptive_message>


Shall you need to split changes within a file into separate commits, use

.. code-block:: console

   $ git add <file> -p

After all changed files are committed, you can push the branch to your fork
and create a PR

.. code-block:: console

   $ git push <you_origin> -u



How to fix a bad rebase by "cherry-picking" commits:
----------------------------------------------------

Say you are working on a branch ``feature1``. It has several commits and is
ready to be merged. However, there are a few minor merge conflicts and so
you are asked to rebase onto ``develop`` upstream branch. Occasionally, it
happens so that a contributor rebases not on top of the upstream branch, but
on his/her local outdated copy of it. This would lead to an inclusion of the
whole lot of duplicated history and of course can not be merged as-is.

One way to get out of troubles is to ``cherry-pick`` important commits. To
do that, first checkout a temporary back-up branch:

.. code-block:: console

  git checkout -b tmp

Now look at logs and save hashes of commits you would like to keep

.. code-block:: console

  git log

Next, go back to the original branch and reset it to ``develop``.
Before doing so, make sure that you local ``develop`` branch is up-to-date
with the upstream.

.. code-block:: console

  git checkout feature1
  git reset --hard develop

Now you can cherry-pick relevant commits

.. code-block:: console

  git cherry-pick  <hash1>
  git cherry-pick  <hash2>


push the modified branch to your fork

.. code-block:: console

  git push -f

and if everything looks good, delete the back-up:

.. code-block:: console

  git branch -D tmp