Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!

Bug 244144

Summary: mail-filter/p3scan: CC variable not respected
Product: Gentoo Linux Reporter: Diego Elio Pettenò (RETIRED) <flameeyes>
Component: New packagesAssignee: Net-Mail Packages <net-mail>
Status: RESOLVED FIXED    
Severity: normal CC: arttuv69, hwoarang
Priority: High    
Version: unspecified   
Hardware: All   
OS: Linux   
Whiteboard:
Package list:
Runtime testing required: ---
Bug Depends on:    
Bug Blocks: 243502    
Attachments: tries to change the p3scan-2.3.2 ebuild to respect CC and CFLAGS
Replacement patch to p3scan-2.3.2 ebuild to address LDFLAGS issue, in addition to changes from comment #1

Description Diego Elio Pettenò (RETIRED) gentoo-dev 2008-10-24 19:49:41 UTC
I'm filing this bug (from a template, mind you) because the ebuild I'm reporting it against failed to build once I removed my /usr/bin/{gcc,cc,c++,c99} binaries. This means that the ebuild is relying on gcc or cc as compiler, while it should use "$(tc-getCC)" so that user choices are respected, and cross-compiling works as intended.

This usually comes down to one of these tasks:

- use emake CC="$(tc-getCC)" for building, to ovewrite make's CC variable (defaults to "cc", some upstream rewrites it);
- the above plus replacing explicit "gcc" (or similar) calls with $(CC) so that the variable is actually respected;
- tc-export CC in src_compile before eventual econf.

For C++, you'd have to replace CC with CXX everywhere above, of course.

If your package is a special case on this, please let me know.

Thanks,
Diego
Comment 1 Arttu Valo 2010-01-03 16:52:25 UTC
Created attachment 215041 [details, diff]
tries to change the p3scan-2.3.2 ebuild to respect CC and CFLAGS

Suggested changes to p3scan-2.3.2.ebuild. Tries to fix this bug (respecting CC), bug #240786 (respecting CFLAGS), and also silence the jobserver availability warnings which don't have a bug open yet, methinks.

Unfortunately one of the root causes is that p3scan seems to ship and build a copy of ripmime (1.4.0.6, current stable in portage tree as net-mail/ripmime) as a library. The ripmime in portage however is only built as a program, not as a library. So fully removing the bundled library from p3scan would require work on net-mail/ripmime as well.
Comment 2 Kevin Pyle 2010-01-04 05:12:22 UTC
A few comments on the proposed fix:

For the CC= line, you could instead delete the line via -e '/^CC=/d', rather than commenting it out.  Either way works.
For the CFLAGS line, changing it to += results in user CFLAGS preceding the Makefile CFLAGS, so some user CFLAGS may be overridden.  Based on inspecting the Makefile, this is probably a non-issue for this package, since the only flags that survive the sed are preprocessor defines specific to this package.  I point it out only as a potential pitfall that could arise in later versions or other packages.
You listed ripmime/Makefile twice in the sed that fixes it.
Your sed of ripmime/Makefile uses an unescaped dot, which is a regular expression single character wildcard.  Use of a slash after it prevents matching lines you did not want, but it would be better to escape the dot.
You set CC= and CFLAGS= in the src_compile emake:
- You use tc-getCC but do not inherit toolchain-funcs.  This happens to work because eutils inherits multilib, and multilib inherits toolchain-funcs.  Although I doubt the Gentoo developers will change this cascaded inheritance, it would be better to get in the habit of using an explicit inherit for any eclasses you need.
- Setting a variable on the make command line causes it to ignore += directives, so setting CFLAGS= on the command line suppresses the preprocessor directives from $(LOGSET).  Without inspecting the package, I do not know the consequences of this, but it was probably not what you wanted.

Finally, it is worth noting that there is yet another QA violation in the upstream package: use of LDFLAGS to pass libraries.  Do not feel bad about missing this one, since it mostly works.  However, if you are feeling ambitious, it would be worth fixing, so that libraries are passed at the end of the rule.
Comment 3 Markos Chandras (RETIRED) gentoo-dev 2010-07-10 22:05:01 UTC
Do you have a new proposed patch for this one in order to respect LDFLAGS as well?
Comment 4 Kevin Pyle 2010-07-10 23:20:36 UTC
(In reply to comment #3)
> Do you have a new proposed patch for this one in order to respect LDFLAGS as
> well?

In light of the time lag between when I posted my remarks on attachment #215041 [details, diff] and now, I do not even have the relevant package installed.  My comments were intended to guide Arttu Valo (and anyone else who happened across the patch) so that future work would be of even higher quality than the patch in attachment #215041 [details, diff], which is already sufficient to address both the reported issue and the others that he cited.  In particular, since LDFLAGS mistakes are so pervasive in upstream packages, I wanted to advise Arttu about the potential problems so that he could be on the lookout for them in any other packages he might work on.

Markos, if you need help, I can reinstall the package and attempt to draft an amended patch to address the LDFLAGS issue as well.  It is probably as simple as just renaming the assignment in the Makefile.
Comment 5 Arttu Valo 2010-07-11 11:47:22 UTC
I was working on a patch set ~6 months ago. I was going to rip out the bundled ripmime entirely. This also needed work on net-mail/ripmime as currently it doesn't install libs.

Unfortunately, I must've gotten occupied with something else, and currently I don't believe I can do much for at least a few weeks (absolute minimum, no promises). Feel free to proceed with more immediate solutions.

From my 6-months-old home vcs comments I deduce that ripping out the bundled ripmime is not just a security and QA improvement. Having it there causes unnecessary grief wrt CC, CFLAGS and LDFLAGS as well, and that's why I was going to remove it.
Comment 6 Markos Chandras (RETIRED) gentoo-dev 2010-07-11 11:56:40 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Do you have a new proposed patch for this one in order to respect LDFLAGS as
> > well?
> 
> In light of the time lag between when I posted my remarks on attachment #215041 [details, diff]
> and now, I do not even have the relevant package installed.  My comments were
> intended to guide Arttu Valo (and anyone else who happened across the patch) so
> that future work would be of even higher quality than the patch in attachment
> #215041 [details], which is already sufficient to address both the reported issue and the
> others that he cited.  In particular, since LDFLAGS mistakes are so pervasive
> in upstream packages, I wanted to advise Arttu about the potential problems so
> that he could be on the lookout for them in any other packages he might work
> on.
> 
> Markos, if you need help, I can reinstall the package and attempt to draft an
> amended patch to address the LDFLAGS issue as well.  It is probably as simple
> as just renaming the assignment in the Makefile.
> 

I 'd appreciate it if you do that. Having a working patch will help me commit the fix at no time.
I do not maintain this package but I want to fix that on behalf of the QA team. If you don't have time to do that, I will try to find sometime withing the next days to fix it.
Comment 7 Kevin Pyle 2010-07-11 23:15:02 UTC
Created attachment 238365 [details, diff]
Replacement patch to p3scan-2.3.2 ebuild to address LDFLAGS issue, in addition to changes from comment #1

I took the changes from comment #1, applied them to p3scan-2.3.2.ebuild, and did some additional work to create the ebuild described by this patch.  The diff is against the 2.3.2 ebuild in Portage, so it should be possible to use this patch without first applying attachment #215041 [details, diff].

Per comment #5, I decided to remove the bundled ripmime as well.  p3scan 2.3.2 is still marked ~arch on the architectures where it runs at all, so I took advantage of the ~arch ripmime-1.4.0.9 prepared by dertobi123@g.o which _does_ install a (static) library, unlike the ripmime-1.4.0.6 that Arttu was using.  This made it fairly easy to unbundle ripmime.  It would be nice to link to a shared library, but ripmime does not install one yet.  I specified linkage as -lripmime, so ld should switch to a shared library if someone modifies ripmime to provide one.

For the changes to this ebuild:
- I applied the CC/CFLAGS changes I described in comment #2.
- I moved all the libraries from LDFLAGS to a target-specific LDLIBS.  The -L. was dropped, since it was only needed due to the bundled ripmime, which is now gone.
- I deleted the Makefile rules for libripmime.a, for .c.o, and p3scan.  C source files are now built using the Make built-in rule, which respects $CPPFLAGS, unlike the p3scan rule.  libripmime is not built at all.  The built-in linking rule used for p3scan places LDFLAGS at the left edge of the line, so that position sensitive options like --as-needed are effective.
- I explicitly deleted the bundled ripmime, to ensure that it is not used.
- I preserved the existing CC= from Arttu's patch, but dropped the CFLAGS= per the concern I raised in comment #2.

The modified ebuild is able to build, link, and install p3scan.  I lack the facilities to test whether it works as expected.
Comment 8 Markos Chandras (RETIRED) gentoo-dev 2010-07-12 09:05:38 UTC
+*p3scan-2.3.2-r1 (12 Jul 2010)
+
+  12 Jul 2010; Markos Chandras <hwoarang@gentoo.org>
+  +p3scan-2.3.2-r1.ebuild:
+  Rebump which fixes CC/C{XX}FLAGS/LDFLAGS. Unbundle ripmime library.
+  thanks to Kevin Pyle for all the work and the patches he provided
+  Fixes bug #244144
+