[Python-apps-team] Bug#750871: Bug 750871 - patch

[Faheem Mitha]

Hi Javi (and Max),

On Wed, 18 Jun 2014, Max Bowsher wrote:

> On 17/06/14 23:40, Javi Merino wrote:
>> Control: tags -1 - pending + wontfix
>>
>> On Sat, Jun 07, 2014 at 09:35:43PM +0100, Max Bowsher wrote:
>>> The problem here is that some manual sed hackery munging the /usr/bin/hg
>>> shebang was removed in PAPT SVN r10748, with the justification that
>>> dh_python2 would take care of it automatically.
>>>
>>> Unfortunately, it was not considered that the package currently circumvents
>>> large portions of dh_python2's multiple python version support by calling
>>> the upstream Makefile instead of setup.py.
>>
>> My guess is that the dh_python2 in sid doesn't have the problem
>> described in the bug, that's why I didn't see it.
>
> That's not it - the bug is masked in sid by there only being one version
> of Python 2.

Right. When there are two versions of Python 2 available, the bug can show 
up. Otherwise, it can't.

>>> Fortunately the solution is relatively simple:
>>>
>>> * Drop package-local Makefile constructs handling multiple python versions
>>
>> True, that needs to be simplified.  It's been in my todo list for a
>> while now.
>>
>>> * Explicitly call the python_distutils debhelper buildsystem plugin
>>
>> No, this is not a good idea.
>>
>>> * Use override hooks to call only the upstream Makefile targets which handle
>>> non-distutils parts of the build.

>> I don't want to replicate upstream's Makefile in debian/rules.
>> Today this may mean calling "make doc" and "make tests" by hand but
>> you never know what can change in upstream's Makefile that will be
>> lost because we're not using it any more.  Mercurial is meant to be
>> built using the Makefile so calling distutils directly should be
>> reduced to the minimum.

> I suppose you can get away with this for now since upstream Python have
> declared there will never be a Python 2.8.
>
> However, by doing so you block debhelper's automatic support for
> supplying the right options to setup.py, and end up having to re-add
> things like --install-layout=deb within debian/rules explicitly.
>
> I'd suggest that's as much or more of an awkward tie into the internals
> of the buildsystem than my proposed patch.
>
> Max.

I'd have to go with Max here.  I asked about this bug on #mercurial on
freenode. Max happened to be in the channel, and very kindly took some
time to work out a patch. I think this patch merits serious
consideration.

Your main objection seems to be based on the assumption that Debian
Mercurial packaging *must* call upstream's Makefile.  To be precise,
you wrote "Mercurial is meant to be built using the Makefile so
calling distutils directly should be reduced to the minimum."  I don't
see why this is so. You say "but you never know what can change in
upstream's Makefile that will be lost because we're not using it any
more". As far as I know, the Mercurial Makefile is not going
anywhere. One can easily check it periodically. Also, it does not
change that often. I see the upstream Makefile is called in
override_dh_auto_build: and nowhere else. It is called as

     $(MAKE) all

Annotate run on the relevant lines shows:

     2244: all: build doc
     [....]
     2235: build:
     18056:  $(PYTHON) setup.py $(PURE) build $(COMPILER:%=-c %)
      2235:
      2235: doc:
      2235:  $(MAKE) -C doc


Now 18056 was

     changeset:   18056:7c9b07f0da73
     parent:      18050:5522a7951bd7
     user:        Bryan O'Sullivan <bryano at fb.com>
     date:        Tue Dec 11 13:44:00 2012 -0800
     files:       Makefile
     description:
     makefile: allow local builds to work on windows/mingw32

and the relevant change was

-       $(PYTHON) setup.py $(PURE) build
+       $(PYTHON) setup.py $(PURE) build $(COMPILER:%=-c %)

Which afaict on Debian is a no-op. Before that, annotate shows

     2244: all: build doc
     [....]
     2235: build:
     7706:  $(PYTHON) setup.py $(PURE) build
     2235:
     2235: doc:
     2235:  $(MAKE) -C doc

This corresponds to changeset

     changeset:   7706:0ae7f0b312ea
     user:        Martin Geisler <mg at daimi.au.dk>
     date:        Sat Jan 24 01:47:36 2009 +0100
     files:       .hgignore Makefile
     description:
     use PURE option in Makefile

with change.

  build:
-       $(PYTHON) setup.py build
+       $(PYTHON) setup.py $(PURE) build

Also a no-op. So, I think we can conclude this portion of the
Makefile, at least, does not change very often.

Max's main point, I think, is that it makes sense to make use of
debhelper's built-in functionality for handling things like multiple
Python versions correctly, and it does not make sense to reinvent the
wheel. (I don't presume to speak for Max here, so Max, please correct
me if this is not right).

The only reason not to do this is the overriding principle that the
Makefile should be called correctly, and I have already attempted to
point out there is no very compelling reason to do so.

Granted, the fate of the world does not depend on this
patch. Regardless, I suggest asking Mercurial devs directly what they
think. Matt Mackall, at least, is not shy about expressing his
opinion, and will tell you if he thinks the Makefile should be called
directly. Also, you could talk to the other people who have done work
on packaging Debian in the past. Vincent Danjean and Tristan Seligmann
come to mind.

                                                        Regards, Faheem