Developing Freeciv |
---|
So you want to become a Freeciv contributor! Welcome on board.
Please note that if you have files to contribute, create a new patch report on Freeciv's ticket tracker and attach them there. Do not post large or binary contributions to our mailing lists.
Pieces of Freeciv source code, however, such as bugfixes and new features, must be entered into the OSDN ticket tracking system - see how.
If you want to help out with the Freeciv code, then you should read Freeciv hacker's guide and the Style Guide. There are also other documents of interest in the doc/ directory. You may want to consult the mailing list archives to see if the answers to your questions are buried there.
The Freeciv source code is maintained with Git. See Freeciv source code repository for details.
As of 2012 July 03, project developers now prefer patches to be named *.patch rather than *.diff. The software tools now in use can automatically handle a .patch while a .diff requires more time/attention to check in.
How to create patch file with Git[]
This method can be used if you are working with a source code checkout in git. If you don't have git checkout, but have the source code e.g. as a freeciv release tarball, see next chapter about creating a patch without git's internal tools.
In most cases contributions to freeciv should be sent as a patch file to ticket tracker. To develop such a patch, start from the head of the git branch you want to develop your change to. Make sure you're in the head by pulling latest changes in.
$ git pull
Create local branch, branched from your current commit, i.e, from the head you just got in to, to make your development on
$ git checkout -b [branch-name]
e.g.
$ git checkout -b main-work
Do your changes for one patch, and create a commit. To make a commit add files you've changed by 'git add' and once they are all there, do 'git commit'. 'git status' might help you see what are the files you want to add.
e.g.
$ git status ... modified: common/game.c modified: common/game.h ... $ git add common/game.c common/game.h $ git commit -s
Optionally develop more commits the same way.
Once you want patch files created from all your commits, use 'git log' to find out commit hash of the commit just before your first commit, i.e., the commit your work is relative to. Use 'git format-patch' to write the patch files.
$ git log -> commit 08a17d90455031387d9e488d6a4a3edec529f28d $ mkdir ../patch $ git format-patch -o ../patch -C -M 08a17d90455031387d9e488d6a4a3edec529f28d
Now you should have patches for all your commits in ../patch, with filenames consisting of the number telling the order of patches and first line of the commit message.
If you want, you can now switch back to the branch you can sync from github
$ git checkout main
If you later need to revisit the commits you've made, for example after they have been reviewed, there's two ways. If you need to adjust just the latest commit (maybe the only one), you can do 'commit --amend' for it after adding your changes.
$ git checkout main-work # do your changes $ git status ... modified: common/game.h ... $ git add common/game.h $ git commit --amend $ git format-patch -o ../patch -C -M 08a17d90455031387d9e488d6a4a3edec529f28d
Second option is the create a new branch and cherry-pick commits there and do changes there. You have to use this method if you are changing commits other than the last, or if you want to rebase your patches (against newer head commit of the github branch)
$ git pull $ git checkout -b main-workB # check commit hashes of commits from old branch $ git log main-work -> commit e7b04ed05c9fcc3010a034b210cc7a670ea5c23a # cherry-pick first commit $ git cherry-pick e7b04ed05c9fcc3010a034b210cc7a670ea5c23a # cherry-pick further commits until you have one that you want to change. Then do modifications. $ git add common/game.h $ git commit --amend # cherry-pick further commits # See commit hash your changes are relative to now $ git log -> commit fe726dbe3758c74a1f269a429b2a9b8641d15220 $ git format-patch -o ../patch -C -M fe726dbe3758c74a1f269a429b2a9b8641d15220
Once you've finished with a work branch (or two), you can delete them.
$ git checkout main $ git branch -D main-work $ git branch -D main-workB
How to create patches using "diff"[]
This section describes how to create patches using the program called "diff". It is also possible to create patches using Git; see section V. There is a GNU version of "diff", and there are some commercial-Unix versions of varying quality. The GNU version is the best, of course.
Make sure your patch is made against the latest version of the source code you can possibly get, preferably from Git or a Git snapshot. Be sure to indicate exactly which version of the source code your patch should be applied to e.g., "version 1.7.2" or "source tree from March 16".
Make sure the patch adheres to the coding style agreed for Freeciv code. The script mkpatch works fairly well: you can create a patch by typing mkpatch srcdirectory > my.patch; see instructions. The patch will work, but it is not exactly in the desired format. If this doesn't suffice, you have to know the details of running diff, so read on.
There are three different "diff" formats, in descending order of goodness: unified diff "diff -u", context diff "diff -c" and the original diff "diff". The GNU version does unified diffs; most commercial versions will only do context diffs and some won't even do those. Use unified diff unless this is absolutely impossible.
You need a copy of the original file(s) as well as your modified file(s). If you've forgotten to make a copy of the original source code file(s) you've modified, before you modified them, then you'll need to re-extract them or re-download them. See also the sections on Subversion.
Be sure to specify the original file(s) first, and the modified file(s) second. Otherwise your diffs will be "backwards". If you've only modified 1 file, or a small number of files all in the same directory, then you can run diff against the file(s) individually, concatenating them if necessary:
% diff -u original_file modified_file >> my_patch
In this case, the "modified_file" should be the normal name of the file in question -- for example,
% diff -u unithand.c.orig unithand.c >> my_patch # RIGHT
Don't do this:
% diff -u unithand.c unithand.c.modified >> my_patch # WRONG!
because that will confuse the program/person applying the patch.
Please use concatenation only when all the files are in a single directory, otherwise the person applying the patch will have to break the patch into single-directory pieces.
If you've modified several files, it's probably best to let diff find them all for you. In this case, you'll want two completely separate directory trees -- one with the original code, and one with your modified version. Then you can use the "recursive" option to diff:
% diff -r -u original_directory modified_directory > my_patch
When you do this, you'll want to make sure there aren't any object files or programs in these directories -- if there are, then diff will include references to them in the patch it creates. In that case, either remove those files and rerun the diff, or edit the references out of the patch file see below.
It is also best to specify only short, relative paths to the "diff -r" command. Using long and/or absolute paths just makes it harder to apply the patch. For example, do this:
% diff -r -u orig_dir mod_dir > my_patch # RIGHT
Instead of this:
% diff -r -u prj/orig/fc prj/mods/fc > my_patch # WRONG!
Especially, don't do this:
% diff -r -u /home/me/orig_fc /home/me/mod_fc > my_patch # WRONG!
If you've created new files, or deleted or renamed files, then there are some special options you may have to feed to diff -- check the docs. If a file foo exists in your original_directory and you want it to be copied in your modified directory do:
% diff -Nur original_directory modified_directory > my_patch
If you use the -N flag you must be especially careful that the patch does not include more new files than you intend; see the next point.
Often you want files to be ignored in both directories. Binary files or output files or whatever files can be ignored. Use the command:
% diff -Nur -Xdiff_ignore original_directory modified_directory > my_patch
Where diff_ignore is a file that contains a list of files to be ignored during the "diff" (one per line; may use wildcards).
A suggested diff_ignore file is included in the Freeciv distribution.
After generating the patch file, read it. Make sure it looks sane. You can use a text editor to correct filenames, or to remove "Binary files differ" and "Only in" lines. And, please apply it and rebuild Freeciv to check if all goes smoothly. See section III for how to apply the patch.
Small patches are easier to integrate that big ones. If you have a big patch try to split it into several little ones. See the next section for how to submit patches to the ticket tracker.
As an example, suppose you have two directories: freeciv-git, which contains a snapshot of the Git repository, and freeciv-mod, which contains your working copy. And, you have made a small change to your working copy (adding a comment). From the directory that contains freeciv-git and freeciv-mod (and diff_ignore), you could use the following to create a patch:
% diff -Nur -Xdiff_ignore freeciv-git freeciv-mod > /tmp/barb_comment.diff
The resultant patch would look something like:
diff -Nur -Xdiff_ignore freeciv-git/common/game.h freeciv-mod/common/game.h --- freeciv-git/common/game.h Sat Jan 1 14:30:12 2000 +++ freeciv-mod/common/game.h Fri Jan 14 12:32:52 2000 @@ -62,6 +62,8 @@ int player_idx; struct player *player_ptr; struct player players[MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS]; + /* barbarians are ai players; + use is_barbarian() to test for a player being a barbarian */ int global_advances[A_LAST]; /* a counter */ int global_wonders[B_LAST]; /* contains city id's */ /* global_wonders[] may also be (-1), or the id of a city
See section V for a way to create this patch using Subversion.
How to submit patches to the bug tracker[]
Developers now expect all patches to be entered into the Freeciv bug tracking system.
- You can submit them by using our bug tracker at https://osdn.net/projects/freeciv/ticket . A patch or anything else can be attached to the new ticket created in the Web interface. Remember to include information about who you are and how you can be contacted.
- Before it can be accepted into Git, your patch must be reviewed by others. Reviewing isn't very popular; it's more fun to do your own patches. Therefore, you must provide maximal incentive for others to look at your patch.
- implement one feature at a time (this is essential!)
- comment it well
- follow the style guide
- explain it in your post (and detail carefully all rules it changes in the game)
How to use the patch command[]
Once you start creating patches you should also learn to use patch, which applies patches. There is a GNU version of patch, and there are some commercial-Unix versions of varying quality. The GNU version is the best, of course. This will help to test and manage your own patches, and also let you apply patches from other people.
For more detailed or definitive information, see the patch man page.
Here is the example patch we presented in section I:
diff -Nur -Xdiff_ignore freeciv-git/common/game.h freeciv-mod/common/game.h --- freeciv-git/common/game.h Sat Jan 1 14:30:12 2000 +++ freeciv-mod/common/game.h Fri Jan 14 12:32:52 2000 @@ -62,6 +62,8 @@ int player_idx; struct player *player_ptr; struct player players[MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS]; + /* barbarians are ai players; + use is_barbarian() to test for a player being a barbarian */ int global_advances[A_LAST]; /* a counter */ int global_wonders[B_LAST]; /* contains city id's */ /* global_wonders[] may also be (-1), or the id of a city
Recall that we saved this patch in a file called /tmp/barb_comment.diff.
Now, take a look at the line that begins with "+++". This indicates from where the modified file came. It is also the preferred file to be patched on your machine. Notice that the path includes the "root" Freeciv directory (in this case, "freeciv-mod"). This is common (though, not universal) for recursive diffs.
Anyway, you may not keep your working copy of the Freeciv source in "freeciv-mod". Suppose it is in "freeciv". To apply this patch to your working copy, "cd" into your "freeciv" directory, and type:
% patch -u -p1 < /tmp/barb_comment.diff
The "-p1" tells patch to skip the initial directory in the path to the file being patched (in this case "freeciv-mod/").
Patch will respond with something like:
Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |diff -Nur -Xdiff_ignore freeciv-git/common/game.h freeciv-mod/common/game.h |--- freeciv-git/common/game.h Sat Jan 1 14:30:12 2000 |+++ freeciv-mod/common/game.h Fri Jan 14 12:32:52 2000 -------------------------- Patching file common/game.h using Plan A... Hunk #1 succeeded at 62. done
Patch has succeeded in doing its thing, modified common/game.h, and saved the original in common/game.h.orig.
Suppose now you want to restore the file to its previous state. You want to apply the patch "in reverse". To do this, use the "-R" argument to patch:
% patch -u -p1 -R < /tmp/barb_comment.diff
Patch will respond with something very much like before:
Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |diff -Nur -Xdiff_ignore freeciv-git/common/game.h freeciv-mod/common/game.h |--- freeciv-git/common/game.h Sat Jan 1 14:30:12 2000 |+++ freeciv-mod/common/game.h Fri Jan 14 12:32:52 2000 -------------------------- Patching file common/game.h using Plan A... Hunk #1 succeeded at 62. done
And, patch will have restored your file.
Another common kind of patch to receive is one generated using the Git command "git diff" (see section V, below).
Here is the example patch from section V:
Index: common/game.h =================================================================== RCS file: /home/freeciv/CVS/freeciv/common/game.h,v retrieving revision 1.46 diff -u -r1.46 game.h --- game.h 2000/01/01 19:29:46 1.46 +++ game.h 2000/01/14 18:07:56 @@ -62,6 +62,8 @@ int player_idx; struct player *player_ptr; struct player players[MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS]; + /* barbarians are ai players; + use is_barbarian() to test for a player being a barbarian */ int global_advances[A_LAST]; /* a counter */ int global_wonders[B_LAST]; /* contains city id's */ /* global_wonders[] may also be (-1), or the id of a city
Here, the "+++" line specifies only the file name. The directory information is communicated by the "Index: " line. Notice that this time, there is no "root" directory specified. Therefore, we don't need to skip one by using "-p1". So, to apply this patch to your working copy, "cd" into your "freeciv" directory, and type:
% patch -u < /tmp/barb_comment_svn.diff
Patch will respond with something like:
Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |Index: common/game.h |=================================================================== |RCS file: /home/freeciv/CVS/freeciv/common/game.h,v |retrieving revision 1.46 |diff -u -r1.46 game.h |--- game.h 2000/01/01 19:29:46 1.46 |+++ game.h 2000/01/14 18:07:56 -------------------------- Patching file common/game.h using Plan A... Hunk #1 succeeded at 62. done
(Applying the patch in reverse works just like before, by adding "-R".)
Sometimes, you'll get a patch of just a single file, which might look something like:
--- ../../freeciv-svn/common/game.h Sat Jan 1 14:30:12 2000 +++ game.h Fri Jan 14 12:32:52 2000 @@ -62,6 +62,8 @@ int player_idx; struct player *player_ptr; struct player players[MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS]; + /* barbarians are ai players; + use is_barbarian() to test for a player being a barbarian */ int global_advances[A_LAST]; /* a counter */ int global_wonders[B_LAST]; /* contains city id's */ /* global_wonders[] may also be (-1), or the id of a city
Again, the "+++" line includes only the file name ("game.h"), but there is nothing like the "Index: " line to give the directory. So, in this case, you can just "cd" to where you know the file to be (i.e., "common"), and run patch as follows:
% patch -u < /tmp/barb_comment_game.h.diff
Patch will respond with something like:
Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |--- ../../freeciv-svn/common/game.h Sat Jan 1 14:30:12 2000 |+++ game.h Fri Jan 14 12:32:52 2000 -------------------------- Patching file game.h using Plan A... Hunk #1 succeeded at 62. done
You may also get a patch (from someone who hasn't read this :) which uses a long relative or absolute path to specify the file(s) to be patched. An example of a long relative path is:
diff -Nur -Xdiff_ignore freeciv-svn/common/game.h freeciv-mod/common/game.h --- prj/freeciv/svn/common/game.h Sat Jan 1 14:30:12 2000 +++ prj/freeciv/mod/common/game.h Fri Jan 14 12:32:52 2000 @@ -62,6 +62,8 @@ int player_idx; struct player *player_ptr; struct player players[MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS]; + /* barbarians are ai players; + use is_barbarian() to test for a player being a barbarian */ int global_advances[A_LAST]; /* a counter */ int global_wonders[B_LAST]; /* contains city id's */ /* global_wonders[] may also be (-1), or the id of a city
In this case, you will need to use a different value for the "-p" argument to "diff". Just "cd" into your "freeciv" directory, and type:
% patch -u -p3 < /tmp/barb_comment.diff
The "-p3" tells patch to skip the first three directories in the path to the file being patched (in this case "prj/freeciv/mod/").
In the case of an absolute path, for example:
diff -Nur -Xdiff_ignore freeciv-svn/common/game.h freeciv-mod/common/game.h --- /home/me/svn/common/game.h Sat Jan 1 14:30:12 2000 +++ /home/me/mod/common/game.h Fri Jan 14 12:32:52 2000 @@ -62,6 +62,8 @@ int player_idx; struct player *player_ptr; struct player players[MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS]; + /* barbarians are ai players; + use is_barbarian() to test for a player being a barbarian */ int global_advances[A_LAST]; /* a counter */ int global_wonders[B_LAST]; /* contains city id's */ /* global_wonders[] may also be (-1), or the id of a city
you will need to use a value for the "-p" argument to "diff" that is one greater than the number of leading directories (to account for the root directory). So, in this case "cd" into your "freeciv" directory, and type:
% patch -u -p4 < /tmp/barb_comment.diff
The "-p4" tells patch to skip the root directory and the first three named directories in the path to the file being patched (in this case "/home/me/mod/").
If patch ever can't determine what file to patch, it will prompt you for the file to be patched:
Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |--- ../../freeciv-svn/common/game.h Sat Jan 1 14:30:12 2000 |+++ game.h Fri Jan 14 12:32:52 2000 -------------------------- File to patch:
Just enter the path to the file to be patched.
Everything doesn't always go so smoothly. For example, if your working file has been modified, and patch can't figure out how to apply the patch, you may see something like:
% patch -u -p1 < /tmp/barb_comment.diff Hmm... Looks like a unified diff to me... The text leading up to this was: -------------------------- |diff -Nur -Xdiff_ignore freeciv-svn/common/game.h freeciv-mod/common/game.h |--- freeciv-svn/common/game.h Sat Jan 1 14:30:12 2000 |+++ freeciv-mod/common/game.h Fri Jan 14 12:32:52 2000 -------------------------- Patching file common/game.h using Plan A... Hunk #1 failed at 62. 1 out of 1 hunks failed--saving rejects to common/game.h.rej done
In this case patch didn't succeed in applying the patch: notice it reported that Hunk #1 failed at line 62. Your files are now in the following states:
- common/game.h is the file modified by the hunks which were applied with success.
- common/game.h.orig is the same file before any change to it.
- common/game.h.rej is composed of the hunks which have failed.
(A hunk is a block of text from line number x to line number y to be substituted by a new block of text that will go from line number x' to line number y'.)
In our case, common/game.h.rej looks like:
*************** *** 62,67 **** int player_idx; struct player *player_ptr; struct player players[MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS]; int global_advances[A_LAST]; /* a counter */ int global_wonders[B_LAST]; /* contains city id's */ /* global_wonders[] may also be (-1), or the id of a city --- 62,69 ---- int player_idx; struct player *player_ptr; struct player players[MAX_NUM_PLAYERS + MAX_NUM_BARBARIANS]; + /* barbarians are ai players; + use is_barbarian() to test for a player being a barbarian */ int global_advances[A_LAST]; /* a counter */ int global_wonders[B_LAST]; /* contains city id's */ /* global_wonders[] may also be (-1), or the id of a city
Here, there is one problematic hunk. The original code runs from line 62 through line 67, and is displayed after the "*** 62,67 ****". The new code was to run from line 62 through 69, and these new lines are displayed after the "--- 62,69 ----".
Unfortunately, now you need to edit common/game.h somewhere near line 62, and manually make the rejected changes.
How to set up Git[]
See Freeciv source code repository.
- FIXME: add documentation about git usage similar what we had at the time we used subversion.
Freeciv policy[]
Some efforts have been made to make Freeciv development easier. We advise you to use the tools which are described here and located in the Freeciv sources.
utility/mem.c: fc_malloc(), fc_realloc(), fc_calloc() and mystrdup() can be used instead of classic C malloc(), realloc(), calloc() and strdup() functions.
These functions test the return value, and if the allocation failed, the file and line number where the function was called is displayed.
utility/support.c: Replacements for functions which are not available on all platforms. Where the functions are available natively, these are just wrappers.
Freeciv routine | mystrcasecmp | mystrncasecmp | mystrerror | myusleep | mystrlcpy | mystrlcat | my_snprintf | my_vsnprintf |
Replaces | strcasecmp | strncasecmp | strerror | usleep | strlcpy | strlcat | snprintf | vsnprintf |
We also have two convenience routines, sz_strlcpy and sz_strlcat, for when dest is an array.
utility/capability.c: The capability string is a list of options which are supported. When a client connects to a server, we compare the two capability strings. Any capabilities which are missing are deactivated. This allows compability between differents versions. (The capability string itself is defined in common/capstr.c .)
For example, if we have a client with a capabilty string "+1.8 nuke" and a server with a capability string "+1.8", the client won't be able to use the "nuke" option whatever that is.
Some capabilities are mandatory. They begin by a '+'. In the example, the client can't connect to a "+1.8pre2" server.
In the server, tests look like this:
if (has_capability("autoattack1", pc->capability))
The server checks if the client knows about autoattack1. If the client knows about autoattack1, the server will send data needed by autoattack1.
In the client,
if (!has_capability("nuke", aconnection.capability)) { return; }
The client will ignore the nuke command if the server doesn't know about nuke.
utility/log.c: freelog(int level, char *message, ...) is our Freeciv logging function. There are 5 levels:
- define LOG_FATAL 0 Something very wrong. Freeciv immediately exits.
- define LOG_ERROR 1 Something went wrong.
- define LOG_NORMAL 2 Freeciv did something usual.
- define LOG_VERBOSE 3 More information. Not shown by default.
- define LOG_DEBUG 4 Debugging information. Suppressed unless DEBUG defined.
For example, in game.c:
freelog(LOG_DEBUG, "game_remove_city %d", pcity->id);
Except for the level, you use it as a printf().
The server and client have a default log level which is used to filter the freelog output. You can set it via the function log_set_level(int level).
log_set_level(LOG_FATAL) will output only LOG_FATAL freelog messages. log_set_level(LOG_ERROR) will output LOG_FATAL and LOG_ERROR freelog messages. log_set_level(LOG_NORMAL) will output LOG_FATAL, LOG_ERROR and LOG_NORMAL freelog messages. log_set_level(LOG_VERBOSE) will output LOG_FATAL, LOG_ERROR, LOG_NORMAL and LOG_VERBOSE freelog messages. log_set_level(LOG_DEBUG) will output all freelog messages. Same as LOG_VERBOSE if DEBUG is not defined.
If you are debugging a function, you can put log_set_level(LOG_DEBUG) at the beginning of the function and log_set_level(LOG_NORMAL) at the end.
You can use command line options to specify a file to write log messages to, and the log level. For example:
civclient --log cli_log.log --debug 2 civserver --log ser_log.log --debug 2
In this example, all the freelog output will be written to the files "cli_log.log" and "ser_log.log".
Mini Style Guide[]
If you want to hack Freeciv, and want your patches to be accepted, it helps to follow some simple style rules. For more detailed instructions, see the Coding Style guide.
- Use K&R indentation style with indentation 2 (when in doubt, use "indent -kr -i2"). However, do not re-indent areas of code you are not modifying or creating.
- Order include files consistently: First state all system include files with <> in alphabetic order, then all Freeciv include files with "", sorted by directory (common, server, ...) and then by alphabetic order, with a blank line between the sections. This helps to avoid adding unnecessary or duplicated include files.
- If you use a system specific feature, don't add #ifdef __CRAY__ or something like that. Rather write a check for that feature for configure.ac, and use a meaningful macro name in the source.
- Always prototype global functions in the appropriate header file. Local functions should always be declared as static.
- Don't use C++-style comments (i.e., // comments).
A handy Emacs formatting macro to help you with this is available from a posting to freeciv-dev.