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.

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

Reduce AsciiDoctor closure size #77149

Merged
merged 5 commits into from Jan 10, 2020
Merged

Reduce AsciiDoctor closure size #77149

merged 5 commits into from Jan 10, 2020

Conversation

@alyssais
Copy link
Member

@alyssais alyssais commented Jan 6, 2020

Motivation for this change

In the documentation format wars, a frequent criticism of AsciiDoctor has been its use closure size. On master, it’s currently 968.1M. With all these changes, it’s 125M. At this point I think we’ve hit the point of diminishing returns — it’s possible to shave off a few more megabytes, but it’s probably not worth it.

Some of these changes are a little risky. I’d like to test with nix-review that nothing that builds before this change fails to build after it. Enough things depend on AsciiDoctor that I think this provides reasonable test coverage. I haven’t done this yet.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Notify maintainers

cc @

@alyssais alyssais requested a review from zimbatm as a code owner Jan 6, 2020
@alyssais
Copy link
Member Author

@alyssais alyssais commented Jan 6, 2020

@GrahamcOfBorg build asciidoctor

@domenkozar
Copy link
Member

@domenkozar domenkozar commented Jan 7, 2020

<3 <3 <3

@zimbatm
Copy link
Member

@zimbatm zimbatm commented Jan 7, 2020

I think the defaultGemConfig changes could be merged straight to master. removing the reference to CC seems a bit risky but worth a try.

@zimbatm
Copy link
Member

@zimbatm zimbatm commented Jan 7, 2020

in fact, probably changing libxml2 is what creates the biggest rebuild

@alyssais
Copy link
Member Author

@alyssais alyssais commented Jan 8, 2020

@alyssais alyssais force-pushed the alyssais:asciidoctor branch from 422ab53 to df649f4 Jan 8, 2020
@Br1ght0ne
Copy link
Contributor

@Br1ght0ne Br1ght0ne commented Jan 8, 2020

wc -l on the latest ofBorg eval says that there are 81097 changed paths in total, and 21076 changed paths on x86_64-linux alone.

@zimbatm
Copy link
Member

@zimbatm zimbatm commented Jan 8, 2020

I suspect there will be packages that relied on the libxml2 Python
bindings being propagated, but don't know how I'd identify them. The
libxml2 change is too big for nix-review to handle. Any ideas?

Can you make this PR without libxml2? Potentially that PR could merged straight into master and then send the libxml2 changes to staging.

Another solution would be to add a withPython parameter to libxml2 and then override the version without the python packages just for ruby and its dependencies.

@alyssais alyssais force-pushed the alyssais:asciidoctor branch from df649f4 to 114ad86 Jan 9, 2020
@alyssais alyssais changed the base branch from staging to master Jan 9, 2020
alyssais added 5 commits Jan 6, 2020
I'm not sure why this was disabled, but it looks like a pretty
harmless way to bring down closure size and remove references to
compilers and stuff.
This makes RbConfig["CC"] return an invalid path, but I hope nothing
is depending on that anyway...
ext/ isn't needed once the extensions have been built, contains
references to a bunch of huge dependencies, and contains megabytes of
tests.
(Except on JRuby, where these are presumably important.)
@alyssais alyssais force-pushed the alyssais:asciidoctor branch from 114ad86 to 1ac11cc Jan 9, 2020
@alyssais
Copy link
Member Author

@alyssais alyssais commented Jan 9, 2020

@ofborg ofborg bot requested review from marsam, manveru and vrthra Jan 9, 2020
@Br1ght0ne
Copy link
Contributor

@Br1ght0ne Br1ght0ne commented Jan 9, 2020

Results of nix-review pr 77149 --package-regex 'rubyPackages_2_7.*':

[255 built (1 failed), 351 copied (605.2 MiB), 195.2 MiB DL]
https://github.com/NixOS/nixpkgs/pull/77149
8 packages failed to build:
rubyPackages_2_7.atk rubyPackages_2_7.cairo-gobject rubyPackages_2_7.gdk_pixbuf2 rubyPackages_2_7.gio2 rubyPackages_2_7.glib2 rubyPackages_2_7.gobject-introspection rubyPackages_2_7.gtk2 rubyPackages_2_7.pango
245 packages built:
rubyPackages_2_7.CFPropertyList rubyPackages_2_7.RubyInline rubyPackages_2_7.ZenTest rubyPackages_2_7.actionmailer rubyPackages_2_7.actionpack rubyPackages_2_7.actionview rubyPackages_2_7.activejob rubyPackages_2_7.activemodel rubyPackages_2_7.activerecord rubyPackages_2_7.activesupport rubyPackages_2_7.addressable rubyPackages_2_7.arel rubyPackages_2_7.ast rubyPackages_2_7.atomos rubyPackages_2_7.awesome_print rubyPackages_2_7.bacon rubyPackages_2_7.builder rubyPackages_2_7.byebug rubyPackages_2_7.cairo rubyPackages_2_7.camping rubyPackages_2_7.charlock_holmes rubyPackages_2_7.claide rubyPackages_2_7.clamp rubyPackages_2_7.cld3 rubyPackages_2_7.cocoapods rubyPackages_2_7.cocoapods-acknowledgements rubyPackages_2_7.cocoapods-art rubyPackages_2_7.cocoapods-bin rubyPackages_2_7.cocoapods-browser rubyPackages_2_7.cocoapods-bugsnag rubyPackages_2_7.cocoapods-check rubyPackages_2_7.cocoapods-clean rubyPackages_2_7.cocoapods-clean_build_phases_scripts rubyPackages_2_7.cocoapods-core rubyPackages_2_7.cocoapods-coverage rubyPackages_2_7.cocoapods-deintegrate rubyPackages_2_7.cocoapods-dependencies rubyPackages_2_7.cocoapods-deploy rubyPackages_2_7.cocoapods-disable-podfile-validations rubyPackages_2_7.cocoapods-downloader rubyPackages_2_7.cocoapods-expert-difficulty rubyPackages_2_7.cocoapods-fix-react-native rubyPackages_2_7.cocoapods-generate rubyPackages_2_7.cocoapods-git_url_rewriter rubyPackages_2_7.cocoapods-keys rubyPackages_2_7.cocoapods-no-dev-schemes rubyPackages_2_7.cocoapods-open rubyPackages_2_7.cocoapods-packager rubyPackages_2_7.cocoapods-packager-pro rubyPackages_2_7.cocoapods-playgrounds rubyPackages_2_7.cocoapods-plugins rubyPackages_2_7.cocoapods-prune-localizations rubyPackages_2_7.cocoapods-rome rubyPackages_2_7.cocoapods-search rubyPackages_2_7.cocoapods-sorted-search rubyPackages_2_7.cocoapods-static-swift-framework rubyPackages_2_7.cocoapods-stats rubyPackages_2_7.cocoapods-tdfire-binary rubyPackages_2_7.cocoapods-testing rubyPackages_2_7.cocoapods-trunk rubyPackages_2_7.cocoapods-try rubyPackages_2_7.cocoapods-try-release-fix rubyPackages_2_7.cocoapods-update-if-you-dare rubyPackages_2_7.cocoapods-whitelist rubyPackages_2_7.cocoapods-wholemodule rubyPackages_2_7.coderay rubyPackages_2_7.colorator rubyPackages_2_7.colored2 rubyPackages_2_7.concurrent-ruby rubyPackages_2_7.crass rubyPackages_2_7.curb rubyPackages_2_7.curses rubyPackages_2_7.daemons rubyPackages_2_7.data_objects rubyPackages_2_7.dep-selector-libgecode rubyPackages_2_7.diff-lcs rubyPackages_2_7.digest-sha3 rubyPackages_2_7.do_sqlite3 rubyPackages_2_7.docile rubyPackages_2_7.domain_name rubyPackages_2_7.dotenv rubyPackages_2_7.em-websocket rubyPackages_2_7.erubis rubyPackages_2_7.escape rubyPackages_2_7.ethon rubyPackages_2_7.eventmachine rubyPackages_2_7.excon rubyPackages_2_7.faraday rubyPackages_2_7.ffi rubyPackages_2_7.ffi-compiler rubyPackages_2_7.ffi-rzmq-core rubyPackages_2_7.fog-core rubyPackages_2_7.fog-dnsimple rubyPackages_2_7.fog-json rubyPackages_2_7.formatador rubyPackages_2_7.forwardable-extended rubyPackages_2_7.fourflusher rubyPackages_2_7.fuzzy_match rubyPackages_2_7.gh_inspector rubyPackages_2_7.gitlab-markup rubyPackages_2_7.globalid rubyPackages_2_7.gpgme rubyPackages_2_7.hashie rubyPackages_2_7.highline rubyPackages_2_7.hike rubyPackages_2_7.hitimes rubyPackages_2_7.hpricot rubyPackages_2_7.http-accept rubyPackages_2_7.http-cookie rubyPackages_2_7.httpclient rubyPackages_2_7.i18n rubyPackages_2_7.iconv rubyPackages_2_7.idn-ruby rubyPackages_2_7.jaro_winkler rubyPackages_2_7.jbuilder rubyPackages_2_7.jekyll rubyPackages_2_7.jekyll-sass-converter rubyPackages_2_7.jekyll-watch rubyPackages_2_7.jmespath rubyPackages_2_7.json rubyPackages_2_7.jwt rubyPackages_2_7.kramdown rubyPackages_2_7.kramdown-parser-gfm rubyPackages_2_7.libv8 rubyPackages_2_7.libxml-ruby rubyPackages_2_7.liquid rubyPackages_2_7.listen rubyPackages_2_7.loofah rubyPackages_2_7.mab rubyPackages_2_7.magic rubyPackages_2_7.mail rubyPackages_2_7.markaby rubyPackages_2_7.mercenary rubyPackages_2_7.method_source rubyPackages_2_7.mime-types rubyPackages_2_7.mime-types-data rubyPackages_2_7.mini_magick rubyPackages_2_7.mini_mime rubyPackages_2_7.mini_portile2 rubyPackages_2_7.minitest rubyPackages_2_7.molinillo rubyPackages_2_7.msgpack rubyPackages_2_7.multi_json rubyPackages_2_7.multipart-post rubyPackages_2_7.mysql2 rubyPackages_2_7.nanaimo rubyPackages_2_7.nap rubyPackages_2_7.native-package-installer rubyPackages_2_7.ncursesw rubyPackages_2_7.net-scp rubyPackages_2_7.net-ssh rubyPackages_2_7.netrc rubyPackages_2_7.nio4r rubyPackages_2_7.nokogiri rubyPackages_2_7.opus-ruby rubyPackages_2_7.osx_keychain rubyPackages_2_7.ovirt-engine-sdk rubyPackages_2_7.parallel rubyPackages_2_7.parser rubyPackages_2_7.pathutil rubyPackages_2_7.patron rubyPackages_2_7.pcaprub rubyPackages_2_7.pg rubyPackages_2_7.pkg-config rubyPackages_2_7.polyglot rubyPackages_2_7.pry rubyPackages_2_7.pry-byebug rubyPackages_2_7.pry-doc rubyPackages_2_7.public_suffix rubyPackages_2_7.puma rubyPackages_2_7.rack rubyPackages_2_7.rack-protection rubyPackages_2_7.rack-test rubyPackages_2_7.rails rubyPackages_2_7.rails-deprecated_sanitizer rubyPackages_2_7.rails-dom-testing rubyPackages_2_7.rails-html-sanitizer rubyPackages_2_7.railties rubyPackages_2_7.rainbow rubyPackages_2_7.rake rubyPackages_2_7.rb-fsevent rubyPackages_2_7.rb-inotify rubyPackages_2_7.rb-readline rubyPackages_2_7.rbnacl rubyPackages_2_7.re2 rubyPackages_2_7.redcarpet rubyPackages_2_7.redis rubyPackages_2_7.redis-rack rubyPackages_2_7.redis-store rubyPackages_2_7.ref rubyPackages_2_7.rest-client rubyPackages_2_7.rmagick rubyPackages_2_7.rouge rubyPackages_2_7.rpam2 rubyPackages_2_7.rspec rubyPackages_2_7.rspec-core rubyPackages_2_7.rspec-expectations rubyPackages_2_7.rspec-mocks rubyPackages_2_7.rspec-support rubyPackages_2_7.rubocop rubyPackages_2_7.rubocop-performance rubyPackages_2_7.ruby-graphviz rubyPackages_2_7.ruby-libvirt rubyPackages_2_7.ruby-lxc rubyPackages_2_7.ruby-macho rubyPackages_2_7.ruby-progressbar rubyPackages_2_7.ruby-terminfo rubyPackages_2_7.ruby-vips rubyPackages_2_7.ruby_dep rubyPackages_2_7.rubyzip rubyPackages_2_7.rugged rubyPackages_2_7.safe_yaml rubyPackages_2_7.sassc rubyPackages_2_7.scrypt rubyPackages_2_7.semian rubyPackages_2_7.sequel rubyPackages_2_7.sequel_pg rubyPackages_2_7.simplecov rubyPackages_2_7.simplecov-html rubyPackages_2_7.sinatra rubyPackages_2_7.slather rubyPackages_2_7.slop rubyPackages_2_7.snappy rubyPackages_2_7.sprockets rubyPackages_2_7.sprockets-rails rubyPackages_2_7.sqlite3 rubyPackages_2_7.taglib-ruby rubyPackages_2_7.terminal-table rubyPackages_2_7.thor rubyPackages_2_7.thread_safe rubyPackages_2_7.thrift rubyPackages_2_7.tilt rubyPackages_2_7.tiny_tds rubyPackages_2_7.treetop rubyPackages_2_7.typhoeus rubyPackages_2_7.tzinfo rubyPackages_2_7.unf rubyPackages_2_7.unf_ext rubyPackages_2_7.unicode-display_width rubyPackages_2_7.uuid4r rubyPackages_2_7.whois rubyPackages_2_7.xcodeproj rubyPackages_2_7.xctasks rubyPackages_2_7.yard rubyPackages_2_7.zookeeper

Seems that rubyPackages_2_7.glib2 fails to build:

build failure
builder for '/nix/store/myijs4b6fjvdg6bz86ndwmvvywzmm5if-ruby2.7.0-glib2-3.3.7.drv' failed with exit code 1; last 10 log lines:
        |                              ^~~~~~
  cc1: warning: unrecognized command line option '-Wno-self-assign'
  cc1: warning: unrecognized command line option '-Wno-parentheses-equality'
  cc1: warning: unrecognized command line option '-Wno-constant-logical-operand'
  make: *** [Makefile:244: glib-enum-types.o] Error 1

  make failed, exit code 2

  Gem files will remain installed in /nix/store/dv588hh3pijbhmda5b48rr4v83df08ab-ruby2.7.0-glib2-3.3.7/lib/ruby/gems/2.7.0/gems/glib2-3.3.7 for inspection.
  Results logged to /nix/store/dv588hh3pijbhmda5b48rr4v83df08ab-ruby2.7.0-glib2-3.3.7/lib/ruby/gems/2.7.0/extensions/x86_64-linux/2.7.0/glib2-3.3.7/gem_make.out

gem_make.out


Didn't try to build other packages, as it would take a long time.

@zimbatm
Copy link
Member

@zimbatm zimbatm commented Jan 10, 2020

rubyPackages_2_7.gtk2 seems to be broken on master as well. I remember seeing another PR that was fixing this.

@zimbatm zimbatm merged commit ddc83e6 into NixOS:master Jan 10, 2020
16 checks passed
16 checks passed
Evaluation Performance Report Evaluator Performance Report
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details
ruby on aarch64-linux Success
Details
ruby on x86_64-darwin Success
Details
ruby on x86_64-linux Success
Details
@alyssais alyssais deleted the alyssais:asciidoctor branch Jan 10, 2020
@alyssais alyssais mentioned this pull request Jan 10, 2020
4 of 10 tasks complete
@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Feb 5, 2020

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/january-2020-in-nixos/5771/1

@nomeata
Copy link
Contributor

@nomeata nomeata commented Mar 4, 2020

@zimbatm, It seems this broke stuff, I now get

Error: Could not find or load main class org.asciidoctor.diagram.CommandServer
asciidoctor: ERROR: index.adoc: line 70: Failed to generate image: end of file reached

when I use -r asciidoctor-diagram and have plantuml diagrams, and the error message seems to relate to the removal of the JARs.

Can I get the JARs from somewhere else now?

@alyssais
Copy link
Member Author

@alyssais alyssais commented Mar 4, 2020

@nomeata
Copy link
Contributor

@nomeata nomeata commented Mar 4, 2020

I don't think so, just calling asciidoctor in the buildPhase of a derivation, and it broke compared to the 19.09 release branch. Let me try to reproduce in isolation.

@alyssais
Copy link
Member Author

@alyssais alyssais commented Mar 4, 2020

@nomeata
Copy link
Contributor

@nomeata nomeata commented Mar 4, 2020

This is how you can test it

nix-build -E '(let pkgs = import <nixpkgs> {}; in pkgs.runCommandNoCC "foo" { nativeBuildInputs = [ pkgs.asciidoctor pkgs.jre ]; } '"''"'asciidoctor -r asciidoctor-diagram --failure-level WARN -v <(echo "[plantuml]"; echo ....; echo "a --> b"; echo ....; ) -o $out'"''"' )'
zimbatm added a commit that referenced this pull request Mar 4, 2020
This reverts commit 1ac11cc.

asciidoctor-diagram starts Java processes, so the JARs are necessary
on all platforms.

See #77149 (comment).
zimbatm added a commit that referenced this pull request Mar 4, 2020
This reverts commit 1ac11cc.

asciidoctor-diagram starts Java processes, so the JARs are necessary
on all platforms.

See #77149 (comment).

(cherry picked from commit 89a0945)
skykanin added a commit to skykanin/nixpkgs that referenced this pull request Mar 5, 2020
This reverts commit 1ac11cc.

asciidoctor-diagram starts Java processes, so the JARs are necessary
on all platforms.

See NixOS#77149 (comment).
dasj19 added a commit to dasj19/nixpkgs that referenced this pull request Mar 12, 2020
This reverts commit 1ac11cc.

asciidoctor-diagram starts Java processes, so the JARs are necessary
on all platforms.

See NixOS#77149 (comment).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

7 participants
You can’t perform that action at this time.