native gems are built with ExtensionTask.no_native=true
#2125
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
| warnings | ||
| end | ||
|
|
||
| def to_hash |
There was a problem hiding this comment.
Method to_hash has a Cognitive Complexity of 16 (exceeds 5 allowed). Consider refactoring.
| warnings | ||
| end | ||
|
|
||
| def to_hash |
There was a problem hiding this comment.
Method to_hash has 39 lines of code (exceeds 25 allowed). Consider refactoring.
|
Code Climate has analyzed commit c50a658 and detected 2 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 82.5% (80% is the threshold). This pull request will bring the total coverage in the repository to 94.3% (0.0% change). View more on Code Climate. |
flavorjones
force-pushed
the
2078-fix-rake-compiler-native-things
branch
from
0336d36
to
c50a658
Compare
also: - extract VersionInfo into a separate file - make lib/nokogiri/version.rb require_relative these two new files so that we can easily find it from Rakefile now, and potentially a standalone gem spec later. (This was driven out because Hoe does a questionable thing when inferring the gem version, which is to grep through files in order of Manifest.txt; and this is leading to using the LIBXML_REQUIRED_VERSION as the VERSION. ¯\_(ツ)_/¯)
and add some filtering for common temp files to reduce noise
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
force-pushed
the
2078-fix-rake-compiler-native-things
branch
2 times, most recently
from
c7e8633
to
c91ece6
Compare
That is, darwin tasks now mirror the linux and windows tasks even though there's no guest container (only a child process). This way we avoid having darwin-specific handling from the POV of the person invoking rake. The native platform tasks previously named "guest" are now named "builder" because that's a more descriptive and more accurate name, especially since (on darwin) there's no actual guest.
flavorjones
force-pushed
the
2078-fix-rake-compiler-native-things
branch
from
c91ece6
to
440dc4e
Compare
This was referenced Dec 3, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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=truewithin the rake-compiler-dock container ("guest") so that:cross_compilingblock is calledand packaged (Native gems: eliminate the "extra" dll #2076)
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.rbinto two new files:lib/nokogiri/version/constant.rblib/nokogiri/version/info.rband
require_relatives them both fromversion.rb. This is so that Hoe doesn't pull inREQUIRED_LIBXML_VERSIONfromextconf.rbafter 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.