Created attachment 147632 [details] Proposed patch (since 367888 revision) Patch to update games/oolite port from 1.76 to 1.80 version. - Added license information to Makefile and removed it from pkg-descr file. - Added files/oolite.in script to run openapp oolite GNUStep application, which also used by desktop shortcut. Removed associated files/pkg-message.in file. - Added some sed fixes, which used for building the port. - Converted to dynamically generated package list by using PORTDATA, PORTDOCS and PLIST_FILES variables. - Removed firefox-4.0.source.js-only.tbz distfile, because of internal using of deps/mozilla sources from main distfile archive. Added/adapted necessary patches from lang/spidermonkey185 port (mainly, to build with LLVM/Clang compiler). - While the port's archive contains src/Core/MiniZip sources, added library dependency from archivers/minizip port, where there is new version with proper fixes. Adapted port's GNUmakefile accordingly.
Maintainer CC'd
Created attachment 147633 [details] The poudriere testport log (FreeBSD 10 amd64) Added poudriere testport log for FreeBSD 10 amd64.
This is so great, thank you very much! By the way, if you'd like to have this port I will gladly give away my maintainership. Thanks again.
Created attachment 147660 [details] Proposed patch (since 367888 revision) Thanks for review. Currently, I don't intend to "collect" other ports. Recently, I made games/pioneer port, which is related to Elite genre, like Oolite does. So, it just some interest. There is error after downloading expansion pack(s). The application cannot create intermediate directories after "~/GNUstep/Library" directory, where it need "~/GNUstep/Library/ApplicationSupport/Oolite/ManagedAddOns". I tried to fix it with "withIntermediateDirectories:YES" for "createDirectoryAtPath" at "oo_createDirectoryAtPath" function inside of "src/Core/NSFileManagerOOExtensions.m" source file, but this crashes the application. So, I fixed it by creating intermediate directories after "~/GNUstep/Library" directory inside of "src/Core/OOOXZManager.m" source file. Please, review the patch again.
Created attachment 147661 [details] The poudriere testport log (FreeBSD 10 amd64)
I want to reword my last reply: I appreciate your work as creator/maintainer of this port. I also have an interest for space simulators. And I glad, that I could help with port updating process. On the other hand, the long term maintainership is not my priority, but I thank you for such a proposal.
Assigning to Andriy
(In reply to John Marino from comment #7) I am not a ports committer. As a maintainer I've already given my approval.
I'm taking it. I'll commit after a little testing.
Created attachment 148273 [details] modified patch The proposed patch does not work on any version except 10. It was making the clang compiler crash on 11 and it errors out about the include c++/v1/tgmath.h not being found on 8.4 and 9.3. I "solved" this with the attached patch, forcing the port to use clang 3.5 on 11 and making the sed for the tgmath.h include conditional. After this the port is failing again on 9.3 with clang crashing. I'll attach logs of the crashes. On 8.4 it fails with errors from clang33, log will follow. I can solve this by forcing clang 3.5 in those versions too, I'm going to test this solution later and ask for approval if that works fine. If we choose such solution I'll force clang 3.5 for 10.0 too for consistency. If anyone is able to I can try helping debugging the issue with clang.
Created attachment 148274 [details] Failure with clang from base on 11-current
Created attachment 148275 [details] Failure with clang from base on 9.3-RELEASE
Created attachment 148276 [details] Failure with clang33 on 8.4-RELEASE
Created attachment 148287 [details] Proposed patch (since 367888 revision) Thanks for testing Oolite 1.80 on different FreeBSD versions, Guido Falsi. (In reply to Guido Falsi from comment #10) > It was making the clang compiler crash on 11 and it errors out about the > include c++/v1/tgmath.h not being found on 8.4 and 9.3. It is on FreeBSD 10 and just includes complex.h and math.h files, because of using modified tgmath.h from FreeBSD causes errors. The solution is simple: replacing of tgmath.h include to complex.h and math.h includes. (In reply to Guido Falsi from comment #10) > I "solved" this with the attached patch, forcing the port to use clang 3.5 > on 11 and making the sed for the tgmath.h include conditional. > > After this the port is failing again on 9.3 with clang crashing. I'll attach > logs of the crashes. The mentioned logs shows critical error (which causes core dump) for FreeBSD LLVM/Clang 3.4.1 version. The 3.4.1 version used on 9.3, 10.1 and 11-current versions by default. The 3.5.0 version wasn't released on date (4 Sep 2014), when Oolite 1.80 released (1 July 2014) and it isn't default on current FreeBSD versions. The FreeBSD 10 logs shows successful build on FreeBSD 10 LLVM/Clang 3.3 default compiler. Therefore, I propose to use conditional dependency on LLVM/Clang 3.3 port's version for FreeBSD versions before 10 and after 10.1 (inclusively).
Created attachment 148288 [details] The poudriere testport log (FreeBSD 9.3 amd64)
Created attachment 148289 [details] The poudriere testport log (FreeBSD 10 amd64)
Strange enough I have had it failing anyway with clang33 on 8.4 and 9.3, maybe we are doing something different, can you send me the diff from svn of thee port you have tested with? Thanks.
(In reply to Guido Falsi from comment #17) > Strange enough I have had it failing anyway with clang33 on 8.4 and 9.3, > maybe we are doing something different, can you send me the diff from svn of > thee port you have tested with? On your log for 8.4 you had tgmath.h error, which is related to modified /usr/include/tgmath.h from FreeBSD. The /usr/include/c++/v1/tgmath.h (on FreeBSD 10), which I used, just includes complex.h and math.h files. Therefore, I fixed it by direct patch method, instead of using sed for c++/v1/tgmath.h. On your log for 9.3 you had LLVM/Clang 3.4.1 error, which is fixed by using previous LLVM/Clang 3.3 stable compiler. I attached mentioned difference file into comment #14. The new poudriere testport logs is for newer proposed patch version. The difference between old and new proposed patches is here: https://bugs.freebsd.org/bugzilla/attachment.cgi?oldid=147660&action=interdiff&newid=148287&headers=1
Created attachment 148367 [details] Newer patch Hi again, I've found out another occurrence of tgmath.h in an header file, I added the patch file patch-src_Core_OOPointMaths.h. I also narrowed down a little the versison checks: Minimum set to 1000100, 10.0 version branch, since there isn't a bump for when clang 3.3 was imported. Maximum set to 1000703, when clang 3.4 was imported. With this patch I've been able to build it on 8.4, 9.3, 10.0 and head in poudriere. I think this patch is ready to be committed, I'd like one final approval for this patch from maintainer. Thanks in advance.
Created attachment 148368 [details] Proposed patch (since 367888 revision) I tested building Oolite port on LLVM/Clang v3.2, v3.3 and v3.4.2 from lang/clang32, lang/clang33 and lang/clang34 ports, respectively. Also, Guido Falsi tested 3.5.0 version from lang/clang35 port. They are unaffected to bug of LLVM/Clang v3.4.1, which is used by default on FreeBSD 9.3-RELEASE, 10.1-RC2 and 11-CURRENT. Therefore, I propose to check Clang 3.4.1 version and use other tested Clang compiler in this case, while there is no patch or new Oolite version, which fixes this issue. (In reply to Guido Falsi from comment #19) > I've found out another occurrence of tgmath.h in an header file, I added the > patch file patch-src_Core_OOPointMaths.h. Also included in new attached patch. Thanks.
(In reply to lightside from comment #20) > Created attachment 148368 [details] > Proposed patch (since 367888 revision) > > I tested building Oolite port on LLVM/Clang v3.2, v3.3 and v3.4.2 from > lang/clang32, lang/clang33 and lang/clang34 ports, respectively. Also, Guido > Falsi tested 3.5.0 version from lang/clang35 port. They are unaffected to > bug of LLVM/Clang v3.4.1, which is used by default on FreeBSD 9.3-RELEASE, > 10.1-RC2 and 11-CURRENT. clang 3.5 did give me some problems in 9.x and 8.x, so I discadrded it in favor of 3.3. If lang/clang34 works on all versions I think using that would be better. I'll test that case too.
(In reply to Guido Falsi from comment #21) > (In reply to lightside from comment #20) > > Created attachment 148368 [details] > > Proposed patch (since 367888 revision) > > > > I tested building Oolite port on LLVM/Clang v3.2, v3.3 and v3.4.2 from > > lang/clang32, lang/clang33 and lang/clang34 ports, respectively. Also, Guido > > Falsi tested 3.5.0 version from lang/clang35 port. They are unaffected to > > bug of LLVM/Clang v3.4.1, which is used by default on FreeBSD 9.3-RELEASE, > > 10.1-RC2 and 11-CURRENT. > > clang 3.5 did give me some problems in 9.x and 8.x, so I discadrded it in > favor of 3.3. > > If lang/clang34 works on all versions I think using that would be better. > I'll test that case too. After some more tests I can confirm clang33 is the only one working in all releases, so your patch in comment #20 is the final one I'd like to commit.
Created attachment 148391 [details] oolite.diff While testing other ports I found out the CCVERSION assignment causes problems on 8.4 when clang33 isn't present. In that case the second part of the conditional isn't evaluated, so the execution of ${CC} --version is deferred and is executed later when CC has already been set to the new value. This causes this error in poudriere: ====>> Calculating ports order and dependencies ====>> Sanity checking the repository /usr/local/bin/clang33: not found "Makefile", line 55: warning: "/usr/local/bin/clang33 --version" returned non-zero status /usr/local/bin/clang33: not found "Makefile", line 55: warning: "/usr/local/bin/clang33 --version" returned non-zero status /usr/local/bin/clang33: not found "Makefile", line 55: warning: "/usr/local/bin/clang33 --version" returned non-zero status /usr/local/bin/clang33: not found "Makefile", line 55: warning: "/usr/local/bin/clang33 --version" returned non-zero status I fixed this adding a conditional to avoid that assignment on 8.4. Attaching new patch.
Created attachment 148444 [details] Proposed patch After more testing on current FreeBSD release versions, I found different approach. It checks "/usr/bin/clang" and uses it, instead of depending on ${CC} changes across port's system stages.
Created attachment 148445 [details] The poudriere testport log (FreeBSD 8.4 amd64)
Created attachment 148446 [details] The poudriere testport log (FreeBSD 9.3 amd64)
Created attachment 148447 [details] The poudriere testport log (FreeBSD 10 amd64)
(In reply to lightside from comment #24) > Created attachment 148444 [details] > Proposed patch > > After more testing on current FreeBSD release versions, I found different > approach. It checks "/usr/bin/clang" and uses it, instead of depending on > ${CC} changes across port's system stages. I sincerely do not like this convoluted "include" hack, I'd rather have a linear simple if logic in a block in the main Makefile, makes more sense. Can't you rework it in the main makefile as a simpler if/else structure?
Created attachment 148477 [details] Proposed patch (In reply to Guido Falsi from comment #28) > I sincerely do not like this convoluted "include" hack This is not a "hack". This is a "suggestion" for such a USES file, which could determine full Clang version. The current Mk/Uses/compiler.mk works for MAJOR.MINOR, but not for MAJOR.MINOR.PATCH versions. But anyway, the changed file attached.
(In reply to lightside from comment #29) > Created attachment 148477 [details] > Proposed patch > > (In reply to Guido Falsi from comment #28) > > I sincerely do not like this convoluted "include" hack > > This is not a "hack". This is a "suggestion" for such a USES file, which > could determine full Clang version. The current Mk/Uses/compiler.mk works > for MAJOR.MINOR, but not for MAJOR.MINOR.PATCH versions. > I did not understand this, anyway this is a bug report about oolite, so patches sent here should only address the problem. If you want to propose a change to compiler.mk you should file a specific PR with reasons why it should be changed. Before committing such a change an exprun will most probably be required. Also, just changing the COMPILER_VERSION variable to having three numbers in it isn't viable, you should provide compatibility for ports expecting it to be a two number value, or provide patches for all ports expecting that. Anyway such an infrastructure change requires it's own PR since a lot of other problems need to be addressed which are definitely out of scope here. Let's concentrate on the problem at hand and produce a solution to make this one port work. The solution can be generalized later if needed. > But anyway, the changed file attached. I['m seeing some new strange errors with this patch. I'm not sure why. I'm investigating. Using variables with the same names as the ones in compiler.mk in the port makefile could cause strange problems though, since you risk polluting the uses "namespace". It's always advisable to try to keep variables names unique. My preference is to find a simple solution to fix this one port for all supported releases. I'm not sure if this is the cause of these strange errors though, I still have to look better into it.
(In reply to Guido Falsi from comment #30) > I did not understand this The USES is just a different way to include *.mk files. It was the same in my case. (In reply to Guido Falsi from comment #30) > If you want to propose a change to compiler.mk you should file a specific PR with reasons why it should be changed. I didn't propose such changes to top level make files, but use the modified part of them to solve the build. (In reply to Guido Falsi from comment #30) > Using variables with the same names as the ones in compiler.mk in the port makefile could cause strange problems though I don't use "USES+= compiler" in this case, therefore there is no error. (In reply to Guido Falsi from comment #30) > Let's concentrate on the problem at hand and produce a solution to make this one port work. From my side, this port update is already done for current FreeBSD release versions (8.4, 9.3, 10). At least, this is what poudriere testport logs shows. You already have the solution with the same result also.
Created attachment 148551 [details] Proposed patch (since 371287 revision) Created patch for changes after 371287 revision.
Created attachment 148552 [details] Proposed patch (since 371287 revision) Sorted USES.
Created attachment 148570 [details] Proposed patch (since 371287 revision) I found possible cause of error for LLVM/Clang 3.4.1 compiler, related to OOJavaScriptEngine.m file. It was optimization for speed (e.g. -O2, -O3). I fixed it by using optimization for size (-Os). The -O1 is also works. The new patch attached.
Created attachment 148571 [details] The poudriere testport log (FreeBSD 9.3 amd64)
(In reply to lightside from comment #34) > Created attachment 148570 [details] > Proposed patch (since 371287 revision) > > I found possible cause of error for LLVM/Clang 3.4.1 compiler, related to > OOJavaScriptEngine.m file. It was optimization for speed (e.g. -O2, -O3). I > fixed it by using optimization for size (-Os). The -O1 is also works. > > The new patch attached. Sorry for my delay in answering. This looks like a much better and cleaner fix. I'm testing it and will report back soon.
A commit references this bug: Author: madpilot Date: Thu Oct 23 11:52:13 UTC 2014 New revision: 371389 URL: https://svnweb.freebsd.org/changeset/ports/371389 Log: - Update to 1.80 - Add LICENSE information - Convert to using PLIST_FILES, PORTDOCS, PORTDATA - Use external minizip - Force space optimization (in place of -O2) when compiling with clang 3.4.1 to avoid build problems PR: 193886 Submitted by: lightside <lightside at gmx.com> Approved by: avg@ (maintainer) Changes: head/games/oolite/Makefile head/games/oolite/distinfo head/games/oolite/files/oolite.in head/games/oolite/files/patch-GNUmakefile head/games/oolite/files/patch-deps_Cross-platform-deps_mozilla_js_src_config_system-headers head/games/oolite/files/patch-deps_mozilla-bug771281 head/games/oolite/files/patch-deps_mozilla_js_src_configure head/games/oolite/files/patch-deps_mozilla_js_src_configure.in head/games/oolite/files/patch-deps_mozilla_js_src_jsscript.h head/games/oolite/files/patch-deps_mozilla_js_src_jsstr.cpp head/games/oolite/files/patch-src_Core_NSFileManagerOOExtensions.h head/games/oolite/files/patch-src_Core_OOMaths.h head/games/oolite/files/patch-src_Core_OOOXZManager.m head/games/oolite/files/patch-src_Core_OOPointMaths.h head/games/oolite/files/pkg-message.in head/games/oolite/pkg-descr head/games/oolite/pkg-plist
I committed your last patch. Thanks! I only changed it to use clang 3.4.2, affter some testing. I did this because I noticed that the default compiler for openstep has been changed to clang 3.4.2 recently [1], so it looks reasonable. [1] http://svnweb.freebsd.org/ports?view=revision&revision=371153