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

Translate GLib critical assertions mc into error messages #4581

Open
mc-butler opened this issue Aug 31, 2024 · 9 comments
Open

Translate GLib critical assertions mc into error messages #4581

mc-butler opened this issue Aug 31, 2024 · 9 comments
Labels
area: core Issues not related to a specific subsystem prio: medium Has the potential to affect progress

Comments

@mc-butler
Copy link

Important

This issue was migrated from Trac:

Origin https://midnight-commander.org/ticket/4581
Reporter zaytsev (@zyv)
Mentions egmont (@egmontkob)

The root cause of #4576 (visual glitches in the editor) turned out to be GLib critical asserts caused by wrong usage of GLib function (calling it on a NULL pointer). These asserts are printed on the standard streams and usually swallowed by Midnight Commander. Probably many graphical artifacts have those as a root cause.

GLib people say that there is a way to capture these assertions:

With the initial proviso that critical warnings always indicate application bugs, and should always be fixed rather than ignored or hidden…

Critical warnings are logged using GLib’s standard logging framework (g_log_structured()), so you can intercept them by calling g_log_set_writer_func() and providing your own log writer function. It will have to handle all log messages from GLib (e.g. all calls to g_debug(), g_message(), g_warning(), etc.), not just critical warnings. The default writer function is g_log_writer_default() and you can see what it does here.

You could, for example, set the writer function to (the built-in) g_log_writer_journald() to send all the messages to the systemd-journal rather than stdout/stderr.

https://gitlab.gnome.org/GNOME/glib/-/issues/3459#note_2208789

I'm not familiar with the error handling infrastructure of mc, but it does show red error messages from time to time. It would be good to translate GLib critical assertions (and maybe something else, like warnings? depending on how much, how useful and how annoying it is) to red error boxes in mc, so that instead of being ignored or causing graphical glitches they can be noticed and fixed.

@mc-butler
Copy link
Author

Changed by ossi (@ossilator) on Aug 31, 2024 at 6:20 UTC (comment 1)

message boxes would be a terrible user experience. users should not be confronted with uncontrollable barrages of crap caused by the application's bugs.

journald would be better in this regard, but no-one would even notice it. i suppose one could print a single "there were internal errors; check the log" message to stderr on exit.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Aug 31, 2024 at 7:08 UTC

Replying to zaytsev (#4581):

g_log_structured()
g_log_set_writer_func()
g_log_writer_default()

glib >= 2.50

You could, for example, set the writer function to (the built-in) g_log_writer_journald() to send all the messages to the systemd-journal rather than stdout/stderr.

glib >= 2.50 too. Fortunately, there are some distros withowt crapd.

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Aug 31, 2024 at 7:13 UTC (comment 3)

message boxes would be a terrible user experience. users should not be confronted with uncontrollable barrages of crap caused by the application's bugs.

Well, it's not known how many of those are there in the code in the first place. Maybe there are none (even though I doubt it).

As a developer I'd like to have them pop up just like the immediate crashes of the sanitizers, so that I can connect it with what I just did. Also, I'm running mc instances for weeks if not months in a row, so when I exit and get a log full of those, it will be very hard to debug, because I literally exercised every single feature in the meantime.

But maybe warning boxes should be made optional in the first place with the default of being disabled. This way users won't get visual corruption, because the assertions are captured, but also if it can be enabled, causes of strange behaviors are much easier to identify and developers can just run it with warning translation AND alerting enabled by default.

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Aug 31, 2024 at 7:19 UTC (comment 4)

Fortunately, there are some distros withowt crapd.

macOS comes to mind ;-)

glib >= 2.50

We can write a translator with conditional compilation for glib >= 2.50, without having to bump glib, if we decide to do it in the first place.

I'm more interested in the warning infrastructure we have in mc, but I don't have time to look into it - still pending replies to other tickets. Not sure we even have something like an internal error queue.

Anyways, as we have learned, swallowing glib internal assertions is no good. If we don't have infrastructure to translate and don't want to build it, maybe we can just (optionally) crash at least.

I wanted to create this ticket to save this point. Couldn't find any other one on the subject.

@mc-butler
Copy link
Author

Changed by egmont (@egmontkob) on Dec 18, 2024 at 20:06 UTC (comment 1.5)

Replying to ossi:

journald would be better in this regard, but no-one would even notice it. i suppose one could print a single "there were internal errors; check the log" message to stderr on exit.

Alternatively, mc could collect these errors in memory (up to a safety limit, let's say 100 messages) and print all those actual messages to stderr on exit. Maybe with a preface text saying something like "these errors occurred while running mc, please report them".

Also, this would have no systemd dependency whatsoever.

@mc-butler
Copy link
Author

Changed by egmont (@egmontkob) on Dec 18, 2024 at 20:11 UTC (comment 4.6)

glib >= 2.50

We can write a translator with conditional compilation for glib >= 2.50, without having to bump glib, if we decide to do it in the first place.

Why this obsession with supporting ancient systems?

glib 2.50 was released in Sep 2016, i.e. 8+ years ago. Realistically it'll be at least 9 if not 10 years old or even older by the time someone implements this feature and it's released in an mc tarball. Is this not enough??

IMO requiring a 2-3 year old version of an underlying library would be absolutely fair game.

Anyone wishing to stick to an ancient system AND also wishing to use the newest shiniest mc could invest the necessary work themselves, and let upstream developers work on things that actually matter in the long run.

Or they can use a somewhat old mc just fine.

Let's please drop this attitude of wishing to support ancient base libraries forever, let's please not waste any effort on this. GLib requirement bumped to 2.50, users still get an enormous window of 8+ years, problem solved, move on.

@mc-butler
Copy link
Author

Changed by egmont (@egmontkob) on Dec 18, 2024 at 20:12 UTC (comment 7)

  • Cc set to egmont

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Dec 19, 2024 at 5:43 UTC (comment 8)

Relates to #4611 (another missed assertion found).

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Dec 19, 2024 at 5:58 UTC (comment 6.9)

Replying to egmont:

glib >= 2.50

We can write a translator with conditional compilation for glib >= 2.50, without having to bump glib, if we decide to do it in the first place.

Why this obsession with supporting ancient systems?

One's views often depend on one's experience. I ported mc to AIX this summer and discovered a few annoying problems with GLib:

  1. Modern GLib has moved away from autotools to a new build system, which you have to port to in order to build it at all. You also have to learn it in order to patch other problems, of which there are many.
  1. At some point, GLib started requiring PCRE to build GLib regex, so I had to fight another package to be able to build it (and it's always a fight).

So I was lucky enough to be able to use GLib 2.46 and save a lot of time. Of course, you or any other user wouldn't care about this, and you'd be right. If you were the maintainer, I'd have to use an older version of mc on AIX if I wanted to save myself some trouble - and I'd take it. It's just that I'm the one still doing some maintenance, so I do care. It's as simple as that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: core Issues not related to a specific subsystem prio: medium Has the potential to affect progress
Development

No branches or pull requests

1 participant