Message ID | 8c518a46ea2d8332ccbb55cb00574b75db982558.camel@disroot.org |
---|---|
State | New |
Headers | show |
Series | [bug#60153,v2,1/3] gnu: python-pygame: Update to 2.1.2. | expand |
On 12/18/22 23:51, Adam Faiz wrote: > + (add-after 'unpack 'fix-build-config > + (lambda _ > + (substitute* "buildconfig/config_unix.py" > + (("origincdirs = \\[.*\\]") > + "origincdirs = os.environ['C_INCLUDE_PATH'].split(\":\")") > + (("ORIGLIBDIRS") "LIBRARY_PATH") > + (("incdirs = \\[\\]") "incdirs = origincdirs") > + (("libdirs = \\[\\]") "libdirs = origlibdirs")))) > + (add-after 'unpack 'fix-sdl2-headers > + (lambda _ > + (substitute* "buildconfig/config_unix.py" > + (("SDL_ttf.h") "SDL2/SDL_ttf.h") > + (("SDL_image.h") "SDL2/SDL_image.h") > + (("SDL_mixer.h") "SDL2/SDL_mixer.h")) > + (substitute* "src_c/imageext.c" > + (("SDL_image.h") "SDL2/SDL_image.h")) > + (substitute* "src_c/font.h" > + (("SDL_ttf.h") "SDL2/SDL_ttf.h")) > + (substitute* "src_c/mixer.h" > + (("SDL_mixer.h") "SDL2/SDL_mixer.h")) > + (substitute* "src_c/_sdl2/mixer.c" > + (("SDL_mixer.h") "SDL2/SDL_mixer.h"))))))) I don't see why these should go in phases, when I think going in a snippet would be better. From "Snippets and Phases" in the Guix manual: > The source derived > from an origin should produce a source that can be used to build the > package on any system that the upstream package supports (i.e., act as > the corresponding source). In particular, origin snippets must not > embed store items in the sources; such patching should rather be done > using build phases. The 'fix-build-config is what I consider to be a serious bugfix, it doesn't matter that it was originally fixed as part of guix packaging. The 'fix-sld2-headers phase might stay in a build phase, if most systems that pygame supports doesn't keep these headers where guix would locate them. But I would consider that a bug in other systems for not storing the headers in the file path upstream places them. The 'fix-sdl2-headers doesn't embed store items, so there's no other justification other than the reason given above.
On 12/24/22 07:20, Adam Faiz wrote: > + (("incdirs = \\[\\]") "incdirs = origincdirs") > + (("libdirs = \\[\\]") "libdirs = origlibdirs")))) I didn't do this kind of thing, because I didn't want to hardcode these. Most systems that pygame supports want to stay with the defaults. > (substitute* "buildconfig/config_unix.py" > (("localbase.d") "d" > #~(modify-phases %standard-phases > ;; Pass the dependencies to the configure script > ;; through environment variables. > (add-before 'build 'set-library-paths > (lambda _ > (setenv "LOCALBASE" " ")))))) I added this so that Guix as well as other systems can choose to pass the dependencies at the precise locations, otherwise it uses the defaults and doesn't affect those that use the defaults. I hope this clarifies my understanding of this issue. I like that Guix can provide patched and sometimes enhanced package sources while Guix-specific things stay in the build phases.
On 12/24/22 07:43, Adam Faiz wrote: >> (substitute* "buildconfig/config_unix.py" >> (("localbase.d") "d" >> #~(modify-phases %standard-phases >> ;; Pass the dependencies to the configure script >> ;; through environment variables. >> (add-before 'build 'set-library-paths >> (lambda _ >> (setenv "LOCALBASE" " ")))))) > I added this so that Guix as well as other systems can choose to pass > the dependencies at the precise locations, otherwise it uses the > defaults and doesn't affect those that use the defaults. This part is actually flawed, what was done as part of 'fix-build-config is better. My mistake.
Am Samstag, dem 24.12.2022 um 07:20 +0800 schrieb Adam Faiz: > On 12/18/22 23:51, Adam Faiz wrote: > > + (add-after 'unpack 'fix-build-config > > + (lambda _ > > + (substitute* "buildconfig/config_unix.py" > > + (("origincdirs = \\[.*\\]") > > + "origincdirs = > > os.environ['C_INCLUDE_PATH'].split(\":\")") > > + (("ORIGLIBDIRS") "LIBRARY_PATH") > > + (("incdirs = \\[\\]") "incdirs = origincdirs") > > + (("libdirs = \\[\\]") "libdirs = origlibdirs")))) > > + (add-after 'unpack 'fix-sdl2-headers > > + (lambda _ > > + (substitute* "buildconfig/config_unix.py" > > + (("SDL_ttf.h") "SDL2/SDL_ttf.h") > > + (("SDL_image.h") "SDL2/SDL_image.h") > > + (("SDL_mixer.h") "SDL2/SDL_mixer.h")) > > + (substitute* "src_c/imageext.c" > > + (("SDL_image.h") "SDL2/SDL_image.h")) > > + (substitute* "src_c/font.h" > > + (("SDL_ttf.h") "SDL2/SDL_ttf.h")) > > + (substitute* "src_c/mixer.h" > > + (("SDL_mixer.h") "SDL2/SDL_mixer.h")) > > + (substitute* "src_c/_sdl2/mixer.c" > > + (("SDL_mixer.h") "SDL2/SDL_mixer.h"))))))) > I don't see why these should go in phases, when I think going in a > snippet would be better. From "Snippets and Phases" in the Guix > manual: > > > The source derived from an origin should produce a source that can > > be used to build the package on any system that the upstream > > package supports (i.e., act as the corresponding source). In > > particular, origin snippets must not embed store items in the > > sources; such patching should rather be done using build phases. > The 'fix-build-config is what I consider to be a serious bugfix, it > doesn't matter that it was originally fixed as part of guix > packaging. That part of the manual is sadly in need of a serious overhaul (see [1] for a more extended discussion). In any case, patching the source so that it finds certain headers or libraries is typically a #:phases thing, not a snippet one. The move to #:phases reflects that. Cheers [1] https://issues.guix.gnu.org/57598
Am Samstag, dem 24.12.2022 um 07:43 +0800 schrieb Adam Faiz: > On 12/24/22 07:20, Adam Faiz wrote: > > + (("incdirs = \\[\\]") "incdirs = origincdirs") > > + (("libdirs = \\[\\]") "libdirs = origlibdirs")))) > I didn't do this kind of thing, because I didn't want to hardcode > these. > Most systems that pygame supports want to stay with the defaults. Here, "using the defaults" equates to having both origincdirs and origlibdirs being empty, i.e. expanded from non-existing or empty environment variables, right? In that case, the build config will still add the default /usr stuff – it just won't start out empty. I feel as though we could do without this patch by supplying PYGAME_EXTRA_BASE as a configure flag. Wouldn't that be better than patching? Cheers
On 12/24/22 13:51, Liliana Marie Prikler wrote: > Am Samstag, dem 24.12.2022 um 07:43 +0800 schrieb Adam Faiz: >> On 12/24/22 07:20, Adam Faiz wrote: >>> + (("incdirs = \\[\\]") "incdirs = origincdirs") >>> + (("libdirs = \\[\\]") "libdirs = origlibdirs")))) >> I didn't do this kind of thing, because I didn't want to hardcode >> these. >> Most systems that pygame supports want to stay with the defaults. > Here, "using the defaults" equates to having both origincdirs and > origlibdirs being empty, i.e. expanded from non-existing or empty > environment variables, right? In that case, the build config will > still add the default /usr stuff – it just won't start out empty. Yes, I hadn't considered that when sending the patch. > I feel as though we could do without this patch by supplying > PYGAME_EXTRA_BASE as a configure flag. Wouldn't that be better than > patching? Indeed, it's better than patching and should be done if it works for you. I feel it's suboptimal though because the include path doesn't deserve to be called "extra". > Cheers
diff --git a/gnu/packages/game-development.scm b/gnu/packages/game-development.scm index 8fec474d0b..07b2c6f3fe 100644 --- a/gnu/packages/game-development.scm +++ b/gnu/packages/game-development.scm @@ -1194,82 +1194,49 @@ (define-public quesoglc (define-public python-pygame (package (name "python-pygame") - (version "1.9.4") + (version "2.1.2") (source (origin (method url-fetch) (uri (pypi-uri "pygame" version)) (sha256 (base32 - "1dn0nb86jl7yr8709cncxdr0yrmviqakw7zx3g8jbbwrr60if3bh")))) + "0g6j79naab7583kymf1bgxc5l5c9h5laq887rmvh8vw8iyifrl6n")))) (build-system python-build-system) (arguments `(#:tests? #f ; tests require pygame to be installed first #:phases (modify-phases %standard-phases - ;; Set the paths to the dependencies manually because - ;; the configure script does not allow passing them as - ;; parameters. This also means we can skip the configure - ;; phase. - (add-before 'build 'set-library-paths - (lambda* (#:key inputs outputs #:allow-other-keys) - (let ((sdl-ref (assoc-ref inputs "sdl")) - (font-ref (assoc-ref inputs "sdl-ttf")) - (image-ref (assoc-ref inputs "sdl-image")) - (mixer-ref (assoc-ref inputs "sdl-mixer")) - (smpeg-ref (assoc-ref inputs "libsmpeg")) - (png-ref (assoc-ref inputs "libpng")) - (jpeg-ref (assoc-ref inputs "libjpeg")) - (freetype-ref (assoc-ref inputs "freetype")) - (v4l-ref (assoc-ref inputs "v4l-utils")) - (out-ref (assoc-ref outputs "out"))) - (substitute* "Setup.in" - (("SDL = -I/usr/include/SDL") - (string-append "SDL = -I" sdl-ref "/include/SDL -I."))) - (substitute* "Setup.in" - (("FONT = -lSDL_ttf") - (string-append "FONT = -I" font-ref "/include/SDL -L" - font-ref "/lib -lSDL_ttf"))) - (substitute* "Setup.in" - (("IMAGE = -lSDL_image") - (string-append "IMAGE = -I" image-ref "/include/SDL -L" - image-ref "/lib -lSDL_image"))) - (substitute* "Setup.in" - (("MIXER = -lSDL_mixer") - (string-append "MIXER = -I" mixer-ref "/include/SDL -L" - mixer-ref "/lib -lSDL_mixer"))) - (substitute* "Setup.in" - (("SMPEG = -lsmpeg") - (string-append "SMPEG = -I" smpeg-ref "/include/smpeg -L" - smpeg-ref "/lib -lsmpeg"))) - (substitute* "Setup.in" - (("PNG = -lpng") - (string-append "PNG = -I" png-ref "/include -L" - png-ref "/lib -lpng"))) - (substitute* "Setup.in" - (("JPEG = -ljpeg") - (string-append "JPEG = -I" jpeg-ref "/include -L" - jpeg-ref "/lib -ljpeg"))) - - (substitute* "Setup.in" - (("FREETYPE = -lfreetype") - (string-append "FREETYPE = -I" freetype-ref "/include/freetype2 -L" - freetype-ref "/lib -lfreetype"))) - - (substitute* "Setup.in" - (("^pypm") "#pypm")) - ;; Create a path to a header file provided by v4l-utils. - (system* "mkdir" "linux") - (system* "ln" "--symbolic" - (string-append v4l-ref "/include/libv4l1-videodev.h") - "linux/videodev.h") - (system* "ln" "--symbolic" "Setup.in" "Setup"))))))) + (add-after 'unpack 'fix-build-config + (lambda _ + (substitute* "buildconfig/config_unix.py" + (("origincdirs = \\[.*\\]") + "origincdirs = os.environ['C_INCLUDE_PATH'].split(\":\")") + (("ORIGLIBDIRS") "LIBRARY_PATH") + (("incdirs = \\[\\]") "incdirs = origincdirs") + (("libdirs = \\[\\]") "libdirs = origlibdirs")))) + (add-after 'unpack 'fix-sdl2-headers + (lambda _ + (substitute* "buildconfig/config_unix.py" + (("SDL_ttf.h") "SDL2/SDL_ttf.h") + (("SDL_image.h") "SDL2/SDL_image.h") + (("SDL_mixer.h") "SDL2/SDL_mixer.h")) + (substitute* "src_c/imageext.c" + (("SDL_image.h") "SDL2/SDL_image.h")) + (substitute* "src_c/font.h" + (("SDL_ttf.h") "SDL2/SDL_ttf.h")) + (substitute* "src_c/mixer.h" + (("SDL_mixer.h") "SDL2/SDL_mixer.h")) + (substitute* "src_c/_sdl2/mixer.c" + (("SDL_mixer.h") "SDL2/SDL_mixer.h"))))))) + (native-inputs + (list pkg-config)) (inputs `(("freetype" ,freetype) - ("sdl" ,sdl) - ("sdl-image" ,sdl-image) - ("sdl-mixer" ,sdl-mixer) - ("sdl-ttf" ,sdl-ttf) - ("sdl-gfx" ,sdl-gfx) + ("sdl2" ,sdl2) + ("sdl2-image" ,sdl2-image) + ("sdl2-mixer" ,sdl2-mixer) + ("sdl2-ttf" ,sdl2-ttf) + ("sdl2-gfx" ,sdl2-gfx) ("libjpeg" ,libjpeg-turbo) ("libpng" ,libpng) ("libX11" ,libx11)