Showing posts with label vcs. Show all posts
Showing posts with label vcs. Show all posts

Tuesday, July 02, 2013

Code Review with Rietveld and Mercurial Queues

UPD 2014-07: On Windows, thanks to bug in subprocess that doesn't escape ^ character (caret) you need to escape it manually in all commands, like this ^^.

Teal Deer

    $ hg qser -v
    0 A supported-vcs
    1 U noul

    $ python upload.py --rev "'supported-vcs'^1:'supported-vcs'"

Also FAQ.

Rietveld, pre-commit and post-commit reviews


You probably know what Rietveld is - it allows you to send uncommitted changes for review with upload.py script that you grab from review server:
    $ python upload.py
    Upload server: codereview.appspot.com (change with -s/--server)
    New issue subject: Test
    Issue created. URL: http://codereview.appspot.com/10864043
    Uploading base file for README

This is a pre-commit review, where you discuss changes before they are committed, and add a link to passed review into commit message. In post-commit reviews you usually comment on existing revisions in external project history browser service, such as Google Code or GitHub. I you need a thorough examination of every change, then post-commit review process can become a challenge of catching a running train. If you just need to skim over the committed code and express doubts if anything is unclear for further discussion, the post-commit is ok. In fact we use this way for Spyder IDE development.

Post review is nice in the sense that development is not stopped while you're offline for a vacation, or your braincells are overwhelmed with more time pressing matters. Pre review is good if you want to ensure maximum stability for your system. Ideal development process should have convenient means to support both simultaneously.

--rev hack


But back to the Rietveld. Little is know that it supports reviewing existing changesets and revision ranges with the --rev argument of upload.py For example:
    $ python upload.py --rev "2869^1:2869"
    Upload server: codereview.appspot.com (change with -s/--server)
    New issue subject: Spyder IDE changeset 2869
    Issue created. URL: http://codereview.appspot.com/10866043
    Uploading base file for spyderlib/plugins/configdialog.py
    Uploading base file for spyderlib/utils/external/lockfile.py

This will upload revision 2869 (5a3d6821eabe) from Spyder repository for review. Why --rev is a hack? As you may see there is no original commit message, no revision number, no parent revision info. Actually, Rietveld doesn't know anything about that. What is gets from upload.py is a patchset as a plain SVN diff. Data loss could be prevented with extensible changeset format, which will greatly improve tool interoperability, but I don't know any entity that could support the time required for that development.

The --rev is a hack not only because of information loss, but also because it serves double purpose. The purpose is either:
  1. --rev REV specify the base revision to compare current change to
  2. --rev REV1:REV2 specify revision range to create the diff
To implement it the proper way, the upload.py needs -c CSET, --changeset CSET argument that will extract revision diff from the current version control history, propose to supply commit message as an issue description, save parent revision and changeset hash info and maybe cache the issue number for that changeset locally.

Uploading stuff from Mercurial Queue


Mercurial Queue is just a bunch of patch files in .hg/patches/ directory that can be applied or reverted in order specified by series file. This may change in future, but everybody knows and uses this fact. There is also a less obvious fact (that took long time for me to discover) that every applied patch is also a full fledged Mercurial revision. While it can be applied/reverted with qpush/qpop, as long as it applied, all other operations are working on it as well. In addition, every such revision gets a tag that is essentially this patch name.

So, to upload a patch from Mercurial Queue, make sure it is applied:
    $ hg qser -v
    0 A supported-vcs
    1 U noul

And use its name as --rev range argument:
    $ python upload.py --rev "'supported-vcs'^1:'supported-vcs'"

Make sure it is escaped as shown, because otherwise Mercurial will read is as a subtraction of tag named vcs from tag supported.

With Mercurial Queues it is also much-much easier to send patch updates. For that make sure your patch is active:
    $ hg qseries -v
    0 U supported-vcs
    1 U noul

It is not, so apply it:
    $ hg qpush 
    applying supported-vcs
    now at: supported-vcs

Do your modifications according to review comments and refresh the patch:
    $ hg qrefresh

Remember Rietveld issue number and use upload.py to update existing issue:
    $ python upload.py -i 10822044 --rev "'supported-vcs'^1:'supported-vcs'"

Stuff ToDo and Bitcoin Extortion


As I said Mercurial Queue integration can be improved. It is the 9th most requested feature in Rietveld at the moment and at least for me there is a clear reason why. The workflow for upload.py should look like the following:

(no arguments specified)
  • check that current VCS is Mercurial
  • check that hg diff output is empty
  • check that there is MQ patch applied
  • check if there is Rietveld issue number for the patch
    (needs local storage for this number)
  • check if the patch is not already uploaded
    (actual if issue number is found)
  • propose to upload a patch
  • propose to use commit message for patch description
    (propose to open editor to edit description)
  • submit

This is just an entrypoint for further enhancement to reach the ideal workflow, which will require more fixes not only in upload.py, but also on server side.

I am not sure I will be able to code this given the fact that I am to be sold into full time job slavery for my debts. That's why I leave this research here for further development. With that decomposition it should be easy to implement it and send a patch in exchange for some credits.

For those who'd appreciate the feature, but whose time/money ratio is leaning towards 0, there is a way to show the interest through a timeless fund (tracked here) with a total goal limit of 4 BTC for improved MQ support in Rietveld. I could name a few reasons why you should do this including the chance to trade money back for some life time in future, not only for yourself, but for the whole lot of open source projects that use Mercurial with Rietveld (such as core Python development). While all these are argument good, this is not a sponsored development, as I will likely to do this stuff sooner or later regardless of the goals met. I just need some Bitcoins to experiment with, and because I don't have anything else to trade, here is a good opportunity to give something back in exchange.

Thursday, May 19, 2011

Why Subversion conflict resolution suxx?

Because conflict resolution choice is confusing (for non-English natives for sure):
$ svn up
U    bitten\slave.py
G    bitten\tests\queue.py
Conflict discovered in 'bitten/queue.py'.
Select: (p) postpone, (df) diff-full, (e) edit,
        (mc) mine-conflict, (tc) theirs-conflict,
        (s) show all options:

Should you select "theirs conflict" or "mine confict" to fix your code? In situation when you feel that incoming change is right you may want to select "mine conflict" as the change that conflicts and should be removed as a result. But the correct answer it to select "theirs conflict" as the change to be incorporated in your code. Subversion developers think that you're are not removing conflicts, but incorporating them into your code. If they could think of user action - it is removing or resolving conflict in high-level terminology, or selecting correct change in low-level description.

Let's see the help:
Conflict discovered in 'bitten/slave.py'.
Select: (p) postpone, (df) diff-full, (e) edit,
        (mc) mine-conflict, (tc) theirs-conflict,
        (s) show all options: s
  (e)  edit             - change merged file in an editor
  (df) diff-full        - show all changes made to merged file
  (r)  resolved         - accept merged version of file
  (dc) display-conflict - show all conflicts (ignoring merged version)
  (mc) mine-conflict    - accept my version for all conflicts (same)
  (tc) theirs-conflict  - accept their version for all conflicts (same)
  (mf) mine-full        - accept my version of entire file (even non-conflicts)
  (tf) theirs-full      - accept their version of entire file (same)
  (p)  postpone         - mark the conflict to be resolved later
  (l)  launch           - launch external tool to resolve conflict
  (s)  show all         - show this list


Frankly, I can't understand how (mc) is different from (mf) if both versions accept all changes. (same) marker doesn't add much sense either. But even if help doesn't describe anything - these options were still added for a reason, and if we meditate a bit on them starting from the bottom comment to (mf) help, it becomes clear that:

1. (mf) throws away any incoming changes for the given file even if some of them were merged successfully
2. that means (mc) probably keeps the changes that were merged successfully
3. that means (df) shows more changes than (dc) output

Recommendations? Introduce `change` and `conflict` terminology earlier, i.e. show info like `3 changes, 2 conflicts`. It will also allow incremental conflict resolution, like if you (e)dited the counter is changed to `4 changes, 1 conflict` etc. Final recommendation if to replace `(ignoring merged version)` with `(ignoring merged changes)` string. There are surely more ways to improve this.

Wednesday, September 22, 2010

hgsubversion: Installing on Windows

        I use Windows primarily, and it's not easy to install Mercurial extensions there if they are not bundled with Mercurial installer itself. I am using plain .msi installer, which doesn't include hgsubversion, so I'll show you how to get this extension in place manually. Suppose you have Mercurial installed into C:\Mercurial Go there and notice library.zip archive. This is where you should add hgsubversion. library.zip has the following structure: / +-- email +-- encodings +-- hgext +-- logging ... __future__.pyc _abcoll.pyc _elementtree.pyd ... zipextimporter.pyc zipfile.pyc Contents of this archive is the whole Mercurial code. Code is written in Python, and is compiled into .pyc and .pyd files for speed and packed into library.zip for convenience. For interpreted languages like Python it is not necessary to compile source files for execution, but they did it. Get latest hgsubversion sources from http://bitbucket.org/durin42/hgsubversion/src by pressing link "get source" in upper right corner. Or download released version from http://pypi.python.org/pypi/hgsubversion/1.1.2 Unpack archive. You should have directory structure like: / +-- hgsubversion +-- notes +-- tests +-- tools .hgignore ... setup.py Now put hgsubversion/ dir from this structure into library.zip archive, inside hgext/ directory, where Mercurial expects to find its extensions. Enable extension by uncommenting line ";hgsubversion =" in Mercurial.ini that is usually located in your profile directory, i.e. C:\Users\yourname If it is not there - copy one from your C:\Mercurial\hgrc.d directory. Try to checkout some subversion repository. You will get an error like this: C:\p>hg clone http://google-twitter.googlecode.com/svn/trunk (falling back to static-http) (falling back to Subversion support) destination directory: trunk abort: subvertpy 0.7.3 or later required, but not found! You need Subversion bindings for Python. They advertise subvertpy, but it is also possible to use official bindings. Those bindings is just a set of libraries that allows Python scripts to use Subversion binary code directly. Unfortunately, after migration to apache.org, Subversion bindings are not compiled anymore, so there is no official place where you can download them. But thanks to http://alagazam.net/ it is possible to get unofficial . Download svn-win32-1.6.12_py.zip or more up-to-date distribution and extract it. You will get a directory structure like: / +-- python +-- libsvn +-- svn README.txt In libsvn/ rename _client.dll to _client.pyd. Do the same with _core.dll, _delta.dll, _ra.dll Maybe you'll have to rename other .dll files, but these were enough for me. Now put svn/ and libsvn/ into the library.zip directly in the root. I also had to copy intl3_svn.dll from an old 1.6.6 installation of SVN into library.zip root (you may find this file in Windows binaries archive from http://alagazam.net/) and place libsvn_swig_py-1.dll from libsvn/ dir also in library.zip root. It should work now. 

Wednesday, August 27, 2008

Bazaar Experience

Not so long ago I decided to play with launchpad - site for open source collaboration. It is built around decentralized Bazaar version control system that reached version 1.6 in its development up to this moment. I read that centralized VCS are bad and DVCS are good, but never had enough free time to check this in practice and finally..

The first thing I liked in Bazaar is that it is written in Python - that means that bugs can be found and fixed rather easily. This was also the thing that I disliked, because standalone bzr distribution includes Python and takes 14Mb and package for installed Python distribution doesn't add bzr to PATH

Accessibility. I often work through tunnels, proxies, firewalls and other stuff that kind people tend to put here and there in each Local Area Network. Not only I have to deal with them myself, but sometimes also need to teach others. In SVN proxy settings are stored in documented configuration file called 'servers'. In Bazaar there is no such file and all configuration is done with environment variables and Python settings that are not documented. To use Bazaar through a proxy - set environment variables:

set http_proxy=127.0.0.1:1080
set https_proxy=127.0.0.1:1080

These proxy settings are rather common for linux/unix machines, but cause particular pain with windows machines in a domain. If SVN is able to transparently authenticate against domain proxy without asking sensitive passwords then Bazaar lacks such feature and needs another authorization proxy server (NTLMAPS) middleware.

Usability. In general distributed version control systems should be better than centralized ones. A lot of articles and tutorials describe the theory very well, but in practice there are many things is are not that brilliant. For example, I needed to fix stubborn DevPak plugin for Code:Blocks that didn't want to install one specific version of a curl library. In SVN I would do a partial checkout, make modifications and upload patch to the forum. Not very convenient. In Bazaar I would have to make a branch of the whole Code::Blocks repository, start patching plugin and then merge the whole branch back. But branching the whole repository is an overkill for such little plugin and you can't make a branch out of subtree in Bazaar. More then that - Bazaar doesn't even allow to export subtree of repository. Download either the whole thing or nothing. Not too effective, especially when the traffic is 0.02 cents for Mb.

So, I manually created directory structure for the plugin similar to existing project and started a new Bazaar repository from scratch. After a couple of revisions I released fixed plugin for stable Code::Blocks and would like to switch to latest development code and merge changes made to plugin in trunk into my version. Again, Bazaar disappointed me. I knew that SVN won't allow merging with different repository, but at least I could grab files from the source tree and merge them manually. Bazaar didn't allow me even that. With almost 100 commands it just couldn't export some files.

It may be that Bazaar is a good system for some small projects, but in my case it appeared unwieldy and too expensive even being open and free.