New core plan for R #571

baggerspion · 2017-05-24T19:50:38Z

Signed-off-by: Paul Adams paul@baggerspion.net

thesentinels · 2017-05-24T19:50:40Z

Thanks for the pull request! Here is what will happen next:

Your PR will be reviewed by the maintainers
If everything looks good, one of them will approve it, and your PR will be merged.

Thank you for contributing!

stevendanna · 2017-05-24T20:51:35Z

👍 This looks like a good start to me. It would be nice if we could get some of the graphical output devices enabled:

  Interfaces supported:
  External libraries:        readline, curl
  Additional capabilities:   NLS
  Options enabled:           shared BLAS, R profiling

  Capabilities skipped:      PNG, JPEG, TIFF, cairo, ICU
  Options not enabled:       memory profiling

  Recommended packages:      yes

Because of dependency shenanigans I've had trouble getting some of those enabled without at least getting some parts of xorg packaged, which is started over in the xorg org.

I don't think we need those immediately, just wanted to mention it as I'm happy to help get those enabled over time.

baggerspion · 2017-05-24T21:10:47Z

Thanks for the heads-up!

I can enable PNG, JPEG, TIFF and ICU with no problem. Will need to work with others to get core/cairo shipped.

baggerspion · 2017-05-25T14:25:41Z

I'm going to leave this PR open to keep conversation going (other people are interested in this core/R). But if we want image export, I, at least, additionally need to work on:

core/cairo
core/Xlibs

This is currently a do-not-merge PR.

robbkidd · 2017-05-25T15:15:00Z

@therealpadams,

baggerspion · 2017-05-26T15:22:03Z

Status update:
When compiling R, during the .configure step, pkg-config cannot find cairo (despite being correctly present in PKG_PATH). There is some anecdotal evidence that this happens when pango is not present.

So now I am working on:

core/cairo
core/pango
core/harfbuzz
core/R

Thanks to @eeyun and @stevendanna for their help in the hack day yesterday. This PR is staying open for further comment.

stevendanna · 2017-05-26T17:14:39Z

I was able to get cairo recognized with a small fix to pixman (in another PR) and by explicitly adding some transitive dependencies to the R pkg_deps array.

stevendanna · 2017-05-26T17:15:22Z

PR for pixman: #577

baggerspion · 2017-05-28T13:17:49Z

@stevendanna That's great news. Do you have a branch somewhere that I can merge into this PR so I can grab your R plan.sh?

baggerspion · 2017-05-28T22:40:35Z

** Status update **
Builds:

core/pango
core/harfbuzz
core/cairo

WIP:

core/R

In terms of builds, I'm pretty close to wrapping this up now. Once everything is building nicely, there will need to be some work to cleanup the plans. Note: this PR assumes that #577 is merged.

baggerspion · 2017-05-29T12:56:56Z

@stevendanna when you have a moment, I'd hugely appreciate a review of this PR. I cannot work out why I am not getting R to build with all the graphical device support. I assume that once cairo works, jpeg, png etc will all enable, too.
Cannot work out why cairo is not being enabled, though.

stevendanna · 2017-05-29T14:27:37Z

@therealpadams Thanks for pushing this forward. I'll pull down your recent changes and see what I can find.

stevendanna · 2017-05-30T01:25:16Z

OK. I've made some progress, but don't get too excited, lots of caveats below:

> capabilities()
       jpeg         png        tiff       tcltk         X11        aqua
       TRUE        TRUE        TRUE       FALSE       FALSE       FALSE
   http/ftp     sockets      libxml        fifo      cledit       iconv
       TRUE        TRUE        TRUE        TRUE        TRUE        TRUE
        NLS     profmem       cairo         ICU long.double     libcurl
       TRUE        TRUE        TRUE        TRUE        TRUE        TRUE
> png("foo.png", type="cairo")
> plot(c(1,2,3,4,5))
> dev.off()
null device
          1
>
Save workspace image? [y/n/c]: n
[157][default:/src:0]# file foo.png
foo.png: PNG image data, 480 x 480, 8-bit colormap, non-interlaced

This is available on my branch which I forked off of yours: baggerspion/core-plans@core/R...stevendanna:core/R-ssd

Here is a brief summary of where I've gotten:

A lot of things were in pkg_build_deps that needed to be in pkg_deps. I've sorted most of them out, but we'll want to do another pass once everything is working.
I've added some "transitive" deps directly to pkg_deps so that they are correctly added to PKG_CONFIG_PATH. This is how I got the configure script recognizing pango and cairo. A good trick for figuring out why pkg-config isn't finding something is to have it tell you by exporting the same PKG_CONFIG_PATH that the build is using and then running pkg-config directly with the --print-errors flag:
```
[52][default:/src:0]# /hab/pkgs/core/pkg-config/0.29/20170513212944/bin/pkg-config cairo --print-errors
Package fontconfig was not found in the pkg-config search path.
Perhaps you should add the directory containing `fontconfig.pc'
to the PKG_CONFIG_PATH environment variable
Package 'fontconfig', required by 'cairo', not found
```
The configure script that R ships, expect that cairo-xlib is there no matter how cairo has been compiled. Here are the two tests programs it might execute:

Pango and Cairo:
```
#include <pango/pango.h>
#include <pango/pangocairo.h>
#include <cairo-xlib.h>
#if CAIRO_VERSION  < 10200
#error cairo version >= 1.2 required
#endif
int main(void) {
    cairo_t  *CC = NULL; // silence picky compilers
    cairo_arc(CC, 0.0, 0.0, 1.0, 0.0, 6.28);
    pango_cairo_create_layout(CC);
    pango_font_description_new();
    return 0;
 }
```
Cairo-only:
```
#include <cairo.h>
#include <cairo-xlib.h>
#if CAIRO_VERSION  < 10200
#error cairo version >= 1.2 required
#endif
int main(void) {
    cairo_t  *CC;
    cairo_arc(CC, 0.0, 0.0, 1.0, 0.0, 6.28);
    cairo_select_font_face (CC, "Helvetica", CAIRO_FONT_SLANT_NORMAL,
                             CAIRO_FONT_WEIGHT_BOLD);
    return 0;
 }
```
To get around this I removed pango so I just had to deal with one of the tests and then ran sed on the configure script to remove the include <cairo-xlib.h>. The test still passes without this include. My guess is the same is true for the pango+cairo test, the cairo-only test just seemed like the place to start.

To be honest, this makes me a bit nervous since I don't know much about cairo and whether not having cairo-xlib will be a problem down the road. I might be "OK" with it for now until we can get xlib in core.
Even after all that, lots of code won't work "out-of-the-box" since the default "type" parameter for devices like png still seems to be "xlib" even though we compiled it completely without xlib. It would be nice if we could figure out how to make the default "cairo".
Editing the configuration file also led to some failures during building some package that requires java config. I haven't dug into it at all yet. It might have to do with the timestamp on the configure script or it could have been environmental.

baggerspion · 2017-05-30T13:57:22Z

We now successfully have all graphics drivers enabled and cairo enabled as the default bitmap type:

> capabilities()
       jpeg         png        tiff       tcltk         X11        aqua 
       TRUE        TRUE        TRUE       FALSE       FALSE       FALSE 
   http/ftp     sockets      libxml        fifo      cledit       iconv 
       TRUE        TRUE        TRUE        TRUE        TRUE        TRUE 
        NLS     profmem       cairo         ICU long.double     libcurl 
       TRUE        TRUE        TRUE        TRUE        TRUE        TRUE 
> options("bitmapType")
$bitmapType
[1] "cairo"

Signed-off-by: Paul Adams <paul@baggerspion.net>

Signed-off-by: Paul Adams <paul.adams@endocode.com>

baggerspion · 2017-05-31T12:12:50Z

These plans now have all their pkg_build_deps and pkg_deps corrected and the EXPORTS removed. I reckon these are now OK to merge.

…over these plans. Signed-off-by: Paul Adams <paul@baggerspion.net>

Signed-off-by: Paul Adams <paul@baggerspion.net>

baggerspion · 2017-06-01T17:21:54Z

So here's a problem:
It's one thing to have FontConfig installed. It's totally another to have fonts installed...

I was able to show that the output is correct, by installing some fonts manually into my studio:

Signed-off-by: Paul Adams <paul@baggerspion.net>

baggerspion · 2017-06-03T07:36:27Z

This PR is getting a little crazy :) It now contains:
core/cairo
core/harfbuzz
core/pango
core/R
core/liberation-fonts-ttf

The Liberation Fonts pkg simply downloads and untars the Liberations Fonts for use by, for example, R. Problem, we need to run fc-cache $(pkg_path_for core/liberation-fonts-ttf) at some point before the fonts can actually be used. Not entirely sure when.

bdangit · 2017-06-03T18:03:57Z

While we could do this in init but I suspect this is more of a cli command than a service that is to be managed underneath Hab sup.

Alternatively, you could always make a "R" wrapper where it will execute the command you need as well setup any other additional exports/variables/etc.

bdangit

Looking good!

bdangit · 2017-06-03T18:05:39Z

R/plan.sh

+  core/diffutils
+  core/file
+  core/gcc
+  core/icu


No need to call this out here if it's defined in pkg_deps

bdangit · 2017-06-03T18:06:20Z

R/plan.sh

+  core/file
+  core/gcc
+  core/icu
+  core/liberation-fonts-ttf


If you need this during runtime, you'll want to make sure this is placed in pkg_deps

bdangit · 2017-06-03T18:06:51Z

R/plan.sh

+  core/icu
+  core/liberation-fonts-ttf
+  core/libjpeg-turbo
+  core/libpng


Remove here if called out in pkg_deps

bdangit · 2017-06-03T18:07:20Z

R/plan.sh

+  core/gcc
+  core/icu
+  core/liberation-fonts-ttf
+  core/libjpeg-turbo


Remove this if defined in pkg_deps

bdangit · 2017-06-03T18:08:07Z

R/plan.sh

+do_build() {
+    sed -i '/#include.*<cairo-xlib.h>/d' ./configure
+    ./configure --prefix="${pkg_prefix}" \
+		--with-x=no \


Alignment seems off by one space

These alignment errors are super weird... don't see them in VIM. Will guess my way around them :)

bdangit · 2017-06-03T18:08:18Z

R/plan.sh

+    sed -i '/#include.*<cairo-xlib.h>/d' ./configure
+    ./configure --prefix="${pkg_prefix}" \
+		--with-x=no \
+                --disable-java \


bdangit · 2017-06-03T18:09:30Z

harfbuzz/plan.sh

+pkg_origin=core
+pkg_version=1.3.1
+pkg_maintainer="The Habitat Maintainers <humans@habitat.sh>"
+pkg_license=('mit')


Minor nit but would Uppercase to MIT

bdangit · 2017-06-03T18:09:48Z

harfbuzz/plan.sh

+
+do_build() {
+  ./configure --prefix="$pkg_prefix" \
+	      --with-gobject=yes


Alignment off by one space

bdangit · 2017-06-03T18:10:50Z

pango/plan.sh

+  core/patch
+  core/perl
+  core/pkg-config
+   core/util-linux


Alignment by one space

Signed-off-by: Paul Adams <paul@baggerspion.net>

baggerspion · 2017-06-04T11:55:06Z

Thanks for the review, @bdangit. Alignment issues all fixed and I've sorted out pkg_deps vs pkg_build_deps (thought I'd already done that).

I think this is mergeable now. The issue with updating the font-cache is something that can be fixed with documentation.

bdangit · 2017-06-04T19:21:25Z

👍

stevendanna · 2017-06-07T11:47:08Z

@eeyun Wondering if we can remove the DO-NOT-MERGE label? This looks good to me. While there is the fc-cache issue, I think it is best to "document and iterate" on that issue.

eeyun · 2017-06-07T13:27:53Z

LGTM!

eeyun · 2017-06-23T17:52:07Z

core/pixman/0.34.0/20170623171747 has been built and uploaded to the depot. 💖
core/cairo/1.14.8/20170623172008 has been built and uploaded to the depot. 💖
core/liberation-fonts-ttf/2.00.1/20170623172912 has been built and uploaded to the depot. 💖
core/harfbuzz/1.3.1/20170623172916 has been built and uploaded to the depot. 💖
core/pango/1.40.6/20170623172955 has been built and uploaded to the depot. 💖
core/R/3.4.0/20170623173042 has been built and uploaded to the depot. 💖

The following reverse dependencies in the chain have also been rebuilt:
core/qemu/2.7.0/20170623173821 has been built and uploaded to the depot. 💖
core/grub/2.02/20170623174156 has been built and uploaded to the depot. 💖

raskchanky added the DO NOT MERGE label May 25, 2017

New core plan for R

4206558

Signed-off-by: Paul Adams <paul@baggerspion.net>

baggerspion force-pushed the core/R branch from 0512ed9 to 4206558 Compare May 30, 2017 14:53

Fix: R plan cleaning

9254916

Signed-off-by: Paul Adams <paul.adams@endocode.com>

Fixed dependencies in order to remove the ugly EXPORTS that were all …

8ee9e78

…over these plans. Signed-off-by: Paul Adams <paul@baggerspion.net>

baggerspion force-pushed the core/R branch from 291be10 to 8ee9e78 Compare May 31, 2017 12:46

Added missing upstream URLS and descriptions

b7bee46

Signed-off-by: Paul Adams <paul@baggerspion.net>

Add core/liberation-fonts-ttf pkg and update harfbuzz and R to use it

4e1e2f5

Signed-off-by: Paul Adams <paul@baggerspion.net>

baggerspion force-pushed the core/R branch from e009654 to 4e1e2f5 Compare June 3, 2017 07:32

bdangit suggested changes Jun 3, 2017

View reviewed changes

Fixes from @bdangit's comments

1089b13

Signed-off-by: Paul Adams <paul@baggerspion.net>

baggerspion force-pushed the core/R branch from 2b70ae2 to 1089b13 Compare June 4, 2017 11:52

eeyun added S-DO-NOT-MERGE and removed DO NOT MERGE labels Jun 6, 2017

eeyun removed the S-DO-NOT-MERGE label Jun 7, 2017

eeyun merged commit 8585f28 into habitat-sh:master Jun 23, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New core plan for R #571

New core plan for R #571

baggerspion commented May 24, 2017

thesentinels commented May 24, 2017

stevendanna commented May 24, 2017

baggerspion commented May 24, 2017

baggerspion commented May 25, 2017

robbkidd commented May 25, 2017

baggerspion commented May 26, 2017 •

edited

stevendanna commented May 26, 2017

stevendanna commented May 26, 2017

baggerspion commented May 28, 2017

baggerspion commented May 28, 2017 •

edited

baggerspion commented May 29, 2017

stevendanna commented May 29, 2017

stevendanna commented May 30, 2017 •

edited

baggerspion commented May 30, 2017

baggerspion commented May 31, 2017

baggerspion commented Jun 1, 2017

baggerspion commented Jun 3, 2017

bdangit commented Jun 3, 2017

bdangit left a comment

bdangit Jun 3, 2017

bdangit Jun 3, 2017

bdangit Jun 3, 2017

bdangit Jun 3, 2017

bdangit Jun 3, 2017

baggerspion Jun 4, 2017

bdangit Jun 3, 2017

bdangit Jun 3, 2017

bdangit Jun 3, 2017

bdangit Jun 3, 2017

baggerspion commented Jun 4, 2017

bdangit commented Jun 4, 2017

stevendanna commented Jun 7, 2017

eeyun commented Jun 7, 2017

eeyun commented Jun 23, 2017

New core plan for R #571

New core plan for R #571

Conversation

baggerspion commented May 24, 2017

thesentinels commented May 24, 2017

stevendanna commented May 24, 2017

baggerspion commented May 24, 2017

baggerspion commented May 25, 2017

robbkidd commented May 25, 2017

baggerspion commented May 26, 2017 • edited

stevendanna commented May 26, 2017

stevendanna commented May 26, 2017

baggerspion commented May 28, 2017

baggerspion commented May 28, 2017 • edited

baggerspion commented May 29, 2017

stevendanna commented May 29, 2017

stevendanna commented May 30, 2017 • edited

baggerspion commented May 30, 2017

baggerspion commented May 31, 2017

baggerspion commented Jun 1, 2017

baggerspion commented Jun 3, 2017

bdangit commented Jun 3, 2017

bdangit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

baggerspion commented Jun 4, 2017

bdangit commented Jun 4, 2017

stevendanna commented Jun 7, 2017

eeyun commented Jun 7, 2017

eeyun commented Jun 23, 2017

baggerspion commented May 26, 2017 •

edited

baggerspion commented May 28, 2017 •

edited

stevendanna commented May 30, 2017 •

edited