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.

Sign up for GitHub

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

Jump to bottom

Native gems: remove the C source code from the precompiled gem #2077

Closed
flavorjones opened this issue Sep 8, 2020 · 2 comments
Closed

Native gems: remove the C source code from the precompiled gem #2077

flavorjones opened this issue Sep 8, 2020 · 2 comments
Labels
packaging/native-gem

Comments

@flavorjones
Copy link
Member

Because the gem is precompiled, there's no need to also ship the C source and header files.

Package the gem without the C code.

Related to #2075.
@flavorjones flavorjones added this to the v1.11.0 milestone Sep 8, 2020
@flavorjones flavorjones mentioned this issue Sep 8, 2020
Closed
@flavorjones
Copy link
Member Author

I'll note here that we'll need to be careful with header files to allow Nokogumbo to compile properly on installation, see #1788 for context.

@flavorjones flavorjones added help wanted release-blocker Blocking a milestone release labels Dec 1, 2020
flavorjones added a commit that referenced this issue Dec 2, 2020
@flavorjones
fix: native gems are built with ExtensionTask.no_native=true
9e38a0d 
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 (#2089)
  - 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 2, 2020
@flavorjones
fix: native gems are built with ExtensionTask.no_native=true
0336d36 
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 flavorjones mentioned this issue Dec 2, 2020
Merged
flavorjones added a commit that referenced this issue Dec 3, 2020
@flavorjones
fix: native gems are built with ExtensionTask.no_native=true
c50a658 
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
@flavorjones
fix: native gems are built with ExtensionTask.no_native=true
17c84fc 
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
@flavorjones
Merge pull request #2125 from sparklemotion/2078-fix-rake-compiler-na…
b54c3fb 
…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.
flavorjones added a commit that referenced this issue Dec 22, 2020
@flavorjones
feat: remove C source code from the precompile gem
30676cc 
to reduce the size of the gem file. This probably will annoy people
who use local rdocs (because we also have to remove the C files from
the set of extra_rdoc_files), but let's wait and see whether people
complain.

Closes #2077.
@flavorjones
Copy link
Member Author

Hmm. After some thought, it's helpful to have the C extension files present as extra_rdoc_files, and they compress down to about 43KB which isn't much compared to the size of the 3.4MB gemfile. I'm going to close this and not actually do it, to preserve the local rdoc experience and avoid surprise.

@flavorjones flavorjones removed help wanted release-blocker Blocking a milestone release labels Dec 23, 2020
@flavorjones flavorjones removed this from the v1.11.0 milestone Dec 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
packaging/native-gem
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@flavorjones