Ticket #4502 (closed defect: fixed)
File bindings don't work with GLib 2.77.3
Reported by: | sachahony | Owned by: | andrew_b |
---|---|---|---|
Priority: | major | Milestone: | 4.8.31 |
Component: | mc-config-ini | Version: | 4.8.30 |
Keywords: | Cc: | ||
Blocked By: | Blocking: | ||
Branch state: | merged | Votes for changeset: | committed-master |
Description
I have updated my glib2 from 2.77.2 to 2.77.3. After the update I can no longer enter into tar.bz2 files. Reverting back 2.77.2 solves the issue. Also the view archive shows a "binary" view rather than a "direcory" as if the file is not recognised.
Changelog of glib: https://download.gnome.org/sources/glib/2.77/glib-2.77.3.news
Change History
comment:2 Changed 3 weeks ago by andrew_b
- Component changed from mcview to mc-config-ini
The significant change in GLib-2.77.3 is gkeyfile: Fix overwriting of GError.
Commit 71b7efd08.
Solution: escape backslashes in mc.ext.ini: replace single \ with double one: \\.
comment:3 Changed 3 weeks ago by Toolybird
We just hit this in Arch Linux because that commit made it into stable release glib2-2.76.5
comment:4 Changed 3 weeks ago by dimich
I can confirm the issue with glib-2.76.5 in Arch: https://bbs.archlinux.org/viewtopic.php?pid=2118521
Not only compressed tar archives affected but other suffixes too, e.g. .mp4, .odt
comment:5 follow-up: ↓ 7 Changed 3 weeks ago by zaytsev
Oh man, why they would handle escape sequences in INI files in the first place o_O ?
Maybe we can just switch from g_key_file_get_string to g_key_file_get_value API, because otherwise every stupid backslash would have to be escaped and it will make a bloody hell out of our regexes.
comment:6 Changed 3 weeks ago by ossi
the ini parser is also used to parse .desktop files and such stuff, so supporting escapes in strings makes perfect sense.
it is a bit surprising that such a seemingly obvious bugfix has such side effects. i guess mc does something ... "interesting"?
comment:7 in reply to: ↑ 5 Changed 3 weeks ago by andrew_b
Replying to zaytsev:
Maybe we can just switch from g_key_file_get_string to g_key_file_get_value API, because otherwise every stupid backslash would have to be escaped and it will make a bloody hell out of our regexes.
g_key_file_* API is used in mc indirectly via mcconfig engine. Changing g_key_file_get_string to g_key_file_get_value in mcconfig may break the handling of other INI files.
We already have INI files with escaped backslashes: mc.lib, Syntax (it is not INI but nonetheless), probably some others. I think mc.ext.ini should be consistent with them.
comment:8 follow-up: ↓ 15 Changed 3 weeks ago by zaytsev
it is a bit surprising that such a seemingly obvious bugfix has such side effects. i guess mc does something ... "interesting"?
I'm not sure I would define this as "interesting". I think it's just missing error handling, the usual C stuff.
We already have INI files with escaped backslashes: mc.lib, Syntax (it is not INI but nonetheless), probably some others. I think mc.ext.ini should be consistent with them.
You are right, but exactly for this reason I find them to be disgusting and actually see no reason why we should support escape sequences in the configuration files. So I'd rather break everything while we are at it :-) But it's up to you.
comment:9 Changed 3 weeks ago by andrew_b
- Summary changed from VFS of compressed tar archives appears broken with GLib 2.77.3 to File bindings don't work with GLib 2.77.3
comment:10 Changed 3 weeks ago by andrew_b
Probably the story isn't finished yet: https://gitlab.gnome.org/GNOME/glib/-/issues/3095
comment:11 Changed 3 weeks ago by zaytsev
comment:12 Changed 3 weeks ago by andrew_b
Ticket #4505 has been marked as a duplicate of this ticket.
comment:13 Changed 3 weeks ago by birdie
I thought I was going crazy. Thanks, Andrew.
comment:14 Changed 3 weeks ago by birdie
A quick temporary fix for the affected users:
sudo cp -a /etc/mc/mc.ext.ini /etc/mc/mc.ext.ini.bak sudo sed '/Regex=/s/\\/\\\\/g' -i /etc/mc/mc.ext.ini
comment:15 in reply to: ↑ 8 Changed 2 weeks ago by andrew_b
So, I propose the following:
- escape backslashes in mc.ext.ini to be consistent with other files were regexps are used;
- use g_key_file_get_value instead of g_key_file_get_string to avoid parsing of key value in GLib.
Replying to zaytsev:
actually see no reason why we should support escape sequences in the configuration files
Escape sequence is a part of key definition for various terminals: https://midnight-commander.org/browser/misc/mc.lib#L15
comment:16 Changed 2 weeks ago by andrew_b
- Status changed from new to accepted
- Owner set to andrew_b
- Branch state changed from no branch to on review
- Milestone changed from Future Releases to 4.8.31
Branch: 4502_mc.ext.ini
Initial changeset:2da7ef347d1c48e4ad8e73f5135cfe8a363e6c1f
comment:17 Changed 2 weeks ago by andrew_b
- Branch state changed from on review to on rework
Some tests failed.
comment:18 Changed 2 weeks ago by andrew_b
- Branch state changed from on rework to on review
The usage g_key_file_get_value instead of g_key_file_get_string requires a lot of changes in test. Let's leave it as it is.
comment:19 Changed 7 days ago by andrew_b
- Votes for changeset set to andrew_b
- Branch state changed from on review to approved
comment:20 Changed 7 days ago by andrew_b
- Status changed from accepted to testing
- Votes for changeset changed from andrew_b to committed-master
- Resolution set to fixed
- Branch state changed from approved to merged
Merged to master: [e8c4677b7e10c43f673e6542a86d3ea3ccf4f95e].
Are any other regressions with glib-2.77.3 besides tar?
Why component is mcview?