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.

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

devilutionx: 1.0.1 -> 1.1.0 @ 1432fbc8e #101633

Conversation

@karolchmist
Copy link
Contributor

@karolchmist karolchmist commented Oct 25, 2020

Motivation for this change

This fixes the issue of devilutionx build failing.

The fix was committed upstream, and there is not yet an official release containing it, so I used this commit temporarily. As soon as a new release is available, we should update to it.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.
Copy link
Member

@SuperSandro2000 SuperSandro2000 left a comment

Fails to build for darwin:

-- The C compiler identification is Clang 7.1.0
-- The CXX compiler identification is Clang 7.1.0
-- Detecting C compiler ABI info
-- Detecting C compiler ABI info - done
-- Check for working C compiler: /nix/store/gkcr1p2x0psdc31bavmqbfb2nwq0d6jn-clang-wrapper-7.1.0/bin/clang - skipped
-- Detecting C compile features
-- Detecting C compile features - done
-- Detecting CXX compiler ABI info
-- Detecting CXX compiler ABI info - done
-- Check for working CXX compiler: /nix/store/gkcr1p2x0psdc31bavmqbfb2nwq0d6jn-clang-wrapper-7.1.0/bin/clang++ - skipped
-- Detecting CXX compile features
-- Detecting CXX compile features - done
-- Looking for pthread.h
-- Looking for pthread.h - found
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD
-- Performing Test CMAKE_HAVE_LIBC_PTHREAD - Success
-- Found Threads: TRUE
-- Found sodium: /nix/store/k5gzm3szsjrnp14l3yg8mayxa0zvkrwm-libsodium-1.0.18/lib/libsodium.dylib (found version "1.0.18")
CMake Error at CMake/FindSDL2.cmake:254 (set_property):
  set_property could not find TARGET SDL2::SDL2.  Perhaps it has not yet been
  created.
Call Stack (most recent call first):
  CMakeLists.txt:150 (find_package)


CMake Error at /nix/store/z7qvx6wbvmjvs767g06h03pkzr5aqn6i-cmake-3.18.2/share/cmake-3.18/Modules/FindPackageHandleStandardArgs.cmake:165 (message):
  Could NOT find SDL2 (missing: SDL2_SDLMAIN_LIBRARY)
Call Stack (most recent call first):
  /nix/store/z7qvx6wbvmjvs767g06h03pkzr5aqn6i-cmake-3.18.2/share/cmake-3.18/Modules/FindPackageHandleStandardArgs.cmake:458 (_FPHSA_FAILURE_MESSAGE)
  CMake/FindSDL2.cmake:152 (find_package_handle_standard_args)
  CMakeLists.txt:153 (find_package)


-- Configuring incomplete, errors occurred!
See also "/private/var/folders/r4/k85s52jx3555ckj0wty615s00000gp/T/nix-build-devilutionx-1.1.0-432fbc8e.drv-0/source/build/CMakeFiles/CMakeOutput.log".
See also "/private/var/folders/r4/k85s52jx3555ckj0wty615s00000gp/T/nix-build-devilutionx-1.1.0-432fbc8e.drv-0/source/build/CMakeFiles/CMakeError.log".
builder for '/nix/store/zd70lwcq8z3nni78lsnzpqffzlnkizln-devilutionx-1.1.0-432fbc8e.drv' failed with exit code 1
error: build of '/nix/store/zd70lwcq8z3nni78lsnzpqffzlnkizln-devilutionx-1.1.0-432fbc8e.drv' failed

Copy link
Member

@rapenne-s rapenne-s left a comment

The game runs fine on Nixos amd64
Changes in the nix file looks fine to me.

@karolchmist
Copy link
Contributor Author

@karolchmist karolchmist commented Nov 1, 2020

Fails to build for darwin:

Hey @SuperSandro2000, thanks for checking this up. I don't know how to fix it though... Any ideas? Otherwise I'd remove the darwin build until someone has a fix.

@SuperSandro2000
Copy link
Member

@SuperSandro2000 SuperSandro2000 commented Nov 1, 2020

Hey @SuperSandro2000, thanks for checking this up. I don't know how to fix it though... Any ideas? Otherwise I'd remove the darwin build until someone has a fix.

No clue. Could be an upstream issue.

@karolchmist karolchmist force-pushed the devilutionx-1.1.0-432fbc8e-fixed-dynamic-sdl2 branch from 570dc46 to bdf4666 Nov 7, 2020
@karolchmist
Copy link
Contributor Author

@karolchmist karolchmist commented Nov 7, 2020

Disabled darwin build. Seems that it works fine on linux. Can it be merged?

@karolchmist
Copy link
Contributor Author

@karolchmist karolchmist commented Nov 19, 2020

Hello @Mic92, could you please merge this?

@dasJ dasJ requested a review from ajs124 Dec 10, 2020
@aanderse
Copy link
Member

@aanderse aanderse commented Apr 1, 2021

@karolchmist can you please update this to follow the package naming conventions? I'm more than happy to merge once we have that in place.

Thank you!

@ajs124 ajs124 removed their request for review Apr 1, 2021
@karolchmist karolchmist force-pushed the devilutionx-1.1.0-432fbc8e-fixed-dynamic-sdl2 branch from bdf4666 to c846444 Apr 4, 2021
@karolchmist
Copy link
Contributor Author

@karolchmist karolchmist commented Apr 4, 2021

Hello @aanderse , I changed the naming according to the convention, sorry for that :) I can squash the commits if it's ok with you now.

@aanderse
Copy link
Member

@aanderse aanderse commented Apr 4, 2021

Sounds good, thanks! I appreciate your efforts on this.

Set devilutionx to commit 432fbc8e that fixes build with dynamic SDL2. Disable failing devilutionx build on darwin.
@karolchmist karolchmist force-pushed the devilutionx-1.1.0-432fbc8e-fixed-dynamic-sdl2 branch from c846444 to 8479a42 Apr 4, 2021
@karolchmist
Copy link
Contributor Author

@karolchmist karolchmist commented Apr 4, 2021

@aanderse squashed, thanks!

@aanderse aanderse merged commit 60d4ac8 into NixOS:master Apr 4, 2021
19 checks passed
version = "1.0.1";
pname = "devilutionx";
version = "2020-10-20";
pname = "devilutionx-unstable";
Copy link
Member

@SuperSandro2000 SuperSandro2000 Apr 4, 2021

The pname should not change here. unstable should be part of the version if we only have one package for devolutionx.

"-DBINARY_RELEASE=ON"
];

enableParallelBuilding = true;
Copy link
Member

@SuperSandro2000 SuperSandro2000 Apr 4, 2021

cmake sets this automatically.

@aanderse
Copy link
Member

@aanderse aanderse commented Apr 5, 2021

Sorry for the hassle @SuperSandro2000. Thanks for the catch and cleanup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants