Ticket #189 (closed bug: fixed)

Opened 12 years ago

Last modified 12 years ago

Small Sconscript tweaks (DESTDIR and BUILD_TESTS fixes)

Reported by: slack Assigned to: arnonym
Priority: major Milestone: FFADO 2.0
Component: Version: FFADO 2.0-rc1 (1.999.40)
Keywords: Cc:
The device the bug applies to:

Description

Problem 1:

If someone actually does build libffado as root (libffado is one of the few real examples why this *IS* a bad idea), any existing installation of libffado will be destroyed.

It is just wrong to try to remove installed files while building! This should only be done during installing/uninstalling.

- If the user account used for building has no permission to remove installed files, a permission error happens. - The building system is normally not the system the resulting libffado package will be installed.

Therefore as a workaround I have commented out "env.Execute( env.Action( "rm -f %s/ffadomixer" % envbindir? ) )".

Problem 2:

While installing, scons ignores "BUILD_TESTS".

The fixes work for me while building my Slackware packages, I hope they are ok for inclusion.

Thanks

Attachments

buildsystem.fixes.diff (1.3 kB) - added by slack on 12/25/08 05:01:29.

Change History

12/25/08 05:01:29 changed by slack

  • attachment buildsystem.fixes.diff added.

12/25/08 08:07:48 changed by nettings

that one was added by arnold at my request. the problem was that ffadomixer got renamed to ffado-mixer for consistency, which would leave migrating users with an outdated ffadomixer binary that wouldn't get overwritten. i never looked at how it was done, just tested that i was rid of the old one :) but i agree, it should not happen during build, only during scons install. maybe arnold had a particular reason for doing it this way?

12/25/08 10:46:32 changed by arnonym

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

I don't know which version you are trying to package (or build) but I can't find that convenience-function neither in the 2.0-branch nor in trunk. And it got removed from the branch before the rc was build afaik.

It was only a convenience-function for developers (which should know how to install libffado without ever being root) and it should not complain when the file is not there or can't be deleted because of missing permissions. But I didn't test the second.

Anyway, the first part of this bug-report is invalid as far as I see it... I will take a look at the second point. Basicly the tests-dir shouldn't even be touched when BUILDS_TESTS is false. So the fix to this is not indie the tests-dir but in the toplevel SConstruct.

12/25/08 11:04:52 changed by arnonym

(In [1512]) - Somehow there where two checks for the xdg-tools. One is enough.

  • Reword the failure message for the mixers a bit.
  • Don't touch the tests-dir when BUILD_TESTS is false. See #189

12/25/08 11:08:19 changed by arnonym

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

12/26/08 02:36:26 changed by slack

First, thank you all for the very fast responses to this report.

I also believe your fix for problem 2 in the toplevel SConstruct is the right way to fix it.

But I don't completely understand your comment about problem 1 (which version?):

wget http://www.ffado.org/files/libffado-2.0-rc1.tar.gz tar xvzf libffado-2.0-rc1.tar.gz cd libffado-2.0-rc1

Then in SConstruct, line 565 you will find:

env.Execute( env.Action( "rm -f %s/ffadomixer" % envbindir? ) )

But you are right, in the latest svn tree [1513], it isn't there anymore.

So, I also can report: Building works ok now.

P.S.: Merry christmas and a happy new year to you all, and thank you for the good work.

12/26/08 03:58:29 changed by arnonym

Ah, so the change in the branch was after the tagging of rc1. Sorry, did overlook this... Just remove that command from the SConscript when you do your packages.

ppalmers, I think its time for either the next rc or the release...