-
-
Notifications
You must be signed in to change notification settings - Fork 898
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
Native gems: ensure that mini_portile2 is not a dependency #2078
Labels
Milestone
Comments
I've bumped the upstream issue, rake-compiler/rake-compiler#171 |
flavorjones
added a commit
that referenced
this issue
Dec 2, 2020
Specifically, following the advice of @kou at rake-compiler/rake-compiler#171 we set `Rake::ExtensionTask.no_native=true` within the rake-compiler-dock container ("guest") so that: - the ExtensionTask `cross_compiling` block is called - so that we don't package the dependencies' tarballs in /ports - so that we don't have mini_portile2 as a dependency (#2078) - so that we don't have an extra nokogiri.so/nokogiri.bundle built and packaged (#2076) - we no longer have to hotfix rake-compiler at build time This also will enable us to more easily do things like removing the C extension source code from the native gem package (#2077).
flavorjones
added a commit
that referenced
this issue
Dec 3, 2020
Specifically, following the advice of @kou at rake-compiler/rake-compiler#171 we set `Rake::ExtensionTask.no_native=true` within the rake-compiler-dock container ("guest") so that: - the ExtensionTask `cross_compiling` block is called - so that we don't package the dependencies' tarballs in /ports - so that we don't have mini_portile2 as a dependency (#2078) - so that we don't have an extra nokogiri.so/nokogiri.bundle built and packaged (#2076) - we no longer have to hotfix rake-compiler at build time This also will enable us to more easily do things like removing the C extension source code from the native gem package (#2077).
flavorjones
added a commit
that referenced
this issue
Dec 3, 2020
Specifically, following the advice of @kou at rake-compiler/rake-compiler#171 we set `Rake::ExtensionTask.no_native=true` within the rake-compiler-dock container ("guest") so that: - the ExtensionTask `cross_compiling` block is called - so that we don't package the dependencies' tarballs in /ports - so that we don't have mini_portile2 as a dependency (#2078) - so that we don't have an extra nokogiri.so/nokogiri.bundle built and packaged (#2076) - we no longer have to hotfix rake-compiler at build time This also will enable us to more easily do things like removing the C extension source code from the native gem package (#2077).
flavorjones
added a commit
that referenced
this issue
Dec 3, 2020
…tive-things native gems are built with `ExtensionTask.no_native=true` --- **What problem is this PR intended to solve?** Specifically, following the advice of @kou at rake-compiler/rake-compiler#171, we set `Rake::ExtensionTask.no_native=true` within the rake-compiler-dock container ("guest") so that: - the ExtensionTask `cross_compiling` block is called - so that we don't package the dependencies' tarballs in /ports - so that we don't have mini_portile2 as a dependency (#2078) - so that we don't have an extra nokogiri.so/nokogiri.bundle built and packaged (#2076) - we no longer have to hotfix rake-compiler at build time This also will enable us to more easily do things like removing the C extension source code from the native gem package (#2077). This patch set also breaks out `lib/nokogiri/version.rb` into two new files: - `lib/nokogiri/version/constant.rb` - `lib/nokogiri/version/info.rb` and `require_relative`s them both from `version.rb`. This is so that Hoe doesn't pull in `REQUIRED_LIBXML_VERSION` from `extconf.rb` after 652c6fd extracted that value into a constant. This patch set also updates how Darwin native gems are built, to mirror the same rake task structure that's used for Linux and Windows; and it renames to "builder" the rake tasks that used to be "guest". **Have you included adequate test coverage?** I'm satisfied with the level of testing we have now on different platforms, though it could always be better. I will add some testing to the packaged/installed gem when I merge #1788 which introduces that test coverage pretty nicely. **Does this change affect the behavior of either the C or the Java implementations?** Should only impact how the native (precompiled) gems are built and packaged.
#2125 fixed this, but I'd like to add test coverage before closing. |
Merged
flavorjones
added a commit
that referenced
this issue
Dec 23, 2020
…kaging fine tune packaging --- **What problem is this PR intended to solve?** There are a few open issues and pull requests about the packaging and installation of Nokogiri which this PR is trying to address: - [Native gems: eliminate the "extra" dll · Issue #2076 · sparklemotion/nokogiri](#2076) - [Native gems: ensure that mini_portile2 is not a dependency · Issue #2078 · sparklemotion/nokogiri](#2078) - [Nokogumbo should be able to compile against Nokogiri by finding headers in extensions directory by stevecheckoway · Pull Request #1788 · sparklemotion/nokogiri](#1788) More specifically, the code that's been doing the gem packaging is ad-hoc, brittle, and not very discoverable, and has little-to-no test coverage. This PR embarks on cleaning up some of that code, and erecting tests around the files that are packaged and installed, so that I have the confidence to refactor and update this code going forward. I want to give a shoutout to @stevecheckoway whose original PR at #1788 inspired this approach, though unfortunately I didn't use most of that PR because the precompiled native gems, added since that PR, complicated things. **Have you included adequate test coverage?** Ayuh. Notably two new scripts have been added and will be run in CI: - `scripts/test-gem-file-contents` which makes assertions on the contents of the tarball and the packaged gemspec - `scripts/test-gem-installation` which asserts on the filesystem left behind by the gem installation process **Does this change affect the behavior of either the C or the Java implementations?** This affects only installation-time, the functional behavior of Nokogiri is not changed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
When we ship precompiled native gems, we no longer need mini_portile2 as an install-time dependency.
Previously @larskanis and I did some work on this, resulting in:
The workarounds are brittle; this didn't seem to work for the Darwin native gem. Let's figure out a way to do this reliably.
Related to #2075.
The text was updated successfully, but these errors were encountered: