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

Update Mac build process. Fixes #751 #1322

Merged
merged 5 commits into from Oct 8, 2012
Merged

Conversation

dmcdougall
Copy link
Member

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.

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.
@dmcdougall
Copy link
Member Author

Regarding the discussion in #751, I should perhaps add some kind of automation regarding setting the PKG_CONFIG_PATH environment 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.
@dmcdougall
Copy link
Member Author

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

if pkgconfig_path is None:
return

pkgconfig_path += '/pkgconfig'
Copy link
Member

Choose a reason for hiding this comment

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

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...

Copy link
Member

Choose a reason for hiding this comment

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

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'

Copy link
Member Author

Choose a reason for hiding this comment

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

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?

@dmcdougall
Copy link
Member Author

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.

@@ -267,6 +281,10 @@ def has_pkgconfig():
#print 'environ', os.environ['PKG_CONFIG_PATH']
Copy link
Member

Choose a reason for hiding this comment

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

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

@pelson
Copy link
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

@pelson
Copy link
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.

@pelson
Copy link
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

@mdboom
Copy link
Member

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.

@pelson
Copy link
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).

@dmcdougall
Copy link
Member Author

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

@dmcdougall
Copy link
Member Author

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.

@dmcdougall
Copy link
Member Author

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

@mdboom
Copy link
Member

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.

@pelson
Copy link
Member

pelson commented Oct 8, 2012

+1.

efiring added a commit that referenced this pull request Oct 8, 2012
Update Mac build process. Fixes #751
@efiring efiring merged commit 58d8310 into matplotlib:v1.2.x Oct 8, 2012
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

5 participants