[RFC/MERGE] Implement DirState parsing in Pyrex

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Robert Collins wrote:
> On Mon, 2007-05-07 at 18:37 -0500, John Arbash Meinel wrote:
>> The main reason this is RFC is because it is introducing a (potential)
>> dependency on compiled code. I tried to make sure it was genuinely
>> worth
>> it before I advocated it. But I think our disk format is fairly stable
>> (and the parser is actually easier to modify in pyrex because of the
>> lack of hacks).
> 
> I agree.
> 
>> Originally, I know Robert was thinking to version the .c file, along
>> with the .pyx files. However, this will cause a lot of commit churn,
>> because when I build them it includes my system paths
>> (/home/jameinel/...foo/bar), and when the PQM builds them, it will
>> create different paths there.
>>
>> What I would rather do, is change our *release* process so that we
>> include them in the final tar file.
> 
> I dont like this, it means that tarballs will be easier to install from
> than our source tree. I'd rather fix the .c source - e.g. post process
> it to have relative paths. (Absolute paths are not needed AFAIK).

They are only included in the comments:
 /* /path/to/foo/bar.pyx line: 298 */

So I would be okay with post-processing them. They are nice hints when
debugging the pyrex code, but as you say, relative paths would be just fine.

I'm more concerned about pyrex compilation mismatch. So if I have pyrex
0.9.4.1 and you have 0.9.5, is there any chance that the output is
"stable"? Or does it mean everytime I grab bzr.dev I'm going to commit
new .c code, and then when I submit to the PQM, it is going to generate
something different.

I honestly don't think it is that much more onerous. We basically are
saying that if you want to compile from the source tree, you need 1 more
tool (pyrex).

It is the same idea as not versioning Makefile.in because you use
Makefile.am. And people who want to compile from source have to have
automake.

Releases will include the .in files, so people only need configure and gcc.

...

> 
>> One other possibility is to have a 'bzr.dev-pyrex' branch. Which would
>> let us add things like this on the side, and only merge them once we
>> have decided it is worthwhile.
> 
> I dont think having a separate 'official' branch makes sense: Either its
> good enough or its not :).

Well, the idea is to get some people testing it, without requiring
everyone. But I guess we can just call that 'bzr.dev' and revert the
changes if we decide we don't want it.

> 
>> Also, for now I've gone with writing functions such that both the
>> python
>> code and the pyrex code are tested by ./bzr selftest. Rather than
>> having
>> 'make check' run one time without them, and one time with them.
> 
> I like this.
> 
>> Oh, a few other things...
...

> 
> Personally I'd prefer to see the .pyx and .py in the same place - e.g.
> dirstate_core.{c,py,pyx} and in dirstate you do from dirstate_core
> import *. Or you can look at my readdir pyrex branch to see how I've
> done it there.

I am okay with having "dirstate_core_py.py" and "dirstate_core_c.pyx".
But I don't think we want them to have an identical prefix.

*Because* we want to be able to test both of them. And you can't "import
bzrlib.dirstate_core" and select whether you will get the python
implementation or the C implementation.

The code I did was in 'dirstate.py' I have:

def foo_py():
  ...
foo = foo_py

try:
  from bzrlib.compiled.dirstate_helpers import foo_c
except ImportError:
  pass
else:
  foo = foo_c

I would be happy to change that to:

try:
  from bzrlib.dirstate_core_c import foo
except ImportError:
  from bzrlib.dirstate_core_py import foo

But I need a way to import a specific implementation in the test suite.

> 
>> This also gets my feet a bit wet on writing (fast) pyrex code. One
>> interesting part is that the intermediate C code allows you to see how
>> your code is being compiled. Sort of like reading the assembly output
>> for C code, only I don't have to read assembly :).
>> It has helped in a few cases where I didn't realize the overhead of
>> one type of function. 
> 
> Generally I'm +1 on the concept, though I haven't read the detail of the
> patch yet, and I'd like use to reach agreement on the layout stuff
> before its merged.
> 
> Rob

I agree.

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.3 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGQK1tJdeBCYSNAAMRAnuxAKCY8IBN2EyAd/YomzAfqukP5tBn2QCgkXao
U95HJs/9BSKvDwrGtvZ/8pU=
=VTlT
-----END PGP SIGNATURE-----