[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
https://bugzilla.redhat.com/show_bug.cgi?id=694287 Richard Shaw changed: What|Removed |Added Flags|fedora-review? |fedora-review+ --- Comment #67 from Richard Shaw --- Review flag was left ?, fixing. -- You are receiving this mail because: You are always notified about changes to this product and component ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #66 from Fedora Update System 2011-06-02 15:04:11 EDT --- openCOLLADA-0-5.svn838.fc14 has been pushed to the Fedora 14 stable repository. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 Fedora Update System changed: What|Removed |Added Fixed In Version|openCOLLADA-0-5.svn838.fc15 |openCOLLADA-0-5.svn838.fc14 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #65 from Fedora Update System 2011-06-02 06:56:02 EDT --- openCOLLADA-0-5.svn838.fc14 has been pushed to the Fedora 14 stable repository. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #64 from Fedora Update System 2011-05-03 00:45:07 EDT --- openCOLLADA-0-5.svn838.fc15 has been pushed to the Fedora 15 stable repository. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 Fedora Update System changed: What|Removed |Added Status|ON_QA |CLOSED Fixed In Version||openCOLLADA-0-5.svn838.fc15 Resolution||ERRATA Last Closed||2011-05-03 00:45:13 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 Fedora Update System changed: What|Removed |Added Status|MODIFIED|ON_QA --- Comment #63 from Fedora Update System 2011-04-29 00:32:09 EDT --- openCOLLADA-0-5.svn838.fc15 has been pushed to the Fedora 15 testing repository. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #62 from Fedora Update System 2011-04-28 19:42:10 EDT --- openCOLLADA-0-5.svn838.fc15 has been submitted as an update for Fedora 15. https://admin.fedoraproject.org/updates/openCOLLADA-0-5.svn838.fc15 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 Fedora Update System changed: What|Removed |Added Status|ASSIGNED|MODIFIED -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #61 from Fedora Update System 2011-04-28 19:42:03 EDT --- openCOLLADA-0-5.svn838.fc14 has been submitted as an update for Fedora 14. https://admin.fedoraproject.org/updates/openCOLLADA-0-5.svn838.fc14 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #60 from Richard Shaw 2011-04-28 15:50:13 EDT --- Nevermind... changed perms to 0400... -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #59 from Richard Shaw 2011-04-28 15:48:11 EDT --- Hans, I'm geting an error even though i've uploaded my public SSH key: $ fedpkg clone -B openCOLLADA Cloning into bare repository /home/richard/fedora-scm/openCOLLADA/fedpkg.git... Permission denied (publickey). fatal: The remote end hung up unexpectedly Could not clone: Command '['git', 'clone', '--bare', 'ssh://hobbes1...@pkgs.fedoraproject.org/openCOLLADA', '/home/richard/fedora-scm/openCOLLADA/fedpkg.git']' returned non-zero exit status 128 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #58 from Dennis Gilmore 2011-04-28 12:42:53 EDT --- Git done (by process-git-requests). -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 Richard Shaw changed: What|Removed |Added Flag||fedora-cvs? --- Comment #57 from Richard Shaw 2011-04-28 09:30:58 EDT --- New Package SCM Request === Package Name: openCOLLADA Short Description: Collada 3D import and export libraries Owners: hobbes1069 Branches: f14 f15 InitialCC: -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #56 from Hans de Goede 2011-04-28 09:29:08 EDT --- (In reply to comment #55) > I'm assuming we are not worried about branching for F13, just F14 and F15? Yes, I don't think it is likely we will see blender update using this for F13, so no need for an F13 branch. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #55 from Richard Shaw 2011-04-28 09:17:04 EDT --- I'm assuming we are not worried about branching for F13, just F14 and F15? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #54 from Hans de Goede 2011-04-28 09:03:52 EDT --- (In reply to comment #53) > Woops. Fixed the description, forgot the summary. > > FAS account name is hobbes1069. Ok, you've been added to the packagers group and sponsored, next step is to do a scm admin request to get a module created in git and bugzilla for openCOLLADA, as described here: http://fedoraproject.org/wiki/Package_SCM_admin_requests Note that it may take up to an hour for your new packager right to propagate, so if you get an error when you try to set the cvs flag, wait a bit :) -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 Richard Shaw changed: What|Removed |Added Flag|fedora-review+ |fedora-review? --- Comment #53 from Richard Shaw 2011-04-28 08:52:41 EDT --- Woops. Fixed the description, forgot the summary. FAS account name is hobbes1069. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 Hans de Goede changed: What|Removed |Added Flag|fedora-review? |fedora-review+ --- Comment #52 from Hans de Goede 2011-04-28 04:31:25 EDT --- Hi, (In reply to comment #51) > Everything looks good. The debuginfo file now has some size to it :) > > New spec and SRC RPM uploaded. > > SRC RPM: > https://docs.google.com/leaf?id=0B4A6of0Rl4nUY2UxZGQ4ZDEtY2Q5MC00MTNmLWExOTEtYWVhNjU4NjFhYzgx&hl=en > > SPEC: > https://docs.google.com/leaf?id=0B4A6of0Rl4nUZWVlOWE3ZGEtNDdiMi00MWZmLWE5NmQtOWQ5YTdmYTY5YWFk&hl=en Looks good now, approved! One last small thing, but that can be fixed before / after import into git, you copy pasted the Summary for the doc subpackage from the devel doc package and did not adjust it to describe the doc subpackage. So since this package is approved now it is time to sponsor you, next I'll need to know your FAS account username to create one if needed, see: http://fedoraproject.org/wiki/PackageMaintainers/Join#Get_a_Fedora_Account Then I'll add you to the packagers group and sponsor you. Regards, Hans -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #51 from Richard Shaw 2011-04-27 17:03:10 EDT --- Everything looks good. The debuginfo file now has some size to it :) New spec and SRC RPM uploaded. SRC RPM: https://docs.google.com/leaf?id=0B4A6of0Rl4nUY2UxZGQ4ZDEtY2Q5MC00MTNmLWExOTEtYWVhNjU4NjFhYzgx&hl=en SPEC: https://docs.google.com/leaf?id=0B4A6of0Rl4nUZWVlOWE3ZGEtNDdiMi00MWZmLWE5NmQtOWQ5YTdmYTY5YWFk&hl=en -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #50 from Richard Shaw 2011-04-27 16:18:10 EDT --- (In reply to comment #49) > (In reply to comment #47) > > > > Done. While we on the subject of sub-packages. Should we also make a > > separate > > -doc package? > > No I don't think that is necessary, given that the docs are not all that > large. > (In reply to comment #48) > > Went ahead and created a -doc package. > > > > Hehe, well if you prefer having the htdocs in a separate sub package that is > fine. note that things like README and LICENSE must be in the main package > though. I figured since most people will not be direct users (this package will just be a dependency of the program they are actually using), that it would be good to separate the documentation. Yup, I was careful to only move htdocs to the sub-package. > > Also, I ran rpmlint on all the resulting packages and saw something new: > > > > openCOLLADA-debuginfo.x86_64: E: debuginfo-without-sources > > > > Ah, yes this is caused by you using cmake rather then %cmake when invoking > cmake, > I missed that. This causes rpm_opt_flags to not be used, causing this amongst > other issues. Fixed and rebuilding now. Between taking the smp_flags off and adding whatever the cmake macro adds it's taking MUCH longer to build. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #49 from Hans de Goede 2011-04-27 15:36:58 EDT --- (In reply to comment #47) > Done. While we on the subject of sub-packages. Should we also make a separate > -doc package? No I don't think that is necessary, given that the docs are not all that large. (In reply to comment #48) > Went ahead and created a -doc package. > Hehe, well if you prefer having the htdocs in a separate sub package that is fine. note that things like README and LICENSE must be in the main package though. > Also, I ran rpmlint on all the resulting packages and saw something new: > > openCOLLADA-debuginfo.x86_64: E: debuginfo-without-sources > Ah, yes this is caused by you using cmake rather then %cmake when invoking cmake, I missed that. This causes rpm_opt_flags to not be used, causing this amongst other issues. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #48 from Richard Shaw 2011-04-27 14:34:52 EDT --- Went ahead and created a -doc package. Also, I ran rpmlint on all the resulting packages and saw something new: openCOLLADA-debuginfo.x86_64: E: debuginfo-without-sources Maybe it's always been there since I haven't checked the debuginfo package before. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #47 from Richard Shaw 2011-04-27 14:17:46
EDT ---
(In reply to comment #46)
> Anyways it is best to just not use %{?_smp_mflags} in this case to avoid build
> failures on for example the buildsystem.
Removed.
> > > * 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
Done.
> 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
I got it all in one by doing this:
for dir in COLLADABaseUtils COLLADAFramework COLLADASaxFrameworkLoader \
COLLADAStreamWriter COLLADAValidator Externals/MathMLSolver \
GeneratedSaxParser; do
mkdir -p %{buildroot}%{_includedir}/$(basename $dir)
cp -a $dir/include/* %{buildroot}%{_includedir}/$(basename $dir)
done
> 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.
Done. While we on the subject of sub-packages. Should we also make a separate
-doc package?
Richard
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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)
> > * 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.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #45 from Richard Shaw 2011-04-27 11:17:40
EDT ---
(In reply to comment #44)
> Almost there, but still needs some work:
>
> * Please drop everything in the spec file above Name: except for the %define
> of
> AGE, the rest is no longer needed
Done.
> * Please drop Patch1 and Patch2 they are not needed, they patch SConscript
> files which are only used by scons ...
I think you mean patch 0 and patch1 since patch2 is your patch. Done.
> * Please drop "License:MIT" from the -devel subpackage, since it
> is the same as for the main package (I missed that before)
Done.
> * 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?
> * 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? What about
Externals/MathMLSolver?
> * 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.
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?
Richard
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #44 from Hans de Goede 2011-04-27 10:58:17
EDT ---
Hi,
(In reply to comment #41)
> 1. Right now I'm getting the wrong library permissions. 775 instead of 755.
> I'm
> guessing I could fix this with install instead of cp -a but is there an easy
> way to tell cmake to do the right thing?
Weird, it does not do that for me, maybe a difference in the cmake version we
have (I'm on Fedora 15).
(In reply to comment #43)
> Ok, fixed the permissions on the libraries with "install -p -m 0755..."
Good
> left the symlinks as "cp -a" since they need 777, right?
symlinks are always 777, but anything but cp -a will probably copy / install
the files they point to rather then the links, so keeping cp -a for them is the
right thing to do.
> NEW SRPMS URL:
> https://docs.google.com/leaf?id=0B4A6of0Rl4nUMjJhZDNkNzAtNzI2MC00MmIyLTkyMTktMWQwMWY1ZWZhYzZm&hl=en
Almost there, but still needs some work:
* Please drop everything in the spec file above Name: except for the %define of
AGE, the rest is no longer needed
* Please drop Patch1 and Patch2 they are not needed, they patch SConscript
files which are only used by scons ...
* Please drop "License:MIT" from the -devel subpackage, since it
is the same as for the main package (I missed that before)
* 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} :)
* 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.
* 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
Regards,
Hans
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #43 from Richard Shaw 2011-04-27 10:18:15 EDT --- Ok, fixed the permissions on the libraries with "install -p -m 0755..." left the symlinks as "cp -a" since they need 777, right? Rpmlint output pretty clean now on everything. NEW SRPMS URL: https://docs.google.com/leaf?id=0B4A6of0Rl4nUMjJhZDNkNzAtNzI2MC00MmIyLTkyMTktMWQwMWY1ZWZhYzZm&hl=en SPEC URL Not affected. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #42 from Richard Shaw 2011-04-27 09:48:15 EDT --- (In reply to comment #41) > 2. Ran across this[1] page saying we shouldn't use CMAKE_SKIP_RPATH but I > didn't understand the work around. > [1] http://fedoraproject.org/wiki/Packaging:Cmake Nevermind. Read it again and noticed it's supposed to be handled in by make install but we're not using it. Richard -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #41 from Richard Shaw 2011-04-27 09:47:01 EDT --- Ok. Getting close. 1. Right now I'm getting the wrong library permissions. 775 instead of 755. I'm guessing I could fix this with install instead of cp -a but is there an easy way to tell cmake to do the right thing? 2. Ran across this[1] page saying we shouldn't use CMAKE_SKIP_RPATH but I didn't understand the work around. Richard [1] http://fedoraproject.org/wiki/Packaging:Cmake -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #39 from Hans de Goede 2011-04-27 09:33:28
EDT ---
Hi,
(In reply to comment #37)
> > The lib files should be installed under the same name as you can find them
> > under the ./lib dir, actually you can just do:
> > mkdir -p $RPM_BUILD_ROOT%{_libdir}
> > cp -a lib/* $RPM_BUILD_ROOT%{_libdir}
> >
> > For the so files (still need to also cp the include dirs) from %install
> > since
> > cmake also puts the needed symlinks, etc. under ./lib. Talking about the
> > include dirs, you should now no longer install the include dirs for
> > libBuffer,
> > libftoa and libUTF.
> >
> > The %files entry for the so files in the main package then becomes:
> >
> > %{_libdir}/lib*.so.svn%{AGE}
>
> Shouldn't I just make this "%{_libdir}/lib*"? Otherwise it doesn't pick up the
> symbolic link .so files.
The .so files belong in the -devel sub package, where as the main package gets
the lib*.so.svn%{AGE} files, the .so symlinks are used to determine which lib
to link to when building only, they are not used runtime (the dynamic linker
searches for the soname runtime, which is the lib*.so.svn%{AGE} file).
Regards,
Hans
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #38 from Richard Shaw 2011-04-27 09:31:08
EDT ---
(In reply to comment #37)
> (In reply to comment #36)
> > The %files entry for the so files in the main package then becomes:
> >
> > %{_libdir}/lib*.so.svn%{AGE}
>
> Shouldn't I just make this "%{_libdir}/lib*"? Otherwise it doesn't pick up the
> symbolic link .so files.
Nevermind. I guess those are only used for the -devel package?
Richard
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #40 from Hans de Goede 2011-04-27 09:33:44 EDT --- Right (comments crossed). -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #37 from Richard Shaw 2011-04-27 09:28:34
EDT ---
(In reply to comment #36)
> (In reply to comment #33)
> > (In reply to comment #28)
> > > - Add -p to invocation of cp for installation of header files to preserve
> > > the
> > > timestamps on the files
> >
> > This is already recursive. Would it be appropriate to just use -a here?
>
> Yes
Done.
> (In reply to comment #35)
>
>
> > openCOLLADA.x86_64: E: invalid-soname
> > /usr/lib64/libOpenCOLLADABaseUtils.so.0.838.0
> > libOpenCOLLADABaseUtils.so.svn838
>
> The lib files should be installed under the same name as you can find them
> under the ./lib dir, actually you can just do:
> mkdir -p $RPM_BUILD_ROOT%{_libdir}
> cp -a lib/* $RPM_BUILD_ROOT%{_libdir}
>
> For the so files (still need to also cp the include dirs) from %install since
> cmake also puts the needed symlinks, etc. under ./lib. Talking about the
> include dirs, you should now no longer install the include dirs for libBuffer,
> libftoa and libUTF.
>
> The %files entry for the so files in the main package then becomes:
>
> %{_libdir}/lib*.so.svn%{AGE}
Shouldn't I just make this "%{_libdir}/lib*"? Otherwise it doesn't pick up the
symbolic link .so files.
>
>
> Oh, one other small thing I just noticed, instead of:
> %post -n openCOLLADA -p /sbin/ldconfig
>
> %postun -n openCOLLADA -p /sbin/ldconfig
>
> You can just write:
> %post -p /sbin/ldconfig
> %postun -p /sbin/ldconfig
>
> There is no need for the -n openCOLLADA since that is the main package's name.
>
>
> With all previous fixes + the just cp -a lib thingie I think we're 99% there,
> if you can provide a new spec file + srpm I'll check it out and either approve
> it right away or make a hopefully small list of things left to fix :)
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #36 from Hans de Goede 2011-04-27 01:38:38
EDT ---
(In reply to comment #33)
> (In reply to comment #28)
> > - Add -p to invocation of cp for installation of header files to preserve
> > the
> > timestamps on the files
>
> This is already recursive. Would it be appropriate to just use -a here?
Yes
(In reply to comment #34)
> New problem. I'm getting rpath's in the resultant libraries.
> I added CMAKE_SKIP_RPATH to the cmake command but don't have time to build the
> package just yet to see if it works. Hopefully it's that simple.
I woke up thinking this morning if we don't use cmake for make install, we need
to add this magic value to disable rpath, I'm happy to see you already figured
this out yourself :)
(In reply to comment #35)
> openCOLLADA.x86_64: E: invalid-soname
> /usr/lib64/libOpenCOLLADABaseUtils.so.0.838.0
> libOpenCOLLADABaseUtils.so.svn838
The lib files should be installed under the same name as you can find them
under the ./lib dir, actually you can just do:
mkdir -p $RPM_BUILD_ROOT%{_libdir}
cp -a lib/* $RPM_BUILD_ROOT%{_libdir}
For the so files (still need to also cp the include dirs) from %install since
cmake also puts the needed symlinks, etc. under ./lib. Talking about the
include dirs, you should now no longer install the include dirs for libBuffer,
libftoa and libUTF.
The %files entry for the so files in the main package then becomes:
%{_libdir}/lib*.so.svn%{AGE}
Oh, one other small thing I just noticed, instead of:
%post -n openCOLLADA -p /sbin/ldconfig
%postun -n openCOLLADA -p /sbin/ldconfig
You can just write:
%post -p /sbin/ldconfig
%postun -p /sbin/ldconfig
There is no need for the -n openCOLLADA since that is the main package's name.
With all previous fixes + the just cp -a lib thingie I think we're 99% there,
if you can provide a new spec file + srpm I'll check it out and either approve
it right away or make a hopefully small list of things left to fix :)
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #35 from Richard Shaw 2011-04-26 18:48:00 EDT --- Rebuilt now that I'm home from work. Here's the new rpmlint output from the installed package: (MUCH SMALLER!) # rpmlint openCOLLADA openCOLLADA.x86_64: W: spelling-error Summary(en_US) Collada -> Collard, Collate, Collage openCOLLADA.x86_64: W: spelling-error %description -l en_US opensource -> open source, open-source, outsource openCOLLADA.x86_64: W: spelling-error %description -l en_US validator -> invalidator, validation, validate openCOLLADA.x86_64: W: spelling-error %description -l en_US xml -> XML, cml, ml openCOLLADA.x86_64: W: incoherent-version-in-changelog 0.0-3 ['0-4.svn838.fc14', '0-4.svn838'] openCOLLADA.x86_64: E: invalid-soname /usr/lib64/libOpenCOLLADABaseUtils.so.0.838.0 libOpenCOLLADABaseUtils.so.svn838 openCOLLADA.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libOpenCOLLADABaseUtils.so.0.838.0 /usr/lib64/libpcreposix.so.0 openCOLLADA.x86_64: E: invalid-soname /usr/lib64/libOpenCOLLADAStreamWriter.so.0.838.0 libOpenCOLLADAStreamWriter.so.svn838 openCOLLADA.x86_64: E: invalid-soname /usr/lib64/libMathMLSolver.so.0.838.0 libMathMLSolver.so.svn838 openCOLLADA.x86_64: E: invalid-soname /usr/lib64/libGeneratedSaxParser.so.0.838.0 libGeneratedSaxParser.so.svn838 openCOLLADA.x86_64: E: invalid-soname /usr/lib64/libOpenCOLLADASaxFrameworkLoader.so.0.838.0 libOpenCOLLADASaxFrameworkLoader.so.svn838 openCOLLADA.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libOpenCOLLADASaxFrameworkLoader.so.0.838.0 /usr/lib64/libpcreposix.so.0 openCOLLADA.x86_64: E: invalid-soname /usr/lib64/libOpenCOLLADAFramework.so.0.838.0 libOpenCOLLADAFramework.so.svn838 openCOLLADA.x86_64: W: unused-direct-shlib-dependency /usr/lib64/libOpenCOLLADAFramework.so.0.838.0 /usr/lib64/libMathMLSolver.so.svn838 1 packages and 0 specfiles checked; 6 errors, 8 warnings. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #34 from Richard Shaw 2011-04-26 17:40:03 EDT --- Ok, small update. Apparently cmake installs all the libs a ./lib directory in the build dir so the find command was no longer necessary. Took that out and it built fine. New problem. I'm getting rpath's in the resultant libraries. I added CMAKE_SKIP_RPATH to the cmake command but don't have time to build the package just yet to see if it works. Hopefully it's that simple. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #33 from Richard Shaw 2011-04-26 16:09:25
EDT ---
(In reply to comment #28)
> Needs work:
> ===
>
> Ok, I'm going to divide this into 2 lists, one with small things and one with
> large things. First the small things:
> - rpmlint checks return:
> openCOLLADA.src:108: W: mixed-use-of-spaces-and-tabs (spaces: line 9, tab:
> line 108)
> Please don't use tabs in spec files.
I couldn't find the tab but I ran across 'expand' which seemed to work quite
nicely to fix the problem.
> openCOLLADA.x86_64: W: file-not-utf8 /usr/share/doc/openCOLLADA-0/AUTHORS
> You will need to convert this to utf8, add the following to %prep:
> iconv -f ISO_8859-1 -t utf-8 COLLADAStreamWriter/AUTHORS > \
> COLLADAStreamWriter/AUTHORS.tmp
> touch -r COLLADAStreamWriter/AUTHORS COLLADAStreamWriter/AUTHORS.tmp
> mv COLLADAStreamWriter/AUTHORS.tmp COLLADAStreamWriter/AUTHORS
Done.
> - please drop the following comment (it is the same as Source0, so I see no
> use for it):
> #Source0:%{name}-svn%{AGE}.tar.xz
Already gone. Sorry, I left that in there when I was playing with different
compression (gz, xz, bz2)
> - please drop the following (already commented) command from the spec file:
> #strip --strip-debug *.so.*
> Stripping should *never* be done inside the spec file.
That's left over from Suse. It was always commented out for me. Fixed.
> - please add -p to the invocation of cp for the doc files, to keep the
> original
Fixed.
> timestamps. Also why do you cp LICENSE, where as for AUTHORS you reference
> it with its full path ? This seems inconsistent.
Not sure, unless Dave just wanted AUTHORS in the subdir and LICENSE in the
root. I don't see why AUTHORS couldn't be moved to the install root (is that
the right terminology?)
> - please add -k to the invocation of dos2unix to keep the original timestamps
> on the files
Done.
> - Group: Applications/Productivity
> Is not really appropriate, please use:
> Group: System Environment/Libraries
> For the main package instead.
Apparently Suse has A LOT MORE groups than we do. I guessed on this one but
that was before I really knew what I was doing :)
> - # copy CHANGES.txt
> install -p -m 0644 %{S:1} ./
> This belongs in %prep IMHO
Done.
> - Add -p to invocation of cp for installation of header files to preserve the
> timestamps on the files
This is already recursive. Would it be appropriate to just use -a here?
> - devel does not requires base package n-v-r
> Please change the:
> Requires: %{name} = %{version}
> For the -devel sub-package into:
> Requires: %{name} = %{version}-%{release}
Done.
> - I don't like the way the changelog is formatted, here is a copy paste from
> another review with similar issues:
Done.
Richard
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #32 from Richard Shaw 2011-04-26 15:39:49 EDT --- (In reply to comment #31) > Using cmake with this patch seems to fix all "large things" mentioned in my > full review comment :) I hope you're ok with moving to cmake for building ... Not at all! In fact Dave and I both prefer it but he hadn't had time to learn more cmake so he could figure out what was missing and I don't know it near well enough to take it on. I'll try it out tonight! Richard -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #31 from Hans de Goede 2011-04-26 15:30:29
EDT ---
Created attachment 495019
--> https://bugzilla.redhat.com/attachment.cgi?id=495019
PATCH: add soname to cmake build files, hide util libs
Hi,
So I've been working on the weak non resolved symbols issue, and I noticed
there were CMakeList files already present, and they already contained all the
bits needed to properly link the libs to each other to avoid the weak non
resolved symbols issue.
So I gave the cmake buildsys a spin and it works well, working with cmake is so
much easier then scons :)
So I did a small patch fixing 2 things with the cmake buildsys:
1) Add a proper soname to the build libraries
2) the libBuffer libftoa and libUTF8 libs were only being used in one other
openCOLLADA lib each, and have very generic names. So instead of building
them as libs, I've changed the buildsys to embed them into the libs using
them.
If you use the attached patch (and drop the sconstruct patches) and replace the
%build section with:
%cmake -DUSE_STATIC=OFF -DUSE_SHARED=ON -Dsoversion=svn%{AGE}
# Note building with _smp_mflags does not work for some reason ...
make
Then you'll find properly build libs (including soname + symlinks) under the
lib subdir, make install does not work, it looks like the cmake files need some
work to get this to work, but we can just do it manually for now.
Using cmake with this patch seems to fix all "large things" mentioned in my
full review comment :) I hope you're ok with moving to cmake for building ...
Regards,
Hans
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #29 from Richard Shaw 2011-04-24 12:39:42 EDT --- (In reply to comment #28) > 4) And then there is the issue with the unresolved symbols I've run out of > steam (and time) for now to look into that (blergh scons), maybe I'll look > into > it later today. If not feel free to do a new version in the mean time > addressing all the other concerns. I've been working with Dave Platter, the more or less creator of the *nix version of this library and Suse package maintainer, and he's started working on the undefined-non-weak-symbol issues. Apparently Suse doesn't use rpmlint so he wasn't aware of the problem. He fixed two of the errors by updating the SConscript files. I'll attach the patch as a template. Two down, 243 to go. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #30 from Richard Shaw 2011-04-24 12:40:22 EDT --- Created attachment 494541 --> https://bugzilla.redhat.com/attachment.cgi?id=494541 Patch to fix undefined-non-weak-symbol issues in libbuffer. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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
Hans de Goede changed:
What|Removed |Added
Flag||fedora-review?
--- Comment #28 from Hans de Goede 2011-04-23 11:42:50
EDT ---
Full review done, results below:
Good:
=
- rpmlint checks return:
openCOLLADA.src: W: spelling-error Summary(en_US) Collada -> Collard
openCOLLADA.src: W: spelling-error %description -l en_US opensource -> open
source, open-source, outsource
openCOLLADA.src: W: spelling-error %description -l en_US validator ->
lavatorial
openCOLLADA.src: W: spelling-error %description -l en_US xml -> XML, ml, x ml
openCOLLADA.src:21: W: macro-in-comment %{name}
openCOLLADA.src:21: W: macro-in-comment %{AGE}
openCOLLADA.src: W: invalid-url Source0: openCOLLADA-svn838.tar.xz
openCOLLADA.x86_64: W: spelling-error Summary(en_US) Collada -> Collard
openCOLLADA.x86_64: W: spelling-error %description -l en_US opensource -> open
source, open-source, outsource
openCOLLADA.x86_64: W: spelling-error %description -l en_US validator ->
lavatorial
openCOLLADA.x86_64: W: spelling-error %description -l en_US xml -> XML, ml, x
ml
openCOLLADA.x86_64: W: incoherent-version-in-changelog 0.0-3
['0-3.svn838.fc15', '0-3.svn838']
openCOLLADA-devel.x86_64: W: no-documentation
These can all be ignored.
- package meets naming guidelines
- package meets packaging guidelines
- license (MIT) OK, text in %doc, matches source
- spec file legible, in am. english
- source matches upstream
- package compiles on devel (x86)
- no missing BR
- no unnecessary BR
- no locales
- not relocatable
- owns all directories that it creates
- no duplicate files
- permissions ok
- %clean ok
- macro use consistent
- code, not content
- no need for -docs
- nothing in %doc affects runtime
- no need for .desktop file
- devel package ok
- no .la files
- post/postun ldconfig ok
Needs work:
===
Ok, I'm going to divide this into 2 lists, one with small things and one with
large things. First the small things:
- rpmlint checks return:
openCOLLADA.src:108: W: mixed-use-of-spaces-and-tabs (spaces: line 9, tab:
line 108)
Please don't use tabs in spec files.
openCOLLADA.x86_64: W: file-not-utf8 /usr/share/doc/openCOLLADA-0/AUTHORS
You will need to convert this to utf8, add the following to %prep:
iconv -f ISO_8859-1 -t utf-8 COLLADAStreamWriter/AUTHORS > \
COLLADAStreamWriter/AUTHORS.tmp
touch -r COLLADAStreamWriter/AUTHORS COLLADAStreamWriter/AUTHORS.tmp
mv COLLADAStreamWriter/AUTHORS.tmp COLLADAStreamWriter/AUTHORS
- please drop the following comment (it is the same as Source0, so I see no
use for it):
#Source0:%{name}-svn%{AGE}.tar.xz
- please drop the following (already commented) command from the spec file:
#strip --strip-debug *.so.*
Stripping should *never* be done inside the spec file.
- please add -p to the invocation of cp for the doc files, to keep the original
timestamps. Also why do you cp LICENSE, where as for AUTHORS you reference
it with its full path ? This seems inconsistent.
- please add -k to the invocation of dos2unix to keep the original timestamps
on the files
- Group: Applications/Productivity
Is not really appropriate, please use:
Group: System Environment/Libraries
For the main package instead.
- # copy CHANGES.txt
install -p -m 0644 %{S:1} ./
This belongs in %prep IMHO
- Add -p to invocation of cp for installation of header files to preserve the
timestamps on the files
- devel does not requires base package n-v-r
Please change the:
Requires: %{name} = %{version}
For the -devel sub-package into:
Requires: %{name} = %{version}-%{release}
- I don't like the way the changelog is formatted, here is a copy paste from
another review with similar issues:
"Please put a blank line in the changelog before each data-name-version line
(except for the first). And prefix / itemize items below the
data-name-version line with "- ". IE change:
* Sun Sep 26 2010 Elad Alfassa - 0.6-3
Fix typo.
* Sun Sep 26 2010 Elad Alfassa - 0.6-2
Mark schemas file as config file
* Sun Sep 26 2010 Elad Alfassa - 0.6-1
More spec file fixes
* Thu Sep 23 2010 Elad Alfassa - 0.6-0
Version update, some general spec file fixes.
* Sun Jul 25 2010 Elad Alfassa - 0.3-1
initial build
to:
* Sun Sep 26 2010 Elad Alfassa - 0.6-3
- Fix typo.
* Sun Sep 26 2010 Elad Alfassa - 0.6-2
- Mark schemas file as config file
* Sun Sep 26 2010 Elad Alfassa - 0.6-1
- More spec file fixes
* Thu Sep 23 2010 Elad Alfassa - 0.6-0
- Version update, some general spec file fixes.
* Sun Jul 25 2010 Elad Alfassa - 0.3-1
-initial build"
***
And then now the large things.
1) You're using a soname
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #27 from Richard Shaw 2011-04-21 14:44:50 EDT --- NEW SPEC URL: https://docs.google.com/leaf?id=0B4A6of0Rl4nUZWVlOWE3ZGEtNDdiMi00MWZmLWE5NmQtOWQ5YTdmYTY5YWFk&hl=en NEW SRC RPM URL: https://docs.google.com/leaf?id=0B4A6of0Rl4nUM2IwMzQyOTMtZWZjZS00ZTNmLWEzZTctM2I1NWFjMGJmZTgy&hl=en -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #26 from Hans de Goede 2011-04-21 14:20:11 EDT --- (In reply to comment #25) > > You do a new spec + srpm fixing as much mentioned issues, then I do a full > > review and then we go from there. > > I can post what I have now Yes please. > but I think the only major issue left is all the > undefined-non-weak-symbol warnings from rpmlint. I can probably help with those, once I've an srpm of your latest work to play around with :) -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #25 from Richard Shaw 2011-04-21 14:14:27 EDT --- (In reply to comment #24) > No need for virtual provides, so also no need to worry about versions :) Yeah, I just thought about it. Since the libraries are not in Fedora separately, there's nothing to track. > You do a new spec + srpm fixing as much mentioned issues, then I do a full > review and then we go from there. I can post what I have now, but I think the only major issue left is all the undefined-non-weak-symbol warnings from rpmlint. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #24 from Hans de Goede 2011-04-21 11:06:38 EDT --- Hi, (In reply to comment #23) > Sounds good. A couple of questions: > > 1. Should we add virtual provides for the two bundled libraries? We'll have to > figure out what their version are or fake something. > No need for virtual provides, so also no need to worry about versions :) > 2. Where do we go from here? You do a new spec + srpm fixing as much mentioned issues, then I do a full review and then we go from there. Regards, Hans -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #23 from Richard Shaw 2011-04-21 10:32:21 EDT --- Sounds good. A couple of questions: 1. Should we add virtual provides for the two bundled libraries? We'll have to figure out what their version are or fake something. 2. Where do we go from here? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #22 from Hans de Goede 2011-04-21 04:55:52 EDT --- Hi, (In reply to comment #21) > If there is nothing happening upstream for these two libraries, are they OK > being bundled in openCOLLADA? As long as openCOLLADA is the only user of them, keeping them bundled is fine in this specific case. If however other users turn up in Fedora (unlikely). Things should be coordinated with the package owner of those other users and they should be unbundled. Regards, Hans -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #21 from Richard Shaw 2011-04-20 15:13:16 EDT --- (In reply to comment #20) > (In reply to comment #19) > > (In reply to comment #17) > > > 2 issues: > > > > > > * The tarball contains a lot of bundled libraries (cf, Externals/). > > > > cf? > Compare for the contents of the directory called Externals/ inside of the > tarball. It contains zlib, Cg,, libxml, lib3ds and other libraries. > > A more detailed look into the package tells that openCOLLADA currently only > uses MathMLSolver/ and UTF/. > > => Make sure the other directories are not being used when building for > Fedora. > Brute-force way to do so would be to remove them in %prep (This is what a > recent change to the FPG recommends). I have done this and it still builds. > > > This is problematic twice: > > > - In general, the Fedora package should not not use them. > > > > > > - These packages' licenses need to be checked for whether they are > > > properly > > > licensed and whether these package's licenses are compatible to > > > openCOLLADA's > > > license. > > > > > > From a coarse glance into tarball, I'd suspect Externals/MathMLSolver not > > > to be > > > properly licensed (I can't find any licence). Googling however directed > > > me to > > > http://mathmlsolver.svn.sourceforge.net/viewvc/mathmlsolver/trunk, but I > > > haven't checked details, yet. > > > > Drilled down to the actual SF page at it says it is MIT licensed. > Yes. I meanwhile also found some copyright notices in Externals/MathMLSolver's > headers and found openCOLLADA/Externals/MathMLSolver to be a hacked up version > of the code on sourceforge. > > I don't know why openCOLLADA is doing so - Could be they are "just hacking" > and > don't care about proper integration/packaging, could be the sourceforge > project > is dead. AFAICT, googling indicates openCOLLADA is the only user of > MathMLSolver while the sourceforge project might be dead. Yes, I haven't seen much activity on their SF page so I would assume it's dead. As far as UTF goes. It looks to be a copy of ConvertUTF which also appears to be defunct. I found a ftp link for the code on unicode.org through: http://gears.googlecode.com/svn/trunk/third_party/convert_utf/README.google If there is nothing happening upstream for these two libraries, are they OK being bundled in openCOLLADA? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #20 from Ralf Corsepius 2011-04-14 23:08:43 EDT --- (In reply to comment #19) > (In reply to comment #17) > > 2 issues: > > > > * The tarball contains a lot of bundled libraries (cf, Externals/). > > cf? Compare for the contents of the directory called Externals/ inside of the tarball. It contains zlib, Cg,, libxml, lib3ds and other libraries. A more detailed look into the package tells that openCOLLADA currently only uses MathMLSolver/ and UTF/. => Make sure the other directories are not being used when building for Fedora. Brute-force way to do so would be to remove them in %prep (This is what a recent change to the FPG recommends). > > This is problematic twice: > > - In general, the Fedora package should not not use them. > > > > - These packages' licenses need to be checked for whether they are properly > > licensed and whether these package's licenses are compatible to > > openCOLLADA's > > license. > > > > From a coarse glance into tarball, I'd suspect Externals/MathMLSolver not > > to be > > properly licensed (I can't find any licence). Googling however directed me > > to > > http://mathmlsolver.svn.sourceforge.net/viewvc/mathmlsolver/trunk, but I > > haven't checked details, yet. > > Drilled down to the actual SF page at it says it is MIT licensed. Yes. I meanwhile also found some copyright notices in Externals/MathMLSolver's headers and found openCOLLADA/Externals/MathMLSolver to be a hacked up version of the code on sourceforge. I don't know why openCOLLADA is doing so - Could be they are "just hacking" and don't care about proper integration/packaging, could be the sourceforge project is dead. AFAICT, googling indicates openCOLLADA is the only user of MathMLSolver while the sourceforge project might be dead. > How do we handle that? The formal way would be to ask upstream to add the license file. > I ran into this problem on RPMFusion and the decision was to put > comments above the License: tag explaining which parts had what license. Yes, this is one option to handle such cases. > > * The package naming seems inconsistent to me: > > libOpenCOLLADA vs. OpenCOLLADA-devel > > > > The FPG would recommend using the tarball name, which would mean to name the > > packages openCOLLADA and openCOLLADA-devel > > The current naming was how the Suse maintainer set it up and I'm sure their > rules differ in many areas. For the purpose of the Fedora package I'll change > the name. If you want to add "SUSE compatibility" you can add corresponding "Provides: ...". -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #19 from Richard Shaw 2011-04-14 22:40:40 EDT --- (In reply to comment #17) > 2 issues: > > * The tarball contains a lot of bundled libraries (cf, Externals/). cf? > This is problematic twice: > - In general, the Fedora package should not not use them. > > - These packages' licenses need to be checked for whether they are properly > licensed and whether these package's licenses are compatible to openCOLLADA's > license. > > From a coarse glance into tarball, I'd suspect Externals/MathMLSolver not to > be > properly licensed (I can't find any licence). Googling however directed me to > http://mathmlsolver.svn.sourceforge.net/viewvc/mathmlsolver/trunk, but I > haven't checked details, yet. Drilled down to the actual SF page at it says it is MIT licensed. How do we handle that? I ran into this problem on RPMFusion and the decision was to put comments above the License: tag explaining which parts had what license. > * The package naming seems inconsistent to me: > libOpenCOLLADA vs. OpenCOLLADA-devel > > The FPG would recommend using the tarball name, which would mean to name the > packages openCOLLADA and openCOLLADA-devel The current naming was how the Suse maintainer set it up and I'm sure their rules differ in many areas. For the purpose of the Fedora package I'll change the name. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #18 from Richard Shaw 2011-04-14 22:29:39 EDT --- (In reply to comment #13) > # c++filt _ZN6Common4ftoaEfPc _ZN6Common4dtoaEdPcb > Common::ftoa(float, char*) > Common::dtoa(double, char*, bool) > > Check for a library or header providing these two symbols. Thanks for the hint! I actually created a patch (albeit an UGLY patch) for rpmlint to do this for me. Upstream cleaned it up so it will be included in a future release. https://bugzilla.redhat.com/show_bug.cgi?id=696749 -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #17 from Ralf Corsepius 2011-04-13 13:23:43 EDT --- 2 issues: * The tarball contains a lot of bundled libraries (cf, Externals/). This is problematic twice: - In general, the Fedora package should not not use them. - These packages' licenses need to be checked for whether they are properly licensed and whether these package's licenses are compatible to openCOLLADA's license. >From a coarse glance into tarball, I'd suspect Externals/MathMLSolver not to be properly licensed (I can't find any licence). Googling however directed me to http://mathmlsolver.svn.sourceforge.net/viewvc/mathmlsolver/trunk, but I haven't checked details, yet. * The package naming seems inconsistent to me: libOpenCOLLADA vs. OpenCOLLADA-devel The FPG would recommend using the tarball name, which would mean to name the packages openCOLLADA and openCOLLADA-devel -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #16 from Richard Shaw 2011-04-13 10:07:53 EDT --- (In reply to comment #9) > The output of rpmlint you have attached mention a lot of > undefined-non-weak-symbol. > It mean most library are missing the right library link flags at compile time. > This is usually something that can be fixed within the build tool (SCons). > This have to be reported upstream and is probably rather trivial to fix. I got an answer from the Suse maintainer. Basically opencollada is windows only and he did all the work to turn it into a linux library. It's still a work in progress and he's going to see if he can get the non-weak-symbol errors fixed upstream. I tried looking at the SConscript files but it's over my head. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 Hans de Goede changed: What|Removed |Added Status|NEW |ASSIGNED CC||hdego...@redhat.com Blocks|177841(FE-NEEDSPONSOR) | AssignedTo|nob...@fedoraproject.org|hdego...@redhat.com --- Comment #15 from Hans de Goede 2011-04-13 10:04:07 EDT --- Hi, As discussed by email, I'm willing to sponsor Richard. Nicolas will finish the review he has started as far as possible before he goes on a 2 week vacation this friday, and then I'll take over, (re)review, and if all goes well sponsor Richard. Taking this bug, clearing the needsponsor blocker and changing status to assigned. Regards, Hans -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #14 from Richard Shaw 2011-04-12 14:25:34
EDT ---
Ok, strange. The SConscript for libbuffer appears to include the right
directory which has header files for Common::ftoa and Common::dtoa.
Here's the SConscript file for libbuffer:
#!/usr/bin/env python
Import('env')
libName = 'buffer'
srcDirs = [ 'src/' ]
variantDir = env['objDir'] + env['configurationBaseName'] + '/'
outputDir = env['libDir'] + env['configurationBaseName'] + '/'
targetPath = outputDir + libName
incDirs = ['include/', '../libftoa/include']
src = []
for srcDir in srcDirs:
src += [ variantDir + str(p) for p in Glob(srcDir + '*.cpp')]
VariantDir(variant_dir=variantDir + srcDir, src_dir=srcDir,
duplicate=False)
if env['SHAREDLIB']:
SharedLibrary(target=targetPath, source=src, LINKFLAGS =
'-Wl,--soname=libbuffer.so.0', CPPPATH=incDirs, CCFLAGS=env['CPPFLAGS'])
else:
StaticLibrary(target=targetPath, source=src, CPPPATH=incDirs,
CCFLAGS=env['CPPFLAGS'])
---
SHAREDLIB=yes is specified on the scons command in the spec file...
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 Ralf Corsepius changed: What|Removed |Added CC||rc040...@freenet.de --- Comment #13 from Ralf Corsepius 2011-04-12 13:43:21 EDT --- # c++filt _ZN6Common4ftoaEfPc _ZN6Common4dtoaEdPcb Common::ftoa(float, char*) Common::dtoa(double, char*, bool) Check for a library or header providing these two symbols. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #12 from Richard Shaw 2011-04-12 11:55:35 EDT --- Output: # ldd -r /usr/lib64/libbuffer.so linux-vdso.so.1 => (0x7fff8f6d2000) libstdc++.so.6 => /usr/lib64/libstdc++.so.6 (0x003e4ee0) libm.so.6 => /lib64/libm.so.6 (0x003e4560) libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x003e45e0) libc.so.6 => /lib64/libc.so.6 (0x003e44a0) /lib64/ld-linux-x86-64.so.2 (0x003e4460) undefined symbol: _ZN6Common4ftoaEfPc (/usr/lib64/libbuffer.so) undefined symbol: _ZN6Common4dtoaEdPcb (/usr/lib64/libbuffer.so) Ok, now how do I know what's missing? I'm googling everything I can think of and found some previous bug reports and saw solutions like, "Oh, your missing linking against ." But I can't tell how they knew which one was missing! -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #11 from Nicolas Chauvet (kwizart) 2011-04-12 11:43:24 EDT --- you can show it with ldd -r -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #10 from Richard Shaw 2011-04-12 11:34:29 EDT --- I checked into it but I can't find a solution. When it do a direct "ldd /usr/lib64/" I don't see the error. Is it possible these are loaded on demand and these are false positives by rpmlint? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #9 from Nicolas Chauvet (kwizart) 2011-04-12 11:04:47 EDT --- The output of rpmlint you have attached mention a lot of undefined-non-weak-symbol. It mean most library are missing the right library link flags at compile time. This is usually something that can be fixed within the build tool (SCons). This have to be reported upstream and is probably rather trivial to fix. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #8 from Richard Shaw 2011-04-12 10:33:10 EDT --- Uploaded. The existing link should work. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #7 from Nicolas Chauvet (kwizart) 2011-04-12 10:03:54 EDT --- (In reply to comment #6) ... > I still need your input for the "version" of the package, This still doesn't fit the versioning guideline in the case of svn snapshot. https://fedoraproject.org/wiki/Packaging:NamingGuidelines#Package_Versioning If upstream doesn't use a version, then the Version field is "0". Note that I will be in vacation https://fedoraproject.org /wiki/Vacation If a sponsors wants to continue de review, please do so... -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #6 from Richard Shaw 2011-04-12 09:44:47 EDT --- (In reply to comment #5) > Hi, > > What is good it to have an accurate make install script that install things in > the right place. If it's not done by the developpers, that's move the > "responsability" of the location onto the packager shoulders. > So that's worth for a given release, and that might be right in this case; but > it should also be keep right for others releases and others changes done in > consciousness by the developper team. I had some further communications with the Suse maintainer and I found out he was responsible for getting the scons build to work. The developers mainly cater to Windows so I don't see them trampling on any of the work we do for linux. In fact, whatever improvements we (and the Suse maintainer) make will probably get accepted upstream without issue. > Can you upload an updated spec file somewhere ? Sure. I just wanted to get your response first. I still need your input for the "version" of the package, but I'll assume I was correct for now since you didn't say otherwise. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #5 from Nicolas Chauvet (kwizart) 2011-04-12 06:06:07 EDT --- Hi, What is good it to have an accurate make install script that install things in the right place. If it's not done by the developpers, that's move the "responsability" of the location onto the packager shoulders. So that's worth for a given release, and that might be right in this case; but it should also be keep right for others releases and others changes done in consciousness by the developper team. Can you upload an updated spec file somewhere ? -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #4 from Richard Shaw 2011-04-07 15:31:37 EDT --- (In reply to comment #2) > (In reply to comment #1) > > 10/# Manual install ... > > Are you sure this cannot be installed with scons ? You should fix it within > > scons files then. > > Not sure. I'll ask the Suse maintainer why he chose this route. I got a response, here's my email and his response: --- 04/07/2011 04:03 PM, Richard Shaw wrote: >Dave, > >While trying to get blender accepted for Fedora I got a request that I >submit the openCOLLADA package for review as well. > >One of the questions I couldn't answer is why %install is so manual >and if it's possible to get scons to do the install even if it >requires patching. Not patching writing from scratch. IMHO it's better to have control over installation from the spec file rather than a badly written make install that puts everything in the wrong places. If you're interested in scons and libraries have a look at ffado that's the only library I know of that uses scons to build most use autotools. When I finally have time to switch from scons to cmake, I've already patched SConstruct and the SConscripts heavily, I can get the library versioning and assembly correct. Scons doesn't cater for libs, you have to write everything yourself, whereas cmake does. Collada has SConstruct and CMakeLists.txt, when I originally built it there was only scons and of course vcproj2cmake.rb for visual studio. --- Does this make sense? It's a little over my head. -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #3 from Richard Shaw 2011-04-07 10:00:52 EDT --- Created attachment 490553 --> https://bugzilla.redhat.com/attachment.cgi?id=490553 output of "rpmlint libOpenCOLLADA" for installed package -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 #2 from Richard Shaw 2011-04-07 09:58:13 EDT
---
(In reply to comment #1)
> 1/ Can you show the output of rpmlint on the src.rpm and the installed
> package.
rpmlint for SRPM:
openCOLLADA.src: W: spelling-error Summary(en_US) Collada -> Collard, Collate,
Collage
openCOLLADA.src: W: spelling-error %description -l en_US gtempaccount ->
unaccountable, accountable, accounting
openCOLLADA.src:118: W: mixed-use-of-spaces-and-tabs (spaces: line 8, tab: line
118)
openCOLLADA.src: W: invalid-url Source0: openCOLLADA-svn836.tar.bz2
1 packages and 0 specfiles checked; 0 errors, 4 warnings.
The installed package created a lot of warning that I didn't understand but no
errors. I'll attach the output instead of pasting it here.
> Please fix the empty spaces in the middle of the lines for the %description
Empty lines? Fixed.
> 2/ Please remove Everything that is not usefull in fedora. This reduce
> readability.
> (specially everything before the 'Name' field as this is not usefull in
> fedora).
Those macros are used to build the fake lib version. I can't remove them since
they are used later. What should we do here?
> 3/ The version field is wrong, please follow:
> https://fedoraproject.org/wiki/Packaging:Guidelines#Version_and_Release
I was not sure how to handle this since it has no "version" in the traditional
sense. If I understand the guidelines correctly then this package will always
be at version "1.0" with the svn tag in the release?
> 4/ Uneeded BR: gcc-c++ - It is added by default, please remove.
Fixed.
> 5/ Buildroot is undeed nowdays:
> https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
I saw this before but the spec templates still have it in there so the
guidelines conflict here. Now I know it's truly save to remove...
Isn't one of the other sections, %clean if I remember correctly also now not
needed?
> 6/ Remove spurious 'echo %{buildir}'
Fixed
> 7/ rpm -E %{?jobsflag} is empty. fedora uses %{?_smp_mflags} instead. Please
> remove the previous rpm macro.
Fixed
> 8/ Where does CHANGES.TXT come from ? Please add an url if possible
> ---
> # copy CHANGES.txt
> cp %{S:1} ./
> ---
> It's better to use install -p
It's the svn log which I guess the Suse creator/maintainer of this package
either likes or Suse requires. Do we need this? It appears to only be added to
the -devel package...
> 9/ About:
> # Add some docs, need to fix eol encoding with dos2unix in some files
> -> Please move this in %prep section
>
> 10/# Manual install ...
> Are you sure this cannot be installed with scons ? You should fix it within
> scons files then.
Not sure. I'll ask the Suse maintainer why he chose this route.
> Here a preliminary pass for the review.
Thanks!
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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
Nicolas Chauvet (kwizart) changed:
What|Removed |Added
CC||kwiz...@gmail.com
--- Comment #1 from Nicolas Chauvet (kwizart) 2011-04-06
18:18:08 EDT ---
This is an informal review since I cannot sponsor you.
1/ Can you show the output of rpmlint on the src.rpm and the installed package.
Please fix the empty spaces in the middle of the lines for the %description
2/ Please remove Everything that is not usefull in fedora. This reduce
readability.
(specially everything before the 'Name' field as this is not usefull in
fedora).
3/ The version field is wrong, please follow:
https://fedoraproject.org/wiki/Packaging:Guidelines#Version_and_Release
4/ Uneeded BR: gcc-c++ - It is added by default, please remove.
5/ Buildroot is undeed nowdays:
https://fedoraproject.org/wiki/Packaging:Guidelines#BuildRoot_tag
6/ Remove spurious 'echo %{buildir}'
7/ rpm -E %{?jobsflag} is empty. fedora uses %{?_smp_mflags} instead. Please
remove the previous rpm macro.
8/ Where does CHANGES.TXT come from ? Please add an url if possible
---
# copy CHANGES.txt
cp %{S:1} ./
---
It's better to use install -p
9/ About:
# Add some docs, need to fix eol encoding with dos2unix in some files
-> Please move this in %prep section
10/# Manual install ...
Are you sure this cannot be installed with scons ? You should fix it within
scons files then.
Here a preliminary pass for the review.
--
Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email
--- You are receiving this mail because: ---
You are on the CC list for the bug.
___
package-review mailing list
package-review@lists.fedoraproject.org
https://admin.fedoraproject.org/mailman/listinfo/package-review
[Bug 694287] Review Request: openCOLLADA - 3D import and export libraries
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 Richard Shaw changed: What|Removed |Added Blocks||177841(FE-NEEDSPONSOR) -- Configure bugmail: https://bugzilla.redhat.com/userprefs.cgi?tab=email --- You are receiving this mail because: --- You are on the CC list for the bug. ___ package-review mailing list package-review@lists.fedoraproject.org https://admin.fedoraproject.org/mailman/listinfo/package-review
