-
Notifications
You must be signed in to change notification settings - Fork 9
OFI Cray libfabric recommended workflow and policies
This wiki page documents the recommended workflow and policies we will be using in the libfabric-cray repository.
The first thing to note is that we have renamed the repository after forking ofiwg/libfabric to be libfabric-cray. We did this primarily to aid developers with keeping their repositories straight, since only GNI-specific work will be merged to this fork. The other reason is that the subject line for the github notification emails are prefixed with the repository name only, which makes things confusing if you get notifications from ofiwg/libfabric also. There is currently no way to configure the subject line (though the nice people at github have put in a feature request).
As mentioned above, for the most part only changes in the prov/gni directory should be merged to master. Notable exceptions are the changes needed to build the GNI provider, which need to be made in the top level makefile template and a couple of provider-independent source files. All other work should should go directly to ofiwg/libfabric.
All merges will be done via github reviewed pull requests. More on that below.
This section documents the suggested workflow. There are probably a few different ways to do the same thing, but this should give you an idea.
See https://help.github.com/articles/fork-a-repo/ for more detailed directions on how to fork, clone and set up a remote.
If you have not already forked ofiwg/libfabric, use the github interface to fork the repository under your account (Fork link near the top left of the repository pages). You can fork either ofiwg/libfabric or ofi-cray/libfabric-cray (you cannot fork both). You will need to manage your remotes when changing between the two.
Next, make a local clone of the repository. You can either do this from github from the side bar of your forked repository or via the command line.
Finally, set up your remotes, i.e.,the remote repositories you'll be interacting with. You can set up any number of distinct upstream remotes, for example ofiwg and ofi-cray.
% git remote add ofi-cray https://github.com/ofi-cray/libfabric-cray
% git remote add ofiwg https://github.com/ofiwg/libfabric
If you don't think you'll contribute to or merge from ofiwg, you can skip that one.
In general, you should do work in branches off of your fork. One notable exception is when merging changes from the upstream ofiwg repository, but this is done nightly by a jenkins job so most people shouldn't have to worry about it. There's a lot out there about working with branches, so no need to repeat it here, but here's a typical workflow:
% git checkout master
% git fetch ofi-cray # fetch the latest from ofi-cray
% git checkout ofi-cray/master # check out this (remote) branch (this gives you a "detached HEAD")
% git merge ofi-cray/master # merge latest from ofi-cray (fast-forward, i.e., without local commits)
% git checkout -b my-new-branch # create and checkout a new branch
Remember that your commits will most likely eventually get pushed upstream. The policy of the ofwig group is that all commits should be signed-off. You can easily make sure that all your commits get signed off by using the -s option to git commit. Also, to make it easier for the community developers to tell whether or not your commit(s) impact just the GNI provider, always in the first comment line of your commit, prepend prov/gni: if that is the case. Try to avoid intermingling code changes to the GNI provider with changes to common code.
There is a lot of info out there on fetching, merging, committing, etc. so no need to repeat it here.
Pull requests are a github-specific feature to share your changes with others for discussion. As policy, we will require a pull request for every merge, reviewed by at least one other person.
These articles on pull request cover most of what you need to know:
- https://help.github.com/articles/using-pull-requests/
- https://help.github.com/articles/creating-a-pull-request/
The only slightly tricky part is the destination repository. Make sure it's pointing at the correct master (ofiwg/libfabric or ofi-cray/libfabric-cray) depending on where you want to merge your commits.
Use an @mention in the pull request text to ask for a review from a particular person (setting the Assignee doesn't send a notification to that person).
We've decided to take a loose stance on squashing and rebasing. Having multiple commits per pull request is reasonable when it makes sense, but things like commits that fix type-os or small changes to logic should be squashed. We may revisit this policy later.
Any fix or feature should have an associated github issue opened for it. This will enable us to better track tasks, as well as making tasks available to all developers. When you pick up a task, make yourself the Assignee. When a pull requests closes an issue, mention it in the merge message and the issue will automatically be closed.
New features should be accompanied by new Criterion unit tests whenever possible, or an explanation why it is not possible. Roughly speaking, each individual unit test should not run for more than one minute.
The minimum required testing for any pull request is a complete successful run of gnitest. For more information on building gnitest, see the wiki.
In addition, for significant changes, please run the OSU MPI microbenchmarks against your changes using this [script] (https://github.com/ofi-cray/fab-utils/blob/master/scripts/benchmarks/run_osu). You will need to build Open MPI and the OSU microbenchmarks (info here).
OFI WG has decided to adopt the Linux coding standard. You can perform an automatic check of your patch using a pre-commit hook available here. You will need to download the Linux patch checker. See the comments at the top of the script for details. If you do not already have a pre-commit hook, put the contents of this script into a file called pre-commit in the directory libfabric-cray/.git/hooks. Otherwise, just call this script from your existing pre-commit script.
OFI WG requests that all commits contain a Signed-Off-By line. You can check for this by using the sample commit message hook in libfabric-cray/.git/hooks/commit-msg.sample. Just copy that to a file called commit-msg in the same directory and it will check that every commit message contains a Signed-Off-By line.
If either of these checks fail, your commit will fail.
Reference counting should be handled in the same manner as in the kernel. If a structure could be referenced from within another structure, the reference count of the original structure should be increased. When the reference would become invalid, the reference count should be decreased.
struct A {
/* attributes */
}
struct B {
struct A *ptr;
/* attributes */
}
void alloc_container(struct B *container, struct A *other)
{
container->ptr = other;
atomic_inc(&other->reference_count);
}
void free_container(struct B *container)
{
atomic_dec(&container->ptr->reference_count);
container->ptr = NULL;
}Globally available functions and variables should be prefixed with the 'gnix_' prefix. Functions and variables that are only available within the provider should be prefixed with a '_gnix'. Functions and variables that are only available to the local file should be prefixed with a double underscore('__'). Care should be taken to ensure the local file functions and variables are also static to prevent naming collisions.
As per your organization's policies, your organization's copyright can be added at the top of any source file that you touch. Using a pre-commit script to check for that is also useful.