Dep refactor #2360
Dep refactor #2360
Conversation
|
This looks like a valuable improvement -- only testing in lots of different environments will tell whether it's introducing new obnoxious bugs ;) One immediate problem: One concern: There seems to be a lot of code duplication between the PkgConfig and FreetypeConfig classes. The intent of the PkgConfig class was to be useful for pkg-config and "pkg-config-based" tools, like "freetype-config" -- I see now that had some bugs, but I wonder if we couldn't fix that by inheriting from PkgConfig and reusing the common functionality, rather than copy-and-pasting it. Certain methods, like Speaking from experience, this sort of configuration stuff is always deceptively fiddly, so thanks for taking the time with it. |
|
Thanks for taking a look at this. Good point on removing code duplication. PkgConfig and FreetypeConfig (probably LibpngConfig eventually), should try to share as much of the same implementation as possible. This should wait for the next version to get as much testing abuse as possible before being released into the wild. I'll rebase off of master after #2331 lands. Is there a decision for when MPL drops python 2.6 support? I like the OrderedDict interface, but it showed up in 2.7. Assuming 2.6 support is sticking around, is there any issue with packaging the OrderedDict recipe? This deserves another another discussion, but I think this PR starts laying the infrastructure to download/build/statically-link libpng/freetype if they are not found on the system. At least on *nix-like platforms. |
|
I think Python 2.6 is still reasonably important -- 24% of our users responded they were using it in our survey. The complication here is that the ordered dict would have to be available during build time, and I'm not crazy about increasing the size of the build code by too much. You could go back to using a list, and just creating local references to the entries that matter that are used multiple times (agg, cxx, freetype etc). And yes, having auto download/build/link of freetype and png would be great -- it's the biggest barrier to entry when building from source. |
|
Is #2331 merged into master? I saw it land in the maintenance branch. |
|
I just merged it now. |
|
|
||
| def setup_extension(self, ext, package, default_include_dirs=[], | ||
| default_library_dirs=[], default_libraries=[], |
There was a problem hiding this comment.
Mutable default arguments are dangerous. Unless I hear otherwise, I'm going to assume this is a lurking bug and clean it up.
There was a problem hiding this comment.
Agree. Luckily this only gets run once, but even so I agree - this was a mistake.
|
@mrterry - would you mind rebasing, and we can get on and merge this. Cheers, |
|
@pelson Unless you want to adopt this, I am going to punt this to 1.4.x |
|
@mrterry I am closing this as it is over a year old and there have been a good number of other changes to setup.py between now and then. I suspect re-basing this branch would be more work than just re-doing it. |
It would be nice to have a more configurable (robust?) way to discover and configure extensions/dependencies. Please regard this as a prototype (it is not py26 compliant, for starters), but close enough to start the discussion.
This does a couple things:
OrderedDictofSetupPackageobjectsSetupPackageobjects explicity declare their dependencies on constructionpkg_configlogic into its own classFTConfigclass to usefreetype_configI've seen bugs where extensions can get inconsistent headers and libs because Freetype.check() and Freetype.add_flags() don't duplicate the needed
freetype_configlogic. This is my attempt to make that class of bugs go away.It may introduce another class of more obnoxious bugs, so please let me know your opinions.