GNOME Bugzilla – Bug 475166
Failed assertion when --extract-here is used without --extract-to
Last modified: 2007-12-30 10:13:16 UTC
Steps to reproduce: Run file-roller --extract-here SOME-ARCHIVE The result is a failed assertion: ** (file-roller:14612): CRITICAL **: window_archive__open_extract_here: assertion `dest_dir != NULL' failed Stack trace: gdb didn't seem to want to give me a stack trace since the program technically exits normally. Other information: Shouldn't --extract-here imply that the dest_dir is the current directory (or the location of the archive) or something like that?
at the moment you have to specify the --extract-to option as well, but I agree with you that the --extract-here option should be enough to perform the operation. PS: you can use "run --g-fatal-warnings" in gdb to make it stop when a warning is issued.
Okay, so... I tried to fix the bug myself, but I'm really confused as to what the semantics are supposed to be. The code seems to currently yell at you when --extract-to is omitted, but the data is completely thrown away, and instead the parent directory of the original file is used. Furthermore, an error message is thrown when the archive is given as a local path as opposed to an absolute path or URI. Things to run: file-roller --extract-here foo.tar.bz2 Result: assertion failed file-roller --extract-to=. --extract-here foo.tar.bz2 Result: error message, "Invalid URI" file-roller --extract-to=. --extract-here /full/path/to/foo.tar.bz2 Result: tarball is extracted under /full/path/to file-roller --extract-to=some/other/path --extract-here /full/path/to/foo.tar.bz2 Result: tarball is *still* extracted under /full/path/to Nautilus, when using this option calls it by setting --extract-to to the parent directory of the tarball and passes both arguments as URIs. It seems that better semantics would be to: * If no extract-to directory is given, use the current directory; it says "extract-here", not "extract-at-the-tarballs-parent-directory". This won't break anything as this combination simply never worked before. * If an extract-to directory is given, use that directory. This could potentially break some scripts (I really doubt this option was ever used by people -- it's way too confusing.), but I think that if any script calls file-roller --extract-here --extract-to=/foo /bar/baz.tar.bz2 * Maybe also have an --extract-there option do default at the parent directory of the tarball. (Actually, personally think that there should just be --extract-here and --extract-there, and have them not use --extract-to in the first place, but that would break stuff.) in order to extract into /bar, it's just asking for breakage. This shouldn't break Nautilus, looking at how it calls file-roller, but I don't know about other programs that use file-roller --extract-here, if they exist. This does make me wonder though, what's an extract-here supposed to do? What's the difference between extract-to some directory and extract-here extract-to that same directory?
Created attachment 99588 [details] [review] Fix URI problem and --extract-here without --extract-to keeping old semantics Regarding the URI bug, it seems that archive->uri in both cases (extract-here and extract-to) is never set to a URI and remains a local path. get_desired_destination_from_archive_uri seems to dislike local paths for some reason. Since the code names everything by uri, perhaps it makes sense to convert everything to URIs right away. :-/ I've also "fixed" the bug in this report keeping the old semantics and removing the really confusing extra arguments that get thrown away. Not sure if that was the intent of the code. Interestingly, fr_window_exec_batch_action in the FR_BATCH_ACTION_EXTRACT_HERE case extracts the edata and then completely ignores it. Is there any particular reason for that? It seems that passing NULL instead of calling extract_to_data_new would be just as effective. Still, I think the old semantics seem weird. You'd think that "extract here" means extract *here*, not wherever-the-archive-is-located.
Created attachment 99590 [details] [review] fixed version of patch Oops. That's what I get for being horribly unorganized with my files. Sorry. :-(
patch applied to trunk, thanks.