Bug#750871: Bug 750871 - patch
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 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 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
Bug#750871: Bug 750871 - patch
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. >> 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. -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
Bug#750871: Bug 750871 - patch
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. > 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. Faheem, as I understand it, the backport version of the package will need to reverse apply r10748 [1] http://anonscm.debian.org/viewvc/python-apps/packages/mercurial/trunk/debian/rules?r1=10311&r2=10748&pathrev=10748 Cheers, Javi signature.asc Description: Digital signature
Bug#750871: Bug 750871 - patch
On Sat, 7 Jun 2014, 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. Fortunately the solution is relatively simple: * Drop package-local Makefile constructs handling multiple python versions * Explicitly call the python_distutils debhelper buildsystem plugin * Use override hooks to call only the upstream Makefile targets which handle non-distutils parts of the build. Patch attached. Regards, Max Bowsher. Dear Maintainers, I tested Max's patch. It works, and looks like a clean solution. I hope you will apply it. Regards, Faheemdiff -ru mercurial-3.0/debian/rules mercurial-3.0.fixed/debian/rules --- mercurial-3.0/debian/rules 2014-04-07 22:58:25.0 +0100 +++ mercurial-3.0.fixed/debian/rules 2014-06-07 21:32:25.0 +0100 @@ -5,17 +5,30 @@ #export DH_VERBOSE=1 %: - dh $@ --with python2,bash-completion + dh $@ --with python2,bash-completion --buildsystem python_distutils -PYVERS=$(shell pyversions -vs) PYVER_DEFAULT=$(shell pyversions -vd) DEB_HOST_ARCH := $(shell dpkg-architecture -qDEB_HOST_ARCH) +# This package uses both distutils and Makefile elements in its buildsystem. +# We configure debhelper to use the python_distutils buildsystem plugin, so +# that appropriate Debian-specific invocations of setup.py are used, and then +# call the extra needed Makefile targets from override_dh_auto_* targets. + override_dh_auto_build: - $(MAKE) all + dh_auto_build + $(MAKE) doc # Do not start a line with a word with a dot in a manpage sed -i -e 's,^[.]\(hgignore\|hg/hgrc\),\\fP\1,' doc/hg.1 +override_dh_auto_install: + dh_auto_install + $(MAKE) install-doc DESTDIR=$(CURDIR)/debian/tmp + +override_dh_auto_clean: + dh_auto_clean + $(MAKE) clean + ifneq (,$(filter parallel=%,$(DEB_BUILD_OPTIONS))) NJOBS := $(patsubst parallel=%,%,$(filter parallel=%,$(DEB_BUILD_OPTIONS))) PARALLEL_TEST_JOBS := --jobs $(NJOBS) @@ -44,11 +57,6 @@ rename.ul .deb-backup '' $(CURDIR)/tests/* endif -override_dh_auto_install: $(PYVERS:%=install-python%) - -install-python%: - python$* setup.py install --root $(CURDIR)/debian/tmp --install-layout=deb - override_dh_install: dh_install if test -d $(CURDIR)/debian/mercurial ; then \
Bug#750871: Bug 750871 - patch
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. Fortunately the solution is relatively simple: * Drop package-local Makefile constructs handling multiple python versions * Explicitly call the python_distutils debhelper buildsystem plugin * Use override hooks to call only the upstream Makefile targets which handle non-distutils parts of the build. Patch attached. Regards, Max Bowsher. diff -ru mercurial-3.0/debian/rules mercurial-3.0.fixed/debian/rules --- mercurial-3.0/debian/rules 2014-04-07 22:58:25.0 +0100 +++ mercurial-3.0.fixed/debian/rules 2014-06-07 21:32:25.0 +0100 @@ -5,17 +5,30 @@ #export DH_VERBOSE=1 %: - dh $@ --with python2,bash-completion + dh $@ --with python2,bash-completion --buildsystem python_distutils -PYVERS=$(shell pyversions -vs) PYVER_DEFAULT=$(shell pyversions -vd) DEB_HOST_ARCH := $(shell dpkg-architecture -qDEB_HOST_ARCH) +# This package uses both distutils and Makefile elements in its buildsystem. +# We configure debhelper to use the python_distutils buildsystem plugin, so +# that appropriate Debian-specific invocations of setup.py are used, and then +# call the extra needed Makefile targets from override_dh_auto_* targets. + override_dh_auto_build: - $(MAKE) all + dh_auto_build + $(MAKE) doc # Do not start a line with a word with a dot in a manpage sed -i -e 's,^[.]\(hgignore\|hg/hgrc\),\\fP\1,' doc/hg.1 +override_dh_auto_install: + dh_auto_install + $(MAKE) install-doc DESTDIR=$(CURDIR)/debian/tmp + +override_dh_auto_clean: + dh_auto_clean + $(MAKE) clean + ifneq (,$(filter parallel=%,$(DEB_BUILD_OPTIONS))) NJOBS := $(patsubst parallel=%,%,$(filter parallel=%,$(DEB_BUILD_OPTIONS))) PARALLEL_TEST_JOBS := --jobs $(NJOBS) @@ -44,11 +57,6 @@ rename.ul .deb-backup '' $(CURDIR)/tests/* endif -override_dh_auto_install: $(PYVERS:%=install-python%) - -install-python%: - python$* setup.py install --root $(CURDIR)/debian/tmp --install-layout=deb - override_dh_install: dh_install if test -d $(CURDIR)/debian/mercurial ; then \
