Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix CXXFLAGS and only set the C++ version in CXXFLAGS. #118

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

zetafunction
Copy link

CPPFLAGS are used for building both C and C++ programs, and clang
complains when -std=c++11 is used when the driver is invoked in C mode.
Using CXXFLAGS to set this allows rtorrent-ps to be successfully built
with clang.

Unfortunately, fixing CXXFLAGS requires a new hack to ensure -O2 is
uniformly used. Otherwise, autoconf will break when detecting XMLRPC-C
support with an error like the one in #76.

CPPFLAGS are used for building both C and C++ programs, and clang
complains when -std=c++11 is used when the driver is invoked in C mode.
Using CXXFLAGS to set this allows rtorrent-ps to be successfully built
with clang.

Unfortunately, fixing CXXFLAGS requires a new hack to ensure -O2 is
uniformly used. Otherwise, autoconf will break when detecting XMLRPC-C
support with an error like the one in pyroscope#76.
@zetafunction
Copy link
Author

Would you be OK with this patch to clean up build.sh a little? Touching build config is always a bit risky, since it could break on a platform that didn't get tested: I did test that all builds successfully with the default build command with no additional customizations, and that it also builds with CC/CXX set to clang/clang++.

I understand that clang is not a supported compiler. However, following the normal conventions for CPPFLAGS and not specifying C++-only flags in CPPFLAGS seems better for compatibility, and should be better in the long-run.

Some investigation notes:

  • I noticed that -O2 was being implicitly passed when CXXFLAGS was unset. I'm not familiar enough with autoconf to understand where the default value of -g -O2 is coming from.
  • If -O2 is not passed in the compile command for the XMLRPC-C test program, the build fails due to unresolved references like in Build error on arm32v7/debian:stretch #76
  • The simplest "fix" is to just make sure CFLAGS/CXXFLAGS always includes -O2 for now. I picked -g -O2 for default CFLAGS/CXXFLAGS values, since that's what autoconf was already selecting.
  • However, it's a hack, since the right fix is to not link libtorrent into the autoconf test program. But I haven't figured out where that's coming from yet. I'll poke at this more and see if I can figure out where this is coming from.

@chros73
Copy link
Contributor

chros73 commented Jan 13, 2019

I noticed that -O2 was being implicitly passed when CXXFLAGS was unset. I'm not familiar enough with autoconf to understand where the default value of -g -O2 is coming from.

In case of 0.9.6/0.13.6 the -O2 flag comes from rtorrent/libtorrent. This behavior has been changed since 0.9.7/0.13.7.

Normally, the building environment has to take care of the basic flags if they are not set. If they are then they can override those default flags (at least with certain distributions). Hence the check was added in 0.9.7/0.13.7.

What distribution do you use?

@zetafunction
Copy link
Author

I've been testing on debian stretch, building both 0.9.6/0.13.6 and 0.9.7/0.13.7 (I have a set of local patches to make the latter work).

@chros73
Copy link
Contributor

chros73 commented Jan 13, 2019

and 0.9.7/0.13.7 (I have a set of local patches to make the latter work)

You don't have to do this: rtorrent-ps-ch
The build script is a bit different than pyroscope's.

@zetafunction
Copy link
Author

zetafunction commented Jan 13, 2019

Maybe, but making the minor build.sh tweaks still seems like a step in the right direction. For example, it could have possibly helped with rakshasa/rtorrent#300 (and the associated dupes).

As mentioned, the 'proper' fix is to still make sure the XMLRPC-C test program doesn't build with -ltorrent and only add -ltorrent into the linker flags for rtorrent itself. That's something I'm investigating still though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants