-
Notifications
You must be signed in to change notification settings - Fork 284
Git workflow
We are pretty liberal in accepting contributions. You can either fork https://github.com/tpaviot/oce or ask on our mailing list for write access on our repository. The guidelines described in this page are meant to help everyone contributing. If you believe that they cause more harm than good, please feel free to discuss them.
No development is performed on the master branch, this is an integration branch. Development happens on private branches, and when a developer thinks that her code is ready to merge into master, she requests a merge.
Each developer can create her own branches under a private namespace. We usually choose a namespace based on our initials, but this naming is free as long as it is unambiguous. For instance, John Doe may decide to call his branches jd/fix-issue-13 or jd/mingw-port.
The right part should be as explicit as possible, so that other developers may understand what this branch is about. If you intend to work on long-lived branch, it may also be worth writing a page on this wiki to give more details.
There is no restriction on private branches, developers can do what they want. These branches may be rebased against master at any time without notice, this is why no branches should be based upon private branches of other people. Most of the time, branches are created from master.
You may also create local branches on your harddisk without pushing them on github, these branches are then really private, but the main problem is that someone else may implement different changes without knowing of your work, and this will likely cause conflicts. Thus it is better to always push on github, even if it is unstable and must not be merged as is.
When a developer believes that her branch is ready to be merged into master, she first renames her branch by replacing her own prefix by review/, logs into https://github.com/tpaviot/oce, switches to this new branch, and eventually performs a "Pull Request" by clicking on the top-right button. All OCE developers are then notified of this request, and will comment it. There are mostly three possibilities:
- Branch looks good and can be merged as is. Developers then usually write
+1as a shorthand. - Branch looks good, but could be improved by slight changes. Comment is usually in the form
+1, but please rename variable foo into something more meaningful. - Branch has problems and should not be merged. Then the request will be cancelled and you have to rename your branch to make it a private branch again.
Hopefully after a few days several developers have given their opinion and your branch will be processed. Of course, all developers are encouraged to review branches of other developers.
After the merge , update the Release Notes page in the Wiki, adding a descriptive entry of the work done in the branch.
OCCT git server provides a public web interface, which allows cherry-picking patches. In order to credit original author, we want to import patches with original metadata as much as possible. This will also help when upgrading OCCT version, because we will then knoow that this patch can be dropped because it had already been applied upstream. Here are guidelines to do it right. As an example, we want to apply http://git.dev.opencascade.org/gitweb/?p=occt.git;a=commit;h=4fe5661
First of all, go to the URL above and click on the patch link to download this commit. Other links (like raw) may not work as expected, so take care to take this one. It will download a file called occt.git-4fe56619213b38f852e55d0cff034b0ee9a5bfc4.patch.
We can now try to apply it.
git am occt.git-4fe56619213b38f852e55d0cff034b0ee9a5bfc4.patch
It is likely that it will fail due to conflicts, here is the (truncated) output:
Applying: 0023243: Adapt OpenGL viewer for using in Cocoa applications on Mac OS X
warning: src/InterfaceGraphic/FILES has type 100644, expected 100755
error: src/InterfaceGraphic/InterfaceGraphic.hxx: does not exist in index
warning: src/OpenGl/EXTERNLIB has type 100644, expected 100755
warning: src/OpenGl/FILES has type 100644, expected 100755
error: src/OpenGl/OpenGl_Context.hxx: does not exist in index
error: patch failed: src/OpenGl/OpenGl_Display.cxx:28
error: src/OpenGl/OpenGl_Display.cxx: patch does not apply
[...]
Patch failed at 0001 0023243: Adapt OpenGL viewer for using in Cocoa applications on Mac OS X
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
There are different issues here:
- File permissions do not match, but this is only a warning, it is harmless.
- Header files are missing, this is because we moved them into
inc/directory. - Some files cannot be patched because of conflicting changes.
- Current git state is very similar to the situation when an interactive rebase fails, we have to either abort the current operation, or help git to proceed. We can try to fix things, and as a last resort run
git am --abortif applying this patch is too difficult.
Here is a script file on Unix to solve the 2nd problem.
sed -i -e 's,^\\(---\\|+++\\) \\(.*\\)/src/.*/\\([^/]*.[ghl]xx\\),\\1 \\2/inc/\\3,' \
occt.git-4fe56619213b38f852e55d0cff034b0ee9a5bfc4.patch
Now we can apply the patch by hand:
git apply --ignore-whitespace --reject \
occt.git-4fe56619213b38f852e55d0cff034b0ee9a5bfc4.patch
As the patch fails, the index file is not updated. We must not forget to add new files:
git add inc/Cocoa_LocalPool.hxx inc/Cocoa_Window.hxx \
inc/InterfaceGraphic_Cocoa.hxx src/Cocoa/ \
src/OpenGl/OpenGl_Context_1.mm src/OpenGl/OpenGl_Window_1.mm \
src/ViewerTest/EXTERNLIB src/ViewerTest/ViewerTest_ViewerCommands_1.mm
Now git status displays a list of *.rej files:
# src/OpenGl/OpenGl_Display.cxx.rej
# src/OpenGl/OpenGl_Display_1.cxx.rej
# src/OpenGl/OpenGl_Workspace_2.cxx.rej
# src/TKService/EXTERNLIB.rej
# src/ViewerTest/ViewerTest_ViewerCommands.cxx.rej
These files contain the chunks which could not be applied. One has to apply them by hand.
For instance, to fix src/TKService/EXTERNLIB, we first read the .rej file, then go to OCCT gitweb interface and click on the diff link right to this file. It shows that three lines of text are added, these are the same as in the .rej file, but the file on OCCT git repository has an extra line at the end, this is why the patch could not be applied automatically. We copy the three lines into src/TKService/EXTERNLIB at the right place, then remove src/TKService/EXTERNLIB.rej. When all changes are performed, we can update index and make a commit:
git add -u
git am -r
Author and commit message are now the same as in OCCT git repository. We want to edit this commit message to explain what we did. Run
git commit --amend
and prepend [OCCT] on the first line, to clearly show that this patch comes from OCCT git repository. And at the end, it is better to document which changes were needed in order to apply this patch, for instance add at the end:
Cherry-picked from
http://git.dev.opencascade.org/gitweb/?p=occt.git;a=commit;h=4fe5661
Conflicts resolved in the following files:
src/OpenGl/OpenGl_Display.cxx
src/OpenGl/OpenGl_Display_1.cxx
src/OpenGl/OpenGl_Workspace_2.cxx
src/TKService/EXTERNLIB
src/ViewerTest/ViewerTest_ViewerCommands.cxx
with extra indentation to distinguish from original commit message.
As said in the previous section, we do not use OCCT git server. In order to track our changes, they are transformed into patchsets, and managed within the OCCT-patches branch. For instance, if the last OCCT release is 6.5.3, patches are refreshed from time to time by running these commands:
git checkout OCCT-patches
cp -a patches ../
git checkout official
QUILT_PATCHES=patches-653 quilt push -a
git add --all
git commit -m wip
git diff HEAD master
If there are changes, new patches are written until HEAD and master are the same.
Then patches are committed back into the OCCT-patches branch:
git reset --hard official/6.5.3
git checkout OCCT-patches
rm -rf patches
mv ../patches .
git add patches
git commit
When there is a new OCCT release, it is first unpacked into the official branch and tagged. Then previous patches are applied. Quilt stops when a patch does not apply cleanly. This patch is then either upgraded dropped, if it has been merged upstream. When all patches are applied, all changes are stored into the git index, and a new commit is added to the master branch with this index.