Bug 248118 - emulators/wine-devel: 5.13 - STAGING work again
Summary: emulators/wine-devel: 5.13 - STAGING work again
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Some People
Assignee: Gerald Pfeifer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-07-20 07:32 UTC by Vladimir Druzenko
Modified: 2020-07-30 12:55 UTC (History)
2 users (show)

See Also:


Attachments
STAGING work again (4.67 KB, patch)
2020-07-20 07:32 UTC, Vladimir Druzenko
vvd: maintainer-approval?
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Vladimir Druzenko freebsd_committer freebsd_triage 2020-07-20 07:32:34 UTC
Created attachment 216596 [details]
STAGING work again

Tested with and without STAGING on 12.1 amd64: make check-plist/install.
Comment 1 Gerald Pfeifer freebsd_committer freebsd_triage 2020-07-21 11:59:24 UTC
Having submitted a fix for the build problem upstream and Wine Staging
first not being available and than colliding with that fix, I was going
to wait for the next snapshot/minor release in less than two weeks.

Since you submitted a patch and PR indicates that there is interest and
I'm testing your patch right now and plan on committing (probably tomorrow);
thanks.
Comment 2 Gerald Pfeifer freebsd_committer freebsd_triage 2020-07-21 12:15:15 UTC
First feedback: Your patch does not work as submitted - and if it did, my
original update would have been flawed:

The additions to pkg-plist need to be guarded by %%STAGING%% since they are
specific to that option.  Otherwise the port fails to package when STAGING
is not set.
Comment 3 Vladimir Druzenko freebsd_committer freebsd_triage 2020-07-21 12:17:42 UTC
I "make check-plist" with and without STAGING - no errors.
I can check one more time if you want…
Comment 4 Gerald Pfeifer freebsd_committer freebsd_triage 2020-07-21 12:30:42 UTC
(In reply to VVD from comment #3)
> I "make check-plist" with and without STAGING - no errors.

Unfortunately "make check-plist" is flawed.

I reported https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=220950
on this three years ago (nearly to the date), but there is no agreement
on a full fix.

As one remediation you can run "make package" which will catch those cases
that will prevent packaging.  When you run that (and STAGING _not_ set), do
you get an error?

(I have fixed this in my local tree, no need to submit an updated patch;
this is more for future contributions which I hope we'll see from you. :-)
Comment 5 commit-hook freebsd_committer freebsd_triage 2020-07-22 10:42:08 UTC
A commit references this bug:

Author: gerald
Date: Wed Jul 22 10:41:54 UTC 2020
New revision: 542851
URL: https://svnweb.freebsd.org/changeset/ports/542851

Log:
  The Wine Staging patchset for Wine 5.13 is now available and fixed
  (compared to the previous one).

  It also addresses the build issue for Wine 5.13 that I fixed via
  files/patch-dlls-ntdll-unix-registry.c, so we need to make that
  patch only apply when the Wine Staging patchset is not used.

  PR:		248118
  Submitted by:	vvd@unislabs.com

Changes:
  head/emulators/wine-devel/Makefile
  head/emulators/wine-devel/distinfo
  head/emulators/wine-devel/files/extrapatch-dlls-ntdll-unix-registry.c
  head/emulators/wine-devel/files/patch-dlls-ntdll-unix-registry.c
  head/emulators/wine-devel/pkg-plist
Comment 6 Gerald Pfeifer freebsd_committer freebsd_triage 2020-07-22 10:51:41 UTC
Thank you for the patch!

Note that I did not see lib/wine/fakedlls/winejoystick.drv nor
lib/wine/winejoystick.drv.so in my testing, even with the STAGING
option set, so I did not include those changes to pkg-plist.

Are there any special settings or local changes you have in place
that might enable those?
Comment 7 Vladimir Druzenko freebsd_committer freebsd_triage 2020-07-22 10:59:37 UTC
(In reply to Gerald Pfeifer from comment #6)
OPTIONS_FILE_SET+=CUPS
OPTIONS_FILE_UNSET+=DOCS
OPTIONS_FILE_SET+=DOSBOX
OPTIONS_FILE_SET+=GCC
OPTIONS_FILE_SET+=GECKO
OPTIONS_FILE_SET+=GNUTLS
OPTIONS_FILE_UNSET+=HAL
OPTIONS_FILE_UNSET+=LDAP
OPTIONS_FILE_SET+=LIBXSLT
OPTIONS_FILE_SET+=MONO
OPTIONS_FILE_UNSET+=MPG123
OPTIONS_FILE_UNSET+=OPENAL
OPTIONS_FILE_SET+=STAGING
OPTIONS_FILE_SET+=V4L
OPTIONS_FILE_UNSET+=VKD3D
OPTIONS_FILE_UNSET+=VULKAN
OPTIONS_FILE_SET+=WINEMAKER
OPTIONS_FILE_SET+=X11

Nothing else…
Comment 8 Gerald Pfeifer freebsd_committer freebsd_triage 2020-07-22 11:18:30 UTC
Debugging this further here is a relevant piece of my configure run:

  checking linux/joystick.h usability... no
  checking linux/joystick.h presence... no
  checking for linux/joystick.h... no

I am willing to bet that this is present on your system, which means
configure detects and uses it and that then changes which subdirectories
of wine/dll are processed.  On my build system I see
 
  DISABLED_SUBDIRS    =  \ 
    :
    dlls/winejoystick.drv \

in the toplevel Makefile, so dlls/winejoystick.drv is *not* built, and it
should not be built in a clean build environment.

To avoid this being "randomly" switched on based on the environment a
configure options would be useful and you'll see that I use many of those
in the Makefile of the emulators/wine-devel port - just, there appears none
for this case (cf. `./configure --help` in the Wine source tree, and I tried
--without-winejoystick.drv and --without-winejoystick after reviewing the
source code didn't reveal anything either).
Comment 9 Vladimir Druzenko freebsd_committer freebsd_triage 2020-07-22 11:51:36 UTC
checking linux/joystick.h usability... yes
checking linux/joystick.h presence... yes
checking for linux/joystick.h... yes

$ pkg which /usr/local/include/linux/joystick.h 
/usr/local/include/linux/joystick.h was installed by package evdev-proto-5.3
$ pkg info -r evdev-proto-5.3 v4l_compat-1.18.0
evdev-proto-5.3:
        v4l_compat-1.18.0
v4l_compat-1.18.0:

multimedia/v4l_compat is a build dependency for a lot of ports.
So in real system it installed "a little more often than always"…
Comment 10 Li-Wen Hsu freebsd_committer freebsd_triage 2020-07-22 15:14:54 UTC
(In reply to commit-hook from comment #5)
Note this was reverted due to index breakage:
https://svnweb.freebsd.org/changeset/ports/542857
Comment 11 Vladimir Druzenko freebsd_committer freebsd_triage 2020-07-22 17:50:00 UTC
(In reply to Li-Wen Hsu from comment #10)
What index?
What does it mean?
Comment 12 Li-Wen Hsu freebsd_committer freebsd_triage 2020-07-22 17:52:47 UTC
(In reply to VVD from comment #11)
Error message:
https://lists.freebsd.org/pipermail/freebsd-ports/2020-July/119018.html

See `index` target in ports/Makefile

It can be tested with `make describe` in the port directory.
Comment 13 Vladimir Druzenko freebsd_committer freebsd_triage 2020-07-22 18:01:45 UTC
(In reply to Li-Wen Hsu from comment #12)
Maybe this:
# Include this only if it has not been already included by the
# i386-wine-devel slave port.
.ifndef WINE_SLAVE_BUILD
.include <bsd.port.pre.mk>
.if !${PORT_OPTIONS:MSTAGING}
EXTRA_PATCHES+= ${PATCHDIR}/extrapatch-dlls-ntdll-unix-registry.c
.endif
.endif

Instead of this:
# Include this only if it has not been already included by the
# i386-wine-devel slave port.
.ifndef WINE_SLAVE_BUILD
.include <bsd.port.pre.mk>
.endif

.if !${PORT_OPTIONS:MSTAGING}
EXTRA_PATCHES+= ${PATCHDIR}/extrapatch-dlls-ntdll-unix-registry.c
.endif
Comment 14 Vladimir Druzenko freebsd_committer freebsd_triage 2020-07-23 06:32:51 UTC
Do I need to make new patch with changes in previous comment?
Comment 15 Alex S 2020-07-23 12:04:27 UTC
(In reply to VVD from comment #13)

.include <bsd.port.options.mk>
Comment 16 Vladimir Druzenko freebsd_committer freebsd_triage 2020-07-23 12:38:26 UTC
(In reply to Alex S from comment #15)

# Include this only if it has not been already included by the
# i386-wine-devel slave port.
.ifndef WINE_SLAVE_BUILD
.include <bsd.port.pre.mk>
.else
.include <bsd.port.options.mk>
.endif

.if !${PORT_OPTIONS:MSTAGING}
EXTRA_PATCHES+= ${PATCHDIR}/extrapatch-dlls-ntdll-unix-registry.c
.endif

Correct?
Comment 17 Alex S 2020-07-23 12:57:40 UTC
(In reply to VVD from comment #16)

.ifndef WINE_SLAVE_BUILD
.include <bsd.port.options.mk>
.endif

.if !${PORT_OPTIONS:MSTAGING}
EXTRA_PATCHES+= ${PATCHDIR}/extrapatch-dlls-ntdll-unix-registry.c
.endif

# Include this only if it has not been already included by the
# i386-wine-devel slave port.
.ifndef WINE_SLAVE_BUILD
.include <bsd.port.pre.mk>
.endif
Comment 18 Vladimir Druzenko freebsd_committer freebsd_triage 2020-07-23 13:10:58 UTC
(In reply to Alex S from comment #17)
Failure is in the slave port, not in the master.
Comment 19 Alex S 2020-07-23 13:23:27 UTC
(In reply to VVD from comment #18)

Leave .ifndef WINE_SLAVE_BUILD out then.
Comment 20 Vladimir Druzenko freebsd_committer freebsd_triage 2020-07-23 13:28:03 UTC
(In reply to Alex S from comment #19)

.include <bsd.port.pre.mk>

.if !${PORT_OPTIONS:MSTAGING}
EXTRA_PATCHES+= ${PATCHDIR}/extrapatch-dlls-ntdll-unix-registry.c
.endif

So need to test slave port than…
Comment 21 Gerald Pfeifer freebsd_committer freebsd_triage 2020-07-30 10:22:35 UTC
(In reply to Alex S from comment #17)
> ...

Why not the straightforward approach from comment #13?

That passes `make index` (tested on i386) and allows wine-devel to build
with and without the STAGING option set.

Is there a strong reason not to go that way?
Comment 22 Alex S 2020-07-30 11:14:43 UTC
(In reply to Gerald Pfeifer from comment #21)

> Why not the straightforward approach from comment #13?

Is there any particular reason to apply the patch in question exclusively to the non-i386 ports?
Comment 23 Gerald Pfeifer freebsd_committer freebsd_triage 2020-07-30 11:38:34 UTC
(In reply to Alex S from comment #22)
> Is there any particular reason to apply the patch in question
> exclusively to the non-i386 ports?

The i386 ports have become de facto unmaintained recently and I do not
have a reasonable way to build or test them.

The patch in question is required to get wine-devel to build, but breaks
the build of wine-devel with the STAGING option set (since the maintainers
of the patchset included my patch in their latest release).


So except for the (non-existing) maintainer of i386-wine-devel, which does
require the creation and sharing of pre-built packages, the proposed approach
is a strict improvement.

(I expect a new version to be released in the next days, which will then
make the issue go away in any case, simply did not want to ignore VVD's
contribution.)
Comment 24 Alex S 2020-07-30 12:07:41 UTC
(In reply to Gerald Pfeifer from comment #23)

My logic there is pretty simple: "Malformed conditional" means PORT_OPTIONS is undefined, which usually means that bsd.port.options.mk should be included. This wasn't really intended to be anything other than a quick suggestion.

> The i386 ports have become de facto unmaintained recently

That doesn't seem to bother anyone, unfortunately.
Comment 25 Gerald Pfeifer freebsd_committer freebsd_triage 2020-07-30 12:18:54 UTC
(In reply to Alex S from comment #24)
> [...]

Thanks, Alex!  I'll go ahead with the simple patch I have tested, and
over the weekend we will hopefully see an update that simplifies things
again.

>> The i386 ports have become de facto unmaintained recently
> That doesn't seem to bother anyone, unfortunately.

It's not an easy one, though it has seen a number of improvements (and
updates) earlier this year by Lorenzo.
Comment 26 commit-hook freebsd_committer freebsd_triage 2020-07-30 12:21:02 UTC
A commit references this bug:

Author: gerald
Date: Thu Jul 30 12:20:25 UTC 2020
New revision: 543788
URL: https://svnweb.freebsd.org/changeset/ports/543788

Log:
  Reapply revision 542851 that broke the index via an interaction with
  emulators/i386-wine-devel and was reverted, now with a tweak to avoid
  that breakage:

  The Wine Staging patchset for Wine 5.13 is now available and fixed
  (compared to the previous one).

  It also addresses the build issue for Wine 5.13 that I fixed via
  files/patch-dlls-ntdll-unix-registry.c, so we need to make that
  patch only apply when the Wine Staging patchset is not used.

  PR:             248118 [1]
  Submitted by:   vvd@unislabs.com

Changes:
  head/emulators/wine-devel/Makefile
  head/emulators/wine-devel/distinfo
  head/emulators/wine-devel/files/extrapatch-dlls-ntdll-unix-registry.c
  head/emulators/wine-devel/files/patch-dlls-ntdll-unix-registry.c
  head/emulators/wine-devel/pkg-plist
Comment 27 Alex S 2020-07-30 12:30:00 UTC
Isn't that essentially the same as the reverted commit? Please, test i386-wine-devel with `make describe ARCH=i386`.
Comment 28 Alex S 2020-07-30 12:38:37 UTC
(In reply to Alex S from comment #27)

Ah, I see. I didn't read the diff correctly.
Comment 29 Gerald Pfeifer freebsd_committer freebsd_triage 2020-07-30 12:55:03 UTC
(In reply to Alex S from comment #27)
> Isn't that essentially the same as the reverted commit? Please, test
> i386-wine-devel with `make describe ARCH=i386`.

I had done that. :-)  (Yes, it's a subtle difference, which is why I missed
this originally.)