Bug 210876 - multimedia/assimp: Update to 3.3.1
Summary: multimedia/assimp: Update to 3.3.1
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: Kurt Jaeger
URL:
Keywords: easy, patch
Depends on:
Blocks:
 
Reported: 2016-07-06 18:24 UTC by Yuri Victorovich
Modified: 2016-07-09 07:29 UTC (History)
4 users (show)

See Also:


Attachments
patch (64.56 KB, patch)
2016-07-06 18:24 UTC, Yuri Victorovich
yuri: maintainer-approval+
Details | Diff
patch (65.91 KB, patch)
2016-07-06 18:25 UTC, Yuri Victorovich
no flags Details | Diff
Proposed patch (since 416829 revision) (27.43 KB, patch)
2016-07-08 01:34 UTC, lightside
no flags Details | Diff
The multimedia/assimp port with proposed changes (7.48 KB, application/x-bzip2)
2016-07-08 02:28 UTC, lightside
no flags Details
Proposed patch (since 416829 revision) (18.95 KB, patch)
2016-07-08 18:15 UTC, lightside
no flags Details | Diff
Proposed patch (since 416829 revision) (18.95 KB, patch)
2016-07-08 18:35 UTC, lightside
no flags Details | Diff
Proposed patch (since 416829 revision) (19.37 KB, patch)
2016-07-08 21:10 UTC, lightside
yuri: maintainer-approval+
Details | Diff
The poudriere testport log (FreeBSD 9.3 amd64) (10.01 KB, application/x-bzip2)
2016-07-08 21:11 UTC, lightside
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Yuri Victorovich freebsd_committer freebsd_triage 2016-07-06 18:24:17 UTC
Created attachment 172180 [details]
patch
Comment 1 Yuri Victorovich freebsd_committer freebsd_triage 2016-07-06 18:25:43 UTC
Created attachment 172181 [details]
patch
Comment 2 Kurt Jaeger freebsd_committer freebsd_triage 2016-07-07 18:46:55 UTC
testbuilds@work
Comment 3 lightside 2016-07-08 01:34:04 UTC
Created attachment 172218 [details]
Proposed patch (since 416829 revision)

Hello Yuri Victorovich and Kurt Jaeger.

I noticed this PR recently and analyzed changes for v3.3 release tag:
https://github.com/assimp/assimp/releases/tag/v3.3
https://github.com/assimp/assimp/compare/v3.2...v3.3
I found, that developers didn't (or forgot to) change ASSIMP_VERSION_MINOR from 2 to 3:
https://github.com/assimp/assimp/blob/19769eef8b9612a82fdb55c245db871476e7f178/CMakeLists.txt#L49
while they changed value of MinorVersion variable to 3:
https://github.com/assimp/assimp/commit/19769eef8b9612a82fdb55c245db871476e7f178
Because of this, I propose to use GH_TAGNAME, in case of possible re-tag:
https://github.com/assimp/assimp/compare/v3.2...19769ee

The patch in attachment #172181 [details] contains changes for files/patch-include_assimp_Compiler_pstdint.h file, which updates include/assimp/Compiler/pstdint.h file from 0.1.12 (in sources) to 0.1.15.4 version. Looks like, the patch changes whole file, instead of concrete lines. Therefore, I propose patch, which created by `make makepatch` command, but with some changes "to remove comments with private information". The 0.1.12 version (with custom changes, based on private information and strange wording in comments) was proposed by afiskon, for reference:
https://github.com/assimp/assimp/issues/795

Also, I think (and tested this), this version doesn't require devel/boost-libs library dependency and BOOST option, while there are some traces of "boost" words in sources, but no ASSIMP_ENABLE_BOOST_WORKAROUND cmake's option, as it was before 3.3 version. It requires C++11 features:
https://github.com/assimp/assimp/blob/19769eef8b9612a82fdb55c245db871476e7f178/CMakeLists.txt#L117
which may require to add compiler:c++11-lang to USES.

The proposed patch attached, with following changes (including maintainer's changes):
- Use GH_TAGNAME for current version
- Remove BOOST option and add compiler:c++11-lang to USES
- Add sed patch to change ASSIMP_VERSION_MINOR from 2 to 3 for CMakeLists.txt file
- Add patch for code/BlenderTessellator.cpp file
- Adapt patch for include/assimp/Compiler/pstdint.h file
- Adapt pkg-plist
Comment 4 lightside 2016-07-08 01:35:24 UTC
The multimedia/assimp port is library dependency for games/doomsday and games/pioneer ports. I checked proposed update (with my changes) for games/pioneer port and it works (even without recompile). Not sure, but Danilo Egea Gondolfo might be interested to check games/doomsday port also.

CC: danilo
Comment 5 Yuri Victorovich freebsd_committer freebsd_triage 2016-07-08 01:54:43 UTC
lightside,

For some reason your patch doesn't apply to the Makefile:
Patching file Makefile using Plan A...
Hunk #1 failed at 2.
Hunk #2 failed at 13.
Hunk #3 failed at 22.
3 out of 3 hunks failed--saving rejects to Makefile.rej
Comment 6 lightside 2016-07-08 02:28:15 UTC
Created attachment 172219 [details]
The multimedia/assimp port with proposed changes

(In reply to comment #5)
> For some reason your patch doesn't apply to the Makefile
This may depend on how you apply the patch (and for unchanged sources). For example, if you place assimp.diff file near assimp directory (e.g. multimedia), then to apply the proposed patch you need to run following commands:
% cd multimedia/assimp
% patch -p1 -B '' < ../assimp.diff

I attached assimp.tar.bz2 archive with whole port, just in case.
Comment 7 Yuri Victorovich freebsd_committer freebsd_triage 2016-07-08 08:07:50 UTC
Ok, you need to create patches with 'svn diff', they always apply easy.

Several items handled by the port aren't supposed to be handled here and are best resolved upstream. I asked the upstream to consider fixing them: https://github.com/assimp/assimp/issues/940

Let's put this on hold in the hope that they will re-release it.
Comment 8 Yuri Victorovich freebsd_committer freebsd_triage 2016-07-08 08:08:46 UTC
Ok, you need to create patches with 'svn diff', they always apply easy.

Several items handled by the port aren't supposed to be handled here and are best resolved upstream. I asked the upstream to consider fixing them: https://github.com/assimp/assimp/issues/940

Let's put this on hold in the hope that they will re-release it.
Comment 9 lightside 2016-07-08 08:56:37 UTC
(In reply to comment #8)
> Ok, you need to create patches with 'svn diff', they always apply easy.
I used sources from portsnap and created patch with `diff -ruN assimp.orig assimp > assimp.diff`, the command of which also mentioned in attachment #172218 [details]. I wrote how to apply such kind of patch(es) in comment #6 and there were no problems in other PRs with this.

(In reply to comment #8)
> Let's put this on hold in the hope that they will re-release it.
Agreed, based on description in current v3.3 tag:
https://github.com/assimp/assimp/commit/19769eef8b9612a82fdb55c245db871476e7f178
"Version: prepare 3.3 version.", they (still) prepare it. There are no description of changes for 3.3 version in current commit:
https://github.com/assimp/assimp/blob/19769eef8b9612a82fdb55c245db871476e7f178/CHANGES

You commented on https://github.com/assimp/assimp/issues/940
> pstdint.h isn't of the latest version. You have 0.1.12 and the current
> version is 0.1.15.4. The FreeBSD port for some reason patches it to 0.1.15.4.
> I am not sure why, somebody needed the latest version. But why don't you just
> update it in here, upstream?
The reasons are:
- The 3.2 version in bug 209356 required newer pstdint.h file to fix build on FreeBSD 10.x.
- Between 3.2 and 3.3 versions the pstdint.h was custom patched in https://github.com/assimp/assimp/issues/795 to 0.1.12 version. It gives following warnings on FreeBSD 10.2, for example:
-8<--
In file included from /usr/ports/multimedia/assimp/work/assimp-19769ee/contrib/irrXML/irrXML.cpp:12:
In file included from /usr/ports/multimedia/assimp/work/assimp-19769ee/contrib/irrXML/./../../code/fast_atof.h:29:
/usr/ports/multimedia/assimp/work/assimp-19769ee/include/assimp/Compiler/pstdint.h:678:10: warning: 'UINT_FAST64_MAX' macro redefined
# define UINT_FAST64_MAX UINT_LEAST64_MAX
         ^
/usr/include/x86/_stdint.h:136:9: note: previous definition is here
#define UINT_FAST64_MAX UINT64_MAX
        ^

In file included from /usr/ports/multimedia/assimp/work/assimp-19769ee/contrib/irrXML/irrXML.cpp:12:
In file included from /usr/ports/multimedia/assimp/work/assimp-19769ee/contrib/irrXML/./../../code/fast_atof.h:29:
/usr/ports/multimedia/assimp/work/assimp-19769ee/include/assimp/Compiler/pstdint.h:674:10: warning: 'INT_FAST32_MIN' macro redefined
#define  INT_FAST32_MIN  INT_LEAST32_MIN
         ^
/usr/include/x86/_stdint.h:123:9: note: previous definition is here
#define INT_FAST32_MIN  INT32_MIN
        ^

In file included from /usr/ports/multimedia/assimp/work/assimp-19769ee/tools/assimp_cmd/ImageExtractor.cpp:47:
In file included from /usr/ports/multimedia/assimp/work/assimp-19769ee/include/../code/fast_atof.h:29:
/usr/ports/multimedia/assimp/work/assimp-19769ee/include/assimp/Compiler/pstdint.h:320:12: warning: 'INT8_C' macro redefined
#   define INT8_C(v) ((int8_t) v)
           ^
/usr/include/x86/_stdint.h:45:9: note: previous definition is here
#define INT8_C(c)               (c)
        ^

etc.
-->8-
Therefore, it's advisable to patch it for 3.3 version also. I think, you need to point upstream to official source of pstdint.h file, if you want to resolve this on upstream level:
http://www.azillionmonkeys.com/qed/pstdint.h
Comment 10 lightside 2016-07-08 18:15:05 UTC
Created attachment 172250 [details]
Proposed patch (since 416829 revision)

As you know, the assimp was updated to 3.3.1 version (after fixing of 940 issue):
https://github.com/assimp/assimp/releases/tag/v3.3.1
https://github.com/assimp/assimp/compare/v3.2...v3.3.1

The patch is trivial, with following (possible) changes:
- Remove BOOST option and add compiler:c++11-lang to USES
- Remove patch for pstdint.h file, which fixed by upstream
- Adapt pkg-plist
Comment 11 lightside 2016-07-08 18:35:41 UTC
Created attachment 172253 [details]
Proposed patch (since 416829 revision)

Fixed TIMESTAMP to correct value, based on greatest timestamp of modified files:
% make extract && cd work && find * -type f -not -name '.extract_done*' -print0 | xargs -0 stat -f '%m' | sort -u | tail -1
1467998991
Comment 12 Kurt Jaeger freebsd_committer freebsd_triage 2016-07-08 19:00:45 UTC
It still fails to build on 9.3a -- should this work ?

http://people.freebsd.org/~pi/logs/multimedia__assimp-93a-1468004070.txt
Comment 13 lightside 2016-07-08 21:10:21 UTC
Created attachment 172255 [details]
Proposed patch (since 416829 revision)

(In reply to comment #12)
> It still fails to build on 9.3a -- should this work ?
Looks like, it fails because of broken GCC C++11 support on FreeBSD, described in bug 193528. I used similar approach as Dmitry Marakasov in ports r374985 to fix this.

The new proposed patch attached with following changes:
- Remove BOOST option and add compiler:c++11-lib to USES
- Add "_GLIBCXX_USE_C99" define to CXXFLAGS, in case of GCC C++11 usage (see PR 193528 for details)
- Remove patch for pstdint.h file, which fixed by upstream
- Adapt pkg-plist
Comment 14 lightside 2016-07-08 21:11:14 UTC
Created attachment 172256 [details]
The poudriere testport log (FreeBSD 9.3 amd64)
Comment 15 Kurt Jaeger freebsd_committer freebsd_triage 2016-07-09 03:23:07 UTC
testbuilds@work
Comment 16 commit-hook freebsd_committer freebsd_triage 2016-07-09 03:34:57 UTC
A commit references this bug:

Author: pi
Date: Sat Jul  9 03:34:27 UTC 2016
New revision: 418254
URL: https://svnweb.freebsd.org/changeset/ports/418254

Log:
  multimedia/assimp: 3.2 -> 3.3.1

  PR:		210876
  Changes:	https://github.com/assimp/assimp/releases
  Submitted by:	lightside@gmx.com
  Approved by:	Yuri Victorovich <yuri@rawbw.com> (maintainer)

Changes:
  head/multimedia/assimp/Makefile
  head/multimedia/assimp/distinfo
  head/multimedia/assimp/files/
  head/multimedia/assimp/pkg-descr
  head/multimedia/assimp/pkg-plist
Comment 17 Kurt Jaeger freebsd_committer freebsd_triage 2016-07-09 03:35:05 UTC
Committed, WWW changed to github. Thanks!
Comment 18 lightside 2016-07-09 06:15:46 UTC
Thanks Yuri Victorovich for raising issues in this PR to upstream developer(s), which were resolved for 3.3.1 version. Otherwise, they were fixed, but by different way.
Thanks Kurt Jaeger for timely tests and commit.
Overall, while last proposed patch was submitted by me, it's a collaborative work of persons in this PR and related one (bug 193528).
Comment 19 Yuri Victorovich freebsd_committer freebsd_triage 2016-07-09 07:29:32 UTC
You welcome!