[PATCH 1 of 8] bdiff: add _version to help detect breaking binary changes

Yuya Nishihara yuya at tcha.org
Wed May 3 04:37:31 EDT 2017


On Tue, 2 May 2017 22:15:44 -0700, Jun Wu wrote:
> Excerpts from Yuya Nishihara's message of 2017-05-03 13:18:16 +0900:
> > Maybe rename index to index2 ?
> 
> That's a solution. But developers must rename things recursively - APIs
> using index as input or output also needs to be renamed. That does not look
> pretty, and is not friendly to new developers.

I'm optimistic about that. The code depending directly on C API won't that
large.

> > Should be okay to fail with ImportError or AttributeError as the user
> > requested.
> 
> If that's okay, we should have taken Phil's patches at the first place
> because it works with newer .so + old .py already. I'm doing all of these
> because smf suggested that's not okay.

No. We shouldn't break the default build (unless we agree to.) I said it
should be okay if the user explicitly select the strict 'c' policy.

> > > Mixing python and C modules could lead to surprises if the API changes state
> > > in the moudle. Like "set(x, y)" -> affect C moudle state; "get2(x)" ->
> > > "get2" is only in pure, and the state "x" is not in pure moudle state. 
> > 
> > Yeah. So that's the only difference between my version and yours. I don't
> > say my version is strictly better. The goal of my series is to get rid of
> > our importer hacks, CFFI mess and import cycle around encoding.py. I believe
> > your module versioning patches will get slightly simpler if we had no importer
> > hack.
> 
> I think they do not conflict. We can take this series for policy=c, and also
> move .c to "cext/" (which I think is a very good change). I can do my change
> after the "cext/" movement to resolve merge conflicts on my side.
> 
> I think strong guarantee on .so compatibility is a good thing, and
> dualmodule can give us more trouble than benefit (so I'm -1 on dualmodule):

Well, dualmod is IMHO the easiest hack to move cext modules to new layout
without loosing backward/forward compatibility. We can remove it later if we
prefer using per-module version constant.

It seems getting hard to track what's your point and what's my point. I'll
send first bits of my series, so let's discuss on them later.

>   1. require recursive renaming (pitfall for new contributors)
>   2. possible to have other weird behaviors
> 
> For 2, modules do not have to change its state, exposing state could also be
> troublesome, like (not an issue today though, but not that unrealistic):
> 
>   try:
>       ... # call C or pure API, and may raise
>   except mpatch.mpatchError: # pure mpatchError will miss C exception
>       ...
> 
> In general, I don't think avoiding recompile should take precedence over any
> attempt that makes the code more explicit and consistent.

Honestly I don't need that feature as I always do "make local". ;)


More information about the Mercurial-devel mailing list