AMD64 log: https://pkg-status.freebsd.org/beefy18/data/main-amd64-default/p8e4d68e48915_sb6d3a9644d/logs/mame-0.254.log i386 log: https://pkg-status.freebsd.org/beefy17/data/main-i386-default/p8e4d68e48915_sb6d3a9644d/logs/mame-0.254.log
that code is crashing clang itself
Also had build problems on armv7: mame is now too large to link with lld on systems with a 32 bit address space.
(In reply to Mina Galić from comment #2) > that code is crashing clang itself (In reply to Robert Clausecker from comment #3) > Also had build problems on armv7: mame is now too large to link with lld on systems with a 32 bit address space. My gosh, none of this sounds exciting at all. Someone from some ${LINUX_DISTRO} community contacted me sometime ago about Mame no longer running as expected on 32bit ${LINUX_DISTRO}. Is Mame on 32bit just broken during build, or does it run OK? Mame on amd64-14-CURRENT builds on my host if I set the number of builds to 1, have tested this with poudriere-testport. My thoughts at the moment. Bisect the build environment in the jail guest, attempting to locate a commit related to the build breakage. I have no other ideas at the moment, sorry.
s/My thoughts/My plans/
Possible fixes for the 32 bit link problems: - check if it links with the BFD or Gold linkers instead of LLD - compile with -Os to reduce the binary size a little - if present, remove -g from 32 bit builds to reduce the binary size (even if we strip afterwards, the linker has to deal with debug symbols) - hack the build system to perform an incremental link
(In reply to Robert Clausecker from comment #6) Awesome advice, thanks fuz.
Took a crack at fixing the linking problem on armv7 today. I tried to use the bfd and gold linkers, but neither reduced memory usage, even with all memory saving options enabled. Even compiling with -Os didn't help. Weirdly enough, lld crashes at 2 GB of memory usage. I thought armv7 programs can use 4 GB of virtual address space on arm64 hosts, but it seems like I'm wrong. What finally did the trick was passing LDFLAGS=-s to the makefile, stripping the binary as it was linked. Another solution I had so far not checked might be to unbundle bundled dependencies, reducing the total amount of text and data involved in the link. Note also that as of now, mame ignores CC and CXX passed in from the host and instead hard codes to cc and c++. This is wrong. I've a patch ready that should address this.
Created attachment 242226 [details] emulators/mame: fix build on armv7 The attached patch fixes the build on armv7 by telling the linker not to generate any symbols. As we strip during install anyway, this has no effect on the generated binary. While we are at it, also ensure to obey ${CC} and ${CXX}.
(In reply to Robert Clausecker from comment #9) Thanks Robert. The patch did not cause any breakages on {amd64,i386}-{12.4,13.1,13.2}. I am trying to remove all of the CC ${REINPLACE_CMD} patching in the Makefile, as well as getting the port to use un-bundled libraries. No progress on 14-CURRENT.
(In reply to Alastair Hogge from comment #10) Thanks. If you like, I can go ahead and commit just the LDFLAGS+=-s bit, leaving the ${CC} and so on patching to a future patch for you. Would you also like this change to be MFH'ed?
(In reply to Robert Clausecker from comment #11) Please commit the entire patch as is. Yes, MFH would be awesome, please do. Thanking you very much.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=9fb2d7b2e7473d6bf8bb18d5e457ed4c7ac933df commit 9fb2d7b2e7473d6bf8bb18d5e457ed4c7ac933df Author: Robert Clausecker <fuz@FreeBSD.org> AuthorDate: 2023-05-16 23:36:32 +0000 Commit: Robert Clausecker <fuz@FreeBSD.org> CommitDate: 2023-05-18 08:19:48 +0000 emulators/mame: fix build on armv7 Mame fails to link on armv7 due to an address space exhaustion in lld. As a bandaid, tell lld to not generate symbols. This frees up enough address space to make mame link. PR: 271374 MFH: 2023Q2 Approved by: agh@riseup.net (maintainer) emulators/mame/Makefile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
A commit in branch 2023Q2 references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=9064a6d5dadb703432cccee830b381102205b340 commit 9064a6d5dadb703432cccee830b381102205b340 Author: Robert Clausecker <fuz@FreeBSD.org> AuthorDate: 2023-05-16 23:36:32 +0000 Commit: Robert Clausecker <fuz@FreeBSD.org> CommitDate: 2023-05-18 08:22:19 +0000 emulators/mame: fix build on armv7 Mame fails to link on armv7 due to an address space exhaustion in lld. As a bandaid, tell lld to not generate symbols. This frees up enough address space to make mame link. PR: 271374 MFH: 2023Q2 Approved by: agh@riseup.net (maintainer) (cherry picked from commit 9fb2d7b2e7473d6bf8bb18d5e457ed4c7ac933df) emulators/mame/Makefile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
Leaving this one open as the main issue hasn't been addressed yet.
Please don't assign this to me; the patch I submitted fixes a probably unrelated problem and I am not able to make inroads into the main problem as I don't run CURRENT.
I have a patch that is currently being tested with poudriere.
I thought I had made some progress with cleaning up the Port makefile, specifically tool chain related maintenance, however, Clang is still crashing. The builds with Clang have been progressively difficult to succeed. It is difficult to determine the cause, upstream is aware of cyclic build dependencies, the limitations of the pre-build system GENie, consistent races between generated code and building that code, progress is made addressing these. I do not think building with only one make job as an alternative to avoiding crashes work anymore. It is becoming difficult or impossible now to test build, on multiple platforms and releases at once, with Clang. I used to be able to run 6 x 4 builder poudriere-testport's, now I might be able to get two if I stagger the start. On a AMD Ryzen 9 3950X 16-Core 64GiB of system memory. Everything is built tmpfs. I recall the GCC builds using less memory and will look at those again.
(In reply to Alastair Hogge from comment #18) Note also that the default clang shipped with CURRENT is known to have some bugs. As a workaround, you could try to select a different clang version by adding e.g. USES= llvm:13,build to build with clang 13. You could also try to build with gcc.
Great ideas, thanks. I will get back around to MAME, but it is time to update the system, and local packages.
So, it might be the Qt interactive low-level machine debugger that is causing the problems for Clang on 14-CURRENT. In my WIP I have made it an option. GCC-{10,11,12} are able to build MAME + Qt Debugger on 14-CURRENT, Clang is not.
(In reply to Alastair Hogge from comment #21) You could make it an option and just change the default options depending on OSVERSION. Or you could build with gcc depending on OSVERSION. Or you could do USES=llvm to build with a clang that is known not to be defective.
It looks like LLVM15 is causing issues, tho, I have yet to figure out how MAME selects the LLVM toolchain, because it always picks /usr/bin/c++.
I missed the the ARM64 breakage: https://pkg-status.freebsd.org/ampere2/data/main-arm64-default/peb1aa9292385_sb7afaf8a41/logs/mame-0.254_1.log I have MAME building with base Clang-15 on CURRENT again, with -j16, however, poudriere tests are still failing for CURRENT.
Created attachment 242434 [details] mame-0.254-WIP-makefile-maintenance-build-on-current.diff In an attempt to fix building on 14-CURRENT, I cleaned up the Makefile while going thru the available options in MAME to explicitly rely on libraries provided by Ports, and making correct use of gmake variables declared in MAME for users. Port Makefile maintenance: ∙ explicitly declare more system libraries ∙ port-{clippy,fmt,lint} maintenance ∙ separate out Qt machine debugger via OPTIONS ∙ remove licenses from ${PORTDOCS} as they are catalogued thru various ${LICENSE}* definitions ∙ add conditional logic for when OS ≧ 14 then explicitly build with LLVM ∙ a lot of the work in the post-patch target is on static files, so move most of this to files/ ∙ files/patch-scripts_genie.lua is a fix for GCC I do not like the explicit LLVM dependency, however, I do not have anything better at the moment, sorry.
(In reply to Alastair Hogge from comment #25) Looks cool! Here are some things I found: - do-install-DEBUG-on and do-install-DEBUG-off do the same thing and can be merged; just put the commands into do-install - I recommend that you keep the mention of armv7 in the comment above LDFLAGS. Else a future maintainer might test on amd64 and find that it is not needed, breaking the port once more (you won't believe how often that happens). - the conditionals for the QTDEBUGGER option are commented out. Is this correct? You can rewrite them like such: QTDEBUGGER_MAKE_ENV= USE_QTDEBUG=1 QTDEBUGGER_USES= qt:5 QTDEBUGGER_USE= qt=core,gui,widgets,buildtools:build,qmake:build - in OVERRIDE_CC for FreeBSD > 14, you should not use COMPILER_TYPE as it could be gcc. Just write e.g. clang++${LLVM_VERSION}. - check if you can use DISTVERSION instead of PORTVERSION as per policy - would it make sense to make TOOLS=1 a port option so people can get a leaner mame if they want?
(In reply to Robert Clausecker from comment #26) > Looks cool! Here are some things I found: > > - do-install-DEBUG-on and do-install-DEBUG-off do the same thing and can be merged; just put the commands into do-install Are you sure? I am not. MAME builds "mamed" with DEBUG, and "mame" without DEBUG. Do you think we should just install with the d suffix? > - I recommend that you keep the mention of armv7 in the comment above LDFLAGS. > Else a future maintainer might test on amd64 and find that it is not needed, > breaking the port once more (you won't believe how often that happens). Ohh good point. Will add/restore. Thanks. > - the conditionals for the QTDEBUGGER option are commented out. Yeah, the patch is still WIP (Work In Progress), and I was lazy and left some crumbs lying around, my bad. > Is this correct? You can rewrite them like such: > > QTDEBUGGER_MAKE_ENV= USE_QTDEBUG=1 > QTDEBUGGER_USES= qt:5 > QTDEBUGGER_USE= qt=core,gui,widgets,buildtools:build,qmake:build Oh that is that far more elegant, totally happening, thanks. > - in OVERRIDE_CC for FreeBSD > 14, you should not use COMPILER_TYPE as it could > be gcc. Just write e.g. clang++${LLVM_VERSION}. Ah yes, thanks, again more crumbs. > - check if you can use DISTVERSION instead of PORTVERSION as per policy Good idea, will do. > - would it make sense to make TOOLS=1 a port option so people can get a leaner mame if they want? Farken oath I think it would (I think it is only 3 more binaries), great idea, totally happening. Thanks.
Regarding the DEBUG build, I am picking up what you are putting down now. Will move to do-install target, thanks.
(In reply to Alastair Hogge from comment #28) Oh I totally overlooked that the binary name is different. I see why you had two different targets. Carry on then, nothing to see here.
Created attachment 242488 [details] mame-0.254-WIP-makefile-maintenance-build-on-current.diff I thought I might have been able to collapse the two DEBUG install targets into one, by using a regex on the mame/d binary, tho, I am not sure if that is possible, I am sure of one thing, my regex-fu is of n00b level, and I was not able to pull off the correct incantation. The TOOLS and QTDEBUGGER do not really shave much size of MAME. I am waiting on the amd64 builds to complete with poudriere-testport, then I can move to the i386 builds.
Looks good! Will commit with my next batch.
If I understand your patch correctly, you still unconditionally install Makefiles for the tools, even if the TOOLS option is off. Is this correct and intended?
(In reply to Robert Clausecker from comment #31) I have an update still to upload here that includes an updated .ini. (In reply to Robert Clausecker from comment #32) Ah yes the man pages. Good catch. Will sort that out. Thanks
Created attachment 242495 [details] Makefile maintenance; attempt to restore build on 14 CURRENT
Thanks for the update. I think the mame.ini sample file is small enough that no option is needed, but it's okay to have one. It would be great if you could sort the make targets by roughly the order they run in (so patch, configure, build, install). You can probably remove the explicit GH_PROJECT now that there are no slave ports. Rest looks pretty good!
One last issue I found: Your new state drops USE_SYSTEM_LIB_PUGIXML=1 from MAKE_ARGS, causing pugixml to no longer be unbundled. Yet you still depend on the package. Is this intentional?
(In reply to Robert Clausecker from comment #35) > It would be great if you could sort the make targets by roughly the order they run in (so patch, configure, build, install). Will do. (In reply to Robert Clausecker from comment #36) > One last issue I found: Your new state drops USE_SYSTEM_LIB_PUGIXML=1 from MAKE_ARGS, causing pugixml to no longer be unbundled. Yet you still depend on the package. Is this intentional? Complete mistake, tho, I wonder how long it has been missing for, and why poudriere-testport did not flag that...or maybe it did. Will restore
Created attachment 242512 [details] Makefile maintenance; attempt to restore build on 14 CURRENT Hey Robert, I think have addressed your current criticism: • Remove option EXAMPLES and track mame.ini via @sample in pkg-plist • Restore PUGIXML external dependency • Remove ${GH_PROJECT} • Sort Makefile targets according to order of use (https://docs.freebsd.org/en/books/porters-handbook/book/#options-targets) Ta very much.
Created attachment 242514 [details] Makefile maintenance; attempt to restore build on 14 CURRENT Add two more .ini configuration files.
Cool! Will commit this one with my next batch. I also found that the binary size can be significantly reduced on ARM by compiling with CXXFLAGS+=-mthumb, but that may be something to explore in another patch.
(In reply to Robert Clausecker from comment #40) > Cool! Will commit this one with my next batch. [and there was much rejoicing] > I also found that the binary size can be significantly reduced on ARM by compiling with CXXFLAGS+=-mthumb, but that may be something to explore in another patch. Oh nice, this is very exciting,and am happy to explore anytime from now.
Created attachment 242515 [details] Makefile maintenance; attempt to restore build on 14 CURRENT Move '${MKDIR} ${STAGEDIR}${ETCDIR}' to above the .for loop, to remove being in the scope of the .for loop.
For what it is worth, MAME is now at 0.255.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/ports/commit/?id=84c2e2313e7098468fc7e99ef95527903a7bc0b5 commit 84c2e2313e7098468fc7e99ef95527903a7bc0b5 Author: Alastair Hogge <agh@riseup.net> AuthorDate: 2023-05-31 08:28:33 +0000 Commit: Robert Clausecker <fuz@FreeBSD.org> CommitDate: 2023-06-01 22:29:21 +0000 emulators/mame: fix build on 14-CURRENT - explicitly declare more system libraries - port-{clippy,fmt,lint} maintenance - separate out Qt machine debugger via OPTIONS - separate out tools via OPTIONS - remove licenses from ${PORTDOCS} as they are catalogued thru various ${LICENSE}* definitions - add conditional logic for when OS ≧ 14 then explicitly build with LLVM - a lot of the work in the post-patch target is on static files, so move most of this to files/ - files/patch-scripts_genie.lua is a fix for GCC - sort targets into the order they run in. PR: 271374 emulators/mame/Makefile | 233 +++++++++++---------- .../mame/files/{target.ini.in => mame.ini.in} | 57 +++-- ...arty_genie_build_gmake.freebsd_genie.make (new) | 63 ++++++ .../patch-3rdparty_genie_src_host_scripts.c (new) | 16 ++ .../patch-3rdparty_genie_src_tools_gcc.lua (new) | 15 ++ ...3rdparty_genie_tests_test__gmake__cpp.lua (new) | 15 ++ emulators/mame/files/patch-makefile | 29 ++- emulators/mame/files/patch-scripts_genie.lua (new) | 14 ++ emulators/mame/files/patch-scripts_toolchain.lua | 4 +- emulators/mame/files/pkg-message.in | 15 +- emulators/mame/files/plugin.ini.in (new) | 20 ++ emulators/mame/files/ui.ini.in (new) | 71 +++++++ emulators/mame/pkg-plist | 59 +++--- 13 files changed, 435 insertions(+), 176 deletions(-)
Committed your patch! Please submit the update to 0.255 in a separate bug report.
(In reply to Robert Clausecker from comment #45) Thank you; will do.