{ "cells": [ { "cell_type": "markdown", "metadata": {}, "source": [ "$$\n", "\\def\\CC{\\bf C}\n", "\\def\\QQ{\\bf Q}\n", "\\def\\RR{\\bf R}\n", "\\def\\ZZ{\\bf Z}\n", "\\def\\NN{\\bf N}\n", "$$\n", "# How to contribute to Sage\n", "\n", "## S\u00e9bastien Labb\u00e9\n", "\n", "*If the slide text doesn't fit in your browser window, try decreasing the text size.*\n", "*Type* `T` *if you have trouble viewing this presentation.*\n", "\n", "**Sage Days 28**\n", "\n", "Orsay, France, January 17-19th 2011\n", "\n", "### GNU General Public Licence\n", "\n", "Sage is distributed under the terms of the GNU General Public License version 2 ([GPLv2](http://www.gnu.org/licenses/)) which provides four kinds of freedom:\n", "\n", "- Freedom to run the program\n", "- Freedom to access the code\n", "- Freedom to redistribute the program to anyone\n", "- Freedom to improve the software\n", "\n", "While all users of Sage make use of the first freedom, in this talk, we will see how to appropriate the other three.\n", "\n", "### Twenty Two Easy Steps\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "\n", "
    \n", "
  1. Find a bug
    2. \n", "
    \n", "
  1. Build the documentation
    2. \n", "
    \n", "
  1. Sage trac server
    2. \n", "
    \n", "
  1. Update the current patch
    2. \n", "
    \n", "
  1. Create a ticket
    2. \n", "
    \n", "
  1. Export a patch
    2. \n", "
    \n", "
  1. Clone your Sage
    2. \n", "
    \n", "
  1. Verify the patch
    2. \n", "
    \n", "
  1. Mercurial
    2. \n", "
    \n", "
  1. Upload the patch
    2. \n", "
    \n", "
  1. Enable Mercurial queues
    2. \n", "
    \n", "
  1. More on Mercurial queues
    2. \n", "
    \n", "
  1. Create an empty patch
    2. \n", "
    \n", "
  1. Dowload a patch
    2. \n", "
    \n", "
  1. Fix the bug
    2. \n", "
    \n", "
  1. Edit the series file
    2. \n", "
    \n", "
  1. View your changes
    2. \n", "
    \n", "
  1. Reviewing a patch
    2. \n", "
    \n", "
  1. Test the changes
    2. \n", "
    \n", "
  1. Positive review or Needs work
    2. \n", "
    \n", "
  1. Run tests
    2. \n", "
    \n", "
  1. Do some cleaning
    2. \n", "
\n", "\n", "Are you ready?\n", "\n", "### 1. Find a bug\n", "\n", "That's the easiest part. Choose one amongst this\n", "\n", "> [selection of unreported documentation bugs](http://wiki.sagemath.org/days28-bugs_to_report)\n", "\n", "made for Sage Days 28 or browse the\n", "\n", "> [Open Beginner Tickets](http://trac.sagemath.org/sage_trac/query?status=needs_info&status=needs_review&status=needs_work&status=new&order=priority&col=id&col=summary&col=status&col=type&col=priority&col=milestone&col=component&keywords=~beginner&report=38).\n", "\n", "*During this talk, instead of fixing a bug I am going to introduce one in the inverse method of a permutation.*\n", "\n", "### 2. Sage trac server\n", "\n", "In Sage, modifications are tracked on a web site called [Sage trac](http://trac.sagemath.org). Every bug gets assigned a number. For instance, the number [\\#10484](http://trac.sagemath.org/sage_trac/ticket/10484) refers to the bug called Chinese remainder code raises an error when called\n", "with Python ints. On the ticket, one can see that:\n", "\n", "- The bug was reported and solved by **David Loeffler** (UK) in December 2010.\n", "- The ticket was positively reviewed by **Robert Bradshaw** (USA) and **Mike Hansen**.\n", "- The solution was merged in `sage-4.6.2` by **Jeroen Demeyer** (Belgium) on January 11th 2011.\n", "\n", "One can also look at the solution, download it, test it, etc.\n", "\n", "### 3. Create a ticket\n", "\n", "In order to create a ticket:\n", "\n", "- Create an account on \n", "- Login to your account\n", "- Make sure the ticket does not already exists.\n", "- Create ticket\n", "- In the description field, explain how should someone else understand and/or reproduce the bug.\n", "\n", "*I create the imaginary ticket \\#12345 for introducing a useless* `print` *statement in the method that computes the inverse of a permutation.*\n", "\n", "### 4. Clone your version of Sage\n", "\n", "**Clone Sage and create your branch** (Do it right now because it might take some time) \n", "`sage -clone slabbe`\n", "\n", "This creates a new directory called `sage-slabbe` in the `devel` repository:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "slabbe@pol ~/Applications/sage-4.6.1/devel $ ls -l\n", "drwxr-xr-x 2 slabbe staff 68 14 jan 03:59 old/\n", "lrwxr-xr-x 1 slabbe staff 9 18 jan 15:01 sage -> sage-main/\n", "drwxr-xr-x 23 slabbe staff 782 18 jan 01:42 sage-main/\n", "drwxr-xr-x 24 slabbe staff 816 17 jan 01:50 sage-slabbe/\n", "lrwxr-xr-x 1 slabbe staff 11 14 jan 03:42 sagenb -> sagenb-main/\n", "drwxr-xr-x 21 slabbe staff 714 14 jan 03:41 sagenb-main/" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "> - - `sage -br` --\n", "\n", "### 5. Mercurial\n", "\n", "Sage uses the program Mercurial ( **hg** or **sage -hg** ) to manage all of its source code. Mercurial stores the evolution of every single file of Sage *since the beginning*.\n", "\n", "Since I am too lazy to write **sage -hg** everytime I use Mercurial, I added the following line to my `~/.bashrc` file:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "alias hg='sage -hg'" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "I verify that it works:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "slabbe@pol ~ $ hg --version\n", "Mercurial Distributed SCM (version 1.6.4)\n", "\n", "Copyright (C) 2005-2010 Matt Mackall and others\n", "This is free software; see the source for copying conditions. There is NO\n", "warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE." ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "### 5. Mercurial\n", "\n", "**hg log** \n", "Print the revision history of the specified files or the entire project." ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "slabbe@pol ~/Applications/sage-4.6.1/devel/sage-main $ hg log\n", "\n", "changeset: 15205:f24ce048fa66\n", "tag: tip\n", "user: Jeroen Demeyer\n", "date: Tue Jan 11 08:10:26 2011 +0100\n", "summary: 4.6.1\n", "\n", "\n", "changeset: 0:039f6310c6fe\n", "user: tornaria\n", "date: Sat Feb 11 01:13:08 2006 +0000\n", "summary: [project @ original sage-0.10.12]" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "**hg update** \n", "Update the repository's working directory to the specified changeset.\n", "\n", "### 6. Enable Mercurial queues\n", "\n", "**Mercurial queues** is an extension to Mercurial that allows one to easily work with collections of patches. To allow Mercurial queues, edit (or create) the file `~/.hgrc` and make sure it contains the line hgext.mq = in the extensions section:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "[ui]\n", "username = Sebastien Labbe \n", "[extensions]\n", "hgext.mq =\n", "color =\n", "[alias]\n", "qstatus = status --rev -2:." ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "The line `hgext.mq=` is an indispensable for the next steps.\n", "\n", "### 7. Create an empty patch\n", "\n", "**cd to your branch**:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "cd Applications/sage-4.6.1/devel/sage-slabbe" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "**Create a new empty patch**:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "hg qnew trac_12345-add_useless_print-sl.patch" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "### 8. Fix the bug\n", "\n", "**Find the file containing the bug**\n", "\n", "*Personnally I found the file* `permutation.py` *here*:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "sage-4.6.1/devel/sage-slabbe/sage/combinat/permutation.py" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "**Find the solution to the bug**\n", "\n", "**Edit the source code accordingly, save and quit**\n", "\n", "### 9. View your changes\n", "\n", "Now, you may use the following Mercurial commands to look at your local changes.\n", "\n", "**hg status** shows changed files since last **hg qnew** (or **hg qrefresh**):" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "slabbe@pol ~/Applications/sage-4.6.1/devel/sage-combinat/sage/combinat $ hg status\n", "M sage/combinat/permutation.py" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "**hg diff** shows differences since last **hg qnew** (or **hg qrefresh**)" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "slabbe@pol ~/Applications/sage-4.6.1/devel/sage-combinat/sage/combinat $ hg diff\n", "diff --git a/sage/combinat/permutation.py b/sage/combinat/permutation.py\n", "--- a/sage/combinat/permutation.py\n", "+++ b/sage/combinat/permutation.py\n", "@@ -1208,6 +1208,7 @@ class Permutation_class(CombinatorialObj\n", " sage: Permutation([2, 4, 1, 5, 3]).inverse()\n", " [3, 1, 5, 2, 4]\n", " \"\"\"\n", "+ print \"YO !!!! Let's inverse some permutations !!!\"\n", " w = range(len(self))\n", " for i,j in enumerate(self):\n", " w[j-1] = i+1" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "### 10. Test the changes\n", "\n", "**Build sage** \n", "`sage -b`\n", "\n", "**Verify the effects of the modification** \n", "Run `sage`" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [ { "data": { "text/plain": [ "YO !!!! Let's inverse some permutations !!!\n", "[5, 3, 2, 1, 4]" ] }, "execution_count": 1, "metadata": {}, "output_type": "execute_result" } ], "source": [ "p = Permutation([4,3,2,5,1])\n", "p.inverse()" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "That's great: we are now able to modify Sage.\n", "\n", "### 11. Run tests\n", "\n", "**Make sure that all examples in the source code still work** \n", "`sage -t `" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "slabbe@pol ~/Applications/sage-4.6.1/devel/sage-slabbe/sage/combinat $ sage -t permutation.py\n", "sage -t \"devel/sage-slabbe/sage/combinat/permutation.py\"\n", "**********************************************************************\n", "File \"/Users/slabbe/Applications/sage-4.6.1/devel/sage-slabbe/sage/combinat/permutation.py\", line 1206:\n", " sage: Permutation([3,8,5,10,9,4,6,1,7,2]).inverse()\n", "Expected:\n", " [8, 10, 1, 6, 3, 7, 9, 2, 5, 4]\n", "Got:\n", " YO !!!! Let's inverse some permutations !!!\n", " [8, 10, 1, 6, 3, 7, 9, 2, 5, 4]\n", "----------------------------------------------------------------------\n", "The following tests failed:\n", " sage -t \"devel/sage-slabbe/sage/combinat/permutation.py\"\n", "Total time for all tests: 10.4 seconds" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "If tests failed, one should edit files again...\n", "\n", "### 12. Build the documentation\n", "\n", "**Build the documentation and make sure there are no errors or warnings**:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "sage -b && sage -docbuild reference html" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "**Open the html version of documentation in your browser and make sure the documentation looks OK**:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "open ~/Applications/sage-4.6.1/devel/sage/doc/output/html/en/reference/sage/combinat/permutation.html" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "### 13. Update the current patch\n", "\n", "When the bug is fixed, once we made sure every tests pass and that the documentation builds fine, then we can update the current patch with **hg qrefresh** to reflect the changes:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "hg qrefresh" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "No changes are shown anymore by **hg status** or **hg diff**:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "hg status\n", "hg diff" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "Modifications are now in the patch. See **hg qstatus** or **hg qdiff**:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "hg qstatus\n", "hg qdiff" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "### 14. Export a patch\n", "\n", "Add a commit message to the patch:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "hg qrefresh -m \"#12345: add a useless print in the inverse method of a permutation\"" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "Export the patch with **hg export**:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "hg export trac_12345-add_useless_print-sl.patch >\n", " ~/Documents/tmp/trac_12345-add_useless_print-sl.patch" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "The command **hg export** also adds informations in the patch (author name, date, ...).\n", "\n", "Personnaly, I added the following alias to my `~/.bashrc` :" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "alias qtoptotmp='hg export `hg qtop` > ~/Documents/tmp/`hg qtop`'" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "### 15. Verify the content the patch\n", "\n", "Here is an example of a patch exported by Mercurial for the imaginary ticket \\#12345. It contains information about the author, the date, the commit message we just wrote and finally the complete diff.\n", "\n", "**trac\\_12345-add\\_useless\\_print-sl.patch**:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "# HG changeset patch\n", "# User Sebastien Labbe \n", "# Date 1295311529 -3600\n", "# Node ID 4a6379cf0c965e1ce309846cbcb9f864932a3b6c\n", "# Parent 83e5e45a8935ac627c45ed14042bbebafeb1a800\n", "#12345: add a useless print in the inverse method of a permutation\n", "\n", "diff --git a/sage/combinat/permutation.py b/sage/combinat/permutation.py\n", "--- a/sage/combinat/permutation.py\n", "+++ b/sage/combinat/permutation.py\n", "@@ -1208,6 +1208,7 @@ class Permutation_class(CombinatorialObj\n", " sage: Permutation([2, 4, 1, 5, 3]).inverse()\n", " [3, 1, 5, 2, 4]\n", " \"\"\"\n", "+ print \"YO !!!! Let's inverse some permutations !!!\"\n", " w = range(len(self))\n", " for i,j in enumerate(self):\n", " w[j-1] = i+1" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "### 16. Upload the patch on Sage trac\n", "\n", "**Upload the patch on Sage trac**\n", "\n", "> You can mention things like *\"tested on sage-4.6.1\"* in the text box when uploading the ticket.\n", ">\n", "> Make sure the patch was correctly uploaded by looking at it directly on the web page.\n", "\n", "**Set the ticket to needs review**\n", "\n", "> You may ask somebody to review your ticket.\n", "\n", "### 17. More on Mercurial queues\n", "\n", "Other useful Mercurial commands when patches multiplies:\n", "\n", "**hg qnew** \n", "Create a new patch\n", "\n", "**hg qnew** \n", "...\n", "\n", "**hg qpop** \n", "Move a patch from the applied stack to the unapplied one\n", "\n", "**hg qpush** \n", "Move a patch from the unapplied stack to the applied one\n", "\n", "**hg qtop** \n", "Show the current patch\n", "\n", "**hg qseries** \n", "Print all of the patches in order\n", "\n", "> **hg qunapplied** \n", "> Print the unapplied stack\n", "\n", "### 18. Dowload a patch\n", "\n", "A feature available on a Sage Trac ticket interests you? You want to review a ticket?\n", "\n", "Download a patch!\n", "\n", "Insert a patch into the series after the last applied patch with **hg qimport**:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "hg qimport ~/Downloads/trac_65321-nice-feature-AA.patch" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "Do NOT use the command **hg import** as it will import the changes in the current patch.\n", "\n", "### 19. Edit the series file\n", "\n", "You can change the order in which the patches are applied. To do so, simply edit the **series** file:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "slabbe@pol ~/Applications/sage-4.6.1/devel/sage-slabbe $ cd .hg/patches/\n", "slabbe@pol ~/Applications/sage-4.6.1/devel/sage-slabbe/.hg/patches $ vim series" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "Make sure the patch you are reviewing is the first patch to be applied:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "slabbe@pol ~/Applications/sage-4.6.1/devel/sage-slabbe/.hg/patches $ cat series\n", "trac_65321-nice-feature-AA.patch\n", "A.patch\n", "B.patch\n", "C.patch" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "Patches might not commute, for example if they edit the exact same line.\n", "If conflicts occur after editing the series file and doing **hg qpush**, simply edit the series file and try again.\n", "\n", "### 20. Reviewing a patch\n", "\n", "Visit the [Reviewing a patch](http://www.sagemath.org/doc/developer/walk_through.html#reviewing-a-patch) Section of the Sage Developer's Guide. Also, make sure you read William Stein's [blog post](http://sagemath.blogspot.com/2010/10/how-to-referee-sage-trac-tickets.html) about reviewing a Sage trac ticket.\n", "\n", "Make sure the patch applies on Sage without conflicts: \n", "`hg qpush` and `hg qpop`\n", "\n", "Experiment the functionality proposed in the patch.\n", "\n", "- Make sure the bug described in the ticket is fixed.\n", "- Make sure the patch does not introduce any new bug.\n", "\n", "Run tests on the affected files. \n", "`sage -t `\n", "\n", "### 20. Reviewing a patch\n", "\n", "Test the entire Sage library. \n", "`sage --testall --long`\n", "\n", "Ensure that the documentation builds fine: \n", "`sage -docbuild reference html`\n", "\n", "Check for full 100% doctest coverage: \n", "`sage -coverage `\n", "\n", "Once you\u2019ve tested the patch, report any failures on the Trac page for the ticket. Make suggestions about simplifying the code or fixing typos you noticed.\n", "\n", "### 21. Positive review or Needs work\n", "\n", "Three cases may happen:\n", "\n", "**Needs work** \n", "Mark it as **needs work** if there is anything to do.\n", "\n", "**Positive review** \n", "Otherwise, mark it as positive review, and mention in a comment all the things you checked.\n", "\n", "**Delegate** \n", "If you don\u2019t feel experienced enough for that, add a comment on the Trac page explaining what you have checked, what the results were, and that you think someone more experienced should take a look.\n", "\n", "### 21. Positive review or Needs work\n", "\n", "In Sage, a **negative review** does not exist!\n", "There is always place for work and improvement!\n", "\n", "### 22. Do some cleaning\n", "\n", "Delete an (unapplied) patch from the queue:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "hg qdelete trac_65321-nice-feature-AA.patch" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "Erase your branch. Of course, do this only if you don't care about your local changes:" ] }, { "cell_type": "code", "execution_count": null, "metadata": {}, "outputs": [], "source": [ "rm -rf sage-slabbe" ] }, { "cell_type": "markdown", "metadata": {}, "source": [ "### References\n", "\n", "[Sage](http://www.sagemath.org)\n", "\n", "[Sage trac]()\n", "\n", "[Sage Developer's Guide](http://www.sagemath.org/doc/developer/index.html)\n", "\n", "[Reviewing a Sage trac ticket](http://sagemath.blogspot.com/2010/10/how-to-referee-sage-trac-tickets.html), William Stein's blog post, October 31, 2010.\n", "\n", "This talk was generated\n", "\n", "- by [Docutils](http://docutils.sourceforge.net/)\n", "- from [ReStructuredText](http://docutils.sourceforge.net/rst.html) source\n", "- to a [Simple Standards-based Slide Show System (S5)](http://docutils.sourceforge.net/docs/user/slide-shows.html) format." ] } ], "metadata": { "kernelspec": { "display_name": "sagemath", "name": "sagemath" } }, "nbformat": 4, "nbformat_minor": 2 }