Bug 166252

New version:

http://www.perl.me.uk/downloads/modules/perl-Gnome2-Canvas-1.002-3.src.rpm
http://www.perl.me.uk/downloads/modules/md5sums

# rpmbuild -ba perl-Gnome2-Canvas.spec
..
Unrecognized argument in LIBS ignored: '-pthread'
..

Wow! glib2's gthread-2.0.pc is bugged:
/usr/lib/pkgconfig/gthread-2.0.pc

(In reply to comment #2)
> # rpmbuild -ba perl-Gnome2-Canvas.spec
> ..
> Unrecognized argument in LIBS ignored: '-pthread'
> ..
> 
> Wow! glib2's gthread-2.0.pc is bugged:
> /usr/lib/pkgconfig/gthread-2.0.pc

did you do this build on a rawhide box an x86-64 rawhide box?  I have a slightly
dated rawhide x86 config and perl-Gnome2-Canvas-1.002-3.src.rpm is building on
that system just fine, as well as building on one of my x86 fc4 desktops. 

-jef

okay it builds in mock and outside of mock on a development i386 box
rpmlint perl-Gnome2-Canvas-1.002-3.fc5.i386.rpm
E: perl-Gnome2-Canvas zero-length
/usr/share/doc/perl-Gnome2-Canvas-1.002/canvas_demo/ENTRYPOINT_IS_canvas.pl

 Is this really a zero-length file or a strange build error? If its really zero
length its probably best to exclude it.

W: perl-Gnome2-Canvas devel-file-in-non-devel-package
/usr/lib/perl5/vendor_perl/5.8.6/i386-linux-thread-multi/Gnome2/Canvas/Install/gnomecanvasperl-autogen.h
W: perl-Gnome2-Canvas devel-file-in-non-devel-package
/usr/lib/perl5/vendor_perl/5.8.6/i386-linux-thread-multi/Gnome2/Canvas/Install/gnomecanvasperl-version.h
W: perl-Gnome2-Canvas devel-file-in-non-devel-package
/usr/lib/perl5/vendor_perl/5.8.6/i386-linux-thread-multi/Gnome2/Canvas/Install/gnomecanvasperl.h

Ignorable for perl packages as best that i can determine looking at other perl
packages in extras.


Good:
packagename meets perl naming guidelines
the spec file name matches the packagename
license tag matches included License file
legible spec file
source and upstream source md5sums match 93405a987ba4bbd03c2f91592b88f5cb
buildrequires look sane
no shared libraries in linker path
packages owns all the directories it creates appropriately
permissions look appropriate.. no executable scripts are part of this package.
clean and check correct in specfile


I take it chmod -R u+w $RPM_BUILD_ROOT/* in the specfile is there to work around
a problem with the make test or the clean?

I'm not seeing any major blockers here. I suspect the problems in comment 2 are
some sort of arch specific problem in Core development which I can't see on x86.

I'll hold off on flipping the approval bit for 12 hours or so, to see if any
other blocker comments show up.  The only minor problem i see is the one zero
length file.

-jef

(In reply to comment #3)
> (In reply to comment #2)
> > # rpmbuild -ba perl-Gnome2-Canvas.spec
> > ..
> > Unrecognized argument in LIBS ignored: '-pthread'
> > ..

BTW: The warning originates from ExtUtils::MakeMaker composing its linker flags.
It doesn't reckognize -pthread as library, and filters it out. On FC4/i386,
-pthread is a linker short cut to -lpthread, i.e. -pthread is a valid linker
flag and should not be filtered out.

Whether having -pthread in libs is clever design is a different question.

> did you do this build on a rawhide box an x86-64 rawhide box?
Nope - FC4/i386

> that system just fine, as well as building on one of my x86 fc4 desktops. 

When you check the build logs, you'll notice that -pthread is being used during
compilation:
...
 gcc ... -pthread ...
...

but is being dropped during linkage:
...
gcc  -shared -L/usr/local/lib xs/GnomeCanvas.o xs/GnomeCanvasBpath.o
xs/GnomeCanvasItem.o xs/GnomeCanvasPathDef.o xs/GnomeCanvasRichText.o
xs/GnomeCanvasShape.o xs/GnomeCanvasUtil.o -o
blib/arch/auto/Gnome2/Canvas/Canvas.so  
-lgnomecanvas-2 -lart_lgpl_2 -lpangoft2-1.0 -lgtk-x11-2.0 -lgdk-x11-2.0
-latk-1.0 -lgdk_pixbuf-2.0 -lm -lpangoxft-1.0 -lpangox-1.0 -lpango-1.0
-lgobject-2.0 -lgmodule-2.0 -ldl -lglib-2.0 -lgthread-2.0
...

=> Canvas.so is not explicitly linked against -lpthread.

The question now is, if this has a functional impact on this package or not.

- Blocker: *spec doesn't acknowledge RPM_OPT_FLAGS (try rpmbuild --target=i686).

I'd propose this change:

 %build
-%{__perl} Makefile.PL INSTALLDIRS=vendor OPTIMIZE="$RPM_OPT_FLAGS"
+%{__perl} Makefile.PL INSTALLDIRS=vendor
 make %{?_smp_mflags}

Sorry, reverted diff, of cause this way:

-%{__perl} Makefile.PL INSTALLDIRS=vendor
+%{__perl} Makefile.PL INSTALLDIRS=vendor OPTIMIZE="$RPM_OPT_FLAGS"

(In reply to comment #5)
> When you check the build logs, you'll notice that -pthread is being used 
> during compilation:

grrr... i totally missed that when i looked over the build log.. i must of fat
fingered the grep for pthread when i looked for this earlier. Sorry about that.
I think we are definitely going to have to have an impact assessment before this
leaves review.

One thing to note ldd Canvas.so  shows
libpthread.so.0 => /lib/libpthread.so.0 (0x009d9000)

and the demo canvas.pl script seems to be operating well. Everything in the
demos seems to work and its throwing some rather mundane errors:
Use of uninitialized value in subtraction (-) at canvas-primitives.pl line 386
Use of uninitialized value in subroutine entry at canvas-primitives.pl line 410
Use of uninitialized value in subroutine entry at canvas-primitives.pl line 400
Use of uninitialized value in numeric eq (==) at canvas-curve.pl line 138

What do you think? Is this a good enough assessment as to impact on
functionality? Or do we need to ask the submitter to attempt to get
clarification from the upstream author about the  LIBS ignored: '-pthread' message?

(In reply to comment #6 and #7)
good catch. One question, the spectemplate-perl.spec from fedora-rpmdevtools 
uses make %{?_smp_mflags} OPTIMIZE="$RPM_OPT_FLAGS"  I take it either form is
equivalent? 


-jef

Yes, OPTIMIZE appears to work both in "perl Makefile.PL ..." and "make ...", 
and unless someone finds a case where it doesn't, I'll move it to the 
Makefile.PL invocation in the next rpmdevtools release. 

So where are we?

Do I need to change anything here?

Thanks,

Gavin.

(In reply to comment #10)
> So where are we?
> 
> Do I need to change anything here?


Summary of comment #5-#8

2 blockers:
1) there is build error happening "Unrecognized argument in LIBS ignored:
'-pthread'"  the Canvas.so might not be linking as the upstream authors expected
and this may or may not have an impact on functionality. You might want to ask
the upstream developer about that build time error message.

I have tested the demo program that comes with the page and it appears to work,
but I'm not if this is enough assurance.  I was hoping Ralf would comment on my
demo testing and see if that satified him since he originally found the build
error.  

Though if you can make this build error go away via specfile manipulation that
would be satifactory solution. We either need to fix the build error, get an
explanation as to why this okay to allow, or we feel confident that this problem
isn't going to affect functionality. 

2) 
-%{__perl} Makefile.PL INSTALLDIRS=vendor
+%{__perl} Makefile.PL INSTALLDIRS=vendor OPTIMIZE="$RPM_OPT_FLAGS"

to make --target  work.  For future reference this(or equivalent) is part of the
perl template spec file in fedora-rpmdevtools package.


-jef

(In reply to comment #10)

Okay so I've done some more testing beyond the supplied demo program. Whatever
the build error is that is producing the "Unrecognized argument in LIBS ignored:
'-pthread'" message it doesn't appear to be causing a functional problem on my
system.  Of course I'd like to clean this up.. but it doesn't appear to be
having a functional impact at all from my simplistic functionality tests. I'm
inclined to let this through since there is no functional impact.

Instead of waiting for upstream to look into this, perhaps its most appropriate
to apply a simple patch the the generated Makefile.  You can correct for
whatever mistake  perl Makefile.PL is doing by adding "-pthread" to
the EXTRALIBS and LDLOADLIBS definitions. 

-pthread is a valid argument to both the linker and to gcc so I'm not really
sure why Makefile.PL is throwing the arguments away. 

So if you can regenerated a spec that includes the OPTIMIZE patch and the patch
to add -pthread to the generated Makefile, I will feel comfortable getting this
review finished up.

-jef

As soon as I get a spare moment, I will finish this.

Totally up to my eyes at the mo'

Gavin.

(In reply to comment #12)
> (In reply to comment #10)
> 
> Okay so I've done some more testing beyond the supplied demo program. Whatever
> the build error is that is producing the "Unrecognized argument in LIBS ignored:
> '-pthread'" message it doesn't appear to be causing a functional problem on my
> system.

-pthread, when being passed to the linker, is expands to -lpthread on i386
[It is a gcc-spec macro; c.f. the lib: rule in gcc -dumpspecs]

>  but it doesn't appear to be having a functional impact
As long as -pthread expands only to '-l*' containing libraries on the library
search path, this won't have any functional impact. 

However, you can't rely upon this, because GCC can change this at any time and
because GCC can apply target specific linker flags there.

I.e. Gnone2-Canvas probably is functional on all targets where GCC expands
-pthread to -lpthread, but is likely not to work on other targets and also is
not unlikely to break with future GCC updates.

> -pthread is a valid argument to both the linker and to gcc so I'm not really
> sure why Makefile.PL is throwing the arguments away. 
ExtUtils::MakeMaker only lets -l and -L flags pass through and filters out
anything else.

Added OPTIMIZE="$RPM_OPT_FLAGS" and rebuilt

Not sure where we stand with -pthread

New version:

http://www.perl.me.uk/downloads/modules/perl-Gnome2-Canvas-1.002-4.src.rpm
http://www.perl.me.uk/downloads/modules/perl-Gnome2-Canvas.spec
http://www.perl.me.uk/downloads/modules/md5sums

Okay rpmbuild --target i686 works on my rawhide box  and it builds in mock
against development tree.

The -pthread issue is not a blocker for this package to get into cvs since this
does not seem to be impacting the packaged demos on my 32bit box. But you might
go out of your way and dig up some 64bit pc and ppc testers to see if this
result in operational problems on those other arches to be sure. Ralf does make
a compelling argument about the fragility of the situation long term...but the
problem he brings up is really with the glib2 pkgconfig files using -pthread. It
doesn't make a lot of since to try to correct this in your package if its a more
general problem.  I'll re-open that specific issue for discussion as a glib2 bug
and see where that goes. 

You can choose to do something in this package like manually patching the
makefile as suggested in comment #12 as a work around, but I'm not going to
demand it. If ppc or x86_64  users see functionality problems you might have to
do this patch anyways.


One small thing.. and you can do this after you import it to cvs...exclude the
zero length file:
/usr/share/doc/perl-Gnome2-Canvas-1.002/canvas_demo/ENTRYPOINT_IS_canvas.pl

Approved for import into cvs.

-jef

Noted. I'll do the change after import, but I'll also wait to see where the
glibs bug goes.

Built and available in development since September 2005, please CLOSE as
NEXTRELEASE.

done.