Contribute: Difference between revisions

From SambaWiki
(Fill in more detail on the First Merge Request steps)
m (→‎Change the default CI file: Update Gitlab doc URL and add anchor to target precise section)
 
(64 intermediate revisions by 4 users not shown)
Line 1: Line 1:
== How to contribute to Samba? ==
= How to contribute to Samba? =


Like all OpenSource projects, Samba is reliant on volunteers.
Like all OpenSource projects, Samba is reliant on volunteers.
Line 6: Line 6:
There are several category groups you can work on, e.g.:
There are several category groups you can work on, e.g.:
* Improve documentation
* Improve documentation
* Answer user questions and triage issues on the [https://lists.samba.org/mailman/listinfo/samba Samba users mailing list]
* Update/provide Wiki articles
* Update/provide Wiki articles
* Help testing, particularly of [[Samba_Release_Planning#Upcoming_Release|the upcoming release or master]] so we find bugs before our final release.
* Help testing
* Report bugs
* Report bugs in [http://bugzilla.samba.org Bugzilla]
* Help to verify patches in [http://bugzilla.samba.org Bugzilla]
* Help to verify patches in [http://bugzilla.samba.org Bugzilla] and in [https://gitlab.com/samba-team/samba/-/merge_requests merge requests]
* Create patches (C and Python developers)
* Create patches (C and Python developers)
* Anything else you can imagine
* Anything else you can imagine


'''Whenever participating in the Samba community, please follow and respect the guidelines in [[How to do Samba: Nicely]]'''
== 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 CI 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]].
===First Merge Request===


==Prepare your development environment==
The [[Using Git for Samba Development]] page has details on how to prepare patches using git.


Check out a copy of Samba locally:
====Obtain a GitLab.com account====
Samba development is done on [http://gitlab.com GitLab.com] so you will need to [https://gitlab.com/users/sign_in#register-pane Register a new user account]. You can sign in with you [https://github.com GitHub account if you have one]


git clone https://git.samba.org/samba.git
====Fork the Samba repo (just until we get to know you)====


You should develop new Samba features and fix bugs first in the '''master''' branch before attempting any [[Samba_Release_Planning#Release_Branch_Checkin_Procedure|backports]].
First fork the [https://gitlab.com/samba-team/samba samba git repo].


Run the [[Package_Dependencies_Required_to_Build_Samba#Verified_Package_Dependencies|bootstrap.sh script]] for your operating system from '''bootstrap/generated-dists/''' to install build dependencies and prepare your system to build samba.
By default projects on gitlab.com have a '''1 hour timeout''' set on pipelines. This '''must be changed''' in the [https://docs.gitlab.com/ee/user/project/pipelines/settings.html project settings].


==First Merge Request==
We suggest using a timeout of '''3 hours''', which is still permitted on the free runners.


Making your first merge request is straight-forward, provided you follow these steps carefully.
Otherwise, you will see errors like this:

===Start small===

Unless you are just submitting a spelling fix or a similar ''one line'' change, then you '''must''' review the '''[[Creating_Gitlab_Merge_Requests|full instructions on creating Samba patch series]]''' for instructions on what will need to be ''in'' the merge request. Do this '''before''' creating your patches to avoid rework.

===Obtain a GitLab.com account===
Samba development is done on [http://gitlab.com GitLab.com] so you will need to [https://gitlab.com/users/sign_in#register-pane Register a new user account]. You can [https://gitlab.com/users/sign_in sign in] with your [https://github.com GitHub] account if you have one.

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

This is sufficient for occasional small contributions, or for your first contribution to the project. For ongoing contributions and more complex work, you should [[Samba_on_GitLab#Getting_Access|request access]] to the samba-devel repository.

First fork the [https://gitlab.com/samba-team/samba 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 [https://docs.gitlab.com/ee/ci/pipelines/settings.html#set-a-limit-for-how-long-jobs-can-run 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
ERROR: Job failed: execution took longer than 1h0m0s seconds
The script exceeded the maximum execution time set for the job


====Change the default CI file====
The script exceeded the maximum execution time set for the job


By default a new fork on gitlab.com will copy an inappropriate '''CI/CD configuration file''' setting from the master project. The '''must also be changed''' in the [https://docs.gitlab.com/ee/ci/pipelines/settings.html#specify-a-custom-cicd-configuration-file project settings]. Set '''CI/CD configuration file''' back to an empty value.
====Prepare your patches and push back your first merge request====
After [[Using Git for Samba Development|preparing your patches]] [https://docs.gitlab.com/ee/gitlab-basics/start-using-git.html#send-changes-to-gitlabcom pushing back to your fork on gitlab.com] and [https://docs.gitlab.com/ee/user/project/merge_requests/creating_merge_requests.html submitting a merge request], the patches will be reviewed by samba team members who will post comments on your merge request.


===Subsequent Merge Requests===
===Prepare your patches===


Samba has coding standards, please read [https://gitlab.com/samba-team/samba/-/blob/master/README.Coding.md README.Coding.md] in your source checkout.
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. Pushing branches to samba devel will initiate a CI build test.


* Each patch should be as small as possible, i.e. changes only one thing. This makes review easier.
===Mailing patches to samba-technical===
* The patch should have an appropriate commit description.
* Patches that fix a bug should contain an appropriate <code>BUG</code> tag.


==== Samba Copyright and Community Policies ====
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].


'''Whenever participating in the Samba community, please follow and respect the guidelines in [[How to do Samba: Nicely]]'''
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>.


Samba has a [https://www.samba.org/samba/devel/copyright-policy.html Copyright Policy] that requires mandatory tags on every git commit. When [[Creating_a_Samba_patch_series|preparing your patches]] '''please ensure you add [[CodeReview#commit_message_tags|Signed-off-by tags]] to your commits'''.
The disadvantage to this approach is that your patches risk being missed by an interested samba team member. Submitting a merge request is preferred, because your patches become persistent and will not be forgotten.


Samba is a project with distributed copyright ownership and so [https://www.samba.org/samba/devel/copyright-policy.html following our Copyright Policy] ensures we are all clear that you have the permission and intention to contribute to Samba, and the licence you make those contributions under.
===Patch Reviews===

==== A good commit message ====

{{:A_Good_Git_Commit_Message}}

==== Include tests ====
{{:Writing_Tests_summary}}

===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 [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.

Review the [[Creating_Gitlab_Merge_Requests#Examples|examples of a good/bad merge request]] prior to submitting your merge request.

==Subsequent Merge Requests (and complex first requests)==

Because the Samba Team operates additional GitLab runners to support the full testsuite, we have a slightly unusual process with a single shared ''sandbox'' shared development repository, rather than the per-user fork you will have just created.

* After your first merge request has been reviewed, and now we know you are a genuine contributor, you may be '''[[Samba_on_GitLab#Getting_Access|granted access]]''' to the [https://gitlab.com/samba-team/devel/samba samba devel gitlab repo].
* If your first MR is complex, it is likely that your reviewer will push it again there (and link to the pipeline), or ask you to.
* If you do so, just close the original MR and open a new one, saying ''This replaces !123 submitted from a private fork''.

Likewise, subsequent merge requests should also be made from '''[https://gitlab.com/samba-team/devel/samba this repository]'''.

===Building a Samba patch series===

By this point you may be making fairly complex changes to Samba, so review '''[[Creating_a_Samba_patch_series|creating a samba patch series]]''' for detailed information on how to build [[Creating_a_Samba_patch_series|a polished series of changes to Samba]] ready for [[CodeReview|code review]].

===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 [[CodeReview|Code Review]] as your patches will work the first time, and they can see proof of that.

If you describe your work in the branch name, this will make [[Samba_CI_on_gitlab#Creating_a_merge_request|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 [[Samba CI on gitlab/Debugging CI failures|Debugging CI failures]]==

Pushing branches to [https://gitlab.com/samba-team/devel/samba samba devel gitlab repo] will initiate a full CI build test.

* CI results for changes are here: [https://gitlab.com/samba-team/devel/samba/pipelines Pipelines]
* [https://gitlab.com/samba-team/samba/merge_requests|All merge requests] show a link to the Pipeline (CI) results for each patch series.
* [[Samba CI on gitlab/Under the hood|How it works under the hood]]
* '''[[Samba CI on gitlab/Debugging CI failures|Debugging CI failures]]'''

=Code Review and using autobuild to merge into master=
==Effective Review==

As the submitter, it is '''your responsibility''' to make the most of [[CodeReview|code review]]. The reviewer will likely only point out a '''single instance''' of any given problem. You are expected to find every instance of this mistake and fix it throughout the code before re-submitting.

==Review process==

Once submitted, Samba Team members (in particular) will [[CodeReview|review your changes]], and post comments to you on your merge request. Broader discussions happen on our mailing list, so please ensure you are [https://lists.samba.org/mailman/listinfo/samba-technical 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.
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, whether on the mailing list, or reviewing merge requests on GitLab.
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=

A very small number of patches to Samba are contributed directly via the mailing list. 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].

'''Please include your [[CodeReview#commit_message_tags|Signed-off-by tag]] per the [https://www.samba.org/samba/devel/copyright-policy.html Samba copyright policy]'''.

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 advantages to this approach is that:
* '''This is an e-mail based process''' similar to that historically used by other Free Software projects of our era.

The disadvantages to this approach are that:
* '''This is essentially a manual process''' at our end.
* '''Subscription is still required''' to post to <code>samba-technical</code> (and you will need to follow our [https://www.samba.org/samba/devel/copyright-policy.html Samba copyright policy]).
* 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: A Samba developer will submit the patch to GitLab for you and advise you of the (URL to the) results.

'''Please submit a merge request if you can''', 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 ==

Using [https://git-scm.com/docs/git-send-email 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 '''strongly''' 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 ''See Also'' on the bug).
===Policies===
Regardless of how you send us your patches, please ensure you have added [[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]


Bugzilla is used however for the [[Samba_Release_Planning#Release_Branch_Checkin_Procedure|Release Branch Check-in Procedure]].
Whenever participating in the Samba community, please follow and respect the guidelines in [[How to do Samba: Nicely]]

Latest revision as of 11:15, 25 January 2022

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.:

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

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 before attempting any backports.

Run the bootstrap.sh script for your operating system from bootstrap/generated-dists/ to install build dependencies and prepare your system to build samba.

First Merge Request

Making your first merge request is straight-forward, provided you follow these steps carefully.

Start small

Unless you are just submitting a spelling fix or a similar one line change, then you must review the full instructions on creating Samba patch series for instructions on what will need to be in the merge request. Do this before creating your patches to avoid rework.

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 your GitHub account if you have one.

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

This is sufficient for occasional small contributions, or for your first contribution to the project. For ongoing contributions and more complex work, you should request access to the samba-devel repository.

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

Change the default CI file

By default a new fork on gitlab.com will copy an inappropriate CI/CD configuration file setting from the master project. The must also be changed in the project settings. Set CI/CD configuration file back to an empty value.

Prepare your patches

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

  • Each patch should be as small as possible, i.e. changes only one thing. This makes review easier.
  • The patch should have an appropriate commit description.
  • Patches that fix a bug should contain an appropriate BUG tag.

Samba Copyright and Community Policies

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

Samba has a Copyright Policy that requires mandatory tags on every git commit. When preparing your patches please ensure you add Signed-off-by tags to your commits.

Samba is a project with distributed copyright ownership and so following our Copyright Policy ensures we are all clear that you have the permission and intention to contribute to Samba, and the licence you make those contributions under.

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 backports to a released branch, so please file a bug first)
  • The description for the release notes starting with "RN: ". Please make sure that administrators understand the issue.
  • Your Signed-off-by tag per the 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
RN: Fix problem xyz.

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.

See writing and running Samba tests but in particular:

  • 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 lib/ldb/tests and are run from make test within lib/ldb
  • CTDB: Tests for CTDB are written as shell scripts under ctdb/tests and are run from make test within ctdb

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.

Review the examples of a good/bad merge request prior to submitting your merge request.

Subsequent Merge Requests (and complex first requests)

Because the Samba Team operates additional GitLab runners to support the full testsuite, we have a slightly unusual process with a single shared sandbox shared development repository, rather than the per-user fork you will have just created.

  • After your first merge request has been reviewed, and now we know you are a genuine contributor, you may be granted access to the samba devel gitlab repo.
  • If your first MR is complex, it is likely that your reviewer will push it again there (and link to the pipeline), or ask you to.
  • If you do so, just close the original MR and open a new one, saying This replaces !123 submitted from a private fork.

Likewise, subsequent merge requests should also be made from this repository.

Building a Samba patch series

By this point you may be making fairly complex changes to Samba, so review creating a samba patch series for detailed information on how to build a polished series of changes to Samba ready for code review.

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 the 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

Effective Review

As the submitter, it is your responsibility to make the most of code review. The reviewer will likely only point out a single instance of any given problem. You are expected to find every instance of this mistake and fix it throughout the code before re-submitting.

Review process

Once submitted, Samba Team members (in particular) will review your changes, and post comments to you on your merge request. Broader discussions happen on our mailing 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

A very small number of patches to Samba are contributed directly via the mailing list. To submit patches, use git format-patch and mail your patches to the samba-technical mailing list.

Please include your Signed-off-by tag per the Samba copyright policy.

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 advantages to this approach is that:

  • This is an e-mail based process similar to that historically used by other Free Software projects of our era.

The disadvantages to this approach are that:

  • This is essentially a manual process at our end.
  • Subscription is still required to post to samba-technical (and you will need to follow our Samba copyright policy).
  • 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: A Samba developer will submit the patch to GitLab for you and advise you of the (URL to the) results.

Please submit a merge request if you can, 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 strongly 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 See Also on the bug).

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