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

umu-launcher[-unwrapped]: init at 1.1.4 #369259

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

umu-launcher[-unwrapped]: init at 1.1.4 #369259

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions pkgs/by-name/um/umu-launcher-unwrapped/makefile-installer-prefix.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/Makefile.in b/Makefile.in
index b82053d..3e4379e 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -90,7 +90,7 @@ umu-dist: $(OBJDIR)/.build-umu-dist
umu-dist-install: umu-dist
$(info :: Installing umu )
install -d $(DESTDIR)$(PYTHONDIR)/$(INSTALLDIR)
- $(PYTHON_INTERPRETER) -m installer --destdir=$(DESTDIR) $(OBJDIR)/*.whl
+ $(PYTHON_INTERPRETER) -m installer --prefix=$(DESTDIR) $(OBJDIR)/*.whl
diniamo marked this conversation as resolved.
Show resolved Hide resolved

ifeq ($(FLATPAK), xtrue)
umu-install: version-install umu-dist-install
22 changes: 22 additions & 0 deletions pkgs/by-name/um/umu-launcher-unwrapped/no-umu-version-json.patch
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
diff --git a/Makefile.in b/Makefile.in
index b82053d..37db8c9 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -50,7 +50,7 @@ fix_shebangs:
umu/umu_version.json: umu/umu_version.json.in
$(info :: Updating $(@) )
cp $(<) $(<).tmp
- sed 's|##UMU_VERSION##|$(shell git describe --always --long --tags)|g' -i $(<).tmp
+ sed 's|##UMU_VERSION##|@version@|g' -i $(<).tmp
diniamo marked this conversation as resolved.
Show resolved Hide resolved
Comment on lines +9 to +10
Copy link
Contributor

Choose a reason for hiding this comment

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

This patch shouldn't be needed once Open-Wine-Components/umu-launcher#289 makes it into a release.

Maybe gate the patch file behind lib.versionOlder "1.1.5" or `version == "1.1.4"? Assuming this PR gets merged before there's another upstream release.

That way anyone overriding this package with a newer version (or commit) won't also need to disable the patch.

mv $(<).tmp $(@)

.PHONY: version
@@ -151,7 +151,7 @@ clean:
@rm -rf -v $(OBJDIR) umu/umu_version.json ./$(RELEASEDIR) $(RELEASEDIR).tar.gz


-RELEASEDIR := $(PROJECT)-$(shell git describe --abbrev=0)
+RELEASEDIR := $(PROJECT)-@version@
Comment on lines +18 to +19
Copy link
Contributor

Choose a reason for hiding this comment

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

This patch could be avoided by overriding RELEASEDIR explicitly in makeFlags.

If it matters, we know $(PROJECT) should always be umu-launcher. But I think for our purposes RELEASEDIR could be any string?

$(RELEASEDIR):
mkdir -p $(@)

65 changes: 65 additions & 0 deletions pkgs/by-name/um/umu-launcher-unwrapped/package.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
{
python3Packages,
fetchFromGitHub,
lib,
bash,
hatch,
scdoc,
replaceVars,
}:
python3Packages.buildPythonPackage rec {
pname = "umu-launcher-unwrapped";
version = "1.1.4";

src = fetchFromGitHub {
owner = "Open-Wine-Components";
repo = "umu-launcher";
rev = "refs/tags/${version}";
hash = "sha256-TOsVK6o2V8D7CLzVOkLs8AClrZmlVQTfeii32ZIQCu4=";
};

patches = [
./makefile-installer-prefix.patch
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add a comment to explain why this patch is needed? And/or a TODO/FIXME if you think there's a better approach?

(personally, I don't really understand why we need --prefix instead of --destdir here)

(replaceVars ./no-umu-version-json.patch { inherit version; })
];

nativeBuildInputs = [
python3Packages.build
hatch
scdoc
python3Packages.installer
];

pythonPath = [
python3Packages.filelock
python3Packages.xlib
];

pyproject = false;
configureScript = "./configure.sh";

# The Makefile variables are a big mess, but this combination (and the patches) more or less do what we want
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: could we avoid vague negativity in comments? It could be helpful to list issues we have with the upstream variables, but I think this comment as-is may be counter-productive.

preBuild = ''
makeFlagsArray+=(
"PYTHON_INTERPRETER=${lib.getExe python3Packages.python}"
"SHELL_INTERPRETER=${lib.getExe bash}"
diniamo marked this conversation as resolved.
Show resolved Hide resolved
"PREFIX="
"PYTHONDIR=/${python3Packages.python.sitePackages}"
"DESTDIR=$out"
)
'';
Comment on lines +42 to +50
Copy link
Contributor

Choose a reason for hiding this comment

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

I know we touched on this before, but my preference would be to set this via makeFlags instead of having a preBuild script.

I think this would make the package easier to override. Probably not a big deal, but it feels cleaner.

This could be done by either

  • setting DESTDIR to some local path, then later during the installPhase moving the files to $out,
  • or by setting DESTDIR=${placeholder "out"}.


meta = {
description = "Unified launcher for Windows games on Linux using the Steam Linux Runtime and Tools";
changelog = "https://github.com/Open-Wine-Components/umu-launcher/releases/tag/${version}";
homepage = "https://github.com/Open-Wine-Components/umu-launcher";
license = lib.licenses.gpl3;
mainProgram = "umu-run";
maintainers = with lib.maintainers; [
diniamo
MattSturgeon
fuzen
];
platforms = lib.platforms.linux;
};
}
15 changes: 15 additions & 0 deletions pkgs/by-name/um/umu-launcher/package.nix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{ buildFHSEnv, umu-launcher-unwrapped }:
Copy link
Contributor

Choose a reason for hiding this comment

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

May be worth copying heroic

nixpkgs/pkgs/games/heroic/fhsenv.nix

Lines 4 to 5 in 235caa7

extraPkgs ? pkgs: [ ],
extraLibraries ? pkgs: [ ],

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor

Choose a reason for hiding this comment

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

To make it easier for end-users to add extra packages to the FHS environment by overriding.

This could be done in a follow-up PR if preferred

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I what the code does, but in what scenario would that be useful

I assume heroic launcher also supports native games, in which case it would make sense, but as far as I know, umu is for windows games only

Copy link
Contributor

Choose a reason for hiding this comment

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

Needed for passing graphics and fixing a cursor issue. Graphics is needed for dxvk-nvapi to work

nixpkgs/nixos/modules/programs/steam.nix

Lines 51 to 57 in e24b4c0

extraLibraries = pkgs: let
prevLibs = if prev ? extraLibraries then prev.extraLibraries pkgs else [ ];
additionalLibs = with config.hardware.graphics;
if pkgs.stdenv.hostPlatform.is64bit
then [ package ] ++ extraPackages
else [ package32 ] ++ extraPackages32;
in prevLibs ++ additionalLibs;

buildFHSEnv {
pname = "umu-launcher";
inherit (umu-launcher-unwrapped) version meta;

targetPkgs = pkgs: [ pkgs.umu-launcher-unwrapped ];

executableName = "umu-run";
Copy link
Contributor

Choose a reason for hiding this comment

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

IDK if this is good practice, but this could be set as:

Suggested change
executableName = "umu-run";
executableName = umu-launcher-unwrapped.meta.mainProgram;

runScript = "${umu-launcher-unwrapped}/bin/umu-run";
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
runScript = "${umu-launcher-unwrapped}/bin/umu-run";
runScript = lib.getExe umu-launcher-unwrapped;


extraInstallCommands = ''
ln -s ${umu-launcher-unwrapped}/lib $out/lib
ln -s ${umu-launcher-unwrapped}/share $out/share
'';
}