Ardour new B-D

[Jaromír Mikeš]

2016-12-06 23:09 GMT+01:00 James Cowgill <jcowgill at debian.org>:
> On 06/12/16 02:15, Jaromír Mikeš wrote:
Hi,

>> Can you have a look on my initial packaging of qm-dsp?
>
> d/rules
>> LDFLAGS+=-Wl,--as-needed
>
> Use DEB_MAINT_LD_FLAGS_APPEND

Done!

>> override_dh_auto_configure:
>>       qmake-qt4 -project PREFIX=/usr -config release
>>       $(MAKE)
>>       dh_auto_configure
>
> Why are you using qmake? What does it gain you?

Yes it is nonsense ...  :(
Fixed!

> You should not run the build as part of the configure step.
>
> I think dh_auto_configure is a no-op here.

Fixed!

> d/patches/01-Makefile.patch
>
> Why do you need to patch a makefile in? Most of this stuff could be done
> in debian/rules or by dh_install etc.

I will move instructions from "my makefile" to d/rules file latter ...
for now I have just improved patch.

>> +include build/general/Makefile.inc
>
> IMO you should use make recursion instead of including this makefile.

I must investigate how to do properly ... not working here for now :(

>> +
>> +PREFIX ?= /usr
>> +SUFFIX := $(shell uname -m | sed -e 's/^unknown/$//' -e 's/^i.86/$//' -e 's/^x86_64/$/64/')

> This is broken on non-x86 machines. Why do you need it in the first
> place?
>
> As a side note - using uname -m detects the _build_ architecture and
> not the _host_ architecture (which is what you want here).

LIBDIR is overwritten by d/rules

>> +LIBDIR ?= lib$(SUFFIX)
>> +
>> +
>> +MAJVERS = 0
>> +MINVERS = 0.0
>> +VERSION = $(MAJVERS).$(MINVERS)
>
> Unused variable?

Fixed!

>> +CFLAGS += -DNDEBUG -O3 -fPIC -ffast-math -ftree-vectorize -DUSE_PTHREADS -Ibuild/linux/amd64
>
> The headers in build/linux/amd64 look bundled so I don't think you
> should use them.

Will be fixed ... need some time to figure out how to do it

>> +QM-DSP_MIN = $(QM-DSP_MAJ).$(MINVERS)
>> +QM-DSP_DEP =
>> +QM-DSP_H = base/*.h
>> +
>> +
>> +$(QM-DSP_MAJ):       $(QM-DSP_MAJ)
>
> Circular dependency. You probably want to depend on libqm-dsp.a.

Fixed!

>> +     gcc -shared $(LDFLAGS) -o $(QM-DSP_MAJ) -Wl,-soname,$(QM-DSP_MAJ) -Wl,--whole-archive libqm-dsp.a -Wl,--no-whole-archive
>
> I think adding -Wl,--no-undefined is a good idea here. You then need to
> add the other libraries qm-dsp uses so you don't need to add them manually.

Build fails with -Wl,--no-undefined :(

>> +
>> +install:     $(QM-DSP_MAJ)
>> +     install -d $(DESTDIR)$(PREFIX)/include/base
>> +     install -d $(DESTDIR)$(PREFIX)/$(LIBDIR)
>> +     install -m 644 $(QM-DSP_H) $(DESTDIR)$(PREFIX)/include/base
>
> I think "/usr/include/base" is a bit too generic. I notice that fedora
> installs them into "/usr/include/qm-dsp".

Hmmm but when building Ardour base/Pitch.h is searched ...

> http://pkgs.fedoraproject.org/cgit/rpms/qm-dsp.git/tree/qm-dsp-install.patch
>
>> +     install -m 755 $(QM-DSP_MAJ) $(DESTDIR)$(PREFIX)/$(LIBDIR)
>> +     /sbin/ldconfig -n $(DESTDIR)$(PREFIX)/$(LIBDIR)
>
> This line does nothing because libqm-dsp.so.0 already exists.

Removed!

>> +     ln -sf $(QM-DSP_MIN) $(DESTDIR)$(PREFIX)/$(LIBDIR)/$(QM-DSP_SO)
>> +
>> +clean:
>> +     /bin/rm -f *~ *.o *.a *.d *.so.*
>
> Make doesn't like this because there is already a clean target defined
> in Makefile.inc.

Removed!

mira