-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
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
umu-launcher: 1.1.4 -> 1.2.3 & init pyzstd #381975
base: master
Are you sure you want to change the base?
Conversation
116479c
to
81ebee8
Compare
I don't think a cherry-picked commit from another PR should be merged as a part of this. We should try to get that one merged first, and keep this as a draft until then. Edit: oh, the other PR adds it as a dep as well. Well, it might be fine then, but separating it to a different PR might be more ideal still. |
I disagree. It's a minor dependency for that PR, this one, and also a third one. Including minor dependencies in a larger PR is fine IMO.
#365111 has been inactive for a few weeks and still needs some cleanup work before being merged. It also has merge conflicts. #356882 is another PR that packages |
At least indicate it in the title then? |
|
81ebee8
to
4298aae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Diff looks good
4298aae
to
9d70269
Compare
|
Feel free to fixup pyzstd. I won't be able to get to it for awhile. |
9d70269
to
d64242b
Compare
Looks like umu also has some tests, mostly in the form of shell scripts. I don't want to block this PR on it, but it'd be nice for those to also be part of the package. Looking at the CI workflows, most are run in the e2e workflow, while My first thought is that some tests should be included in the package build, and if some of the other tests are more intensive they can be |
842da74
to
f82983a
Compare
Co-authored-by: OTABI Tomoya <[email protected]>
https://github.com/Open-Wine-Components/umu-launcher/releases/tag/1.2.0 https://github.com/Open-Wine-Components/umu-launcher/releases/tag/1.2.1 https://github.com/Open-Wine-Components/umu-launcher/releases/tag/1.2.2 https://github.com/Open-Wine-Components/umu-launcher/releases/tag/1.2.3 Incorporates work done in: Open-Wine-Components/umu-launcher#345
f82983a
to
f3c8ed5
Compare
Added a check phase using I had to disable 5/97 tests and configure a writable Not sure how much of the 5 skipped tests are upstream issues vs environment issues. Build log with all tests enabled:
|
postPatch = '' | ||
# pyzst specifies setuptools<74 because 74+ drops `distutils.msvc9compiler`, | ||
# required for Python 3.9 under Windows | ||
substituteInPlace pyproject.toml \ | ||
--replace-fail '"setuptools>=64,<74"' '"setuptools"' | ||
''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried using pythonRelaxDepsHook
instead:
diff --git a/pkgs/development/python-modules/pyzstd/default.nix b/pkgs/development/python-modules/pyzstd/default.nix
index 62e77d821d08..f403a5338b00 100644
--- a/pkgs/development/python-modules/pyzstd/default.nix
+++ b/pkgs/development/python-modules/pyzstd/default.nix
@@ -3,6 +3,7 @@
fetchFromGitHub,
lib,
pytestCheckHook,
+ pythonRelaxDepsHook,
setuptools,
zstd-c,
}:
@@ -18,13 +19,6 @@ buildPythonPackage rec {
hash = "sha256-Az+0m1XUFxExBZK8bcjK54Zt2d5ZlAKRMZRdr7rPcss=";
};
- postPatch = ''
- # pyzst specifies setuptools<74 because 74+ drops `distutils.msvc9compiler`,
- # required for Python 3.9 under Windows
- substituteInPlace pyproject.toml \
- --replace-fail '"setuptools>=64,<74"' '"setuptools"'
- '';
-
nativeBuildInputs = [
setuptools
];
@@ -33,6 +27,10 @@ buildPythonPackage rec {
setuptools
];
+ pythonRelaxDeps = [
+ "setuptools"
+ ];
+
buildInputs = [
zstd-c
];
@@ -43,6 +41,7 @@ buildPythonPackage rec {
nativeCheckInputs = [
pytestCheckHook
+ pythonRelaxDepsHook
];
pythonImportsCheck = [
But I don't think this supports relaxing dependencies specified in pyproject.toml
? The docs mention:
pythonRelaxDepsHook
works by modifying the resulting wheel file
Changelogs:
Incorporates work done in the upstream flake: Open-Wine-Components/umu-launcher#345
All patches removed.
Now also uses cargo for some dependencies. See Compiling non-Rust packages that include Rust code in the nixpkgs manual.
Used the new configure flags to specify "vendored" dependencies don't need to be built, since we install them nixpkgs packages.
Adds
withTruststore
andwithDeltaUpdates
package args, that allow configuring the new optional dependencies, as-per the upstream flake.Adds check phase using
pytestCheckHook
.Includes the dependency
pyzstd
, cherry-picked from #365111.Package builds ok. I've not tested using it at runtime yet.
Fixes #381960
Fixes #371859
cc maintainers @LovingMelody @diniamo
cc upstream @R1kaB3rN
Things done
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Add a 👍 reaction to pull requests you find important.