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

Already on GitHub? Sign in to your account

Update Mac build process. Fixes #751 #1322

Merged
merged 5 commits into from Oct 8, 2012

Conversation

Projects
None yet
5 participants
Member

dmcdougall commented Oct 1, 2012

Previously, make.osx installed necessary dependencies. Recent versions
of OS X already include all those dependencies.

The setupext.py basedir list has been updated to include the default
directory to which macports installs libraries.

Update Mac build process. Fixes #751
Previously, make.osx installed necessary dependencies. Recent versions
of OS X already include all those dependencies.

The setupext.py basedir list has been updated to include the default
directory to which macports installs libraries.
Member

dmcdougall commented Oct 2, 2012

Regarding the discussion in #751, I should perhaps add some kind of automation regarding setting the PKG_CONFIG_PATH environment variable.

Make build process set PKG_CONFIG_PATH variable
If the platform has pkg-config then add python's .pc files to the end of
the pkg-config search path. If there was a problem setting it or
pkg-config doesn't exist then do nothing.

It appears that the environment variable change only exists within the
build process.
Member

dmcdougall commented Oct 2, 2012

Ok, I now have setup.py automatically setting the PKG_CONFIG_PATH variable. This works on my Mac, but it would be really helpful if some of the linux people could try this out and let me know if they run into any problems. Specifically, I don't think that changes to PKG_CONFIG_PATH should persist once the build process has completed. Though a change to PKG_CONFIG_PATH does not persist on my machine, I would like to know if this is not the case on linux machines. To check, do the following:

$ echo $PKG_CONFIG_PATH # look at the output of this
/some/libs:/may/get/listed:/here
$ python setup.py build
...
$ echo $PKG_CONFIG_PATH # should be the same
/some/libs:/may/get/listed:/here

@pelson pelson and 2 others commented on an outdated diff Oct 2, 2012

@@ -258,6 +258,20 @@ def get_win32_compiler():
else:
std_libs = ['stdc++', 'm']
+def set_pkgconfig_path():
+ pkgconfig_path = sysconfig.get_config_var('LIBDIR')
+ if pkgconfig_path is None:
+ return
+
+ pkgconfig_path += '/pkgconfig'
@pelson

pelson Oct 2, 2012

Member

os.path.join is the preferred way to do this, but then, I'm not sure that a Windows build can ever get to here...

@WeatherGod

WeatherGod Oct 2, 2012

Member

On Tue, Oct 2, 2012 at 10:49 AM, Phil Elson notifications@github.comwrote:

In setupext.py:

@@ -258,6 +258,20 @@ def get_win32_compiler():
else:
std_libs = ['stdc++', 'm']

+def set_pkgconfig_path():

  • pkgconfig_path = sysconfig.get_config_var('LIBDIR')
  • if pkgconfig_path is None:
  •    return
    
  • pkgconfig_path += '/pkgconfig'

os.path.join is the preferred way to do this, but then, I'm not sure that
a Windows build can ever get to here...

os.path.join() is also good in case the first string is empty

os.path.join('', 'foobar')
'foobar'

@dmcdougall

dmcdougall Oct 2, 2012

Member

Cheers @pelson and @WeatherGod. I was clearly in noob-mode when I wrote that. Thanks for the feedback. Did you both try out a build with this new change? If so, how did it go?

Member

dmcdougall commented Oct 3, 2012

As far as I can tell, this and the related issue (#751) are the only 1.2.x milestones left before the rc3 cut on Monday (according to the mpl calendar). This is a gentle reminder to others that this should at least be tried before the weekend so we can iron out any problems with this approach, should they exist.

I realise we originally discussed this should have been done during rc1 ready for rc2, but it slipped my mind. Apologies.

@pelson pelson commented on the diff Oct 3, 2012

setupext.py
@@ -267,6 +281,10 @@ def has_pkgconfig():
#print 'environ', os.environ['PKG_CONFIG_PATH']
@pelson

pelson Oct 3, 2012

Member

We may decide that it will aid debugging if this is uncommented....

Member

pelson commented Oct 3, 2012

Looks sensible to me, and works just fine on RHEL6 (every platform except "win32" will be affected by this change).
I would like to get as many +1's from others as possible before this goes in if we want this to be in the next RC (minimum of @efiring @WeatherGod and @mdboom ).

+1

Member

pelson commented Oct 3, 2012

There is some associated documentation which we have discussed removing which relates to this. In particular, the https://github.com/dmcdougall/matplotlib/blob/master/README.osx file needs to be updated, removed and/or renamed.

Member

pelson commented Oct 3, 2012

There is some thought needed about the whole test directory (i.e. do we still need/want it), in particular regarding this PR is the _buildbot_mac_sage.sh file which uses the make.osx makefile.

FYI from the source root:

$> grep -ir "make.osx" *
MANIFEST.in:include Makefile  make.osx MANIFEST.in MANIFEST
README.osx:The recommended and supported way to build is to use the make.osx file
README.osx:  make -f make.osx PREFIX=/Users/jdhunter/dev PYVERSION=2.6 \
test/_buildbot_mac_sage.sh:make -f make.osx mpl_install
Owner

mdboom commented Oct 3, 2012

+1.

@pelson: I'm not sure if anyone is using the _buildbot_mac_sage.sh anymore. And with Travis now working (mostly) we have even less incentive to keep it working.

Member

pelson commented Oct 3, 2012

I'm not sure if anyone is using the _buildbot_mac_sage.sh anymore.

Any thoughts on the whole test directory? It seems like pretty obsolete stuff (I have never needed to look in there before).

Member

dmcdougall commented Oct 4, 2012

@pelson I'll look at the test directory today.

Member

dmcdougall commented Oct 4, 2012

Looks like the test stuff is for an old buildbot infrastructure. Some of the scripts required python 2.6. I deleted them because I think it's safe. The tests still, unsurprisingly, passed.

There was a TODO file in test/ and some of the ideas were good, I didn't want to just delete it, so I moved it to source root. We could look into both TODO and TODO_TESTS and figure out what needs to be done. A lot of them have been marked as completed. If so, we could look at removing these in future.

Let me know if I horribly broke something.

Member

dmcdougall commented Oct 7, 2012

Just as a reminder, a decision should be made on this today.

Owner

mdboom commented Oct 8, 2012

As a non-OSX user, I'll stay out of the make.osx question and leave that to others.

As for the buildbot stuff, I think that's fine. We haven't been on buildbot for a while, and Travis seems to be the way forward. We'll always have the buildbot configuration in the repo if we decide to go back to it.

Member

pelson commented Oct 8, 2012

+1.

efiring added a commit that referenced this pull request Oct 8, 2012

@efiring efiring merged commit 58d8310 into matplotlib:v1.2.x Oct 8, 2012

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment