vala_0_38, vala_0_40, vala_0_42: add configuration to disable graphviz (to reduce closure size) #53000

andrew-d · 2018-12-28T07:23:23Z

Motivation for this change

This allows building Vala without support for Graphviz; useful for more minimal installs where we don't want to pull it (and transitively, pango, gd, etc.) in as a dependency.

As an example with vala 0.38:

# with Graphviz
$ nix path-info -S /nix/store/wy3ajrqhplx7lnh4gnqv2pwd6w7insml-vala-0.38.9
/nix/store/wy3ajrqhplx7lnh4gnqv2pwd6w7insml-vala-0.38.9	  195998064

# without
$ nix path-info -S /nix/store/1nalx8vpq6hzsp40x83wwqqaw8aa3di7-vala-0.38.9   
/nix/store/1nalx8vpq6hzsp40x83wwqqaw8aa3di7-vala-0.38.9	   55116152

Or, from ~187MiB to ~53MiB! TBD on whether we want to enable this by default; I've left it as-is for now, and will defer on whether to enable this in more places, or create valaMinimal, etc. to any reviewers. At the very least, we probably want to use this for libsecret, which is how I originally noticed this; building libsecret from scratch required building all of X11, since there was the following chain of dependencies:

vala ➡️ graphviz ➡️ libXpm ➡️ xproto ➡️ libX11

Things done

r? @antono @jtojnar @lethalman @peterhoeg (maintainers)

jtojnar · 2018-12-28T07:29:21Z

cc @hedning

Also, was there ever an attempt to get this patch upstream?

andrew-d · 2018-12-28T07:32:26Z

@jtojnar - Following the chain of bug links, I found this, which doesn't really solve the problem; they added support for disabling the build of valadoc entirely, but not building without graphviz. It looks like someone (@albfan ?) asked about using valadoc without graphviz, but there has been no reply yet.

jtojnar · 2018-12-28T07:58:51Z

If https://gitlab.gnome.org/GNOME/gitg/merge_requests/16#note_345578 is correct and gitg is the only program using valadoc, we should be able to disable it outright, since we do not build gitg docs.

albfan · 2018-12-28T08:11:45Z

Here is the patch to build vala with valadoc without graphviz avaliable

https://gitlab.gnome.org/GNOME/gitg/commit/0cfd07f8ab8b2c9e210bb537ac15283ce8c746c2

We use it for a while but finally remove it as we want to keep in sync with fixes and improvements on vala, and there's no interest to maintain that upstream.

Makes sense as build a project and build docs for a project can be done in different jobs. Our idea is to use a docker image with a recent distro to build docs.

From a distro perspective not sure what is the correct approach

andrew-d · 2018-12-28T20:03:15Z

I just force-pushed another iteration of this that avoids checking in the patches, and instead fetches the ones used by openembedded-core plus a small additional patch. OpenEmbedded a relatively well-supported project, so I'm pretty confident that we'll continue to have additional patches from them, but if not, the patches are relatively trivial and I'm happy to own forward-porting.

@jtojnar - As for just disabling valadoc, it looks like libhttpseverywhere uses it, so I don't think we can get away with just disabling it right now. I like the approach in this PR because docs will still mostly work - the only thing that changes is that graphs that would have been rendered with Graphviz are instead just not rendered, but everything else functions as intended.

andrew-d · 2018-12-28T20:04:49Z

Oh, and one more question: this PR looks like it should go to staging; does anyone know the right way to do this? Last time that I changed the PR's base branch, I ended up ccing like 15 people as reviewers because this PR suddenly had a bunch of commits in it.

jtojnar · 2018-12-28T20:13:37Z

Rebase onto some old commit that is both on staging and master, then force push, then change base.

…

On Fri, 28 Dec 2018, 21:04 Andrew Dunham ***@***.*** wrote: Oh, and one more question: this PR looks like it should go to staging; does anyone know the right way to do this? Last time that I changed the PR's base branch, I ended up ccing like 15 people as reviewers because this PR suddenly had a bunch of commits in it. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#53000 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AArCY6AKT1bszSvYdj_I3pLYGYucv-ppks5u9nllgaJpZM4ZjjDG> .

andrew-d · 2019-01-06T04:17:18Z

@jtojnar - Any suggestions on how to move this forward? It's been rebased on staging.

hedning · 2019-01-07T15:40:57Z

I'd say we merge this, and if it becomes a pain to maintain down the road we can look at disabling valadoc altogether. Does that sound reasonable?

jtojnar · 2019-01-07T15:41:59Z

Fine with me.

jtojnar · 2019-01-07T23:28:35Z

@GrahamcOfBorg build vala_0_38 vala_0_40 vala_0_42

hedning · 2019-01-08T08:20:13Z

Ah, wait, we should turn this on by default, no? Not much point in doing this without actually using it.

worldofpeace · 2019-01-08T19:20:31Z

Ah, wait, we should turn this on by default, no? Not much point in doing this without actually using it.

I'd very much like to enable this by default 👍

andrew-d · 2019-01-08T19:21:39Z

Okay, I’ll push an update when I’m back at my computer in ~15 minutes!

…

On Tue, Jan 8, 2019 at 11:20 AM worldofpeace ***@***.***> wrote: Ah, wait, we should turn this on by default, no? Not much point in doing this without actually using it. I'd very much like to enable this by default 👍 — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#53000 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABB3hUtOcgs_j87hIByHpQof_MyUn8dGks5vBO-FgaJpZM4ZjjDG> .

andrew-d · 2019-01-09T09:09:58Z

A bit closer to 15 hours than 15 minutes, but... @worldofpeace and @hedning, enabled by default now!

hedning · 2019-01-09T09:17:08Z

👍

@GrahamcOfBorg build vala_0_38 vala_0_40 vala_0_42

hedning · 2019-01-09T10:28:01Z

patch failed to apply for 0.40

Edit: also fails for 0.42

andrew-d · 2019-01-13T23:41:05Z

@hedning - Okay, the failure on 0.42 was legit; upstream just released 0.42.4 and the patch fails to apply; I've updated the PR to fix that. The failure on 0.40 looks like a build timeout on the Darwin builder.

worldofpeace · 2019-01-14T01:03:16Z

@GrahamcOfBorg build vala_0_38 vala_0_40 vala_0_42

andrew-d · 2019-01-14T02:58:32Z

The 0.42.4 build failure appears unrelated? Error shows as:

warning: unable to download 'https://cache.nixos.org/nar/1mrdj0g4q5zq83jz88dapj47xj47nzkvsg4i7sjamh7p9di3x34x.nar.xz': HTTP error 503; retrying in 331 ms
error 7 while decompressing xz file

As for the 0.40.12 build failure, that's legit; upstream made the same changes as 0.42, so I did the same thing and fixed the patch merge conflict and just force-pushed.

All three versions build successfully when I build them locally with the new versions/hashes from master.

worldofpeace · 2019-01-14T03:09:12Z

Patches for 0.40 and 0.42 don't apply for me locally.

For 0.40 I get

applying patch /nix/store/qpwxga9bxbh7935zkxfazrf00ch0b3nj-disable-graphviz-0.40.12.patch
patching file configure.ac
patching file libvaladoc/Makefile.am
Hunk #1 succeeded at 126 (offset -2 lines).
Hunk #2 succeeded at 152 (offset -2 lines).
Hunk #3 FAILED at 191.
Hunk #4 succeeded at 218 (offset -6 lines).
Hunk #5 succeeded at 228 (offset -6 lines).
1 out of 5 hunks FAILED -- saving rejects to file libvaladoc/Makefile.am.rej
patching file libvaladoc/html/basicdoclet.vala
Hunk #3 succeeded at 1031 (offset -1 lines).
Hunk #4 succeeded at 1051 (offset -1 lines).
patching file libvaladoc/html/htmlmarkupwriter.vala
builder for '/nix/store/fzz0gyiy93fpx2ygxljrli824dx2q9gj-vala-0.40.11.drv' failed with exit code 1

and for 0.42

patching sources
applying patch /nix/store/fdpwlhbq7fwphz450rak888hs09zwqn5-disable-graphviz-0.42.4.patch
patching file configure.ac
patching file libvaladoc/Makefile.am
Hunk #1 succeeded at 126 (offset -2 lines).
Hunk #2 succeeded at 152 (offset -2 lines).
Hunk #3 FAILED at 191.
Hunk #4 succeeded at 218 (offset -6 lines).
Hunk #5 succeeded at 228 (offset -6 lines).
1 out of 5 hunks FAILED -- saving rejects to file libvaladoc/Makefile.am.rej
patching file libvaladoc/html/basicdoclet.vala
Hunk #3 succeeded at 1031 (offset -1 lines).
Hunk #4 succeeded at 1051 (offset -1 lines).
patching file libvaladoc/html/htmlmarkupwriter.vala
builder for '/nix/store/cg0zf21bslzqjyg0413byyyf2s8glxvj-vala-0.42.3.drv' failed with exit code 1

This allows building Vala without support for Graphviz; useful for more minimal installs where we don't want to pull it (and transitively, pango, gd, etc.) in as a dependency.

andrew-d · 2019-01-14T04:33:55Z

@worldofpeace - Yeah, that's because this branch wasn't rebased on latest master/staging, which has upgraded Vala to 0.42.4 and 0.40.12; I tested by changing the versions locally because I'm afraid of rebasing 😛 Just rebased on staging with the new versions and force-pushed.

worldofpeace

très bien @andrew-d 👍

hedning · 2019-01-14T15:42:42Z

@GrahamcOfBorg build vala_0_38 vala_0_40 vala_0_42

pkgs/development/compilers/vala/default.nix

andrew-d force-pushed the andrew/vala-no-graphviz branch from cab5d97 to 3320ec8 Compare December 28, 2018 07:24

GrahamcOfBorg added 10.rebuild-darwin: 101-500 10.rebuild-linux: 501+ labels Dec 28, 2018

andrew-d force-pushed the andrew/vala-no-graphviz branch from 3320ec8 to c817216 Compare December 28, 2018 19:59

andrew-d force-pushed the andrew/vala-no-graphviz branch from c817216 to 1d5abe6 Compare December 28, 2018 20:22

andrew-d changed the base branch from master to staging December 28, 2018 20:22

andrew-d force-pushed the andrew/vala-no-graphviz branch from 1d5abe6 to e488ece Compare January 9, 2019 09:09

andrew-d force-pushed the andrew/vala-no-graphviz branch from e488ece to dc43a5d Compare January 13, 2019 23:40

andrew-d force-pushed the andrew/vala-no-graphviz branch from dc43a5d to 7a61d3e Compare January 14, 2019 02:56

vala_0_38, vala_0_40, vala_0_42: add configuration to disable graphviz

c52362c

This allows building Vala without support for Graphviz; useful for more minimal installs where we don't want to pull it (and transitively, pango, gd, etc.) in as a dependency.

andrew-d force-pushed the andrew/vala-no-graphviz branch from 7a61d3e to c52362c Compare January 14, 2019 04:33

worldofpeace approved these changes Jan 14, 2019

View reviewed changes

hedning reviewed Jan 14, 2019

View reviewed changes

pkgs/development/compilers/vala/default.nix Show resolved Hide resolved

worldofpeace merged commit 263bc89 into NixOS:staging Jan 15, 2019

jtojnar mentioned this pull request Jul 4, 2023

vala: 0.56.7 → 0.56.9 #241298

Merged

12 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

vala_0_38, vala_0_40, vala_0_42: add configuration to disable graphviz (to reduce closure size) #53000

vala_0_38, vala_0_40, vala_0_42: add configuration to disable graphviz (to reduce closure size) #53000

andrew-d commented Dec 28, 2018

jtojnar commented Dec 28, 2018

andrew-d commented Dec 28, 2018

jtojnar commented Dec 28, 2018

albfan commented Dec 28, 2018

andrew-d commented Dec 28, 2018

andrew-d commented Dec 28, 2018

jtojnar commented Dec 28, 2018 via email

andrew-d commented Jan 6, 2019

hedning commented Jan 7, 2019

jtojnar commented Jan 7, 2019

jtojnar commented Jan 7, 2019

hedning commented Jan 8, 2019

worldofpeace commented Jan 8, 2019

andrew-d commented Jan 8, 2019 via email

andrew-d commented Jan 9, 2019

hedning commented Jan 9, 2019

hedning commented Jan 9, 2019 •

edited

andrew-d commented Jan 13, 2019

worldofpeace commented Jan 14, 2019

andrew-d commented Jan 14, 2019

worldofpeace commented Jan 14, 2019

andrew-d commented Jan 14, 2019

worldofpeace left a comment

hedning commented Jan 14, 2019

vala_0_38, vala_0_40, vala_0_42: add configuration to disable graphviz (to reduce closure size) #53000

vala_0_38, vala_0_40, vala_0_42: add configuration to disable graphviz (to reduce closure size) #53000

Conversation

andrew-d commented Dec 28, 2018

Motivation for this change

Things done

jtojnar commented Dec 28, 2018

andrew-d commented Dec 28, 2018

jtojnar commented Dec 28, 2018

albfan commented Dec 28, 2018

andrew-d commented Dec 28, 2018

andrew-d commented Dec 28, 2018

jtojnar commented Dec 28, 2018 via email

andrew-d commented Jan 6, 2019

hedning commented Jan 7, 2019

jtojnar commented Jan 7, 2019

jtojnar commented Jan 7, 2019

hedning commented Jan 8, 2019

worldofpeace commented Jan 8, 2019

andrew-d commented Jan 8, 2019 via email

andrew-d commented Jan 9, 2019

hedning commented Jan 9, 2019

hedning commented Jan 9, 2019 • edited

andrew-d commented Jan 13, 2019

worldofpeace commented Jan 14, 2019

andrew-d commented Jan 14, 2019

worldofpeace commented Jan 14, 2019

andrew-d commented Jan 14, 2019

worldofpeace left a comment

Choose a reason for hiding this comment

hedning commented Jan 14, 2019

hedning commented Jan 9, 2019 •

edited