Difference between revisions of "Contribute"

(mention ban on git send-email)
(Use David's original wording for patches on samba-technical)
(15 intermediate revisions by the same user not shown)
Line 14: Line 14:
 
* Anything else you can imagine
 
* Anything else you can imagine
  
= Submitting Patches =
+
= Submitting Patches via GitLab =
  
 
The preferred method for submitting patches to samba is via [https://gitlab.com/samba-team/samba the official mirror] on [https://gitlab.com/ GitLab].  For more information see [[Samba on GitLab]].
 
The preferred method for submitting patches to samba is via [https://gitlab.com/samba-team/samba the official mirror] on [https://gitlab.com/ GitLab].  For more information see [[Samba on GitLab]].
Line 53: Line 53:
 
  The script exceeded the maximum execution time set for the job
 
  The script exceeded the maximum execution time set for the job
  
===Prepare your patches and push back your first merge request===
+
===Prepare your patches===
After [[Using Git for Samba Development|preparing your patches]] and [https://docs.gitlab.com/ee/gitlab-basics/start-using-git.html#send-changes-to-gitlabcom pushing back to your fork on gitlab.com] you can [https://docs.gitlab.com/ee/user/project/merge_requests/creating_merge_requests.html submit a merge request]. 
 
  
 +
Samba has coding standards, please read [https://git.samba.org/?p=samba.git;a=blob_plain;f=README.Coding README.Coding] in your source checkout.
 +
 +
When [[Using Git for Samba Development|preparing your patches]] please ensure you add [[CodeReview#commit_message_tags|Signed-off-by tags]] to your commits, and follow the [https://www.samba.org/samba/devel/copyright-policy.html Samba copyright policy].
 +
 +
==== A good commit message ====
 +
 +
Good commit messages are very important. Commits should always be separated in meaningful pieces.
 +
Please make sure that your commit messages include at least the following information:
 +
 +
* A '''short''' summary of the fix/feature including the Samba version and component followed by a blank line.
 +
* A more verbose description.
 +
* A bug number if available (required for [[Samba_Release_Planning#Release_Branch_Checkin_Procedure|backports to a released branch]], so please file a bug first)
 +
* Your [[CodeReview#commit_message_tags|Signed-off-by tag]] per the [https://www.samba.org/samba/devel/copyright-policy.html Samba copyright policy]
 +
 +
Example:
 +
 +
s3:smbd: Add new funky shiny and groovy feature
 +
 +
The funky shiny and groovy feature has been added to
 +
be able to do whatever you like. A typical usecase is the
 +
following.
 +
 +
BUG: https://bugzilla.samba.org/show_bug.cgi?id=1
 +
 +
Signed-off-by: Author <author@example.org>
 +
 +
==== Include tests ====
 +
 +
Most changes to Samba should have a test to demonstrate the bug being fixed, or test the feature being added.  Most tests are run using 'make test' from a Samba source tree. 
 +
* [[Writing Torture Tests]]: smbtorture in the ''source4/torture'' directory and provides direct C protocol tests.
 +
* [[Writing Python Tests]]: If the protocol under test is [[DCERPC]], then [[PIDL]] will have already auto-generated [[Python]] bindings.  Likewise LDAP is easily accessed via [[LDB]].
 +
* [[Writing cmocka Tests]]: Idea for unit tests of C functions.
 +
* [[LDB]]: Tests for LDB are in <code>lib/ldb/tests</code> and are run from <code>make test</code> within <code>lib/ldb</code>
 +
 +
===Push back to GitLab and make your first merge request===
 
Push to your repo (rather than git.samba.org, which will be origin) with:
 
Push to your repo (rather than git.samba.org, which will be origin) with:
  
 
  $ git push gitlab-$USER -f
 
  $ git push gitlab-$USER -f
  
The patches will be reviewed by [https://www.samba.org/samba/team/ Samba Team members] who will post comments on your merge request.
+
After [https://docs.gitlab.com/ee/gitlab-basics/start-using-git.html#send-changes-to-gitlabcom pushing back to your fork on gitlab.com] you can [https://docs.gitlab.com/ee/user/project/merge_requests/creating_merge_requests.html submit a merge request].  (The push will give a URL for you to click).
 +
 
 +
The merge request will be [https://gitlab.com/samba-team/samba/-/merge_requests listed for review] by [https://www.samba.org/samba/team/ Samba Team members] who will post comments on your merge request.
  
 
==Subsequent Merge Requests==
 
==Subsequent Merge Requests==
Line 66: Line 102:
 
After your first merge request has been approved, you will be granted access to the [https://gitlab.com/samba-team/devel/samba samba devel gitlab repo]. Subsequent merge requests should be made from this repository, because more tests are run than are possible on the free runners attached to your personal repo.
 
After your first merge request has been approved, you will be granted access to the [https://gitlab.com/samba-team/devel/samba samba devel gitlab repo]. Subsequent merge requests should be made from this repository, because more tests are run than are possible on the free runners attached to your personal repo.
  
===Code of conduct===
+
===Shared development repo: Code of conduct===
  
Use only to develop Samba. Don't overwrite the work of others.  Prefix branches with your gitlab username, eg:
+
Use our shared development repository only to develop Samba. Don't overwrite the work of others.  Prefix branches with your gitlab username, eg:
  
 
  $USER/topic
 
  $USER/topic
Line 99: Line 135:
 
* '''[[Samba CI on gitlab/Debugging CI failures|Debugging CI failures]]'''
 
* '''[[Samba CI on gitlab/Debugging CI failures|Debugging CI failures]]'''
  
=Code Review=
+
=Code Review and using autobuild to merge into master=
  
 
Once submitted, Samba Team members (in particular) will [[CodeReview|review your changes]], and post comments to you either on your merge request.  Broader discussions happen on the list, so please ensure you are [https://lists.samba.org/mailman/listinfo/samba-technical subscribed to samba-technical] also.
 
Once submitted, Samba Team members (in particular) will [[CodeReview|review your changes]], and post comments to you either on your merge request.  Broader discussions happen on the list, so please ensure you are [https://lists.samba.org/mailman/listinfo/samba-technical subscribed to samba-technical] also.
Line 106: Line 142:
  
 
Outside contributors are welcome to review patches also, this is a good way to learn more about Samba!
 
Outside contributors are welcome to review patches also, this is a good way to learn more about Samba!
 +
 +
==Merging patches from GitLab (for Samba Team members)==
 +
 +
If the developer has created a merge request, then to merge, download the patch with (eg)
 +
 +
https://gitlab.com/samba-team/samba/merge_requests/12.patch
 +
 +
[[CodeReview|Add review tags]] and then <code>git autobuild</code> locally.  The merge button
 +
sadly doesn't work.
 +
 +
After the [[autobuild]] completes, please close the merge request using the git hash finally assigned in '''Samba.org's git  [https://git.samba.org/?p=samba.git;a=shortlog;h=refs/heads/master master]''' in the comment like:
 +
 +
Merged into master as <git hash> for Samba <next version>.
 +
 +
See for example [https://gitlab.com/samba-team/samba/merge_requests/9 this closed merge request].
  
 
=Mailing patches to samba-technical=
 
=Mailing patches to samba-technical=
  
While not ideal, submitting patches via the mailing list is possible and they may still be considered. To submit patches, [[Using_Git_for_Samba_Development#Creating_patches_if_you_don.27t_have_write_access_to_git.samba.org_repositories|use git format-patch]] and mail your patches to the [https://lists.samba.org/mailman/listinfo/samba-technical samba-technical mailing list].
+
While not ideal, submitting patches via the mailing list is still considered acceptable. To submit patches, [[Using_Git_for_Samba_Development#Creating_patches_if_you_don.27t_have_write_access_to_git.samba.org_repositories|use git format-patch]] and mail your patches to the [https://lists.samba.org/mailman/listinfo/samba-technical samba-technical mailing list].
  
The preferred format for patch sets is a single-file bundle attached to the email you send to the list. Bundling can be automated by invoking <code>git format-patch</code> with the flag <code>--stdout</code>.
+
The required format for patch sets is a single-file bundle attached to the email you send to the list. Bundling can be automated by invoking:
 +
 +
git format-patch --stdout origin/master > master-featureX-01.patches.txt
  
 
The disadvantages to this approach is that:
 
The disadvantages to this approach is that:
Line 118: Line 171:
 
* CI testing is still required, so a Samba developer will need to submit the patch to GitLab for you
 
* CI testing is still required, so a Samba developer will need to submit the patch to GitLab for you
  
Submitting a merge request is therefore preferred, because the latest version of your patches remain persistent and will not be forgotten.
+
Submitting a merge request is therefore preferred, because the latest version of your patches become persistent in our [https://gitlab.com/samba-team/samba/-/merge_requests list of outstanding merge requests] and so will not be forgotten.
  
 
== git send-email ==
 
== git send-email ==
  
 
Using git send-email in the Samba project is '''not permitted'''.  Doing so will cause, without manual work by the recipient, your patch to be credited to '''samba-technical@...''' not your own e-mail address.
 
Using git send-email in the Samba project is '''not permitted'''.  Doing so will cause, without manual work by the recipient, your patch to be credited to '''samba-technical@...''' not your own e-mail address.
 +
 +
=[https://bugzilla.samba.org Bugzilla]=
 +
 +
We prefer that new patches for master are not submitted to [https://bugzilla.samba.org Bugzilla] alone, for similar reasons.  Please submit the patches to GitLab instead (note the merge request in the URL on the bug). 
 +
 +
Bugzilla is used however for the [[Samba_Release_Planning#Release_Branch_Checkin_Procedure|Release Branch Check-in Procedure]].
  
 
=Policies=
 
=Policies=

Revision as of 23:55, 23 May 2020

How to contribute to Samba?

Like all OpenSource projects, Samba is reliant on volunteers. You don't need special skill to help this project. Everybody can help! :-)

There are several category groups you can work on, e.g.:

Submitting Patches via GitLab

The preferred method for submitting patches to samba is via the official mirror on GitLab. For more information see Samba on GitLab.

Prepare your development environment

Check out a copy of Samba locally:

git clone https://git.samba.org/samba.git

You should develop new Samba features and fix bugs first in the master branch.

Run the bootstrap.sh script for your operating system from bootstrap/generated-dists/.

First Merge Request

The Using Git for Samba Development page has details on how to prepare patches using git.

Obtain a GitLab.com account

Samba development is done on GitLab.com so you will need to Register a new user account. You can sign in with you GitHub account if you have one

Fork the Samba repo (just until we get to know you)

First fork the samba GitLab.com official mirror.

In the git checkout you made earlier, add a remote for your new gitlab fork (the URL will be in the GitLab UI under the blue Clone button):

$ git remote add gitlab-$USER git@gitlab.com:$USER/samba.git

Change the 1hr default CI timeout

By default projects on gitlab.com have a 1 hour timeout set on pipelines. This must be changed in the project settings. We suggest using a timeout of 3 hours, which is still permitted on the free runners.

Otherwise, once you push your changes back, you will see errors like this:

ERROR: Job failed: execution took longer than 1h0m0s seconds
The script exceeded the maximum execution time set for the job

Prepare your patches

Samba has coding standards, please read README.Coding in your source checkout.

When preparing your patches please ensure you add Signed-off-by tags to your commits, and follow the Samba copyright policy.

A good commit message

Good commit messages are very important. Commits should always be separated in meaningful pieces. Please make sure that your commit messages include at least the following information:

Example:

s3:smbd: Add new funky shiny and groovy feature

The funky shiny and groovy feature has been added to
be able to do whatever you like. A typical usecase is the
following.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=1

Signed-off-by: Author <author@example.org>

Include tests

Most changes to Samba should have a test to demonstrate the bug being fixed, or test the feature being added. Most tests are run using 'make test' from a Samba source tree.

Push back to GitLab and make your first merge request

Push to your repo (rather than git.samba.org, which will be origin) with:

$ git push gitlab-$USER -f

After pushing back to your fork on gitlab.com you can submit a merge request. (The push will give a URL for you to click).

The merge request will be listed for review by Samba Team members who will post comments on your merge request.

Subsequent Merge Requests

After your first merge request has been approved, you will be granted access to the samba devel gitlab repo. Subsequent merge requests should be made from this repository, because more tests are run than are possible on the free runners attached to your personal repo.

Shared development repo: Code of conduct

Use our shared development repository only to develop Samba. Don't overwrite the work of others. Prefix branches with your gitlab username, eg:

$USER/topic

In return you get a full CI run using Samba Team provided resources. That in turn makes it easier for Samba Team members doing Code Review as your patches will work first time, and they can see proof of that.

If you describe your work in the branch name, this will make generating a merge request easier, as the branch name becomes the template title and allows ongoing distinct merge requests.

Step by step instructions

Add a git reference to the gitlab remote repository:

$ git remote add gitlab-ci git@gitlab.com:samba-team/devel/samba.git

Name your branch for our shared repo:

$ git checkout -b $USER/foo

Push the current branch, overwriting any other changes there:

$ git push gitlab-ci -f

Pipelines, Successful tests and Debugging CI failures

Pushing branches to samba devel gitlab repo will initiate a full CI build test.

Code Review and using autobuild to merge into master

Once submitted, Samba Team members (in particular) will review your changes, and post comments to you either on your merge request. Broader discussions happen on the list, so please ensure you are subscribed to samba-technical also.

Patches from non-samba team members require a minimum of 2 reviews from samba team members prior to patches being merged.

Outside contributors are welcome to review patches also, this is a good way to learn more about Samba!

Merging patches from GitLab (for Samba Team members)

If the developer has created a merge request, then to merge, download the patch with (eg)

https://gitlab.com/samba-team/samba/merge_requests/12.patch

Add review tags and then git autobuild locally. The merge button sadly doesn't work.

After the autobuild completes, please close the merge request using the git hash finally assigned in Samba.org's git master in the comment like:

Merged into master as <git hash> for Samba <next version>.

See for example this closed merge request.

Mailing patches to samba-technical

While not ideal, submitting patches via the mailing list is still considered acceptable. To submit patches, use git format-patch and mail your patches to the samba-technical mailing list.

The required format for patch sets is a single-file bundle attached to the email you send to the list. Bundling can be automated by invoking:

git format-patch --stdout origin/master > master-featureX-01.patches.txt

The disadvantages to this approach is that:

  • your patches risk being missed by an interested samba team member.
  • No instant feedback to you is possible regarding compilation errors and test failures
  • CI testing is still required, so a Samba developer will need to submit the patch to GitLab for you

Submitting a merge request is therefore preferred, because the latest version of your patches become persistent in our list of outstanding merge requests and so will not be forgotten.

git send-email

Using git send-email in the Samba project is not permitted. Doing so will cause, without manual work by the recipient, your patch to be credited to samba-technical@... not your own e-mail address.

Bugzilla

We prefer that new patches for master are not submitted to Bugzilla alone, for similar reasons. Please submit the patches to GitLab instead (note the merge request in the URL on the bug).

Bugzilla is used however for the Release Branch Check-in Procedure.

Policies

Regardless of how you send us your patches, please ensure you have added Signed-off-by tags to your commits, and follow the Samba copyright policy

Whenever participating in the Samba community, please follow and respect the guidelines in How to do Samba: Nicely