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
Member

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@karolchmist
Copy link
Member Author

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

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 Compare November 7, 2020 14:12
@karolchmist
Copy link
Member Author

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

@karolchmist
Copy link
Member Author

Hello @Mic92, could you please merge this?

@dasJ dasJ requested a review from ajs124 December 10, 2020 10:43
@aanderse
Copy link
Member

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 April 1, 2021 14:35
@karolchmist karolchmist force-pushed the devilutionx-1.1.0-432fbc8e-fixed-dynamic-sdl2 branch from bdf4666 to c846444 Compare April 4, 2021 20:26
@karolchmist
Copy link
Member Author

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 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 Compare April 4, 2021 20:43
@karolchmist
Copy link
Member Author

@aanderse squashed, thanks!

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cmake sets this automatically.

@aanderse
Copy link
Member

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
Development

Successfully merging this pull request may close these issues.

None yet

4 participants