Difference between revisions of "Starting as a Contributor"

From D Wiki
Jump to: navigation, search
m (How can I link a PR with Bugzilla?)
m (How can I link a PR with Bugzilla?)
Line 397: Line 397:
 
=== How can I link a PR with Bugzilla? ===
 
=== How can I link a PR with Bugzilla? ===
  
The integration is handled by [Dlang-Bot https://github.com/dlang-bots/dlang-bot].
+
The integration is handled by [https://github.com/dlang-bots/dlang-bot Dlang-Bot].
 
If you mention a bugzilla issue in your git commit message, you will get linked to the Bugzilla issue.
 
If you mention a bugzilla issue in your git commit message, you will get linked to the Bugzilla issue.
 
For more information, please visit [https://github.com/dlang-bots/dlang-bot Dlang-Bot's documentation].
 
For more information, please visit [https://github.com/dlang-bots/dlang-bot Dlang-Bot's documentation].

Revision as of 07:50, 10 December 2017

This page describes how to build D, put together a correct patch, and contribute it as a GitHub pull request.

Existing tools

There exist tools which can do some of the below steps automatically:

  • tools/setup.sh is a simple script that either installs a new or updates an existing D development tree. Just download the script and run.
  • Digger - can download and build D from any point in its recent history.
  • DVM - can build and locally install D from source code.

Building from source

The build information is split into Posix and Windows pages. Be sure to follow the according guide and continue to this page once your setup is working.

Unittest phobos

Full build

If you want to work on phobos itself, you need to run unittests—either for the full library, a package, or a module. To unittest the entire library:

make -j16 -f posix.mak unittest

Adjust the parameter passed to -j depending on your machine (beefier machines support larger parameters). This command unittests phobos in both debug and release mode. To only test one of them, add BUILD=debug or BUILD=release to the command line, for example:

make -j16 -f posix.mak BUILD=debug unittest

Specifying BUILD makes unittesting faster so it is recommended in iterative development. Just make sure both debug and release builds are tested before e.g. submitting a pull request.

Test a single package

While changing one specific package or module, it's useful to be able to only unittest that particular entity. The following two commands only unittest (in debug mode) the std.algorithm package:

make -j16 -f posix.mak BUILD=debug std/algorithm.test

Several modules, packages, or mix thereof may be specified for testing in the same command line. For example, this command also tests (and also in debug mode) the std.algorithm package and the std.conv module, with better parallelism:

make -j16 -f posix.mak BUILD=debug std/algorithm.test std/conv.test

Test a single package without Phobos

You can test a module directly with rdmd:

rdmddev -unittest -main std/algorithm/sorting.d

However, be aware that this method doesn't recompile Phobos and thus only checks the executed file. Thus you should only use this for fast builds on small changes. Moreover, for this method you will need to define the rdmddev alias or alternatively use dmd-nightly.

alias rdmdev='rdmd --compiler=$HOME/dlang/dmd/generated/linux/release/64/dmd -conf= -L--export-dynamic -I~/dlang/druntime/import -I~/dlang/phobos ~/dlang/phobos/generated/linux/release/64/libphobos2.a'

Run CI checks

The auto-tester will fail your PR if your changes contain trailing whitespace or incorrect line endings. You can run this test locally with the command:

make -j16 -f posix.mak checkwhitespace

CircleCi will also run various checks for the DStyle and ensures that every example is runnable on dlang.org. You can run this test locally with:

make -f posix.mak style

Running Independent Programs

The rig created so far allows making changes and testing dmd, druntime, and phobos by using their respective makefiles.

However, running independent programs (such as by using ~/dlang/dmd/generated/linux/release/64/dmd ~/mytest) will not know where to pick up the libraries from, even though the compiler path is correctly specified. As a consequence such programs will not compile, or (worse) will pick up libraries of a preexisting dmd installation on the target system and will fail in mysterious ways.

The remedy is to specify include and library paths properly. To build mytest.d using ~/dlang/dmd, ~/dlang/druntime, and ~/dlang/phobos, use this command line:

~/dlang/dmd/generated/linux/release/64/dmd -I~/dlang/druntime/import -I~/dlang/phobos -L-L$HOME/dlang/phobos/generated/linux/release/64 mytest.d

On a different OS, you will need to replace the OS name. For example for macOS:

~/dlang/dmd/generated/osx/release/64/dmd -I~/dlang/druntime/import -I~/dlang/phobos -L-L$HOME/dlang/phobos/generated/osx/release/64 mytest.d


The two -I uses bring the import files for druntime and phobos into visibility. The -L directive forwards the rest of the argument to the linker, i.e. the linker will get $HOME/dlang/phobos/generated/*/release/64/ with $HOME expanded properly. Sadly we cannot use the tilde (~) for -L because it won't get expanded properly. (If you need a 32-bit build, replace /64 with /32 and of course use the -m32 flag for the compiler.)

Of course if you need to use this line often, you may want to put it into a batch file or a dmd.conf file (see more info about dmd.conf on Linux, Windows, OSX, and FreeBSD).

Fetch and build dlang.org

Fetch dlang.org

This step is optional. Changes to phobos documentation require that the site (which includes automatically generated phobos documentation) builds successfully. Alternatively to building the documentation locally, you can use the documentation autotester service, which will build documentation, generate a diff of the results, and add a link to your GitHub pull request.

The following will clone the dlang.org repository:

cd ~/dlang
git clone https://github.com/dlang/dlang.org
cd dlang.org

Furthermore, all of dmd, druntime, phobos, tools are needed for the site to build. The instructions below will build the site to ~/dlang/dlang.org/web.


Build new Phobos documentation

If you are just interested in previewing your change to Phobos documentation, the html and phobos-prerelease will save time:

 make -f posix.mak html phobos-prerelease

Of course, parallelizing with -j improves speed as well.

To informally test it, open the appropriate HTML documents in that directory. Note that the currently released phobos would be in ~/dlang/dlang.org/web/phobos, whereas the current (fresh) build of phobos's documentation will reside in ~/dlang/dlang.org/web/phobos-prerelease. So, for example, if you change the embedded documentation in ~/dlang/phobos/std/conv.d, the changes are visible in ~/dlang/dlang.org/web/phobos-prerelease/std_conv.html. (The build process replaces the slashes in submodules with underscores.)

Avoiding network traffic

Note that one of the first lines output during the make run looks like this:

LATEST=2.110.0 <-- place in the command line to skip network traffic.

That's advice worth heeding because fetching LATEST automatically involves network traffic, which adds time to the build. So for future builds use this:

make -f posix.mak LATEST=2.110.0 html phobos-prerelease

Full build

Note that the full build all of the documentation in all forms, including Kindle builds and various other things that may require installing additional tools, and may download/build old versions of DMD. This may take some time and can be run with:

make -f posix.mak

Or if you want to avoid network traffic:

make -f posix.mak LATEST=2.110.0

Ancillary stuff

dconf.org, dub, dub-registry, installer, tools, and visuald

These ancillary repositories are of somewhat specific interest. Their installation mimics that of the repositories described above. If you get to the point where you need to work on one of these, chances are you're already versed in what needs doing. If not, ask away.

Additional Tools

If you cloned dlang/tools.git, you also have a tools folder where small helping programs live. There is no need to build them, you can just compile them using DMD:

dmd rdmd.d;
dmd ddemangle.d;
dmd dtab;
dmd tolf;

rdmd builds your D modules automatically, from the one containing main. It'll deduce dependencies and compile/link them for you. ddemangle will demangle its input, replacing all mangled D symbols with their unmangled form. dtab transforms tabs into spaces in source code. tolf replaces line endings with LF.

Using dtab and tolf is a good idea if you want to contribute to the dlang repos.

Typical Contributor Workflow

There are many ways to use git and GitHub to contribute. Here's a typical one.

First, fork the github repository or repositories you'd like to contribute to (dmd, druntime, phobos etc) by navigating to their respective pages on github.com and clicking "Fork". Then, set up your local git repository to reflect that. For example, consider you want to contribute to phobos and have forked it. Then run these commands:

cd ~/code/phobos
git remote add myfork https://github.com/username/phobos.git
git remote update

(Replace username with your actual github user name.) This adds the "myfork" repository and makes sure everything is synchronized between your local copy and the remote repositories. Then, it's best to work in branches as shown below:

git checkout -b awesome-new-feature
# ... get some good work done here ...
git commit -am "Awesome new feature ..."
git push -f myfork

With this, your work is in your github fork of the phobos (or whichever) repository. After that, visit your fork on github.com, which looks like https://github.com/username/phobos/tree/awesome-new-feature.

Create a pull request

Once you have tested all your changes and pushed them to your fork on GitHub, you are ready to submit a pull request.

  • Navigate to your fork of the project on GitHub.
  • Important: Select the branch that you made your changes in, say issue_1234.
  • Click on the "Pull Request" button.

This will submit your changes for review by the D maintainers. If your changes are approved, they will be merged into the master branch. Otherwise, if the maintainers have some comments or feedback, you can refine your changes by editing and testing in your local workspace, and pushing the new changes to the same git branch. The new changes will be automatically included in your pull request.

Choose a title for your pull request that clearly states what it does. When fixing a bug, the usual thing to do is to use the summary from the bugzilla report. Eg a title like "Fix 3797" or "Issue 3797" contains much less information than "Fix Issue 3797 - Regression(2.038): Implicit conversion between incompatible function pointers" and requires a lot more effort for the reviewers to determine if it is something they are interested in.

Pull request descriptions should contain a hyperlink to the Bugzilla issue that is being fixed. This is usually added at the end of the description.

If the pull request is for a bug fix, the commit message should have the format "fix issue 1234". This enables the dlang-bot to automatically pick up the issue from Bugzilla and post it as a comment on the PR. If the PR is already open, then a git rebase is necessary followed by a force push. During the rebase, the commit message should be renamed to match the one specified.

After the pull request is created, add the 'pull' keyword to the corresponding Bugzilla issue and a link to the pull request posted in a comment.

Characteristics of a Good Pull Request

  • Addresses one topic only.
  • Refactorings should not be mixed in with bug fixes or enhancements.
  • Refactorings should be marked as such in the subject, and must not contain any behavior changes.
  • Larger refactorings need to broken up into smaller PRs.

The problems with large PRs are:

  • They are hard to review.
  • github is very slow at serving pages with large diffs on them.
  • If the PR introduces a regression, a large diff is much harder to debug than a small one.
  • They imply an all or nothing view of it, when it actually may have good parts and bad parts.

Autotester

Pull requests are automatically picked up by the autotester, which compiles the code in the pull request and runs it through the dmd, druntime, and phobos unittests on all supported platforms. Generally, pull requests must pass all tests before they will be merged. The status of the tests can be monitored through the pull request page.

Every user must be manually approved before the autotester will start testing their pull requests. Users can be approved by anyone with commit access.

Rebasing

Sometimes, if a particular change you are working on is taking a long time, or if you encounter a problem that is fixed by a new commit upstream, you may need to sync your local branch with master in order to keep the code up-to-date. In this case, it is recommended that you use git rebase to apply your changes on top of the latest git master, so that when you submit a pull request, the change history will be easier for the reviewers to follow. Using git merge is not recommended, as it may produce a lot of merge commits that may not be relevant to your changes.

For example, you may be working on your changes:

cd /usr/src/d/phobos
git checkout mybranch
vim std/algorithm.d   # apply lots of cool changes here

First, before you resync with master, make sure all your changes are checked in (or stashed):

git commit -a

If you forked from the official D programming language repositories you may need to add an upstream remote to pull in the latest official changes. If this is the case you can add an upstream remote like this:

git remote add upstream git@github.com:dlang/phobos

This adds another remote to your repository called upstream and only needs to be done once. Once the upstream remote is added, you can update your repository's master branch by running the following:

git checkout master
git pull --ff-only upstream master

The --ff-only option is to ensure that your master branch is identical to the official D sources' master branch, since otherwise you will end up with a very messy history that will be hard to clean up (and the reviewers will probably reject your pull request due to having unrelated merge commits).

Now go back to your branch and rebase it:

git checkout mybranch
git rebase master

Now your sources should be up-to-date. Recompile and test everything to make sure it all works.

Note that after rebasing, you will need to force an update to your fork on GitHub with the -f flag, otherwise it will fail because the histories don't match anymore:

git push -f origin mybranch

You may wish to read up on how git rebase works if you're not familiar with the concept.

If, during the 'git rebase' command, you encounter conflicts, you may want to learn how to resolve a conflict during git rebase.

Squashing

After receiving feedback on your PR, it's common for it to have lots of commits that don't add much by being separate. For example, consider the following git history on a PR:

commit [ffffff] Added new function: foobar
commit [aaaaaa] Spelling error fix in foobar docs
commit [cccccc] Clarified Docs for foobar

Nothing is gained from having these as three separate commits as they are all focused on one feature. Instead, they should be one commit so the history looks like this

commit [333333] Added new function: foobar

while still retaining all of your changes. In order to perform this, please consult this guide

You can also directly append to your last commit and force an update of your PR:

git commit --amend
git push -f

Stable Branch

If you are working on a fix for a regression, chances are it should go into the next point release, and not the next major version (e.g. 2.067.1 instead of 2.068). In this case, you should check out the stable branch of each subproject BEFORE you create your topic branch:

cd dmd
git checkout stable
cd ../druntime
git checkout stable
cd ../phobos
git checkout stable

Then follow the instructions for making a branch.

If you forget to do this, or didn't realize it, It's not possible to simply re-target your branch for pulling into the stable branch. GitHub will let you do this, but your branch will include many of the changes from the unstable branch!

In order to fix such a problem, you can rebase your changes from master on top of the stable branch. First you need to pull in the stable branch from your fork on github:

git checkout stable

Then, you go back to your branch, and replay the changes from master using rebase:

git checkout mybranch
git fetch upstream
git rebase --onto upstream/stable upstream/master mybranch

You may have to follow the instructions in the Rebasing section on adding the upstream branch, substituting stable for master, if you need to update to the latest stable changes.

This sometimes may not work, as the changes between the stable and master are too drastic. In this case, you may have to re create your changes after a clean checkout of the stable branch.

When creating a pull request, you need to tell github to target the stable branch instead of master on the upstream repository. This is done via a drop-down at the top of the page, make sure to do this before submitting your pull request as this cannot be changed after the PR is created (you will have to close the PR and create a new one).

If you notice in your PR a whole slew of changes that seem to have nothing to do with your changes, it's likely because you forgot one of these steps.

Reviews

Any pull requests that make language changes must be approved by Walter and Andrei. This includes druntime changes that implement the specification.

Any pull requests that make significant changes to code should be reviewed by more than one person. This means that at least two people need to approve the pull request before it is merged. One person must be a person with commit rights, but the other need not be, as long as that person is trusted within the developer community.

Pull requests that are trivial (typos, obvious minor bug fixes, etc.) may be pulled without a second review.

Please note that any updates pushed to the candidate branch do not automatically notify a subscribed person. If you update your branch to correct an issue, please also put in a comment indicating it.

Contributing FAQ

A file that I made a change on was modified by a different merged PR, and now my PR can't be merged, now what?

What you need to do is rebase your git branch on the master branch. What this does is rewrite the history of your git branch to make it seem like it was merged off of the head of master rather than the older commit where you actually branched. This will include the new commits in your PR so your PR no longer conflicts. See this tutorial for more details.

I would to help, but I don't know how. What is the best way?

Anyone is welcome to contribute as D is very much a volunteer effort! The best place to look for ideas to contribute is the get involved guide. Please also don't hesitate to point out (or even fix) any stumbling blocks you may encounter when starting out.

CodeCov shows a red cross

On every build the generated coverage files are sent to CodeCov for further processing. You can see the non-covered lines by clicking on the link or for further convenience in the future by installing the CodeCov browser extension.

CircleCi shows a red cross

On CircleCi static code analysis is run. For example it lints the Phobos codebase after the DStyle. Moreover, checks are run to ensure that every example on dlang.org is runnable. On Posix, you can run this locally by executing the style target:

make -f posix.mak style

The auto-tester shows a red cross

The auto-tester tests every PR on all supported platforms (Windows, Linux, macOS, and Linux) for 32 and 64-bit builds. Please click on the link of your PR to find out more about the error.

The Project Tester shows a red cross

The Project Tester tests every PR on all supported platforms (Windows, Linux, macOS, and Linux) for 32 and 64-bit builds. Please click on the link of your PR to find out more about the error.

How can I link a PR with Bugzilla?

The integration is handled by Dlang-Bot. If you mention a bugzilla issue in your git commit message, you will get linked to the Bugzilla issue. For more information, please visit Dlang-Bot's documentation.

See Also