Bug 280205 - emulators/86Box: New port: an emulator of x86-based machines
Summary: emulators/86Box: New port: an emulator of x86-based machines
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: amd64 Any
: --- Affects Some People
Assignee: Vladimir Druzenko
URL: https://86box.net/
Keywords:
Depends on:
Blocks:
 
Reported: 2024-07-09 11:50 UTC by gatekeeper
Modified: 2024-08-03 12:28 UTC (History)
4 users (show)

See Also:


Attachments
git patch (2.63 KB, patch)
2024-07-09 11:50 UTC, gatekeeper
no flags Details | Diff
git patch (4.47 KB, patch)
2024-07-15 05:59 UTC, gatekeeper
no flags Details | Diff
git patch (4.36 KB, patch)
2024-07-15 06:12 UTC, gatekeeper
no flags Details | Diff
git patch (3.59 KB, patch)
2024-07-15 17:20 UTC, gatekeeper
no flags Details | Diff
git patch (3.60 KB, patch)
2024-07-25 21:57 UTC, gatekeeper
no flags Details | Diff
git patch (8.11 KB, patch)
2024-07-26 19:23 UTC, gatekeeper
no flags Details | Diff
git patch (8.11 KB, patch)
2024-07-26 19:25 UTC, gatekeeper
no flags Details | Diff
git patch (9.88 KB, patch)
2024-08-01 23:44 UTC, gatekeeper
no flags Details | Diff
v1 (with WAYLAND option) (2.01 KB, text/plain)
2024-08-02 02:03 UTC, Vladimir Druzenko
vvd: maintainer-approval?
Details
git patch (11.05 KB, patch)
2024-08-02 11:50 UTC, gatekeeper
no flags Details | Diff
git patch (10.20 KB, patch)
2024-08-03 02:19 UTC, gatekeeper
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description gatekeeper 2024-07-09 11:50:43 UTC
Created attachment 251949 [details]
git patch

Hi. I would like to contribute a port for the 86Box program (see attachment).
Link to project website: https://86box.net/
Thanks!
Comment 1 commit-hook freebsd_committer freebsd_triage 2024-07-13 08:59:50 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=89914c4ba9cbc18aeffd353473903426fafa3351

commit 89914c4ba9cbc18aeffd353473903426fafa3351
Author:     Jochen Neumeister <joneum@FreeBSD.org>
AuthorDate: 2024-07-13 08:58:26 +0000
Commit:     Jochen Neumeister <joneum@FreeBSD.org>
CommitDate: 2024-07-13 08:59:25 +0000

    www/freenginx-devel: Fix WWW

    PR:     280205
    Sponsored by:   Netzkommune GmbH

 www/freenginx-devel/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Comment 2 Joel Bodenmann freebsd_committer freebsd_triage 2024-07-13 15:58:39 UTC
For historians: The commit that got linked into this PR appears to be unrelated (incorrect PR number in the commit message).
Comment 3 Joel Bodenmann freebsd_committer freebsd_triage 2024-07-13 16:08:08 UTC
I only had a quick look at this so far:
  - Drop the 'v' prefix in DISTVERSION and use DISTVERSIONPREFIX instead
  - For "static lists", use only one BUILD_DEPENDS= and LIB_DEPENDS=, then just backslash for multi-line.
  - Is manual setting of CMAKE_INSTALL_PREFIX really necessary?

Also, have you successfully completed poudriere-testport on this for all supported FreeBSD releases?

More things will probably follow once I get a chance to look at this more closely but it looks like a good start!
Comment 4 Daniel Engberg freebsd_committer freebsd_triage 2024-07-13 17:13:01 UTC
CMAKE_INSTALL_PREFIX is already defined here: https://cgit.freebsd.org/ports/tree/Mk/Uses/cmake.mk#n105

Move to QT6 as QT5 is slowly being sunset or utilize flavors

You don't need to define the same port multiple times as a dependency (x11/libxkbcommon)

Some dependencies seems to differ between AUR (Arch and Gentoo) so you might want to double check and also CMAKE options.
Comment 5 gatekeeper 2024-07-14 15:18:12 UTC
OK, awesome - thank you for the comments. Let me address these and I will come back with an updated patch :-)
Comment 6 gatekeeper 2024-07-15 05:59:19 UTC
Created attachment 252055 [details]
git patch

Hi. Thank you for the review. I have managed to fix the problem with QT6, and made the changes to use flavors two flavors (qt5 and qt6).

I have dropped the v and now use DISTVERSION.

Also, BUILD_DEPENDS and LIB_DEPENDS are now a multi-line "single line" (using backslashes).

I have also completed the testport for both flavors and for FreeBSD 14.1 and 14.0. Unfortunately, for FreeBSD 13.3, after about 10 hours, it is still running for qt5 flavor, compiling llvm15; don't know why poudriere did not pull this package directly from the repository :-(
I am uploading the updated patch now since, however, I do not think that compiling for 13.3 should be a problem - I will report back once the compilation for both flavors is finished for 13.3

Regarding CMAKE_INSTALL_PREFIX, if I do not set the variable, I get a compiler error (see below). I am not sure how to solve this issue otherwise, i.e. without setting the variable, as I have tried several options. Any help here would be greatly appreciated :-)

===>   Generating temporary packing list
[  0% 1/1] cd /wrkdirs/usr/ports/emulators/86Box/work-qt5/.build && /usr/local/bin/cmake -DCMAKE_INSTALL_DO_STRIP=1 -P cmake_install.cmake
-- Install configuration: "Release"
-- Installing: /usr/local/bin/86Box
CMake Error at src/cmake_install.cmake:67 (file):
  file INSTALL cannot copy file
  "/wrkdirs/usr/ports/emulators/86Box/work-qt5/.build/src/86Box" to
  "/usr/local/bin/86Box": Permission denied.
Call Stack (most recent call first):
  cmake_install.cmake:42 (include)


FAILED: CMakeFiles/install/strip.util 
cd /wrkdirs/usr/ports/emulators/86Box/work-qt5/.build && /usr/local/bin/cmake -DCMAKE_INSTALL_DO_STRIP=1 -P cmake_install.cmake
ninja: build stopped: subcommand failed.
Comment 7 gatekeeper 2024-07-15 06:12:39 UTC
Created attachment 252057 [details]
git patch

Sorry, my bad; I should have "portlinted" before uploading.

This new patch contains the fixes necessary to make portlint happy.
As I wrote before, I will report back later in the day about the running on all supported platforms.

Also, I might want to do a better fix on the patch - instead of just commenting the code, I realized that I can just write if(0), which is much more clean.

NOTE - the patch is necessary for Qt6, otherwise the program crashes at start. I confirmed that with the patch applied, it works correctly on both Qt5 and Qt6.
Comment 8 gatekeeper 2024-07-15 17:20:56 UTC
Created attachment 252084 [details]
git patch

Hi.
Now, I can confirm that the proposed patch compiles on all three major (current) releases, in both flavors (qt5 and qt6). I have also simplified the required patch and tested that it is working. Portlint also says that everything "looks fine" :-)
Comment 9 gatekeeper 2024-07-19 21:49:10 UTC
Hi. Any further comments or suggestions, stuff that I should do/look at, or can the current patch be approved?
Thanks :-)
Comment 10 Joel Bodenmann freebsd_committer freebsd_triage 2024-07-23 15:51:11 UTC
Sorry - I'm extremely busy until next week.
If somebody else wants to pick it up that's completely fine from my side.
Comment 11 gatekeeper 2024-07-25 21:57:48 UTC
Created attachment 252288 [details]
git patch

Updates:
  - make the qt6 option default
  - removed ${PREFIX} from PLIST_FILES
  - changes to make portfmt and portlint happy
Comment 12 Daniel Engberg freebsd_committer freebsd_triage 2024-07-26 00:00:12 UTC
A few notes,

Sort FLAVORS=

I would argue that portfmt is incorrect and that USE_XORG, USE_GNOME and USE_SDL should be placed directly under USES=, sorted and in the same section

USES=...
USE_GNOME
USE_SDL
USE_XORG

NLS_* - Missing (menu) OPTION?

Perhaps CMAKE_ARGS= -DUSE_QT6=ON can be changed to CMAKE_ON= USE_QT6 ?

CMAKE_INSTALL_PREFIX --> https://cgit.freebsd.org/ports/tree/Mk/Uses/cmake.mk#n105 (remove it)

Is CFLAGS+= -I${LOCALBASE}/include/freetype2 needed?

If these are added to C/CXXFLAGS they should go (be removed),
https://github.com/86Box/86Box/blob/master/cmake/flags-gcc-aarch64.cmake
https://github.com/86Box/86Box/blob/master/cmake/flags-gcc-armv7.cmake
https://github.com/86Box/86Box/blob/master/cmake/flags-gcc-x86_64.cmake (possibly keep -mstackrealign)
Ditch https://github.com/86Box/86Box/blob/master/cmake/flags-gcc.cmake#L19
See https://docs.freebsd.org/en/books/porters-handbook/book/#dads-cflags

Keep up the good work! :)

Best regards,
Daniel
Comment 13 gatekeeper 2024-07-26 19:23:11 UTC
Created attachment 252306 [details]
git patch

(In reply to Daniel Engberg from comment #12)

Thanks a lot for all the comments.

Changes in this patch:
  - sorted FLAVORS, but kept default at qt6
  - added the USES_* sorted after USES=
  - Use CMAKE_ON= USE_QT6
  - The CFLAGS+=... was really necessary, but I now patched src/print/CMakeList.txt so that this is no longer necessary and no compilation error occurs
  - Removed optimization flags from .cmake files, as suggested

What is still not really clear to me is how to improve the Makefile for aarch64 etc; From what I can understand, it _should_ be possible to compile under these architectures, but I cannot test this now; therefore I restricted to amd64... :-(
Comment 14 gatekeeper 2024-07-26 19:25:48 UTC
Created attachment 252307 [details]
git patch

Re-uploading the patch, as something went wrong with the previous file.
This one should be fine...
Comment 15 gatekeeper 2024-07-26 19:37:24 UTC
(In reply to Daniel Engberg from comment #12)

For the CMAKE_INSTALL_PREFIX, I do not know why, but if I remove it, I get an with poudriere (see below). Any suggestions how to fix the problem introduced by removing CMAKE_INSTALL_PREFIX?

--- ERROR ---
...
=======================<phase: stage          >============================
===== env: DEVELOPER_MODE=yes STRICT_DEPENDS=yes USER=nobody UID=65534 GID=65534
===>  Staging for 86Box-qt6-4.1.1
===>   86Box-qt6-4.1.1 depends on file: /usr/local/libdata/pkgconfig/x11.pc - found
===>   86Box-qt6-4.1.1 depends on file: /usr/local/libdata/pkgconfig/xcb.pc - found
===>   86Box-qt6-4.1.1 depends on file: /usr/local/libdata/pkgconfig/xext.pc - found
===>   86Box-qt6-4.1.1 depends on file: /usr/local/libdata/pkgconfig/xi.pc - found
===>   Generating temporary packing list
[  0% 1/1] cd /wrkdirs/usr/ports/emulators/86Box/work-qt6/.build && /usr/local/bin/cmake -DCMAKE_INSTALL_DO_STRIP=1 -P cmake_install.cmake
-- Install configuration: "Release"
-- Installing: /usr/local/bin/86Box
CMake Error at src/cmake_install.cmake:67 (file):
  file INSTALL cannot copy file
  "/wrkdirs/usr/ports/emulators/86Box/work-qt6/.build/src/86Box" to
  "/usr/local/bin/86Box": Permission denied.
Call Stack (most recent call first):
  cmake_install.cmake:42 (include)


FAILED: CMakeFiles/install/strip.util 
cd /wrkdirs/usr/ports/emulators/86Box/work-qt6/.build && /usr/local/bin/cmake -DCMAKE_INSTALL_DO_STRIP=1 -P cmake_install.cmake
ninja: build stopped: subcommand failed.
*** Error code 1
Comment 16 Vladimir Druzenko freebsd_committer freebsd_triage 2024-08-01 18:01:27 UTC
Update to 4.2: https://github.com/86Box/86Box/releases/tag/v4.2
Comment 17 Vladimir Druzenko freebsd_committer freebsd_triage 2024-08-01 23:26:57 UTC
(In reply to gatekeeper from comment #15)
Can you report this to upstream?
Comment 18 gatekeeper 2024-08-01 23:44:49 UTC
Created attachment 252440 [details]
git patch

Updated to v4.2

For the CMAKE_INSTALL_PREFIX, I do not think it needs to be reported upstream because now all is working properly. I am not very happy that I did not understand why this problem occurred, but the fact of the matter is that now I do not see the problem reported in comment #15 anymore...
Comment 19 Vladimir Druzenko freebsd_committer freebsd_triage 2024-08-02 02:03:52 UTC
Created attachment 252443 [details]
v1 (with WAYLAND option)

Other files same as in your last patch.
Comment 20 gatekeeper 2024-08-02 11:50:40 UTC
Created attachment 252452 [details]
git patch

Thanks for the review.

I have reactivated the MKDIR, otherwise do-install fails installing the icons because of missing destination directories.

Features for this port:
  - version 4.2
  - desktop shortcut: place the default user-created configuration under ~/.86box.cfg
  - default flavor set to qt6
  - include support for wayland
Comment 21 Alastair Hogge 2024-08-03 00:48:10 UTC
(In reply to gatekeeper from comment #20)

Hello,

In files/86Box.desktop.in, is it possible to use the XDG environment variable (XDG_CONFIG_HOME) for the user's config over hard-coding the name? Would it also be possible to reflect that in files/patch-src_unix_assets_net.86box.86Box.desktop?
Comment 22 gatekeeper 2024-08-03 02:19:06 UTC
Created attachment 252472 [details]
git patch

Hi.
As per suggestions over a separate channel, here the changes:
 - removed BUILD_DEPENDS on freetype2
 - ${LOCALBASE} instead of ${PREFIX} for CXXFLAGS
 - removed patch for .desktop file
 - added single-line REINPLACE_CMD to .desktop file

Also, to document a decision:
Due to how 86Box handles configuration files and sets the title of the window, I have placed the configuration file, when running 86Box from the .desktop shortcut, in the file ~/.86box.cfg

If the software is run from the command-line, it will create a file 86box.cfg (not hidden) in the current directory. This seems to be a feature of 86Box.
If you want to run 86Box from the command line, while taking the configuration as set when running through the .desktop shortcut, you can still do it with the command
86Box -C ~/.86box.cfg
Comment 23 commit-hook freebsd_committer freebsd_triage 2024-08-03 11:40:21 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=af9050eeb8d0c504ca1347398222f751bab6f329

commit af9050eeb8d0c504ca1347398222f751bab6f329
Author:     gatekeeper <tiago.gasiba@gmail.com>
AuthorDate: 2024-08-03 11:34:17 +0000
Commit:     Vladimir Druzenko <vvd@FreeBSD.org>
CommitDate: 2024-08-03 11:34:17 +0000

    emulators/86Box: New port: low level x86 emulator based on PCem

    86Box is a low level x86 emulator that runs older operating systems and
    software designed for IBM PC systems and compatibles from 1981 through
    fairly recent system designs based on the PCI bus.
    https://86box.net
    https://github.com/86Box/86Box

    PR:     280205

 emulators/86Box/Makefile (new)                     | 74 ++++++++++++++++++++++
 emulators/86Box/distinfo (new)                     |  3 +
 .../patch-cmake_flags-gcc-aarch64.cmake (new)      | 10 +++
 .../files/patch-cmake_flags-gcc-armv7.cmake (new)  | 10 +++
 .../patch-cmake_flags-gcc-x86__64.cmake (new)      | 12 ++++
 .../86Box/files/patch-cmake_flags-gcc.cmake (new)  | 11 ++++
 .../files/patch-src_printer_CMakeLists.txt (new)   | 10 +++
 .../files/patch-src_qt_qt__mainwindow.cpp (new)    | 10 +++
 emulators/86Box/pkg-descr (new)                    |  3 +
 emulators/86Box/pkg-plist (new)                    | 10 +++
 emulators/Makefile                                 |  1 +
 11 files changed, 154 insertions(+)
Comment 24 Vladimir Druzenko freebsd_committer freebsd_triage 2024-08-03 11:41:00 UTC
Thanks.
Comment 25 commit-hook freebsd_committer freebsd_triage 2024-08-03 12:28:27 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=0ee95e721fa344313379c1600ad944b908f1fcb8

commit 0ee95e721fa344313379c1600ad944b908f1fcb8
Author:     Vladimir Druzenko <vvd@FreeBSD.org>
AuthorDate: 2024-08-03 12:21:22 +0000
Commit:     Vladimir Druzenko <vvd@FreeBSD.org>
CommitDate: 2024-08-03 12:21:22 +0000

    emulators/86Box: use DESKTOPDIR

    PR:     280205
    Fixes:  af9050eeb8d0 (New port: low level x86 emulator based on PCem)

 emulators/86Box/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)