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

glib,gtk: correct CLAGS for plain buildtype #65850

Merged
merged 5 commits into from Aug 5, 2019

Conversation

@worldofpeace
Copy link
Member

commented Aug 3, 2019

Motivation for this change

A little fallout from #58310 but this should correct things to what
I believe they were or should have been.

Noticed this fairly quickly because of #65843.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS)
  • 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 nix-review --run "nix-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.
@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 4, 2019

Guess some discussion might be happening upstream

@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 4, 2019

Looks like we should do G_DISABLE_CAST_CHECKS in gnome-settings-daemon too

Edit: GNOME 3.34 we might want to set this

@worldofpeace worldofpeace force-pushed the worldofpeace:bye-g-disable-checks branch from ffa497a to d45d4df Aug 4, 2019

# Default for release buildtype but passed manually because
# we're using plain
"-DG_DISABLE_CAST_CHECKS"
] ++ optional stdenv.isSunOS "-DBSD_COMP";

This comment has been minimized.

Copy link
@jtojnar

jtojnar Aug 5, 2019

Contributor

Any idea what is this for? It originated in 8ae2a89 but I do not see any mentions in git log -p -SBSD_COMP in Glib repository.

This comment has been minimized.

Copy link
@worldofpeace

worldofpeace Aug 5, 2019

Author Member

Yeah I have no idea either, since we've diverged so much from then I think we can just drop it.

@jtojnar

This comment has been minimized.

Copy link
Contributor

commented Aug 5, 2019

Could you add links to relevant sections of meson.build to the other commit messages as well?

Also please describe in the gtk3 commit message why we need G_ENABLE_DEBUG. It will be very helpful in the future, when I will be trying to figure out which flags to prune using git blame.

Otherwise, it looks fine.

@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 5, 2019

Could you add links to relevant sections of meson.build to the other commit messages as well?

Also please describe in the gtk3 commit message why we need G_ENABLE_DEBUG. It will be very helpful in the future, when I will be trying to figure out which flags to prune using git blame.

Otherwise, it looks fine.

Yep, will do all of that.

worldofpeace added 4 commits Aug 3, 2019
gtk3: defines for debugging, disable cast checks
Because we're using plain buildtype these have to be
passed manually.

See: https://gitlab.gnome.org/GNOME/gtk/blob/3.24.10/meson.build#L59

With autotools the mapping for the the options to the defines was:

yes:     G_ENABLE_DEBUG G_ENABLE_CONSISTENCY_CHECKS
minimum: G_ENABLE_DEBUG G_DISABLE_CAST_CHECKS
no:      G_DISABLE_CAST_CHECKS G_DISABLE_ASSERT G_DISABLE_CHECKS

So we're passing the exact ones that would've been used for minimum.

Additionally it isn't a good idea to pass the equivalents used for "no" 
as it eliminates G_ENABLE_DEBUG which disables pre-condition checks and 
assertions. The actual option only existed to serve people who needed a 
specific build of GTK for very specific environments. And now they are
much better served with meson's plain buildtype and figuring out what to
pass themselves.
glib: add cflag G_DISABLE_CAST_CHECKS
This is what would have been passed before with the release
buildtype.

See: https://gitlab.gnome.org/GNOME/glib/blob/2.60.4/meson.build#L208

@worldofpeace worldofpeace force-pushed the worldofpeace:bye-g-disable-checks branch from d45d4df to 3b085b4 Aug 5, 2019

@worldofpeace

This comment has been minimized.

Copy link
Member Author

commented Aug 5, 2019

@jtojnar rewritten/revised

glib: drop define BSD_COMP
I fail to see where or for what it is useful for.

@ofborg ofborg bot requested review from jtojnar and 7c6f434c Aug 5, 2019

@jtojnar
jtojnar approved these changes Aug 5, 2019

@worldofpeace worldofpeace merged commit a2253f4 into NixOS:staging Aug 5, 2019

15 of 16 checks passed

glib, gnome3.gnome-settings-daemon, gtk3, pantheon.elementary-settings-daemon on x86_64-linux Timed out, unknown build status
Details
Evaluation Performance Report Evaluator Performance Report
Details
glib, gnome3.gnome-settings-daemon, gtk3, pantheon.elementary-settings-daemon on aarch64-linux Success
Details
glib, gnome3.gnome-settings-daemon, gtk3, pantheon.elementary-settings-daemon on x86_64-darwin Success
Details
grahamcofborg-eval ^.^!
Details
grahamcofborg-eval-check-maintainers matching changed paths to changed attrs...
Details
grahamcofborg-eval-check-meta config.nix: checkMeta = true
Details
grahamcofborg-eval-darwin nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A darwin-tested
Details
grahamcofborg-eval-nixos nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release-combined.nix -A tested
Details
grahamcofborg-eval-nixos-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A manual
Details
grahamcofborg-eval-nixos-options nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./nixos/release.nix -A options
Details
grahamcofborg-eval-nixpkgs-manual nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A manual
Details
grahamcofborg-eval-nixpkgs-tarball nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A tarball
Details
grahamcofborg-eval-nixpkgs-unstable-jobset nix-instantiate --arg nixpkgs { outPath=./.; revCount=999999; shortRev="ofborg"; } ./pkgs/top-level/release.nix -A unstable
Details
grahamcofborg-eval-package-list nix-env -qa --json --file .
Details
grahamcofborg-eval-package-list-no-aliases nix-env -qa --json --file . --arg config { allowAliases = false; }
Details

@worldofpeace worldofpeace deleted the worldofpeace:bye-g-disable-checks branch Aug 5, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.