Bug#523842: mplayer: Debdiff with the change to create the nogui package, as well as a manpages package

[fabrice]

Hi,

Reinhard Tartler escribió:
>>  # commands to compile the package
>> -build-arch: build-arch-stamp
>> -build-arch-stamp: configure-arch-stamp
>> +build-gui: build-gui-stamp
>> +build-gui-stamp:
>>  	dh_testdir
>> +	if test -f build-nogui-stamp; then \
>> +	  $(MAKE) distclean; \
>> +	  rm -f build-nogui-stamp; \
>> +	fi
>> +	rm -f configure-arch-stamp
>> +	$(CLEAN_ENV) \
>> +	./configure $(COMMON_CONFIGURE_FLAGS) $(DEB_BUILD_CONFIGURE) --enable-gui
>> +	$(CLEAN_ENV) \
>> +	$(MAKE)
>> +	touch build-gui-stamp
>> +
>> +build-nogui: build-nogui-stamp
>> +build-nogui-stamp: configure-arch-stamp
>> +	dh_testdir
>> +	if test -f build-gui-stamp; then \
>> +	  $(MAKE) distclean; \
>> +	  rm -f build-gui-stamp; \
>> +	fi
>>  	$(CLEAN_ENV) \
>>  	$(MAKE)
>> -	touch build-arch-stamp
>> +	touch build-nogui-stamp
> 
> this is taken from marillat and look unbelievably silly, IMO. it
> obviously does not support paralell builds and looks like someone tried
> to write a shellscript in make.

Well, actually, this is taken from Ubuntu, but the origin of the package 
in Ubuntu is Marillat, so yes.

> how about merging the configure and merge and build targets for all
> variants? We cannot build in parallel anyway and they are all phony

You mean merge the configure, build and install targets? This is more or 
less what I tried, but I can make it cleaner.
The only issue is that we still needs the configure step for the 
documentation, so we will end running 3 times the configure script. we 
can live with that.

>> -build: build-indep build-arch
>> +build: build-indep
>>  
> 
> This target does not seem to be used from anywhere. Perhaps we should
> just drop it?

Done

>> -	$(MAKE) install-gui DESTDIR=$(destdir)
>> -	$(MAKE) install-mplayer-gui-man DESTDIR=$(destdir)
>> +	$(MAKE) install-mplayer DESTDIR=$(CURDIR)/debian/mplayer-nogui
>>  
> so no manpage for the nogui variant?

That's because I split the manpages to a different package. But I 
understand your point bellow, so we can install only the english version 
of the manpage in the mplayer package, and reduce archive size.

>> diff -u mplayer-1.0~rc3+svn20090405/debian/control mplayer-1.0~rc3+svn20090405/debian/control
>> --- mplayer-1.0~rc3+svn20090405/debian/control
>> +++ mplayer-1.0~rc3+svn20090405/debian/control
>> @@ -75,6 +75,9 @@
>>  	 mplayer-skin,
>>  	 ${shlibs:Depends},
>>  	 ${misc:Depends}
>> +Recommends: mplayer-manpages
>> +Conflicts: mplayer-nogui
>> +Replaces: mplayer-nogui
>>  Description: movie player for Unix-like systems
>>   MPlayer plays most MPEG, VOB, AVI, Ogg/OGM, VIVO,
>>   ASF/WMA/WMV, QT/MOV/MP4, FLI, RM, NuppelVideo, yuv4mpeg, FILM, RoQ, PVA files,
> 
> 
> Yes, I know that this is taken from marillat. While reviewing your
> patch, I was think if it wouldn't actually make more sense to not
> make these package conflict, but rather do the following instead:

This is because actually, the 2 packages contain the same executable 
(mplayer), so to avoid issue during installation, we have to make them 
conflict, except if we rename them.

>  - in mplayer: install /usr/bin/mplayer.gui
>  - in mplayer-nogui: install /usr/bin/mplayer.nogui
>  - use update-alternatives(8) in both packages

I like the idea of update-alternatives, and having the 2 packages not 
conflicting by installing the same file. It has to be run in the 
debian/postinst file, right?

>  - make mplayer depend on mplayer.nogui
>  - don't install support files like documentation to gui since it is
>    already shipped with the nogui package.

How can we use update-alternative, if we always install both packages? 
Also, we will end up with more disk size, as we will have 2 times the 
executable. Or is there something I didn't understood?

>> +Package: mplayer-manpages
> is that really necessary? I read from your mail you want to save archive
> space, but I'm not really comfortable with not shipping the manpage in
> the normal package is not really comfortable. How about installing only
> the english manpage to the nogui package and all translations in
> mplayer-doc?

Yes: that's a good idea.

>> +++ mplayer-1.0~rc3+svn20090405/debian/mplayer-nogui.install
>> +++ mplayer-1.0~rc3+svn20090405/debian/mplayer-nogui.examples
>> +++ mplayer-1.0~rc3+svn20090405/debian/mplayer-nogui.mime
>> +++ mplayer-1.0~rc3+svn20090405/debian/mplayer-nogui.preinst
> 
> with the approach outlined above, we don't need to duplicate these
> files. Duplicating files in the package is certainly not acceptable as
> it is too error prone.

Another approach could be to create a mplayer-common package, with all 
that stuff (that's what I did at first).

Thanks for your review! I'll continue working on the package, and upload 
a new debdiff as soon as all changes are performed.

Fabrice