Ticket #278 (closed bug: fixed)

Opened 10 years ago

Last modified 10 years ago

Patches to allow specific build flags, different installation directories and fix gcc45 build issues.

Reported by: plater Assigned to: arnonym
Priority: major Milestone: FFADO 2.1
Component: generic Version: FFADO 2.0.0
Keywords: Cc:
The device the bug applies to:

Description

Hi, I am a multimedia maintainer for openSUSE and I thought you might be interested in three patches I made to package ffado to be used by jack. ffado-2.0.0-gcc45.patch fixes gcc45 build issues. ffado-2.0.0-sconinstall.patch adds the ability to specify installation directories that are different to the ones used in building files eg. libffado.pc so they reference the correct paths and not the buildroot prefixed installation ones. ffado-2.0.0-sconsflags.patch is used because I'm unable to pass build flags that are needed to build the package for the distribution. Strangely enough I can pass xdg variables but not CFLAGS, CCFLAGS and CXXFLAGS.

Attachments

ffado-2.0.0-gcc45.patch (3.3 kB) - added by plater on 05/01/10 09:27:32.
ffado-2.0.0-gcc45.2.patch (3.3 kB) - added by plater on 05/01/10 09:28:01.
ffado-2.0.0-sconinstall.patch (3.4 kB) - added by plater on 05/01/10 09:32:01.
ffado-2.0.0-sconsflags.patch (0.7 kB) - added by plater on 05/01/10 09:33:09.
SConstruct.compile_flags-2.0branch.patch (3.3 kB) - added by arnonym on 05/10/10 13:03:00.
Fullfill distributors dreams of custom flags?

Change History

05/01/10 09:27:32 changed by plater

  • attachment ffado-2.0.0-gcc45.patch added.

05/01/10 09:28:01 changed by plater

  • attachment ffado-2.0.0-gcc45.2.patch added.

05/01/10 09:32:01 changed by plater

  • attachment ffado-2.0.0-sconinstall.patch added.

05/01/10 09:33:09 changed by plater

  • attachment ffado-2.0.0-sconsflags.patch added.

05/01/10 11:31:56 changed by adi

  • priority changed from major to trivial.

For the gcc-4.5 patch, I suggest to use my version in #255 as it does not contain the whitespace change. ;)

http://subversion.ffado.org/attachment/ticket/255/ffado-gcc45.patch

I don't really understand the purpose (use case) of ffado-2.0.0-sconinstall.patch. Shouldn't DESTDIR do the trick?

Just my €0.02

05/03/10 13:02:32 changed by arnonym

  • owner set to arnonym.
  • status changed from new to assigned.

the gcc-fixes are applied in r1824 and r1825

05/03/10 13:07:52 changed by arnonym

The sconinstall-patch is nonsense (unless you can prove me wrong that is). if you want to change the install paths to be different from $prefix/lib or $prefix/bin, use BINDIR=... at the scons command.

While the upper-case variants of the variables are the ones for usage in scons and honor the DESTDIR, the lower-case variants of these variables are only to be used in the source. And there it doesn't make any sense to define them to something different the the upper-case variants. Except for the missing DESTDIR.

I will look at the env-variables thing.

05/03/10 13:10:22 changed by arnonym

Sure enough its the other way round: Upper-case for in-code usage, lower-case for scons (and following destdir).

Still doesn't make any sense to have libdir any different then $DESTDIR/$LIBDIR as this will break finding files...

05/03/10 14:01:07 changed by plater

When I used DESTDIR=$RPM_BUILD_ROOT with rpmbuild on my system, the binaries ended up in my system's /usr/lib64 and /usr/bin and only files under /usr/share where installed in the buildroot. That was the reason I patched in separate paths for the lowercase install paths. There is also a source rpm produced from the openSUSE build and we can't have files outside of the buildroot when a user tries to build the package with a source rpm. As for the flags, I and others have had to patch in build flags in other scons built packages so somewhere there must be a bug in scons. import os env = Environment(ENV = os.environ) is supposed to propagate the entire environment but it doesn't pass any of the CC,CXX or CFLAGS although it could pass xdg the environment. The only package I know of where flags can be passed is blender and that package has specific options for CFLAGS= and CCFLAGS= where CFLAGS sets only CFLAGS and CCFLAGS set both CXX and CFLAGS. I will try the DESTDIR method again and report exactly what happens. I'll also make sure I'm not doing anything wrong.

05/03/10 16:13:43 changed by arnonym

  • priority changed from trivial to major.
  • milestone changed from FFADO 2.x to FFADO 2.1.

So its more like a DESTDIR-bug. Which is strange as it seems to work well for the debian-/ubuntu-, gentoo-, arch- and probably more- guys. At least they didn't yet complain.

Anyway I will check the DESTDIR stuff the next days. And re-read the scons manpage for hints on importing the environment. In the past we had chosen to only import the variables we thought where important. But maybe its better to just import the whole environment, if scons allows that.

05/04/10 11:51:38 changed by arnonym

Hm, configuring my 2.0-branch checkout to have PREFIX=/usr I get the following: Doing "scons install" as users fails (naturally) because of missing rights. Doing "scons install DESTDIR=/home/arnold/tmp" installs everything correctly to DESTDIR/PREFIX while using PREFIX for the compiled-in paths.

I wonder what your problem there is?

05/04/10 12:45:22 changed by plater

It could be me, I will try all possibilities tomorrow and report back.

05/05/10 08:02:26 changed by plater

It was me, I specified DESTDIR at build time and not install time, I've removed that patch. I only have to move the usr/share/libffado/icon directory to usr/share/ now. I didn't have to do that with the install directory patch but that's acceptable.

05/10/10 11:37:55 changed by arnonym

Hm, somehow scons seems to have a problem import C*FLAGS from the environment.

What about adding an option to define extra flags and stuff to be added to the environment via env.MergeFlags()?

env.MergeFlags() has the advantage that it more or less automatically does the right thing.

05/10/10 13:03:00 changed by arnonym

  • attachment SConstruct.compile_flags-2.0branch.patch added.

Fullfill distributors dreams of custom flags?

05/10/10 13:05:23 changed by arnonym

Might the patch fulfill the needs of distributions (and gentoo-users)?

This seems to me much less hacky then importing a bunch of C*FLAGS variables that scons resets upon its initialization. And it states the fact that there is a supported setup of compiler flags and modifying it is asking for non-support.

06/13/10 07:34:36 changed by arnonym

  • status changed from assigned to closed.
  • resolution set to fixed.

Should be fixed r1852 and r1853.