Bug 216212 - graphics/opennurbs: fix on -current
Summary: graphics/opennurbs: fix on -current
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Larry Rosenman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-18 08:20 UTC by Poul-Henning Kamp
Modified: 2017-03-23 01:28 UTC (History)
4 users (show)

See Also:
fernape: maintainer-feedback+


Attachments
fix detection of zlib.h (997 bytes, patch)
2017-02-11 10:13 UTC, Tobias C. Berner
no flags Details | Diff
patch to the ports tree (6.00 KB, patch)
2017-02-14 22:24 UTC, Fernando Apesteguía
no flags Details | Diff
Improved patch (5.96 KB, patch)
2017-02-16 12:05 UTC, Dmitry Marakasov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Poul-Henning Kamp freebsd_committer freebsd_triage 2017-01-18 08:20:01 UTC
libz/zlib fix needs updating after change in src:

Index: Makefile
===================================================================
--- Makefile    (revision 431539)
+++ Makefile    (working copy)
@@ -35,7 +35,7 @@
                > ${WRKSRC}/opennurbs_version.h.tmp || ${TRUE}
        ${MV} ${WRKSRC}/opennurbs_version.h.tmp ${WRKSRC}/opennurbs_version.h
        ${MV} ${WRKSRC}/zlib ${WRKSRC}/zlib_
-       ${CP} -R ${SRC_BASE}/lib/libz ${WRKSRC}/zlib
+       ${CP} -R ${SRC_BASE}/contrib/zlib ${WRKSRC}/zlib
 
 do-install:
        @${MKDIR} ${STAGEDIR}${EXAMPLESDIR} \
Comment 1 Fernando Apesteguía freebsd_committer freebsd_triage 2017-01-18 10:57:55 UTC
(In reply to Poul-Henning Kamp from comment #0)

Thanks for the patch. I don't have a -CURRENT system to test it, but it simple enough so you have my approval.

Thanks
Comment 2 Dmitry Marakasov freebsd_committer freebsd_triage 2017-02-09 18:04:32 UTC
- Wait, why do you copy zlib from src/ at all? If any, you should remove bundled zlib and use system one. Not sure how dependees would handle this

- iconv in pre-build does not seem to do anything. why's it needed?

- do-install is an overcomplication just for examples installation, ports should not do such things; I'd just install example apps, and they should go to examplesdir, not libexec
Comment 3 Fernando Apesteguía freebsd_committer freebsd_triage 2017-02-10 23:11:30 UTC
(In reply to Dmitry Marakasov from comment #2)

The first version of the patch used just the bundled version of zlib which eventually broke QCad. Copying the sources from base was the fastest patch I came up with.

iconv. Agreed. I tested it and it doesn't seem to be necessary right now (not sure if it was in the past)

The simplification of the installation of examples is certainly necessary.

The patch submitted by phk@ needs to be actually a conditional since we want it to work on systems with both paths.

I'll work on it as soon as I can and send a new patch.
Comment 4 Tobias C. Berner freebsd_committer freebsd_triage 2017-02-11 10:13:42 UTC
Created attachment 179871 [details]
fix detection of zlib.h

Tested on current and 103.
Comment 5 Fernando Apesteguía freebsd_committer freebsd_triage 2017-02-14 22:23:10 UTC
(In reply to Tobias C. Berner from comment #4)

Reworked patch. I added Tobias' detection patch.

- Removed iconv related code.
- Examples go to EXAMPLESDIR instead of libexec

I didn't change the copy if of the zlib sources. Linking the directory is not an option (see this: https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=187506). We could try to avoid to include the zlib.o files in the library archive, but then some of the .o files would have unresolved symbols and we would force the final executable to be linked against zlib (-lz). I'm not sure if this violates POLA since linking against zlib is not necessary if the upstream package is used.

Please, let me know what you think.

Thanks.
Comment 6 Fernando Apesteguía freebsd_committer freebsd_triage 2017-02-14 22:24:06 UTC
Created attachment 180001 [details]
patch to the ports tree
Comment 7 Fernando Apesteguía freebsd_committer freebsd_triage 2017-02-14 22:25:29 UTC
Comment on attachment 179871 [details]
fix detection of zlib.h

Superseded by this patch:

https://bugs.freebsd.org/bugzilla/attachment.cgi?id=180001

which includes this one.
Comment 8 Fernando Apesteguía freebsd_committer freebsd_triage 2017-02-14 22:26:31 UTC
Comment on attachment 179871 [details]
fix detection of zlib.h

Superseded by this one:

https://bugs.freebsd.org/bugzilla/attachment.cgi?id=180001
Comment 9 Dmitry Marakasov freebsd_committer freebsd_triage 2017-02-16 12:05:56 UTC
Created attachment 180043 [details]
Improved patch

Hmm, it actually turns out to be hard to unbundle zlib. Ok, let it be taken from src. I've improved a patch a bit: actually removed iconv stuff, made examples installation optional.
Comment 10 Fernando Apesteguía freebsd_committer freebsd_triage 2017-02-16 22:21:06 UTC
(In reply to Dmitry Marakasov from comment #9)

The patch makes the port much cleaner, thanks!

I tested it with and without examples via port test. I couldn't test it with poudriere though. I get this error:

===========================================================================
=======================<phase: patch-depends  >============================
===========================================================================
=======================<phase: patch          >============================
===>  Patching for opennurbs-20130711_3
===>   Converting DOS text files to UNIX text files
===>  Applying FreeBSD patches for opennurbs-20130711_3
Ignoring previously applied (or reversed) patch.
1 out of 1 hunks ignored--saving rejects to opennurbs_system.h.rej
=> FreeBSD patch patch-opennurbs__system.h failed to apply cleanly.
=> Patch(es)  patch-opennurbs__system.h applied cleanly.
*** Error code 1

I suppose is something in my poudriere set up? Other ports build fine though...

I approve this patch. Thanks for improving it.
Comment 11 Fernando Apesteguía freebsd_committer freebsd_triage 2017-02-16 22:22:32 UTC
Comment on attachment 180001 [details]
patch to the ports tree

superseded by https://bugs.freebsd.org/bugzilla/attachment.cgi?id=180043
Comment 12 Fernando Apesteguía freebsd_committer freebsd_triage 2017-02-25 23:32:23 UTC
(In reply to fernando.apesteguia from comment #10)

Just for the record, I fixed my jails and I can confirm the patch builds fine in {10.3,11.0}{i386,amd64}

What's preventing the patch from being committed?
Comment 13 Fernando Apesteguía freebsd_committer freebsd_triage 2017-03-16 22:56:01 UTC
No advance on this?

Is there singing wrong with the patch?
Comment 14 Fernando Apesteguía freebsd_committer freebsd_triage 2017-03-22 17:36:03 UTC
Again,

Why has this stalled?
Comment 15 Bryan Drewery freebsd_committer freebsd_triage 2017-03-22 19:11:37 UTC
(In reply to fernando.apesteguia from comment #14)
> Again,
> 
> Why has this stalled?

https://bz-attachments.freebsd.org/attachment.cgi?id=180043&action=diff&format=raw&headers=1

Is this the final patch?
Comment 16 Fernando Apesteguía freebsd_committer freebsd_triage 2017-03-22 22:43:52 UTC
(In reply to Bryan Drewery from comment #15)
Yes, the final one by amdmi3@
Comment 17 Larry Rosenman freebsd_committer freebsd_triage 2017-03-23 00:05:43 UTC
taking this PR,
Approved By: adamw (mentor)
Comment 18 commit-hook freebsd_committer freebsd_triage 2017-03-23 01:27:56 UTC
A commit references this bug:

Author: ler
Date: Thu Mar 23 01:27:35 UTC 2017
New revision: 436749
URL: https://svnweb.freebsd.org/changeset/ports/436749

Log:
  fix finding of the zlib sources.

  PR:		216212
  Submitted by:	mdmi3
  Approved by:	fernando.apesteguia@gmail.com, phk, adamw (mentor)
  Differential Revision:	https://reviews.freebsd.org/D10110

Changes:
  head/graphics/opennurbs/Makefile
  head/graphics/opennurbs/files/patch-opennurbs__system.h
  head/graphics/opennurbs/pkg-plist
Comment 19 Larry Rosenman freebsd_committer freebsd_triage 2017-03-23 01:28:35 UTC
Committed, Thanks all.

Thank you for the patience.