Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

winePackages{unstable,staging}: 9.0 -> 9.2, cleanup #286580

Merged
merged 4 commits into from
Feb 12, 2024
Merged

winePackages{unstable,staging}: 9.0 -> 9.2, cleanup #286580

merged 4 commits into from
Feb 12, 2024

Conversation

SuperSamus
Copy link
Contributor

@SuperSamus SuperSamus commented Feb 5, 2024 •
edited
Loading

Description of changes

https://gitlab.winehq.org/wine/wine/-/releases/wine-9.1
https://gitlab.winehq.org/wine/wine/-/releases/wine-9.2
The explanations for the other changes are in the commit messages.

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@ofborg ofborg bot added the 8.has: package (new) This PR adds a new package label Feb 5, 2024
@wegank wegank changed the title winePackages{unstable,staging}: 9.0 -> 0,1, cleanup winePackages{unstable,staging}: 9.0 -> 9.1, cleanup Feb 5, 2024
@reckenrode
Copy link
Contributor

reckenrode commented Feb 6, 2024 •
edited
Loading

LGTM. Running nixpkgs-review on Darwin now.

I’m working on rewriting overrideSDK on Darwin. Once that’s merged, it should let me remove some of the workarounds and drop the Metal-compatibility patches. I also have patches for MetalD3D, but that depends on the overrideSDK update.

@reckenrode
Copy link
Contributor

Result of nixpkgs-review pr 286580 run on x86_64-darwin 1

4 packages failed to build:
  • wine64Packages.wayland
  • wine64Packages.waylandFull
  • wineWow64Packages.wayland
  • wineWow64Packages.waylandFull
14 packages built:
  • wine64 (wine64Packages.full ,wine64Packages.stableFull)
  • wine64Packages.base (wine64Packages.stable)
  • wine64Packages.minimal
  • wine64Packages.staging
  • wine64Packages.stagingFull
  • wine64Packages.unstable
  • wine64Packages.unstableFull
  • wineWow64Packages.base (wineWow64Packages.stable)
  • wineWow64Packages.full (wineWow64Packages.stableFull)
  • wineWow64Packages.minimal
  • wineWow64Packages.staging
  • wineWow64Packages.stagingFull
  • wineWow64Packages.unstable
  • wineWow64Packages.unstableFull

@SuperSamus
Copy link
Contributor Author

Alright, now wine-wayland should be unsupported on Darwin like it was before.
Also, x11Support and waylandSupport are now set in wine-packages.nix.
(Note: both of them are false in minimal. Should x11Support be true even on minimal?)

@reckenrode
Copy link
Contributor

reckenrode commented Feb 6, 2024 •
edited
Loading

I’m pretty sure that’s way too aggressive for marking things broken, but I won’t be able to dig into it until later this evening.

The reason for marking Wayland support broken on Darwin is because the wayland derivation builds, so Wine tries to build with support and fails.

I’ll defer to the Linux maintainers whether x11Support should be enabled in the minimal build on Linux. It may be that minimal is meant for headless use cases.

(This exchange makes me wonder if Wine should be building with libpcap on Darwin. Upstream’s build does. I can address that in my Darwin cleanup PR.)

@ofborg ofborg bot requested a review from reckenrode February 6, 2024 14:25
@SFrijters
Copy link
Member

Based on #281905 (comment), can we remove the wayland release (or at least the 8.2 sources)?

If we remove the sources so that everything is on version 9.x I think we could set winePackages.wayland = winePackages.stable (or another flavour) and since it's overriding base wayland support will be enabled anyway (where available) with the changes in this PR?

@SuperSamus
Copy link
Contributor Author

IIRC (I don't remember where I read this), the Collabora fork still has some advantages over regular Wine with Wayland (e.g., some games are perfectly playable, which isn't the case on regular Wine).
Don't know how valid this is today.

Also, I'd rather remove rather than alias to something else.
(Though that can be done in another PR.)

@reckenrode
Copy link
Contributor

My apologies for not getting back sooner re: Darwin supported flags. #287609 ended up taking more time than expected. I looked back over the darwinUnsupportedFlags, and I think the set of flags is fine. Core Text is used on Darwin instead of fontconfig. x11 support should work, but it doesn’t make a lot of sense. The others are all Linux-specific (or not relevant to Darwin).

I’ll take care of libpcap plus the other Darwin cleanups once I’m properly able to specify the SDK version and deployment target. Those cleanups will also include the patches needed to support #236414.

The only other question I have is whether this should be bumped to Wine 9.2, which just came out a few hours ago.

@SuperSamus
Copy link
Contributor Author

wine-staging still doesn't have a release tagged 9.2, I guess it will tomorrow?
(Speaking of staging, shouldn't it be migrated to fetchFromGitLab, since its GitHub repo is a mirror now?)

@7c6f434c
Copy link
Member

Switching to the true-primary sounds good to me.

@SuperSamus
winePackages.{unstable,staging}: 9.1 -> 9.2
62c998c
@SuperSamus
wine: don't default mingwSupport to true outside stable release
2e2211a 
This way, minimal Wine will build without it
@SuperSamus
wine: cleanup Wayland
b2182b9 
Makes base.nix less Wayland-centric. This means:
- Don't specify support in the meta.description (since even regular Wine supports it, it's specified everywhere)
- `x11Support` and `waylandSupport` are now set in `wine-packages.nix`, like every other flag (they are also now disabled on minimal)
- Disable Darwin in supported platforms if an incompatible supportFlag is true (not just `waylandSupport`)
- Add wineRelease name for every release other than "stable" or "unstable", instead of just "wayland"
  - This also fixes an inconsistency on wine-staging, where "-staging" was written after the version
@SuperSamus SuperSamus changed the title winePackages{unstable,staging}: 9.0 -> 9.1, cleanup winePackages{unstable,staging}: 9.0 -> 9.2, cleanup Feb 11, 2024
reckenrode
reckenrode approved these changes Feb 11, 2024
View reviewed changes
Copy link
Contributor

@reckenrode reckenrode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree about switching wine-staging to the upstream GitLab instance. Changes lgtm.

@7c6f434c 7c6f434c merged commit b54cebb into NixOS:master Feb 12, 2024
27 checks passed
@SuperSamus SuperSamus deleted the wine-update branch February 12, 2024 13:11
SuperSamus added a commit to SuperSamus/nix-gaming that referenced this pull request Feb 20, 2024
@SuperSamus
wine: remove unneeded "release" and "waylandSupport" parameters
be49f13 
Since NixOS/nixpkgs#286580, Wayland is enabled by default
SuperSamus referenced this pull request in fufexan/nix-gaming Feb 20, 2024
@fufexan
flake.lock: update and add wineRelease
8f354ef 
Fixes #145
fufexan added a commit to fufexan/nix-gaming that referenced this pull request Feb 20, 2024
@fufexan
pkgs/wine: drop unused wayland flags
d154dc3 
Reason: NixOS/nixpkgs#286580
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@reckenrode reckenrode reckenrode approved these changes

@bendlas bendlas Awaiting requested review from bendlas

@jmc-figueira jmc-figueira Awaiting requested review from jmc-figueira

@7c6f434c 7c6f434c Awaiting requested review from 7c6f434c

@avnik avnik Awaiting requested review from avnik

Assignees
No one assigned
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-linux: 11-100
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@SuperSamus @reckenrode @SFrijters @7c6f434c