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

ImageMagick: Silence flood of deprecation warnings from GCC #195766

Open
wants to merge 1 commit into
base: staging
Choose a base branch
from

Conversation

YoshiRulz
Copy link
Contributor

@YoshiRulz YoshiRulz commented Oct 13, 2022

Description of changes

When building ImageMagick there's a warning on seemingly every source file:

cc1: warning: '-mtune=x86-64' is deprecated; use '-mtune=k8' or '-mtune=generic' instead as appropriate [8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wdeprecated-Wdeprecated8;;]

...obscuring "real" warnings. This PR applies the suggested fix of s/-mtune=x86-64/-mtune=generic/.

Resubmission of #195757 to target branch staging. Apologies for the notif spam. GitHub should really put a warning on that.
And to answer @AndersonTorres' question, I believe this is Nix-specific as the warning didn't appear during a manual build on my Manjaro machine nor in upstream's CI (Ubuntu Docker image).

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.11 Release Notes (or backporting 23.11 and 24.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
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

@rhendric
Copy link
Member

I've been looking into this PR, and I think there's something dodgy about the way nixpkgs is using --with-gcc-arch that should be fixed instead of patching configure as this PR does.

Per the relevant documentation, the --with-gcc-arch flag here drives -mtune, not -march as one might assume (ImageMagick is calling this macro with [yes]). The values recognized by -mtune, as was previously noted in #125150, do not include aarch64, and now it appears that x86-64 is also deprecated.

The --with-gcc-arch-using code seems to have been copied over from the ImageMagick 6 file, in which it was added nine years ago in 6f53886. However, the use of the AX_GCC_ARCHFLAG macro in ImageMagick was switched from [no] to [yes] in ImageMagick/ImageMagick@c7329a2 eight years ago, meaning that when our illustrious elder wrote that arch code it actually did affect -march and not -mtune.

I think the right thing to do here is drop the arch declaration and replace the lib.withFeatureAs line with a constant "--without-gcc-arch" or "--with-gcc-arch=generic"; I'm not sure which is better.

@wegank wegank added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Mar 19, 2024
I believe this is Nix-specific as it didn't appear during manual build on Arch
or in upstream's CI (Ubuntu Docker image).
The message, printed for every source file, is:
cc1: warning: '-mtune=x86-64' is deprecated; use '-mtune=k8' or '-mtune=generic'
instead as appropriate
[8;;https://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html#index-Wdeprecated-Wdeprecated8;;]
@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Jul 31, 2024
@ofborg ofborg bot requested a review from rhendric July 31, 2024 04:11
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.

3 participants