[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries

[Red Hat Bugzilla]

Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug.

https://bugzilla.redhat.com/show_bug.cgi?id=694287

--- Comment #46 from Hans de Goede  2011-04-27 11:47:37 EDT ---
Hi again,

(In reply to comment #45)
> (In reply to comment #44)

<snip>
> > Remove %{?_smp_mflags} from your make invocation, otherwise the build fails
> > at least it does so consistently on my quad core.
> > Funny how you kept the comment from me snippet saying that the build breaks,
> > but re-added the %{?_smp_mflags} :)
>
> When you said it didn't work I didn't think you meant failed to build, just
> that it didn't do anything. It works for me on my dual core with -j4. Maybe we
> should spec a max number of threads conditional?

No, there is some race condition in there somehow, so anything > 1 is too much.
Maybe it only gets triggered with a newer cmake version, who knows ?

Anyways it is best to just not use %{?_smp_mflags} in this case to avoid build
failures on for example the buildsystem.
> > You're still installing header files from the common dir, resulting in
> > unittest and performance test headers ending up under /usr/include, but
> > see the next item for a more radical suggestion for re-arranging the
> > headers.
>
> Should I remove common in addition to the below suggestion?

s/remove/not install headers from/ -> yes
> What about Externals/MathMLSolver?

I think it is ok to include the headers for that.
> > And last, one slightly larger issue (which I should have checked before).
> > I'm not really happy with putting a bunch of the .h files directly under
> > /usr/include. Ideally (IMHO) COLLADAfoo/include/* should end up as
> > /usr/include/COLLADAfoo/* for all variants of foo
>
> Hmm. This sounds like a job for bash and my bashfoo is not that strong :)
>
> I'm thinking some sort of for loop that either inverts foo/include to
> include/foo or that strips the ./include off before copying the files.

Something like this should do the trick:

for i in COLLADABaseUtils COLLADAFramework COLLADAMax COLLADAMaya \
         COLLADASaxFrameworkLoader COLLADAStreamWriter GeneratedSaxParser; do
    mkdir -p $RPM_BUILD_ROOT%{_includedir}/$i
    cp -a $i/include/* $RPM_BUILD_ROOT%{_includedir}/$i
done

And then "manually" do Externals/MathMLSolver since that is a subdir of a
subdir
> Another option would be to create a /usr/include/openCOLLADA and then just dump
> all the files and/or directories under that...
> What do you think?

I think it is good to keep the headers grouped in the way they are grouped
already in the sources. You could replace %{_includedir} with
%{_includedir}/openCOLLADA in the above script, but I don't think that is
necessary.

Regards,

Hans

p.s.

One other thing, it might be good to also package the COLLADAValidator package
in a -utils subpackage, as that may be a useful utility to have.