Package: guix-patches;
Reported by: msglm <msglm <at> techchud.xyz>
Date: Wed, 26 Mar 2025 08:05:03 UTC
Severity: normal
Tags: patch
Message #14 received at 77272 <at> debbugs.gnu.org (full text, mbox):
From: Ian Eure <ian <at> retrospec.tv> To: msglm <msglm <at> techchud.xyz> Cc: 宋文武 <iyzsong <at> envs.net>, 77272 <at> debbugs.gnu.org, Liliana Marie Prikler <liliana.prikler <at> gmail.com>, Adam Faiz <adam.faiz <at> disroot.org> Subject: Re: [bug#77272] [PATCH 2/2] gnu: dolphin-emu: Update to 2503. Date: Sat, 29 Mar 2025 20:31:17 -0700
Hi msglm, msglm <msglm <at> techchud.xyz> writes: > * gnu/packages/emulators.scm (dolphin-emu): Update to 2503, Fix > netplay > * gnu/packages/game-development.scm (sfml-3): New Package > * gnu/packages/game-development.scm (miniaudio): New Package > * gnu/packages/networking.scm (enet): Update to 2.30.9 All these changes need to be separate commits, which are sent as a patch series. See `(guix)Sending a Patch Series' in the Guix manual. The commit messages also need to follow `(standards)Change Logs'. > Change-Id: I5242f46e457db6552663c03c19dc0f227efb80cc > --- > gnu/packages/emulators.scm | 50 ++++++++++++++++-------- > gnu/packages/game-development.scm | 65 > +++++++++++++++++++++++++++++++ > gnu/packages/networking.scm | 4 +- > gnu/packages/sdl.scm | 4 +- > 4 files changed, 103 insertions(+), 20 deletions(-) > > diff --git a/gnu/packages/emulators.scm > b/gnu/packages/emulators.scm > index e71c2803a3..83ae89c172 100644 > --- a/gnu/packages/emulators.scm > +++ b/gnu/packages/emulators.scm > @@ -278,20 +278,19 @@ (define-public desmume > ;; Following commits and revision numbers of beta versions > listed at > ;; https://dolphin-emu.org/download/. > (define-public dolphin-emu > - (let ((commit "f9deb68aee962564b1495ff04c54c015e58d086f") > - (revision "13669")) The `package' form needs to be dedented now that the enclosing `let' is gone. > (source > (origin > (method git-fetch) > (uri (git-reference > (url "https://github.com/dolphin-emu/dolphin") > - (commit commit))) > + (recursive? #t) Is it reasonably possible to do a non-recursive clone? It looks like that’s vendoring dependencies, which Guix strongly prefers to avoid. I also notice that many of them are deleted in the loop in the next hunk. > @@ -300,14 +299,19 @@ (define-public dolphin-emu > (for-each (lambda (dir) > (delete-file-recursively > (string-append "Externals/" dir))) > - '("LZO" "OpenAL" "Qt" "SFML" "bzip2" > + '( > + ;"LZO" "OpenAL" "Qt" "SFML" "bzip2" > + > + "LZO" "OpenAL" "Qt" "bzip2" ;;TODO: > Ensure SFML is removed and update the package to make it happen > + Please remove commented out code and address TODOs. > - "ffmpeg" "fmt" "gettext" > + ;"ffmpeg" "fmt" "gettext" > + "fmt" "gettext" These too. > - "hidapi" "libpng" "libusb" "mbedtls" > - "miniupnpc" "minizip" "MoltenVK" > "pugixml" > + ;"hidapi" "libpng" "libusb" "mbedtls" > + "hidapi" "libusb" "mbedtls" > + ;"miniupnpc" "minizip" "MoltenVK" > "pugixml" > + "miniupnpc" "MoltenVK" "pugixml" > "soundtouch" > - "xxhash" "zlib" "zstd")) > + ;"xxhash" "zlib" "zstd" > + "xxhash" "zstd" And these. > + )) Parens should be nested on the closing line, not left dangling. > @@ -339,12 +347,21 @@ (define-public dolphin-emu > ((".*add_subdirectory.*Externals/enet.*") "") > ((".*add_subdirectory.*Externals/soundtouch.*") > "") > ((".*add_subdirectory.*Externals/xxhash.*") > "")))) > - (patches (search-patches "dolphin-emu-data.patch")))) Please include a patch deleting this file if it’s no longer needed. > + )) Please nest parens. > (build-system cmake-build-system) > (arguments > (list > #:phases > #~(modify-phases %standard-phases > + (add-before 'configure 'set-scm-desc-str > + (lambda _ > + ;;For netplay to work, the > SCM_REV_STR must match the > + ;;commit hash of whatever version > we're on. > + ;;THIS MUST BE UPDATED EVERY UPDATE > SO THAT NETPLAY > + ;;CONTINUES TO FUNCTION! There should be a space between the comment characters text, ex ";; For netplay..." > + (substitute* > "Source/Core/Common/scmrev.h.in" (("#define SCM_REV_STR > \"\\$\\{DOLPHIN_WC_REVISION\\}\"") "#define SCM_REV_STR > \"9763c0a1e2b9db0c3861d25bc2f5a0ace6a15ee3\"")) It looks like this is populated from the $DOLPHIN_WC_REVISION environment variable, can you set that instead of patching the .in template? That would be much cleaner. You might want to let-bind this at the top of the package definition, so it’s easier to see. This line is too long and needs to be wrapped. > + )) > + Please nest closing parens. > diff --git a/gnu/packages/game-development.scm > b/gnu/packages/game-development.scm > index 91369089b9..bb1ae5bc2f 100644 > --- a/gnu/packages/game-development.scm > +++ b/gnu/packages/game-development.scm > @@ -1115,6 +1115,71 @@ (define-public sfml > of five modules: system, window, graphics, audio and network.") > (license license:zlib))) > > +(define-public sfml-3 ;;For dolphin-emu Please remove the comment. If possible, it’d be prefereable to upgrade the existing SFML package. Is 3.0.0 backwards compatible with 2.5.x, or would that break that other things that use it? > + (package > + (inherit sfml) > + (name "sfml-3") > + (version "3.0.0") > + (inputs (modify-inputs (package-inputs sfml) > + (prepend libxcursor > libxi miniaudio))) > + (source (origin > + (method git-fetch) > + (uri (git-reference > + (url > "https://github.com/SFML/SFML") > + (recursive? #t) > + (commit version))) > + (file-name (git-file-name name > version)) > + (sha256 > + (base32 > + > "0y37cmpp490gcibajigxsbjc6icggqv40jrhzj2kwygpc0ppzb3v")) > + (modules '((guix build utils))) > + ;(snippet > + ; '(begin > + ; ;; Ensure system libraries are > used. > + ; (delete-file-recursively > "extlibs") > + ; #t)) Guix packages should basically never have commented code, please delete it. Does this mean that this version of SFML is building with vendored libraries? Guix strongly discourages vendored libraries, so, it shouldn’t. > +;; For sfml-3 Please remove the comment > +(define-public miniaudio > + (package Indentation here is wrong, it should be two characters deeper than the "(define-public" text. > + (name "miniaudio") > + (version "0.11.22") > + (source (origin > + (method git-fetch) > + (uri (git-reference > + (url > "https://github.com/mackron/miniaudio") > + (commit version))) > + (file-name (git-file-name name > version)) > + (sha256 > + (base32 > + > "1pjaiq71x24n9983vkhjxrsbraa24053h727db22b1rb2xyfrzm3")))) > + (build-system cmake-build-system) > + (arguments > + (list > + #:tests? #f > + #:phases > + #~(modify-phases %standard-phases > + (replace 'install > + (lambda _ > + (let ((out > #$output)) > + ;; Ensure > the output directory exists > + (mkdir-p > (string-append out "/lib")) > + ;; Copy the > static libraries to the output directory > + (for-each > (lambda (file) > + > (copy-file file (string-append out "/lib/" file))) ; Ensure the > destination is a file > + > '("libminiaudio.a" > + > "libminiaudio_channel_combiner_node.a" > + > "libminiaudio_channel_separator_node.a" > + > "libminiaudio_ltrim_node.a" > + > "libminiaudio_reverb_node.a" > + > "libminiaudio_vocoder_node.a")))))))) This feels odd. Does the upstream not have a way to install the libraries? Does it only build static libs, and not .so files? > diff --git a/gnu/packages/networking.scm > b/gnu/packages/networking.scm > index 65d44e975a..ff3e032a2b 100644 > --- a/gnu/packages/networking.scm > +++ b/gnu/packages/networking.scm > @@ -2627,14 +2627,14 @@ (define-public proxychains-ng > (define-public enet > (package > (name "enet") > - (version "1.3.17") > + (version "1.3.18") > (source > (origin > (method url-fetch) > (uri (string-append "http://enet.bespin.org/download/" > "enet-" version ".tar.gz")) > (sha256 > - (base32 > "1p6f9mby86af6cs7pv6h48032ip9g32c05cb7d9mimam8lchz3x3")))) > + (base32 > "0djxz2j8248bsvbrs42vr39fhxlrqr3lqbhzs7yb92ync19hr2ia")))) > (build-system gnu-build-system) > (native-inputs > (list pkg-config)) > diff --git a/gnu/packages/sdl.scm b/gnu/packages/sdl.scm > index 80de707819..3a736107c8 100644 > --- a/gnu/packages/sdl.scm > +++ b/gnu/packages/sdl.scm > @@ -72,7 +72,7 @@ (define-module (gnu packages sdl) > (define-public sdl2 > (package > (name "sdl2") > - (version "2.30.8") > + (version "2.30.9") > (source (origin > (method url-fetch) > (uri > @@ -80,7 +80,7 @@ (define-public sdl2 > version ".tar.gz")) > (sha256 > (base32 > - > "0n006l1zds2av8a9p6m6l0mj7jwb3jbr6mq7j0nxg6vblxg2j31q")))) > + > "197bdcfnnsd4k7q91y518kari0p3rcqbdfq40zsn79w73kvp9d94")))) > (build-system gnu-build-system) > (arguments > (list Both these look great. Please: - Address comments, or give a compelling rationale for why the suggestion won’t work / isn’t desirable. - Run `guix lint' on your changes and make sure all issues that raises are fixed. `guix style' may help here. - Send a v2 patch series. Thanks, -- Ian
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.