Re: [SCM] mplayer packaging branch, ubuntu, updated. debian/1.0.rc3++final-0ubuntu2
On Tue, Jun 08, 2010 at 06:59:01AM +0200, Reinhard Tartler wrote: On Fri, Jun 04, 2010 at 09:47:10 (CEST), Jonas Smedegaard wrote: On Thu, Jun 03, 2010 at 01:06:46PM +, siret...@users.alioth.debian.org wrote: build HTML documentation only if not already avaiable in the build tree This approach does not work well in case upstream (perhaps accidentally) shipped with documentation prebuilt, and that documentation was not up-to-date (or maybe varies depending on features enabled or not?). I take that risk. I was directly involved with the rc3 release and intend to push the upcoming rc4 release as well. Better to always build but ensure it is built only once (typically done by the build rule depending on a touch file, and the touch file being a rule doing the actual build + touching). As in always throw away upstream provided documentation by always removing on clean? Hm, yes, that would probably work but also increate build time considerably, at least on my laptop. Debian fundamentally compile from source - instead of trusting upstream to ship precomiled material suitable for us. This applies to both code and documentation. And yes, compiling requires computing ressources :-) - Jonas -- * Jonas Smedegaard - idealist & Internet-arkitekt * Tlf.: +45 40843136 Website: http://dr.jones.dk/ [x] quote me freely [ ] ask before reusing [ ] keep private signature.asc Description: Digital signature ___ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/pkg-multimedia-maintainers
Re: [SCM] mplayer packaging branch, ubuntu, updated. debian/1.0.rc3++final-0ubuntu2
On Fri, Jun 04, 2010 at 09:47:10 (CEST), Jonas Smedegaard wrote: > On Thu, Jun 03, 2010 at 01:06:46PM +, siret...@users.alioth.debian.org > wrote: > >>build HTML documentation only if not already avaiable in the build >> tree > > This approach does not work well in case upstream (perhaps accidentally) > shipped with documentation prebuilt, and that documentation was not > up-to-date (or maybe varies depending on features enabled or not?). I take that risk. I was directly involved with the rc3 release and intend to push the upcoming rc4 release as well. > Better to always build but ensure it is built only once (typically done > by the build rule depending on a touch file, and the touch file being a > rule doing the actual build + touching). As in always throw away upstream provided documentation by always removing on clean? Hm, yes, that would probably work but also increate build time considerably, at least on my laptop. -- Gruesse/greetings, Reinhard Tartler, KeyID 945348A4 ___ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/pkg-multimedia-maintainers
Re: [SCM] mplayer packaging branch, ubuntu, updated. debian/1.0.rc3++final-0ubuntu2
On Thu, Jun 03, 2010 at 06:17:28PM +0200, Reinhard Tartler wrote: On Do, Jun 03, 2010 at 17:08:53 (CEST), Felipe Sateler wrote: However, I'm not sure enabling parallel building by default is a good idea. There have been no complaints this far and makes building packages on my machines faster. For example, buildds probably do more than one build at a time. Really? AFAIUI, at least buildd doesn't support this OOTB. I'd rather leave that in until a buildd admin asks us to stop doing that. As I wrote in an earlier mail today, this is not only a matter of pleasing build daemons, but more generally about following Debian Policy: Our packaging should _support_ the use of multiple CPUs, but we should not by default _enable_ that support but leave that to the user of our packaging. Kind regards, - Jonas -- * Jonas Smedegaard - idealist & Internet-arkitekt * Tlf.: +45 40843136 Website: http://dr.jones.dk/ [x] quote me freely [ ] ask before reusing [ ] keep private signature.asc Description: Digital signature ___ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/pkg-multimedia-maintainers
Re: [SCM] mplayer packaging branch, ubuntu, updated. debian/1.0.rc3++final-0ubuntu2
On Thu, Jun 03, 2010 at 01:06:46PM +, siret...@users.alioth.debian.org wrote: build HTML documentation only if not already avaiable in the build tree This approach does not work well in case upstream (perhaps accidentally) shipped with documentation prebuilt, and that documentation was not up-to-date (or maybe varies depending on features enabled or not?). Better to always build but ensure it is built only once (typically done by the build rule depending on a touch file, and the touch file being a rule doing the actual build + touching). - Jonas -- * Jonas Smedegaard - idealist & Internet-arkitekt * Tlf.: +45 40843136 Website: http://dr.jones.dk/ [x] quote me freely [ ] ask before reusing [ ] keep private signature.asc Description: Digital signature ___ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/pkg-multimedia-maintainers
Re: [SCM] mplayer packaging branch, ubuntu, updated. debian/1.0.rc3++final-0ubuntu2-3-g0e7f691
On Thu, Jun 03, 2010 at 04:27:33PM +, siret...@users.alioth.debian.org wrote: clarify lacking build dependency on git +Only if you are building from the packaging gut branch, then the package +expects you to have the quilt package installed. For this reason, quilt +is not an explicit build dependency. You probably meant "...on quilt" in the commit message :-) Mentioning to help avoid the typo creeping into the changelog too. Kind regards, - Jonas -- * Jonas Smedegaard - idealist & Internet-arkitekt * Tlf.: +45 40843136 Website: http://dr.jones.dk/ [x] quote me freely [ ] ask before reusing [ ] keep private signature.asc Description: Digital signature ___ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/pkg-multimedia-maintainers
Re: [SCM] mplayer packaging branch, ubuntu, updated. debian/1.0.rc3++final-0ubuntu2
On Thu, Jun 03, 2010 at 01:06:31PM +, siret...@users.alioth.debian.org wrote: The following commit has been merged in the ubuntu branch: +# Support multiple makes at once +ifneq (,$(filter parallel=%,$(DEB_BUILD_OPTIONS))) +NUMJOBS = -j$(patsubst parallel=%,%,$(filter parallel=%,$(DEB_BUILD_OPTIONS))) +else +# on i386 and amd64, we query the system unless overriden by DEB_BUILD_OPTIONS +ifeq ($(DEB_HOST_ARCH),i386) +NUMJOBS := -j$(shell getconf _NPROCESSORS_ONLN 2>/dev/null || echo 1) +else ifeq ($(DEB_HOST_ARCH),amd64) +NUMJOBS := -j$(shell getconf _NPROCESSORS_ONLN 2>/dev/null || echo 1) +endif +endif Not sure, but it looks to me that above not only provides _support_ for parallel builds but also _enable_ it by default. I don't know if Ubuntu (which apparently you are targeting here) has different poicy, but I believe above is illegal for Debian: According to Debian Policy 3.8.4 §4.9.1, we should enable parallel builds if possible but leave it "to the package maintainer to decide whether the package build times are long enough and the package build system is robust enough to make supporting parallel builds worthwhile." Kind regards, - Jonas -- * Jonas Smedegaard - idealist & Internet-arkitekt * Tlf.: +45 40843136 Website: http://dr.jones.dk/ [x] quote me freely [ ] ask before reusing [ ] keep private signature.asc Description: Digital signature ___ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/pkg-multimedia-maintainers
Re: [SCM] mplayer packaging branch, ubuntu, updated. debian/1.0.rc3++final-0ubuntu2
On Do, Jun 03, 2010 at 18:41:24 (CEST), Felipe Sateler wrote: > On Thu, Jun 3, 2010 at 12:32, Reinhard Tartler wrote: > # Build architecture-independent packages > binary-indep: install-indep > diff --git a/debian/source/format b/debian/source/format > index d3827e7..163aaf8 100644 > --- a/debian/source/format > +++ b/debian/source/format > @@ -1 +1 @@ > -1.0 > +3.0 (quilt) > diff --git a/debian/source/options b/debian/source/options > new file mode 100644 > index 000..98a0c1a > --- /dev/null > +++ b/debian/source/options > @@ -0,0 +1,2 @@ > +single-debian-patch this fixes the filename of the generated patch. >>> >>> I'm not quite sure this is correct... You already have >>> debian/patches/*. What is your intended purpose with this flag? >> >> ,[From the dpkg-source(1): >> | >> | --single-debian-patch >> | >> | Use debian/patches/debian-changes instead of >> | debian/patches/debian-changes-version for the name of the >> | automatic patch generated during build. This option is >> | particularly useful when the package is maintained in a VCS >> | and a patch set can't reliably be generated. Instead the >> | current diff with upstream should be stored in a single >> | patch. When using this option, it is recommended to create a >> | debian/source/patch-header file explaining how the Debian >> | changes can be best reviewed, for example in the VCS that is >> | used. >> | > > I have already read that, thanks. What I mean is why do you want it > enabled, *if you already have patches in debian/patches*?. In other > words, why is the version suffix not desired? Note that this would > only apply for autogenerated patches (for example, in a NMU). hm. actually, good question. My main motivation was to make the naming not dependent on getting the version number right in debian/changelog. But since those patches are not intended to be committed under this name anyways, this is probably a rather weak argument... -- Gruesse/greetings, Reinhard Tartler, KeyID 945348A4 ___ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/pkg-multimedia-maintainers
Re: [SCM] mplayer packaging branch, ubuntu, updated. debian/1.0.rc3++final-0ubuntu2
On Thu, Jun 3, 2010 at 12:32, Reinhard Tartler wrote: # Build architecture-independent packages binary-indep: install-indep diff --git a/debian/source/format b/debian/source/format index d3827e7..163aaf8 100644 --- a/debian/source/format +++ b/debian/source/format @@ -1 +1 @@ -1.0 +3.0 (quilt) diff --git a/debian/source/options b/debian/source/options new file mode 100644 index 000..98a0c1a --- /dev/null +++ b/debian/source/options @@ -0,0 +1,2 @@ +single-debian-patch >>> >>> this fixes the filename of the generated patch. >> >> I'm not quite sure this is correct... You already have >> debian/patches/*. What is your intended purpose with this flag? > > ,[From the dpkg-source(1): > | > | --single-debian-patch > | > | Use debian/patches/debian-changes instead of > | debian/patches/debian-changes-version for the name of the > | automatic patch generated during build. This option is > | particularly useful when the package is maintained in a VCS > | and a patch set can't reliably be generated. Instead the > | current diff with upstream should be stored in a single > | patch. When using this option, it is recommended to create a > | debian/source/patch-header file explaining how the Debian > | changes can be best reviewed, for example in the VCS that is > | used. > | I have already read that, thanks. What I mean is why do you want it enabled, *if you already have patches in debian/patches*?. In other words, why is the version suffix not desired? Note that this would only apply for autogenerated patches (for example, in a NMU). -- Saludos, Felipe Sateler ___ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/pkg-multimedia-maintainers
Re: [SCM] mplayer packaging branch, ubuntu, updated. debian/1.0.rc3++final-0ubuntu2
On Do, Jun 03, 2010 at 17:04:56 (CEST), Felipe Sateler wrote: > On Thu, Jun 3, 2010 at 09:26, Reinhard Tartler wrote: >> On Do, Jun 03, 2010 at 15:05:04 (CEST), siret...@users.alioth.debian.org >> wrote: >> >>> +clean: >>> dh_testdir >>> dh_testroot >>> [ ! -f config.mak ] || $(MAKE) distclean >>> dh_clean build-arch-stamp configure-arch-stamp snapshot_version >>> + ! test -d .git || quilt pop -a || test $$? = 2 >> >> This line expects that if we are building in a git repo, that quilt is >> installed on the developer system. I guess this to is a reasonable >> assumption. > > I agree, but may be confusing in the case that it does fail. Maybe a > note in README.source would be useful? good idea. done. >> >>> # Build architecture-independent packages >>> binary-indep: install-indep >>> diff --git a/debian/source/format b/debian/source/format >>> index d3827e7..163aaf8 100644 >>> --- a/debian/source/format >>> +++ b/debian/source/format >>> @@ -1 +1 @@ >>> -1.0 >>> +3.0 (quilt) >>> diff --git a/debian/source/options b/debian/source/options >>> new file mode 100644 >>> index 000..98a0c1a >>> --- /dev/null >>> +++ b/debian/source/options >>> @@ -0,0 +1,2 @@ >>> +single-debian-patch >> >> this fixes the filename of the generated patch. > > I'm not quite sure this is correct... You already have > debian/patches/*. What is your intended purpose with this flag? ,[From the dpkg-source(1): | | --single-debian-patch | | Use debian/patches/debian-changes instead of | debian/patches/debian-changes-version for the name of the | automatic patch generated during build. This option is | particularly useful when the package is maintained in a VCS | and a patch set can't reliably be generated. Instead the | current diff with upstream should be stored in a single | patch. When using this option, it is recommended to create a | debian/source/patch-header file explaining how the Debian | changes can be best reviewed, for example in the VCS that is | used. | ` >> >>> +skip-patches >> >> Does anyone object to this? I hoped that it would have effect on >> building a source package, unfortunately, it has not. > > This options should be applied at extract time. It has no meaning at > build time (patches are never applied when building a source package, > they are stored in the debian.tar.gz). Yeah, that's slightly annoying... -- Gruesse/greetings, Reinhard Tartler, KeyID 945348A4 ___ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/pkg-multimedia-maintainers
Re: [SCM] mplayer packaging branch, ubuntu, updated. debian/1.0.rc3++final-0ubuntu2
On Do, Jun 03, 2010 at 17:08:53 (CEST), Felipe Sateler wrote: > On Thu, Jun 3, 2010 at 09:06, wrote: >> +# Support multiple makes at once >> +ifneq (,$(filter parallel=%,$(DEB_BUILD_OPTIONS))) >> +NUMJOBS = -j$(patsubst parallel=%,%,$(filter >> parallel=%,$(DEB_BUILD_OPTIONS))) >> +else >> +# on i386 and amd64, we query the system unless overriden by >> DEB_BUILD_OPTIONS >> +ifeq ($(DEB_HOST_ARCH),i386) >> +NUMJOBS := -j$(shell getconf _NPROCESSORS_ONLN 2>/dev/null || echo 1) >> +else ifeq ($(DEB_HOST_ARCH),amd64) >> +NUMJOBS := -j$(shell getconf _NPROCESSORS_ONLN 2>/dev/null || echo 1) >> +endif >> +endif >> + > > For this particular case, I think what you actually want is > DEB_BUILD_ARCH. correct. I'll fix this in both the mplayer and ffmpeg package. > However, I'm not sure enabling parallel building by default is a good > idea. There have been no complaints this far and makes building packages on my machines faster. > For example, buildds probably do more than one build at a time. Really? AFAIUI, at least buildd doesn't support this OOTB. I'd rather leave that in until a buildd admin asks us to stop doing that. -- Gruesse/greetings, Reinhard Tartler, KeyID 945348A4 ___ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/pkg-multimedia-maintainers
Re: [SCM] mplayer packaging branch, ubuntu, updated. debian/1.0.rc3++final-0ubuntu2
On Thu, Jun 3, 2010 at 09:06, wrote: > +# Support multiple makes at once > +ifneq (,$(filter parallel=%,$(DEB_BUILD_OPTIONS))) > +NUMJOBS = -j$(patsubst parallel=%,%,$(filter > parallel=%,$(DEB_BUILD_OPTIONS))) > +else > +# on i386 and amd64, we query the system unless overriden by > DEB_BUILD_OPTIONS > +ifeq ($(DEB_HOST_ARCH),i386) > +NUMJOBS := -j$(shell getconf _NPROCESSORS_ONLN 2>/dev/null || echo 1) > +else ifeq ($(DEB_HOST_ARCH),amd64) > +NUMJOBS := -j$(shell getconf _NPROCESSORS_ONLN 2>/dev/null || echo 1) > +endif > +endif > + For this particular case, I think what you actually want is DEB_BUILD_ARCH. However, I'm not sure enabling parallel building by default is a good idea. For example, buildds probably do more than one build at a time. -- Saludos, Felipe Sateler ___ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/pkg-multimedia-maintainers
Re: [SCM] mplayer packaging branch, ubuntu, updated. debian/1.0.rc3++final-0ubuntu2
On Thu, Jun 3, 2010 at 09:26, Reinhard Tartler wrote: > On Do, Jun 03, 2010 at 15:05:04 (CEST), siret...@users.alioth.debian.org > wrote: > >> +clean: >> dh_testdir >> dh_testroot >> [ ! -f config.mak ] || $(MAKE) distclean >> dh_clean build-arch-stamp configure-arch-stamp snapshot_version >> + ! test -d .git || quilt pop -a || test $$? = 2 > > This line expects that if we are building in a git repo, that quilt is > installed on the developer system. I guess this to is a reasonable > assumption. I agree, but may be confusing in the case that it does fail. Maybe a note in README.source would be useful? > >> # Build architecture-independent packages >> binary-indep: install-indep >> diff --git a/debian/source/format b/debian/source/format >> index d3827e7..163aaf8 100644 >> --- a/debian/source/format >> +++ b/debian/source/format >> @@ -1 +1 @@ >> -1.0 >> +3.0 (quilt) >> diff --git a/debian/source/options b/debian/source/options >> new file mode 100644 >> index 000..98a0c1a >> --- /dev/null >> +++ b/debian/source/options >> @@ -0,0 +1,2 @@ >> +single-debian-patch > > this fixes the filename of the generated patch. I'm not quite sure this is correct... You already have debian/patches/*. What is your intended purpose with this flag? > >> +skip-patches > > Does anyone object to this? I hoped that it would have effect on > building a source package, unfortunately, it has not. This options should be applied at extract time. It has no meaning at build time (patches are never applied when building a source package, they are stored in the debian.tar.gz). > > > -- > Gruesse/greetings, > Reinhard Tartler, KeyID 945348A4 > > ___ > pkg-multimedia-maintainers mailing list > pkg-multimedia-maintainers@lists.alioth.debian.org > http://lists.alioth.debian.org/mailman/listinfo/pkg-multimedia-maintainers > -- Saludos, Felipe Sateler ___ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/pkg-multimedia-maintainers
Re: [SCM] mplayer packaging branch, ubuntu, updated. debian/1.0.rc3++final-0ubuntu2
On Do, Jun 03, 2010 at 15:05:04 (CEST), siret...@users.alioth.debian.org wrote: > The following commit has been merged in the ubuntu branch: > commit 7a46da8f787ed0f224d3cf851264170edaa0f0cd > Author: Reinhard Tartler > Date: Wed Jun 2 18:07:21 2010 +0200 > > convert to source Format: 3.0 (quilt) FYI, I've now converted the mplayer package in the ubuntu branch to Format 3.0 (quilt). These steps were necessary: > diff --git a/debian/control b/debian/control > index 4d74686..1a16ec5 100644 > --- a/debian/control > +++ b/debian/control > @@ -60,7 +60,6 @@ Build-Depends: debhelper (>= 7), > libxxf86vm-dev, > libvdpau-dev [i386 amd64], > pkg-config, > - quilt, > vstream-client-dev, > x11proto-core-dev, > xsltproc, > diff --git a/debian/rules b/debian/rules > index c9c88c0..953cf73 100755 > --- a/debian/rules > +++ b/debian/rules > @@ -1,7 +1,5 @@ > #!/usr/bin/make -f > > -include /usr/share/quilt/quilt.make > - > # This has to be exported to make some magic below work. > export DH_OPTIONS > > @@ -88,7 +86,7 @@ endif > # https://wiki.ubuntu.com/DistCompilerFlags > CLEAN_ENV=env -u CFLAGS -u CPPFLAGS -u LDFLAGS -u FFLAGS -u CXXFLAGS > > -install-arch: $(QUILT_STAMPFN) > +install-arch: > dh_testdir > dh_prep -a > [ ! -f config.mak ] || $(MAKE) distclean > @@ -133,12 +131,12 @@ install-indep-stamp: > > clean > > -clean: clean-real unpatch > -clean-real: > +clean: > dh_testdir > dh_testroot > [ ! -f config.mak ] || $(MAKE) distclean > dh_clean build-arch-stamp configure-arch-stamp snapshot_version > + ! test -d .git || quilt pop -a || test $$? = 2 This line expects that if we are building in a git repo, that quilt is installed on the developer system. I guess this to is a reasonable assumption. > # Build architecture-independent packages > binary-indep: install-indep > diff --git a/debian/source/format b/debian/source/format > index d3827e7..163aaf8 100644 > --- a/debian/source/format > +++ b/debian/source/format > @@ -1 +1 @@ > -1.0 > +3.0 (quilt) > diff --git a/debian/source/options b/debian/source/options > new file mode 100644 > index 000..98a0c1a > --- /dev/null > +++ b/debian/source/options > @@ -0,0 +1,2 @@ > +single-debian-patch this fixes the filename of the generated patch. > +skip-patches Does anyone object to this? I hoped that it would have effect on building a source package, unfortunately, it has not. -- Gruesse/greetings, Reinhard Tartler, KeyID 945348A4 ___ pkg-multimedia-maintainers mailing list pkg-multimedia-maintainers@lists.alioth.debian.org http://lists.alioth.debian.org/mailman/listinfo/pkg-multimedia-maintainers