Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

subversion: make python bindings build on 10.9. #26156

Closed
wants to merge 4 commits into from

Conversation

andrewdotn
Copy link
Contributor

Need to filter out CPPFLAGS that confuse swig, see #23993.

Subversion’s build system is complicated. build.conf is processed by a python module called by another python script which generates an auto-included Makefile that passes $(CPPFLAGS) to swig, but swig doesn’t know what to do with -F/usr/local/Frameworks or -isystem/usr/local.... This adds some GNU make magic to strip out those extra arguments from the $(CPPFLAGS) passed to swig.

Need to filter out CPPFLAGS that confuse swig, see Homebrew#23993.
# Prevent '-arch ppc' from being pulled in from Perl's $Config{ccflags}
# 1. Prevent '-arch ppc' from being pulled in from Perl's $Config{ccflags}
# 2. Fix python bindings compile on 10.9
# http://subversion.tigris.org/issues/show_bug.cgi?id=4465
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks from the thread that this might not be the right approach.

Based on feedback from the subversion issue tracker, will backport r1535610
instead.

This reverts commit 06031e6.
Need to filter out CPPFLAGS that confuse swig, see Homebrew#23993.
@andrewdotn
Copy link
Contributor Author

Subversion views this as a homebrew problem because you can successfully build subversion from source.

After digging into this more closely, it turns out that there are two related problems that are currently causing all subversion swig bindings to fail to build—try --ruby or --perl and they’ll fail too.

First, Homebrew adds -isystem to CPPFLAGS which swig doesn’t recognize. There used to be code in this formula that used inreplace to strip those arguments from all Makefiles. But that code was deleted in #23829 because it printed spurious warnings, and that code was actually wrong because it stripped -isystem from CPPFLAGS everywhere, not just in the CPPFLAGS passed to swig.

Secondly, if you have python or python3 installed with Homebrew and try to build any subversion swig bindings, including Ruby or Perl ones, a -F option is added to CPPFLAGS to pick up Homebrew’s Python framework. swig doesn’t understand this either and the build fails.

This pull request fixes both of these issues.

On subversion trunk there is now a separate SWIG_CPPFLAGS Makefile variable. Pulling that patch into 1.8.x allows correctly filtering out the -F and -isystem flags from going to swig. We can drop patch #2 when subversion 1.9 is released in 2014-Q2.

What are your thoughts on enabling bindings by default so that the continuous integration bot will automatically catch problems building them?

@MikeMcQuaid
Copy link
Member

I'd rather pull that upstream SWIG_CPPFLAGS patch.

@andrewdotn
Copy link
Contributor Author

That’s exactly what this does—patch 2 is the output of svn log -v -r1535610 --diff http://svn.apache.org/repos/asf/subversion/trunk which is the upstream patch to trunk.

@adamv
Copy link
Contributor

adamv commented Feb 26, 2014

Can this be rebased on master and made to work with 1.8.8?

@adamv adamv added the python label Feb 26, 2014
@andrewdotn
Copy link
Contributor Author

This was actually pulled in 648a5f2. When building 1.8.8 with homebrew now, the part of the patch that’s still in master applies cleanly, and the unit tests for the Python bindings all pass. So I believe this pull request can be closed. Thanks!

==> Patching
patching file subversion/bindings/swig/perl/native/Makefile.PL.in
Hunk #1 succeeded at 76 (offset 7 lines).
patching file configure.ac
==> Downloading http://serf.googlecode.com/svn/src_releases/serf-1.3.4.tar.bz2

@andrewdotn andrewdotn closed this Feb 26, 2014
@Homebrew Homebrew locked and limited conversation to collaborators Feb 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants