nokogiri should compile with system libs #124

Merged
merged 1 commit into from Aug 5, 2013

Conversation

Projects
None yet
2 participants
@hone
Member

hone commented Aug 5, 2013

As of nokogiri 1.6.0, they default to using the vendored libxml2 libs.
This is problematic for two reasons. First, with the way the heroku
build environment works,
sparklemotion/nokogiri#923. This means it
won't link nokogiri.so properly since it's dependent on hardcoded paths
that don't exist during runtime. Second, compiling libxml2 and friends
is unnecessary since we have them already setup. We should skip this to
speed up deploys.

nokogiri should compile with system libs
As of nokogiri 1.6.0, they default to using the vendored libxml2 libs.
This is problematic for two reasons. First, with the way the heroku
build environment works,
sparklemotion/nokogiri#923. This means it
won't link nokogiri.so properly since it's dependent on hardcoded paths
that don't exist during runtime. Second, compiling libxml2 and friends
is unnecessary since we have them already setup. We should skip this to
speed up deploys.
@hone

This comment has been minimized.

Show comment Hide comment
@hone

hone Aug 5, 2013

Member

tests are passing for me locally. Not sure what's up with Travis.

Member

hone commented Aug 5, 2013

tests are passing for me locally. Not sure what's up with Travis.

hone added a commit that referenced this pull request Aug 5, 2013

Merge pull request #124 from heroku/nokogiri
nokogiri should compile with system libs

@hone hone merged commit b6687d1 into master Aug 5, 2013

1 check failed

default The Travis CI build failed
Details

@hone hone deleted the nokogiri branch Aug 5, 2013

@danp

This comment has been minimized.

Show comment Hide comment
@danp

danp Aug 6, 2013

Member

Nice, does this make the slug smaller too?

Member

danp commented Aug 6, 2013

Nice, does this make the slug smaller too?

@hone

This comment has been minimized.

Show comment Hide comment
@hone

hone Aug 6, 2013

Member

it should b/c it doesn't compile the ext/ stuff, but i haven't done comparison tests.

Member

hone commented Aug 6, 2013

it should b/c it doesn't compile the ext/ stuff, but i haven't done comparison tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment