net/freerdp3: new port 3.1.0 + fix net/remmina to build with installed net/freerdp3

[vvd]

Summary

FreeRDP 3.1.0 released: https://github.com/FreeRDP/FreeRDP/releases/tag/3.1.0
Tested build in poudriere 13.2p9 amd64 and build+work on live system.
Added possibility to install both net/freerdp and net/freerdp3 on same host.
Also fixed build of the net/remmina with installed net/freerdp3 on live system.

[diizzy]

Why do we need both versions in tree?

net/freerdp3/Makefile

2

You might want to consider PKGNAMESUFFIX
https://docs.freebsd.org/en/books/porters-handbook/book/#porting-pkgnameprefix-suffix

7

DISTVERSION for consistency

9

Remove section?

29

Use DISTVERSION, also goes for the line below

41

+= --> =

52

Remove?

60

This is a hard requirement for amd64, just make it mandatory and remove the option and is it SSE or SSE2?

76

Use GNOME helper

128

Will fail on armv6 platforms

167

Why isn't CMAKE_BOOL used here too?

[tcberner]

There seems to be a lot of work done to not install manpages... xmlto does not look like such a heavy build-dependency that would call for that effort :)

[vvd]

net/freerdp3/Makefile

2

I can't figure out how to use it in this case.

7

ok

9

I keep it as exqmple, but can remove it.
Examples are in historey of the net/freerdp.

29

ok

41

It's inherited from net/freerdp.
But if you think this needs to be changed, then I will do so.

52

Used for testing - forgot to remove.
ok

60

SSE2.
This part is inherited from net/freerdp and appeared before I became the maintainer.
AFAIU name SSE insterad SSE2 was used for use SSE_DESC from Mk/bsd.options.desc.mk.

76

This part is inherited from net/freerdp.
But I'll change it - already found examples.

128

I don't know correct way - this part is inherited from net/freerdp.

167

I don't know - this part is inherited from net/freerdp.

[vvd]

In D43127#982976, @tcberner wrote:

There seems to be a lot of work done to not install manpages... xmlto does not look like such a heavy build-dependency that would call for that effort :)

?
2 lines only:

MANPAGES_BUILD_DEPENDS= xmlto:textproc/xmlto
MANPAGES_CMAKE_BOOL=    WITH_MANPAGES

A lot of work for install net/freerdp and net/freerdp3 on one host.

[vvd]

In D43127#982922, @diizzy wrote:

Why do we need both versions in tree?

Consumers doesn't support FreeRDP 3.0.0 yet:
net/remmina: https://gitlab.com/Remmina/Remmina/-/merge_requests/2554
net/gnome-connections: https://gitlab.gnome.org/GNOME/gtk-frdp/-/issues/45 (used by net/gnome-connections)
multimedia/vlc: no bug-report and can't registered at https://code.videolan.org/videolan/vlc/-/issues
net/vinagre: deprecated(?) https://gitlab.gnome.org/Archive/vinagre "Archived project! Repository and other project resources are read-only"
net/guacamole-server: https://issues.apache.org/jira/browse/GUACAMOLE-1026
net/krdc: don't know status - don't work for me rdp with FreeRDP 3.0.0 and work with FreeRDP 2.x
net/remotebox: don't know status

[fernape]

I'm OK with the change in remmina.

Thanks!

[vvd]

In D43127#982976, @tcberner wrote:

There seems to be a lot of work done to not install manpages... xmlto does not look like such a heavy build-dependency that would call for that effort :)

Or you say about old patches inherited from net/freerdp 2.x?

[vvd]

Released 3.1.0 with nice features:

• WITH_BINARY_VERSIONING (default OFF) Similar as for libraries the binaries, manpages and resource locations created by FreeRDP project are postfixed with the API version. Recommended if packagers want to install the package alongside FreeRDP 2 without conflicts.
• RDTK_FORCE_STATIC_BUILD (default OFF) Build and link RDTK statically into shadow server. Recommended for packagers as this library is not really used outside of FreeRDP-shadow.
• UWAC_FORCE_STATIC_BUILD (default OFF) Build and link UWAC statically into wlfreerdp. Recommended for packagers as this library is not really used outside of wlfreerdp.

Also I "fixed" build wlfreerdp and reworked some parts of the port.

[vvd]

Fixed typo.

[vvd]

Replaced
DISTNAME= ${PORTNAME:S/3$//}-${DISTVERSION}
with
PKGNAMESUFFIX= 3

Found example in print/scribus-devel.

[vvd]

Optimized manpages install after discussion with upstream developers.

diizzy, please answer on my comments.

[diizzy]

net/freerdp3/Makefile

128

You need to match both armv6 and armv7 in separate statements or possibly use -mfpu=neon instead of -march=armv7-a

[diizzy]

net/freerdp3/Makefile

128

That being said I'm not a fan of ports not relying on CPUTYPE, I'd rather see it fail than we silently trying to work around incorrect settings.

[vvd]

net/freerdp3/Makefile

128

I still don't understand how to do this correct.
I don't know why author (woodsb02@) of the port did this 7 years ago: https://cgit.freebsd.org/ports/commit/?id=d0ea56deb08f32659f5aef555ad218c6773f985e
And I don't have arm hardware for testing.
Who can help with this?

[woodsb02]

net/freerdp3/Makefile

128

The NEON option and associated ARM compatibility logic was submitted by @kevans as part of PR213637, which was rolled up into that commit.
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=213637

@kevans mentioned in the above that this idea was copied from the multimedia/ffmpeg port.

I see that the NEON/SSE options were replaced by the ASM option in multimedia/ffmpeg, so perhaps @jbeich who made that change will be able to help here?
https://cgit.freebsd.org/ports/commit/multimedia/ffmpeg/Makefile?id=791d55b70a35f2e9a7c784a5467647568fd2a358

[jbeich]

net/freerdp3/Makefile

128

My ffmpeg rationale still stands: -march= shouldn't be set via port options.

• elf_aux_info(3) with AT_HWCAP can be used to detect HWCAP_NEON at runtime
• #ifdef __ARM_NEON can be used to detect at compile-time
• FreeBSD armv7 was split from FreeBSD armv6 since 12.0-RELEASE (2018-12-11)
• FreeBSD armv7 defaults to -march=armv7-a and -mfpu=neon (unlike -target armv7-unknown-freebsdX.Y)

$ pkg install qemu-user-static poudriere
$ fetch https://download.freebsd.org/releases/arm/armv7/ISO-IMAGES/13.2/FreeBSD-13.2-RELEASE-arm-armv7-GENERICSD.img.xz
$ xz -d FreeBSD-13.2-RELEASE-arm-armv7-GENERICSD.img.xz
$ trap 'umount /mnt; poudriere jail -kj 132armv7; poudriere jail -dj 132armv7' EXIT INT TERM
$ mount -o ro $(mdconfig -f FreeBSD-13.2-RELEASE-arm-armv7-GENERICSD.img)s2a /mnt
$ poudriere jail -cj 132armv7 -v 13.2-RELEASE -a arm.armv7 -m null -f none -M /mnt
$ poudriere jail -sj 132armv7
$ jexec 132armv7-default login -f root

$ cc -dM -E - </dev/null | fgrep -i arch
#define __ARM_ARCH 7
#define __ARM_ARCH_7A__ 1
#define __ARM_ARCH_ISA_ARM 1
#define __ARM_ARCH_ISA_THUMB 2
#define __ARM_ARCH_PROFILE 'A'

$ cc -dM -E - </dev/null | fgrep -i -e neon -e vfp
#define __ARM_NEON 1
#define __ARM_NEON_FP 0x4
#define __ARM_NEON__ 1
#define __ARM_PCS_VFP 1
#define __ARM_VFPV2__ 1
#define __ARM_VFPV3__ 1
#define __VFP_FP__ 1

[jbeich]

net/freerdp3/Makefile

128

Also (Phabricator doesn't allow editing inline comments):

• 12.0 (094fc1ed0f26) limited armv6 to RPI-B image
• 15.0 plans to remove armv6 altogether

What can be changed here:

• Patch IsProcessorFeaturePresent to return PF_ARM_NEON_INSTRUCTIONS_AVAILABLE with #ifdef __ARM_NEON (similar to #ifdef __SSE3__) on non-Linux, so NEON port option isn't dead code
• Drop OPTIONS_DEFINE_armv6=NEON and NEON_CFLAGS for duplicating armv7
• Add OPTIONS_DEFAULT_armv7=NEON to denote FreeBSD default
• Replace CMAKE_ARGS_aarch64+=-DWITH_NEON=ON with OPTIONS_{DEFINE,DEFAULT}_aarch64=NEON
• Drop CFLAGS_aarch64+=-D__ARM_NEON__=__ARM_NEON due to https://github.com/FreeRDP/FreeRDP/commit/f1bf99f0755a

[vvd]

Updated ARM/NEON options.
Changed man pages path to LOCALBASE/share/man.

[vvd]

net/freerdp3/Makefile

128

I did as you suggested exept I keep: OPTIONS_DEFINE_armv6= NEON.

Patch IsProcessorFeaturePresent to return PF_ARM_NEON_INSTRUCTIONS_AVAILABLE with #ifdef __ARM_NEON (similar to #ifdef __SSE3__) on non-Linux, so NEON port option isn't dead code
This patch do you mean: winpr/libwinpr/sysinfo/sysinfo.c(803):

#elif defined(__APPLE__) // __linux__

        switch (ProcessorFeature)
        {
                case PF_ARM_NEON_INSTRUCTIONS_AVAILABLE:
                case PF_ARM_NEON:
                        ret = TRUE;
                        break;
        }

#elif defined(__FreeBSD__) // __linux__

switch (ProcessorFeature)
        {
                case PF_ARM_NEON_INSTRUCTIONS_AVAILABLE:
                case PF_ARM_NEON:
#ifdef __ARM_NEON
                                ret = TRUE;
#endif
                        break;
                default:
                        break;
        }

#endif // __linux__

[jbeich]

net/freerdp3/Makefile

128

No need for OS gatekeeping as __ARM_NEON is part of compiler pre-defined macros in both Clang and GCC on both armv7 (optional) and aarch64 (mandatory). For example, the following can be sent upstream and would also help OpenBSD and NetBSD:

--- winpr/libwinpr/sysinfo/sysinfo.c.orig
+++ winpr/libwinpr/sysinfo/sysinfo.c
@@ -800,13 +800,15 @@ BOOL IsProcessorFeaturePresent(DWORD ProcessorFeature)
 			break;
 	}
 
-#elif defined(__APPLE__) // __linux__
+#else // __linux__
 
 	switch (ProcessorFeature)
 	{
 		case PF_ARM_NEON_INSTRUCTIONS_AVAILABLE:
 		case PF_ARM_NEON:
+#ifdef __ARM_NEON
 			ret = TRUE;
+#endif
 			break;
 	}
 

[vvd]

net/freerdp3/Makefile

128

Added patch.

[vvd]

Can I commit this?