Bug 210333 - www/node, www/node4, www/node5: Depend on devel/icu instead of the bundled icu-small
Summary: www/node, www/node4, www/node5: Depend on devel/icu instead of the bundled ic...
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Many People
Assignee: Kurt Jaeger
URL:
Keywords: easy, patch, patch-ready
Depends on:
Blocks: 210305
  Show dependency treegraph
 
Reported: 2016-06-16 20:20 UTC by Bradley T. Hughes
Modified: 2016-06-20 08:10 UTC (History)
4 users (show)

See Also:
bhughes: maintainer-feedback+


Attachments
patch from a git commit, apply with patch -p1 (4.43 KB, patch)
2016-06-16 20:20 UTC, Bradley T. Hughes
bhughes: maintainer-approval+
Details | Diff
corrected_patch-www_node (4.03 KB, patch)
2016-06-18 10:38 UTC, Walter Schwarzenfeld
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Bradley T. Hughes freebsd_committer freebsd_triage 2016-06-16 20:20:08 UTC
Created attachment 171492 [details]
patch from a git commit, apply with patch -p1

Fallout from the recent change to use port dependencies instead of
bundled versions has shown that having devel/icu installed will cause
the node.js ports to fail to build. Handle this by depending on
devel/icu. Bump PORTREVISION for the new dependency.

Change from USES=localbase to USES=pkgconfig to pick up include and
library paths for all port dependencies.

PR: 210305
Comment 1 VK 2016-06-16 22:02:33 UTC
Comment on attachment 171492 [details]
patch from a git commit, apply with patch -p1

Thanks, Bradley. Please flag the patch with maintainer-approval(+) so it appears in the "Maintainer Approved" saved search. Also a hint futurewise, you can flag your patches like that at the upload. ;)
Comment 2 VK 2016-06-16 22:05:03 UTC
Summary cleanup, make individual port names appear in search. Remove wrongly assigned maintainer feedback request.
Comment 3 Kurt Jaeger freebsd_committer freebsd_triage 2016-06-18 04:07:23 UTC
The patch has issues ?

Patching file www/node/Makefile using Plan A...
Hunk #1 failed at 3.
Hunk #2 failed at 19.
Hunk #3 succeeded at 27.
Comment 4 Walter Schwarzenfeld 2016-06-18 10:38:14 UTC
Created attachment 171542 [details]
corrected_patch-www_node
Comment 5 Walter Schwarzenfeld 2016-06-18 10:38:36 UTC
This one should work.
Comment 6 Walter Schwarzenfeld 2016-06-18 11:08:16 UTC
Testbuild with poudriere for node,node4, node5  jail 10.3-RELEASE-amd64 successful.
Comment 7 Walter Schwarzenfeld 2016-06-18 11:51:30 UTC
Same for 10.3-RELEASE-i386.
Comment 8 Kurt Jaeger freebsd_committer freebsd_triage 2016-06-18 11:58:52 UTC
testbuild on 9.3a is fine.
Comment 9 commit-hook freebsd_committer freebsd_triage 2016-06-18 12:28:46 UTC
A commit references this bug:

Author: pi
Date: Sat Jun 18 12:28:31 UTC 2016
New revision: 417063
URL: https://svnweb.freebsd.org/changeset/ports/417063

Log:
  www/node, www/node[45]: Depend on devel/icu instead of the bundled icu-small

  PR:		210333
  Submitted by:	Bradley T. Hughes <bradleythughes@fastmail.fm> (maintainer)
  Reviewed by:	w.schwarzenfeld@utanet.at

Changes:
  head/www/node/Makefile
  head/www/node4/Makefile
  head/www/node5/Makefile
Comment 10 Kurt Jaeger freebsd_committer freebsd_triage 2016-06-18 12:29:25 UTC
Committed, with one small makepatch fix.
Comment 11 Adam Weinberger freebsd_committer freebsd_triage 2016-06-18 15:41:23 UTC
Bradley I don't like this change. icu is a HUGE port and depending on it brings absolutely nothing to node that it didn't have already.

If the problem was a build failure, can node just be explicitly told to use the bundled icu-small instead?
Comment 12 Bradley T. Hughes freebsd_committer freebsd_triage 2016-06-18 19:00:12 UTC
Comment on attachment 171542 [details]
corrected_patch-www_node

The post-configure step isn't part of my original patch, and I don't see the need for it.
Comment 13 Walter Schwarzenfeld 2016-06-18 19:16:08 UTC
Oh, sorry overlooked that.
Comment 14 Walter Schwarzenfeld 2016-06-18 20:01:04 UTC
The post-configure was a suggestion from someone in the other PR. I tested it and forgot to remove it.
But, if  I read what Adam Weinberger wrote, is it  considerable to keep it and   change it to an ICU option?
Comment 15 Bradley T. Hughes freebsd_committer freebsd_triage 2016-06-20 08:10:12 UTC
> The post-configure was a suggestion from someone in the other PR. I tested it and forgot to remove it.
> But, if  I read what Adam Weinberger wrote, is it  considerable to keep it and change it to an ICU option?

Ideally I'd like to remove it. It seems fragile and I would prefer to find a way to make the node.js build system actually work wrt to bundled dependencies while a local copy is installed.