Skip to content

eduke32: 20210910 -> 20221026#197798

Merged
trofi merged 1 commit intoNixOS:masterfrom
wamserma:bump-eduke
Nov 11, 2022
Merged

eduke32: 20210910 -> 20221026#197798
trofi merged 1 commit intoNixOS:masterfrom
wamserma:bump-eduke

Conversation

@wamserma
Copy link
Member

@wamserma wamserma commented Oct 25, 2022 •
edited by trofi
Loading

Description of changes

Closes #189158

ZHF: #199919 (see discussion below)

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 pr 197798". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 22.11 Release Notes (or backporting 22.05 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
    • (Release notes changes) Ran nixos/doc/manual/md-to-db.sh to update generated release notes
  • Fits CONTRIBUTING.md.

@wamserma wamserma requested review from delroth and mikroskeem and removed request for delroth October 25, 2022 22:01
@trofi
Copy link
Contributor

trofi commented Oct 25, 2022

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

1 package failed to build:
  • eduke32

@trofi
Copy link
Contributor

trofi commented Oct 25, 2022

Does it build for you? Fails here as:

build flags: -j16 SHELL=/nix/store/dd3713mm8wql4r2d5jxx0f58g16nfm4h-bash-5.1-p16/bin/bash SDLCONFIG=/nix/store/7j59kg0bsr5
9k7bqp0s4h5gybbqd160z-SDL2-2.24.0/bin/sdl2-config
Building object obj/duke3d/astub.o...^MBuilding object obj/duke3d/sounds_mapster32.o...^MBuilding object obj/duke3d/build_
icon.o...^MBuilding object obj/duke3d/m32common.o...^MBuilding object obj/duke3d/m32def.o...^MBuilding object obj/duke3d/m
32exec.o...^MBuilding object obj/duke3d/m32vars.o...^MBuilding object obj/build/build.o...^MBuilding object obj/build/conf
ig.o...^MBuilding object obj/build/startgtk.editor.o...^MBuilding: make  - -j16 --jobserver-auth=4,5 -- SDLCONFIG=/nix/sto
re/7j59kg0bsr59k7bqp0s4h5gybbqd160z-SDL2-2.24.0/bin/sdl2-config SHELL=/nix/store/dd3713mm8wql4r2d5jxx0f58g16nfm4h-bash-5.1
-p16/bin/bash
Building object obj/duke3d/actors.o...^MBuilding object obj/duke3d/anim.o...^MBuilding object obj/duke3d/cheats.o...^MBuil
ding object obj/duke3d/cmdline.o...^MBuilding object obj/duke3d/config.o...^MIn file included from source/duke3d/rsrc/eduk
e32_icon.c:2,
                 from source/duke3d/rsrc/build_icon.c:1:
source/build/include/sdl_inc.h:17:12: fatal error: SDL2/SDL.h: No such file or directory
   17 | #  include <SDL2/SDL.h>
      |            ^~~~~~~~~~~~
compilation terminated.
Failed building obj/duke3d/build_icon.o from source/duke3d/rsrc/build_icon.c!

@ofborg ofborg bot requested a review from svanderburg October 25, 2022 22:11
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Oct 25, 2022
@wamserma
Copy link
Member Author

wamserma commented Oct 25, 2022

It builds when I check out d2325a46aff and there is no commit mentioning SDL2 since then...
But if fails when I do a nixpkgs-review...

@trofi
Copy link
Contributor

trofi commented Oct 27, 2022

Ah, it's because eduke32 is already broken on master. bisect says it was broken by commit 0bea4a1 cc-wrapper/setup-hook.sh: remove duplicate flags in NIX_CFLAGS_COMPILE.

@trofi trofi mentioned this pull request Oct 27, 2022
Merged
13 tasks
trofi added a commit to trofi/nixpkgs that referenced this pull request Oct 27, 2022
@trofi
eduke32: fix build by removing explicit header paths
c3e1a96 
In NixOS#197798 (comment)
we noticed that `eduke32` fails to build as:

    source/build/include/sdl_inc.h:17:12: fatal error: SDL2/SDL.h: No such file or directory
       17 | #  include <SDL2/SDL.h>
          |            ^~~~~~~~~~~~

It statred after NixOS#191724 where
`NIX_CFLAGS_COMPILE` entries were cleaned up slightly.

This change works build failure around. But I think it's a good change:
there should be no reason to set SDL2 offset via forced include paths.
@trofi trofi mentioned this pull request Oct 27, 2022
Merged
13 tasks
@trofi
Copy link
Contributor

trofi commented Oct 27, 2022

Proposed #198019 to remove unused SDL2/ include offsets. It also happens to workaround build failure for eduke32. Should unbock this PR once merged.

@wamserma
Copy link
Member Author

wamserma commented Oct 27, 2022 •
edited
Loading

Result of nixpkgs-review pr 197798 run on aarch64-linux 1

1 package failed to build:
  • eduke32
error: builder for '/nix/store/7gzpraf0kkv1959h6dkhfn4w34yajsps-eduke32-20221018.drv' failed with exit code 2;
       last 10 log lines:
       > source/imgui/src/imgui_impl_sdl.cpp:71:10: fatal error: SDL.h: No such file or directory
       >    71 | #include <SDL.h>
       >       |          ^~~~~~~
       > compilation terminated.
       > Failed building obj/imgui/imgui_impl_sdl.o from source/imgui/src/imgui_impl_sdl.cpp!
       > make: *** [GNUmakefile:864: obj/imgui/imgui_impl_sdl.o] Error 1
       > make: *** Waiting for unfinished jobs....
       > Built object obj/imgui/imgui_demo.o 
       > Built object obj/imgui/imgui_draw.o 
       > Built object obj/imgui/imgui.o 
       For full logs, run 'nix log /nix/store/7gzpraf0kkv1959h6dkhfn4w34yajsps-eduke32-20221018.drv'.

@trofi
Copy link
Contributor

trofi commented Oct 27, 2022

AFAIU SDL/SDL2 headers should always be included as <SDL/SDL.h> / <SDL2/SDL.h>. Sometimes build systems avoid SDL{,2}/ prefix by extending the include search path. Not sure where it's supposed to happen in eduke32.

I suggest fixing headers locally as:

--- a/pkgs/games/eduke32/default.nix
+++ b/pkgs/games/eduke32/default.nix
@@ -57,6 +57,11 @@ in stdenv.mkDerivation rec {
       substituteInPlace source/glad/src/$f \
         --replace libGL.so ${libGL}/lib/libGL.so
     done
+
+    substituteInPlace source/imgui/src/imgui_impl_sdl.cpp \
+        --replace '#include <SDL.h>' '#include <SDL2/SDL.h>' \
+        --replace '#include <SDL_syswm.h>' '#include <SDL2/SDL_syswm.h>' \
+        --replace '#include <SDL_vulkan.h>' '#include <SDL2/SDL_vulkan.h>'
   '';

   makeFlags = [

WDYT?

@wamserma wamserma changed the title eduke32: 20210910 -> 20221018 eduke32: 20210910 -> 20221026 Nov 11, 2022
@trofi
Copy link
Contributor

trofi commented Nov 11, 2022

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

1 package failed to build:
  • eduke32

@wamserma
Copy link
Member Author

wamserma commented Nov 11, 2022

@trofi now ;)

@wamserma
Copy link
Member Author

wamserma commented Nov 11, 2022

Result of nixpkgs-review pr 197798 run on aarch64-linux 1

1 package built:
  • eduke32

@trofi
Copy link
Contributor

trofi commented Nov 11, 2022

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

1 package built:
  • eduke32

@trofi trofi merged commit 3e2bc67 into NixOS:master Nov 11, 2022
@trofi
Copy link
Contributor

trofi commented Nov 11, 2022

Pulled. Thank you!

Apologies for so many sidetracks with include headers.

@revuwa
Copy link

revuwa commented Nov 12, 2022

Big thanks @ all of you ❤️

@wamserma wamserma deleted the bump-eduke branch November 16, 2022 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mikroskeem mikroskeem Awaiting requested review from mikroskeem

@delroth delroth Awaiting requested review from delroth

@svanderburg svanderburg Awaiting requested review from svanderburg

1 more reviewer

@trofi trofi trofi approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

eduke32 update available

3 participants

@wamserma @trofi @revuwa