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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

wxGTK32: fix console crash #269587

Merged
merged 1 commit into from Dec 3, 2023
Merged

Conversation

gador
Copy link
Contributor

@gador gador commented Nov 24, 2023

Description of changes

If console programs use the wxApp class it will
crash with GTK3 with:
"g_object_get: assertion 'G_IS_OBJECT (object)' failed"

This patch fixes it.

See wxWidgets/wxWidgets#23981 and veracrypt/VeraCrypt#1263

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/)
  • 23.11 Release Notes (or backporting 23.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.

Priorities

Add a 馃憤 reaction to pull requests you find important.

@evils
Copy link
Member

evils commented Nov 24, 2023

based on the patch applying and it not breaking KiCad i have no objections to this

i however cannot reproduce the issues linked to
kicad-cli sch export svg works fine (KiCad 7.0.9 in addition to nixos-unstable)
and veracrypt 1.26.7 builds

is anything in nixpkgs affected by this issue?

@gador
Copy link
Contributor Author

gador commented Nov 24, 2023

is anything in nixpkgs affected by this issue?

Yes, building is not the issue, but e.g. running veracrypt --text breaks.

@SuperSandro2000
Copy link
Member

Not sure if wxWidgets/wxWidgets@8ea22b5#commitcomment-133381150 is relevant

@evils
Copy link
Member

evils commented Nov 24, 2023

oh, why does this target NixOS:staging?

@gador
Copy link
Contributor Author

gador commented Nov 24, 2023

it had a lot more rebuilds when I tried running nixpkgs-review. But it seems I was wrong. We can retarget at master

@evils
Copy link
Member

evils commented Nov 25, 2023

on nixos-unstable, updated to 1.26.7, veracrypt --text shows the help text and exits 1, but doesn't crash
veracrypt --text --create prompts for user input, but i don't know how to use this
veracrypt --text --test also works

@gador
Copy link
Contributor Author

gador commented Nov 25, 2023

on nixos-unstable, updated to 1.26.7, veracrypt --text shows the help text and exits 1, but doesn't crash veracrypt --text --create prompts for user input, but i don't know how to use this veracrypt --text --test also works

yes. Because you tested on unstable
Try running:

NIXPKGS_ALLOW_UNFREE=1;nix run --impure nixpkgs#veracrypt -- --text

to see the error on current master. (Independent of veracrypt version)

@evils
Copy link
Member

evils commented Nov 25, 2023

to see the error on current master.

not seeing it, maybe because i'm on sway (wayland)?

also not when building veracrypt from master directly, or veracrypt 1.26.7 on top of master

@gador
Copy link
Contributor Author

gador commented Nov 25, 2023

Not sure if wxWidgets/wxWidgets@8ea22b5#commitcomment-133381150 is relevant

Yes. Since the patch was already committed upstream, I would just use this one. This patch will be obsolete with the next version of wxWidgets either way

@gador
Copy link
Contributor Author

gador commented Nov 25, 2023

to see the error on current master.

not seeing it, maybe because i'm on sway (wayland)?

also not when building veracrypt from master directly, or veracrypt 1.26.7 on top of master

Thats strange. For me it worked on unstable and the error is reproducible on master (and fixed with this PR).

I am using KDE currently. Maybe you're right and the DM plays a role here

EDIT: Upstream reported the error on wayland, too: wxWidgets/wxWidgets#24081 (comment)

@evils
Copy link
Member

evils commented Nov 25, 2023

also no issue on master with either version of veracrypt in a nix-shell --pure

EDIT: or with master reverted to wxWidgets 3.2.3

@gador
Copy link
Contributor Author

gador commented Nov 25, 2023

also no issue on master with either version of veracrypt in a nix-shell --pure

EDIT: or with master reverted to wxWidgets 3.2.3

I have veracrypt fail with --pure.
I just ran a VM with gnome: Same error with veracrypt on current master.
I tried running sway on wayland in a VM, but no luck until now.

Can you try running a VM with other DMs than wayland? It seems there are numerous options for GTK interoperability with sway and wayland (e.g. programs.sway.wrapperFeatures.gtk) and no easy config (see https://nixos.wiki/wiki/Sway), and I'm not sure whether they could play a role here

EDIT:
For reference, this is my "go-to" VM config:

config
{
  description = "Example";

  # inputs.nixpkgs.url = "github:NixOS/nixpkgs/nixos-22.11";
  inputs.nixpkgs.url = "/home/gador/software/git/nixpkgs";

  outputs = inputs@{ self, nixpkgs }:

    let
      system = "x86_64-linux";
      pkgs = nixpkgs.legacyPackages.${system};
    in
    {

      nixosConfigurations = {

        my-machine = nixpkgs.lib.nixosSystem {
          inherit system;
          modules = [{
            services.qemuGuest.enable = true;
            documentation.nixos.enable = false;
            services.xserver = {
              enable = true;
              layout = "de";
              xkbOptions = "eurosign:e,caps:swapescape";
            };

            services.xserver.displayManager.gdm.enable = true;
            services.xserver.desktopManager.gnome.enable = true;

            i18n.defaultLocale = "de_DE.UTF-8";
            time.timeZone = "Europe/Berlin";
            console = {
              font = "Lat2-Terminus16";
              useXkbConfig = true;
            };
            nix = {
              settings.auto-optimise-store = true;
              extraOptions = ''
                experimental-features = nix-command flakes ca-derivations
              '';
              settings = {
                keep-derivations = true;
                builders-use-substitutes = true;
                connect-timeout = 5;
                log-lines = 25;
                fallback = true;
                trusted-users = [ "test" ];
              };
            };

            system.stateVersion = "23.11";

            # due to https://github.com/NixOS/nixpkgs/issues/196755
            virtualisation.vmVariant = {
              #virtualisation.resolution = { x = 1280; y = 1024; };
              virtualisation = {
                memorySize = 4096;
                cores = 4; # Simulate 4 cores.
                diskSize = 4096;
              };
            };

            users.users."test".password = "test";
            users.users."test".isNormalUser = true;
            users.users.root.password = "test";
            users.mutableUsers = false;
            security.sudo.execWheelOnly = true;
            users.users."test".extraGroups = [ "wheel" ];


            # stuff to test
            # use nixos-rebuild build-vm --flake .#my-machine

            # VERACRYPT
            environment.systemPackages = with pkgs; [ veracrypt ];
          }];
        };
      };

      my-machine =
        self.nixosConfigurations.my-machine.config.system.build.toplevel;
    };
}

@evils
Copy link
Member

evils commented Nov 25, 2023

i'm not used to running declarative VMs (or flakes)

but i've failed to reproduce the failure on the gnome and gnome-xorg NixOS test

@evils
Copy link
Member

evils commented Nov 25, 2023

maybe this is a good point to repeat that i don't actually have an objection to including this change...

@gador
Copy link
Contributor Author

gador commented Nov 25, 2023

maybe this is a good point to repeat that i don't actually have an objection to including this change...

Ha, yes, good to know :-D

I managed to test sway now in a VM (for reference: #147392 helped. and the NixOS sway tests).
The error is not present in sway. So I managed to reproduce your working sample.

To sum up:

  • veracrypt --text fails on KDE
  • veracrypt --text fails on Gnome
  • veracrypt --text works on wayland/sway
  • independent of veracrypt or wxWidgets version
  • This patch fixes the failed cases on KDE and Gnome

If console programs use the wxApp class it will
crash with GTK3 with:
"g_object_get: assertion 'G_IS_OBJECT (object)' failed"

This patch fixes it.

See wxWidgets/wxWidgets#23981
and veracrypt/VeraCrypt#1263

Signed-off-by: Florian Brandes <florian.brandes@posteo.de>
@gador gador force-pushed the wxwidgets-fix-console-crash branch from c657c76 to 51e8bd8 Compare November 25, 2023 08:38
@gador gador changed the base branch from staging to master November 25, 2023 08:38
@gador
Copy link
Contributor Author

gador commented Nov 25, 2023

Changed base to master

Copy link
Member

@evils evils left a comment

Choose a reason for hiding this comment

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

nixpkgs-review
i don't think this caused the erlang-ls failure, it also showed up on a review of wxGTK 3.2.4

1 package marked as broken and skipped:
springLobby

1 package failed to build:
erlang-ls

210 packages built:
abracadabra aegisub akkoma amule amule-daemon amule-gui amule-web asc audacity bochs boinc bossa bossa-arduino cemu chirp chirp.dist cl codeblocks codeblocksFull comical couchdb3 cqrlog cubicsdr diff-pdf digikam displaycal displaycal.dist dump1090 dvdstyler easyabc ejabberd electricsheep elixir elixir-ls elixir_1_10 elixir_1_11 elixir_1_12 elixir_1_13 elixir_1_14 elvis-erlang erlang erlang_24 erlang_26 erlang_javac erlang_odbc erlang_odbc_javac erlfmt espanso espanso-wayland espeakedit far2l filezilla fityk flamerobin freedv freefilesync freqtweak fsg gnudatalanguage gnuradio3_8Packages.ais gnuradio3_8Packages.limesdr gnuradio3_8Packages.osmosdr gnuradio3_8Packages.osmosdr.dev gnuradio3_9Packages.osmosdr gnuradio3_9Packages.osmosdr.dev gnuradioPackages.osmosdr gnuradioPackages.osmosdr.dev golly gqrx gqrx-gr-audio gqrx-portaudio grandorgue grass hugin indi-full kicad kicad-small kicad-unstable kicad-unstable-small kicadAddons.kikit kicadAddons.kikit-library kikit kikit.dist klipper-firmware klipper-flash kstars lenmus lfe limesuite livebook loxodo loxodo.dist mavproxy mavproxy.dist mediainfo-gui meerk40t meerk40t.dist megaglest mercury metamorphose2 mix2nix mmex mobilizon mymcplus mymcplus.dist notmuch-bower odamex opencpn openwebrx openwebrx.dist pcem perl536Packages.AlienWxWidgets perl536Packages.AlienWxWidgets.devdoc perl536Packages.AppMusicChordPro perl536Packages.AppMusicChordPro.devdoc perl536Packages.Wx perl536Packages.Wx.devdoc perl536Packages.WxGLCanvas perl536Packages.WxGLCanvas.devdoc perl538Packages.AlienWxWidgets perl538Packages.AlienWxWidgets.devdoc perl538Packages.AppMusicChordPro perl538Packages.AppMusicChordPro.devdoc perl538Packages.Wx perl538Packages.Wx.devdoc perl538Packages.WxGLCanvas perl538Packages.WxGLCanvas.devdoc phd2 plausible playonlinux pleroma poedit pothos printrun printrun.dist pterm pwsafe python310Packages.humblewx python310Packages.humblewx.dist python310Packages.kicad python310Packages.pcbnew-transition python310Packages.pcbnew-transition.dist python310Packages.soapysdr-with-plugins python310Packages.wxPython_4_2 python311Packages.humblewx python311Packages.humblewx.dist python311Packages.kicad python311Packages.pcbnew-transition python311Packages.pcbnew-transition.dist python311Packages.soapysdr-with-plugins python311Packages.wxPython_4_2 qgis qgis-ltr qradiolink quisk quisk.dist rabbitmq-server rabbitmq-server.doc rabbitmq-server.man radiotray-ng rapidsvn rebar rebar3 rehex rtl_433 saga scorched3d sdrangel sigdigger slic3r soapysdr-with-plugins sonic-pi sooperlooper sound-of-sorting spatialite_gui spek srsran survex suscan tenacity therion timeline tqsl treesheets tsung tunnelx urbackup-client urh urh.dist vbam veracrypt welle-io wings woeusb-ng woeusb-ng.dist wxGTK32 wxSVG wxformbuilder wxhexeditor wxmacmolplt wxmaxima wxsqlite3 wxsqliteplus xchm xmlcopyeditor xylib yaws zeroad zeroadPackages.zeroad-unwrapped zod

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-already-reviewed/2617/1276

@wegank wegank merged commit 0b62f5a into NixOS:master Dec 3, 2023
26 checks passed
@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/veracrypt-glib-error/36424/4

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

6 participants