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.

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

gamescope: 3.11.49 -> 3.11.52-beta6 #219548

Merged
merged 3 commits into from
Mar 13, 2023

Conversation

Scrumplex
Copy link
Member

@Scrumplex Scrumplex commented Mar 4, 2023

NOTE: This PR will probably be updated a few times until the eventual release of gamescope 3.11.52

  • openvr: init at 1.23.8:
  • libliftoff: 0.3.0 -> 0.4.0
  • gamescope: 3.11.49 -> 3.11.52-beta6
    • Project page
    • Patches are here to fix the build in its current state. They are probably going to be unneeded for the stable release of 3.11.52
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • 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/)
  • 23.05 Release Notes (or backporting 22.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.

@Scrumplex
Copy link
Member Author

As an aside: I would be interested in co-maintaining gamescope and maybe libliftoff, as I maintained packages for these on the Arch Linux User Repository, before moving to NixOS.

@Scrumplex Scrumplex changed the title gamescope: 3.11.49 -> 3.11.52-beta5 gamescope: 3.11.49 -> 3.11.52-beta6 Mar 5, 2023
@Scrumplex Scrumplex force-pushed the gamescope-3.11.52 branch 3 times, most recently from b9a570f to fd7366b Compare March 12, 2023 11:30
@PedroHLC
Copy link
Member

PedroHLC commented Mar 13, 2023

@Scrumplex note that I have a few of the same in #215343 if you need something, kindly let me know.

UPDATE: Also, Joshua said he does not want us to put vkroots as global package, since he'll never add a stable API to it, he believes it's better for every package to target a specific commit in the repo. But like you I did it anyway.

UPDATE2: Put us both as co-maintainers of everything here 😄

UPDATE3: Since you're adding the patches manually to a release tag, I could totally rebase my POC in this branch and stop bumping gamescope to untagged refs.

UPDATE4: If possible, please take a look at #187507 -- I wish we could resurrect and merge it.

@PedroHLC PedroHLC self-requested a review March 13, 2023 14:18
@PedroHLC
Copy link
Member

Result of nixpkgs-review pr 219548 run on x86_64-linux 1

6 packages built:
  • bottles
  • bottles-unwrapped
  • gamescope
  • libliftoff
  • openvr
  • vkroots

@PedroHLC PedroHLC added the 12.approvals: 1 This PR was reviewed and approved by one reputable person label Mar 13, 2023
@Scrumplex
Copy link
Member Author

@Scrumplex note that I have a few of the same in #215343 if you need something, kindly let me know.

👍

UPDATE: Also, Joshua said he does not want us to put vkroots as global package, since he'll never add a stable API to it, he believes it's better for every package to target a specific commit in the repo. But like you I did it anyway.

I just added vkroots inline in gamescope to address this. Especially as it's a header-only library it shouldn't really matter too much.

UPDATE2: Put us both as co-maintainers of everything here

done :D

UPDATE3: Since you're adding the patches manually to a release tag, I could totally rebase my POC in this branch and stop bumping gamescope to untagged refs.

sure :D

@ofborg ofborg bot requested a review from PedroHLC March 13, 2023 15:19
@ofborg ofborg bot added the 11.by: package-maintainer This PR was created by the maintainer of the package it changes label Mar 13, 2023
@superherointj
Copy link
Contributor

superherointj commented Mar 13, 2023

Would be good to fetchpatch from source patch.

Copy link
Member

@PedroHLC PedroHLC left a comment

Choose a reason for hiding this comment

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

I'll test this in my system, just a sec.

pkgs/development/libraries/libliftoff/default.nix Outdated Show resolved Hide resolved
@superherointj
Copy link
Contributor

superherointj commented Mar 13, 2023

Result of nixpkgs-review pr 219548 run on x86_64-linux 1

3 packages failed to build:
  • bottles
  • bottles-unwrapped
  • gamescope
2 packages built:
  • libliftoff
  • openvr

Error logs for:

@PedroHLC
Copy link
Member

PedroHLC commented Mar 13, 2023

In gamescope:

/nix/store/fhzz4yrdy17czwc9i4swhlpcp445inzb-binutils-2.40/bin/ld: /nix/store/8s3ancq1nkbv1ir7n17yxhpdzvsak6i2-openvr-1.23.8/lib/libopenvr_api.so: undefined reference to `Json::Value::operator=(Json::Value)'
/nix/store/fhzz4yrdy17czwc9i4swhlpcp445inzb-binutils-2.40/bin/ld: /nix/store/8s3ancq1nkbv1ir7n17yxhpdzvsak6i2-openvr-1.23.8/lib/libopenvr_api.so: undefined reference to `Json::Value::operator!() const'

description = "An API and runtime that allows access to VR hardware from multiple vendors without requiring that applications have specific knowledge of the hardware they are targeting";
license = licenses.bsd3;
maintainers = with maintainers; [ pedrohlc Scrumplex ];
platforms = with platforms; unix;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
platforms = with platforms; unix;
platforms = platforms.unix;

@Scrumplex
Copy link
Member Author

Looks like fetchpatch actually breaks patches (from GitHub) that rename files. The OpenVR patch to support the system installation of jsoncpp, also included headers of the in-tree jsoncpp, which is why there was this mismatch.

Am I missing a specific option of fetchpatch here?

@superherointj
Copy link
Contributor

Looks like fetchpatch actually breaks patches (from GitHub) that rename files. The OpenVR patch to support the system installation of jsoncpp, also included headers of the in-tree jsoncpp, which is why there was this mismatch.

Am I missing a specific option of fetchpatch here?

Try to reference the commit not the PR. We don't reference the PR, only commits. Because PR can change.

@Scrumplex
Copy link
Member Author

Seems like this is a known issue (#32084). I also tried using fetchpatch2 and also tried to pull in the commits separately, but it doesn't work.

This is what I got when I tried fetchpatch2:

error: Normalized patch '/build/patch' is empty (while the fetched file was not)!
Did you maybe fetch a HTML representation of a patch instead of a raw patch?
Fetched file was:
From a7a6995d1d6f9ee1a17a3741661156f9706c40ce Mon Sep 17 00:00:00 2001
From: Lubosz Sarnecki <[email protected]>
Date: Thu, 15 Aug 2019 15:05:34 +0200
Subject: [PATCH] thirdparty: Move jsoncpp to thridparty directory.

Even though the previous patch in this patch set enabled the build with
a system wide jsoncpp and worked for me, it still left the internal
jsoncpp includes in the include path, since the includes were placed
in `CMAKE_CURRENT_SOURCE_DIR`. This could cause problems on other systems
when trying to build with a system wide jsoncpp.

In order to remove the internal json.h from the include path,
I moved all jsoncpp files into a thridparty directory amd include
it in the case of `USE_SYSTEM_JSONCPP` not being set.
---
 {src => thirdparty/jsoncpp}/json/json-forwards.h | 0
 {src => thirdparty/jsoncpp}/json/json.h          | 0
 {src => thirdparty/jsoncpp}/jsoncpp.cpp          | 0
 3 files changed, 0 insertions(+), 0 deletions(-)
 rename {src => thirdparty/jsoncpp}/json/json-forwards.h (100%)
 rename {src => thirdparty/jsoncpp}/json/json.h (100%)
 rename {src => thirdparty/jsoncpp}/jsoncpp.cpp (100%)

diff --git a/src/json/json-forwards.h b/thirdparty/jsoncpp/json/json-forwards.h
similarity index 100%
rename from src/json/json-forwards.h
rename to thirdparty/jsoncpp/json/json-forwards.h
diff --git a/src/json/json.h b/thirdparty/jsoncpp/json/json.h
similarity index 100%
rename from src/json/json.h
rename to thirdparty/jsoncpp/json/json.h
diff --git a/src/jsoncpp.cpp b/thirdparty/jsoncpp/jsoncpp.cpp
similarity index 100%
rename from src/jsoncpp.cpp
rename to thirdparty/jsoncpp/jsoncpp.cpp

@superherointj
Copy link
Contributor

superherointj commented Mar 13, 2023

Seems like this is a known issue (#32084). I also tried using fetchpatch2 and also tried to pull in the commits separately, but it doesn't work.

Feel free to include the patch file then. (but only for patches that fetchpatch didn't work.)

@Scrumplex
Copy link
Member Author

Feel free to include the patch file then.

Well in this case, we only really need to deal with a directory being renamed, so I am just going to add a postUnpack stage.

@Scrumplex Scrumplex force-pushed the gamescope-3.11.52 branch 3 times, most recently from e086d09 to f931ff4 Compare March 13, 2023 21:32
@superherointj
Copy link
Contributor

Result of nixpkgs-review pr 219548 run on x86_64-linux 1

5 packages built:
  • bottles
  • bottles-unwrapped
  • gamescope
  • libliftoff
  • openvr

Copy link
Member

@PedroHLC PedroHLC left a comment

Choose a reason for hiding this comment

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

Tested it in a live system, gamescope was able to handle CDPR launcher and the Witcher 3 as expected.

Copy link

@NguyenCuong17012021 NguyenCuong17012021 left a comment

Choose a reason for hiding this comment

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

Tess

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
8.has: package (new) This PR adds a new package 10.rebuild-darwin: 1-10 10.rebuild-darwin: 1 10.rebuild-linux: 1-10 11.by: package-maintainer This PR was created by the maintainer of the package it changes 12.approvals: 1 This PR was reviewed and approved by one reputable person
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants