extfs: rpm: INSTALL is truncated in the viewer

[anatoly.borodin]

GNU Midnight Commander 4.8.19
Built with GLib 2.53.4
Using the S-Lang library with terminfo database
With builtin Editor
With subshell support as default
With support for background operations
With mouse support on xterm and Linux console
With support for X11 events
With internationalization support
With multiple codepages support
Virtual File Systems: cpiofs, tarfs, sfs, extfs, ext2undelfs, ftpfs, sftpfs, fish
Data types: char: 8; int: 32; long: 64; void *: 64; size_t: 64; off_t: 64;

If you try to view the INSTALL file with the internal viewer, only this text is shown:

# Run this script to install this RPM p

The reason: the size of INSTALL / UPDATE / REBUILD is hardcoded as 39 in src/vfs/extfs/helpers/rpm.

[anatoly.borodin]

I can see three possible solutions to the problem:

1) Remove the rpm commands from mcrpmfs_copyout() (have been added in e7ed071be7fda2df59191d103c47bf30a6b98ca8), revert the INSTALL line for consistency (remove the 'script' word). Now the files are 39 bytes again.

2) Set the size of the files to 0 in mcrpmfs_list(). I don't know, if its the right thing to do, but it works with the INFO/* files, and it triggers mcview to show the full file contents.

3) Recalculate the size of the files in mcrpmfs_list(), like it's done for HEADERSIZE. Of course, some refactoring should be done to avoid the code duplication.

NB:

1) I propose the removal of the word 'script' in any case: it is inconsistent with UPDATE and REBUILD.

2) I personally prefer the first solution, as the easiest one. See the attached patch.

3) The second and the third solution add the command to the text. But the command should be fixed as well: the file name is written unescaped, it will not work with whitespace in the path if just copy&pasted.

[anatoly.borodin]

The first solution

[anatoly.borodin]

git pull https://github.com/anatolyborodin/mc 2fbd997a87329410fff6add17ea9334a368f3565:3865_extfs_rpm_install_truncated

[egmont]

Thanks for the bugreport and investigation!

I'd prefer the 2nd approach if it indeed works. Especially given the recent (post-4.8.19) "grow file" fixes why not use this feature if we can? In virtual file systems I think it's pretty common and acceptable not to know the file size in advance and report some fake value (e.g. 0 or 4096) in the file listing.

Any hardcoded number such as 39 is bound to break in the future, let alone it's not translatable by design (not sure if we're planning to translate these strings but at least with the 2nd approach we could if we wanted to).

[anatoly.borodin]

Ok. Should we leave the rpm command call? It differs from other extfs scripts (like e.g. deb), it can be buggy (whitespace etc), and I don't think it's more useful than man rpm.

[egmont]

If you remove the rpm command then the virtual INSTALL, UPGRADE and REBUILD files will remain useless, right? We could remove these files completely, sure that's a way to fix this bug :-) I used to use this feature a lot when I had rpm-based systems and yum/dnf wasn't yet available.

What do you mean "It differs from other extfs scripts (like e.g. deb)"? "deb" also has a virtual INSTALL file, and by the way, also misbehaves if the filename has a special character in it.

I'd rather fix the issue which is probably as simple as adding $(printf %q ...) at the right places.

[anatoly.borodin]

If you remove the rpm command then the virtual INSTALL, UPGRADE and REBUILD files will remain useless, right?

You can still run the corresponding rpm command by pressing Enter on a file. See the function mcrpmfs_run() in src/vfs/extfs/helpers/rpm. They will not become useless.

What do you mean "It differs from other extfs scripts (like e.g. deb)"?

If you go into a deb file, the contents of INSTALL is

                              WARNING
     Don't use this method if you are not willing to reinstall everything...

This is not a real file. It is a way to install the package you are browsing.

To install this package go back to the panel and press Enter on this file.

In Debian systems, a package is automatically upgraded when you install a new
version of it. There is no special upgrade option. Install always works.

No command calls, just text. But thanks to sub mcdebfs_run, it can be run as well.

"deb" also has a virtual INSTALL file, and by the way, also misbehaves if the filename has a special character in it.

Is it so? It's a bug then, and should be fixed, like the bug fixed in 7ddc29649bcdad82dad30e36bed6df136421f519 and f029a529d26c8c9cda13ee6930374aa60b84877e.

Unfortunately, I don't speak Perl. I don't know which special characters can break it, but it works with spaces (in the path and in the file name).

[egmont]

Replying to anatoly.borodin:

You can still run the corresponding rpm command by pressing Enter on a file.

Sorry, I'm lost, or I'm missing something.

If you press Enter on the rpm file itself, it enters the virtual file system and you get to see its contents along with files such as INSTALL. If you press Enter on the virtual file INSTALL, it'll install the archive.

Sounds to me from your words that there's another "automatic" way (not typing the entire "rpm -i ..." in the command line) of installing the rpm as well. What is that?

[anatoly.borodin]

Replying to egmont:

Replying to anatoly.borodin:

You can still run the corresponding rpm command by pressing Enter on a file.

Sorry, I'm lost, or I'm missing something.

If you press Enter on the rpm file itself, it enters the virtual file system and you get to see its contents along with files such as INSTALL. If you press Enter on the virtual file INSTALL, it'll install the archive.

That's what I was talking about: pressing Enter on the INSTALL file. It will execute the installation command, and it doesn't matter what is being shown by F3. So it will not be useless, even if there will be only some text without commands.

[egmont]

My bad. I understand you now. I didn't realize that long version of the text that is truncated now (what this bugreport is about) is also supposed to show the command to the user that (s)he can execute.

I agree that there's not much point in showing this command where they can just run this script. Hence I wouldn't mind removing the rpm command from the copyout function.

[andrew_b]

Branch: 3865_exfs_rpm_scripts.
Initial changeset:e9cc914e2efbc31689338302fa29ff0b7ad8279a

[zaytsev]

I'm ok with removing the commands...

[andrew_b]

Merged to master: [218dcea54320eb0d4ed029dbc0ac661eb4f8fe62].

git log --pretty=oneline 38ce185..218dcea