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

cataclysm-dda: fix gcc 13 build with 3 debian patches. #282444

Merged
merged 1 commit into from
Mar 1, 2024

Conversation

detroyejr
Copy link
Contributor

@detroyejr detroyejr commented Jan 20, 2024

Description of changes

Edit:
Fixed failing gcc 13 build with 3 patches from Debian.

[patch] disable dangling reference warning
[patch] fix build with gcc 13
[patch] cleanup autogenerated prefix.h

Fixed build failures for cataclysm-dda and cataclysm-dda-git by using disable-warnings-if-gcc13 (see #268097).

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
    • sandbox = relaxed
    • sandbox = true
  • Tested, as applicable:
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • 24.05 Release Notes (or backporting 23.05 and 23.11 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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@wegank
Copy link
Member

wegank commented Jan 20, 2024

Instead of disabling warnings, can we apply patches from Debian or from upstream?

@detroyejr
Copy link
Contributor Author

Yes, adding 3 patches to pkgs/games/cataclysm-dda/stable.nix fixes the build as well.

Would you mind telling me if fetchpatch is the best way to apply these or if I should use something else?

    patches = [
      # Unconditionally look for translation files in $out/share/locale
      ./locale-path.patch
      (fetchpatch {
        url = "https://sources.debian.org/data/main/c/cataclysm-dda/0.G-4/debian/patches/gcc13-dangling-reference-warning.patch";
        hash = "sha256-9nPbyz49IYBOVHqr7jzCIyS8z/SQgpK4EjEz1fruIPE=";
      })
      (fetchpatch {
        url = "https://sources.debian.org/data/main/c/cataclysm-dda/0.G-4/debian/patches/gcc13-cstdint.patch";
        hash = "sha256-8IBW2OzAHVgEJZoViQ490n37sl31hA55ePuqDL/lil0=";
      })
      (fetchpatch {
        url = "https://sources.debian.org/data/main/c/cataclysm-dda/0.G-4/debian/patches/gcc13-keyword-requires.patch";
        hash = "sha256-8yvHh0YKC7AC/qzia7AZAfMewMC0RiSepMXpOkMXRd8=";
      })
    ];

@wegank
Copy link
Member

wegank commented Jan 21, 2024

Would you mind telling me if fetchpatch is the best way to apply these or if I should use something else?

Using fetchpatch is preferred, and your code looks good.

@detroyejr detroyejr changed the title cataclysm-dda: fixed build with disable-warnings-if-gcc13 cataclysm-dda: fix gcc 13 build with 3 debian patches. Jan 21, 2024
@detroyejr
Copy link
Contributor Author

Added those 3 patches to the stable.nix and git.nix files in the cataclysm-dda folder. Both build now. Let me know if there's anything else needed.

@DeeUnderscore
Copy link
Contributor

Looks like upstream has these fixed in the master branch as well (independent of the Debian patches):

Haven't checked if they apply cleanly, so the Debian patches might be preferable here. Would be a good idea to include a comment that says something like "Fixes for failing build with GCC13, remove on updating to next release after 0.G"

@DeeUnderscore
Copy link
Contributor

Thanks, looks good, but please squash everything down to one commit with an appropriate commit message.

Release 0.G needs 3 patches to build successfully with gcc 13. These can
be removed after the next release.

[patch] disable dangling reference warning
[patch] fix build with gcc 13
[patch] cleanup autogenerated prefix.h
@detroyejr
Copy link
Contributor Author

Sounds good. Should be good to go.

@wegank
Copy link
Member

wegank commented Mar 1, 2024

@ofborg build cataclysm-dda cataclysm-dda.passthru.tests

@wegank wegank merged commit bd61395 into NixOS:master Mar 1, 2024
21 of 24 checks passed
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.

6 participants