search

joncampbell123 / dosbox-x

DOSBox-X fork of the DOSBox project
GNU General Public License v2.0
2.66k stars 378 forks source link

Mount zip file doesn't work since version 0.83.23 ? #3535

Open aetchris opened 2 years ago

aetchris commented 2 years ago

Describe the bug

Programs mounted in zip files don't launch since after version 0.83.23.

Steps to reproduce the behaviour

Mount a zip file as drive c Try to launch program Dosbox-x hangs and program doesn't launch

Expected behavior

In version 0.83.23, the program launches. After this version, dosbox-x seems to hang up with a black screen

What operating system(s) this bug have occurred on?

Windows 10

What version(s) of DOSBox-X have this bug?

Since 0.83.24

Used configuration

mount C "pathtozipfile\file.zip:\"
C:
cd\
program.exe

Output log

Many "ERROR CPU:Illegal Unhandled Interrupt Called 6" in output

373108514 ERROR CPU:Illegal Unhandled Interrupt Called 6
 373108611 ERROR CPU:Illegal Unhandled Interrupt Called 6
 373108708 ERROR CPU:Illegal Unhandled Interrupt Called 6
 373108805 ERROR CPU:Illegal Unhandled Interrupt Called 6
 373108902 ERROR CPU:Illegal Unhandled Interrupt Called 6
 373108999 ERROR CPU:Illegal Unhandled Interrupt Called 6
 373109096 ERROR CPU:Illegal Unhandled Interrupt Called 6

Additional information

No response

Have you checked that no similar bug report(s) exist?

Code of Conduct & Contributing Guidelines

grapeli commented 2 years ago

I confirm does not work from the moment of introducing these changes #3347.

aetchris commented 2 years ago

The issue seems to be resolved in version 0.84.1. Thanks a lot!

grapeli commented 2 years ago

I have not noticed any improvement. It still does not work.

keen6.zip bomber.7z.txt

dosbox-x -c "mount c ./bomber.7z" -c "c:" -c "cd 3dbomber" -c bomb
dosbox-x -c "mount c ./keen6.zip" -c "c:" -c keen6

After undoing these unfortunate code changes #3347, it works.

aetchris commented 2 years ago

Well, that's strange... The issue seems to persist on some programs, and not others... For example, "Elite" in a zip file works, whereas "Elite plus" in a zip file doesn't.

NebularNerd commented 2 years ago

I have the same issue with an old Dynamix title 'Stellar 7' happy to supply zip if needed.

grapeli commented 2 years ago

I am surprised by the long lack of response to reporting this bug from developers.

For those who need this functionality and are able to compile on their own, I suggest undoing these unfortunate changes to the code. revert-3347-fix-mount-zip.patch.txt

tbr commented 2 years ago

Hi, I'd also like to confirm that this problem persists. An example to trigger this bug is to use (in DosBox-X) "edit somefile.txt" where "somefile.txt" is a file inside a mounted zip file as drive (with an overlay mount on top of it). DosBox-X will hang while spawning the interrupt 6 error on console as described in this issue.

Moreover, I was reviewing merge #3347 and I believe that there is also a mistake made (but I might be wrong): on dos.cpp:line1895 there is now a call to MEM_BlockRead which seems wrong (I think it should be MEM_BlockWrite) as it is the code for 0x3f (File Read) which reads from file (writing) into a memory block.

NebularNerd commented 2 years ago

Looking forward to seeing this fixed, I'm trying to compile the code with @grapeli patch but it's been a while since I did some basic compiling and things are much more complicated now.

The .zip thing has been one of the great things added to DosBox, the ability to enjoy a program or game freely without harm to the original install is ideal for both preserving and running multiple copies of a base install (Say Win 3.1) but with different driver/software combinations as required, any issues occur you can simply delete the overlay directory for that run and start again. With modern systems the speed difference is barely appreciable.

NebularNerd commented 2 years ago

Just seen this was merged last night and tested with build dosbox-x-vsbuild-win64-20220901051309, My copy of Stellar 7 now plays from inside a regular .zip and my stupidly high compressed .7z without issue.

Big thumbs up to @tbr for the fix. 👍😁

tbr commented 2 years ago

While this is now fixed, I'm seeing still some issues when combining ZIP/7Z drive mounts with an overlay folder (for writing changes, like save games). So, more debugging is probably advised.

NebularNerd commented 2 years ago

While this is now fixed, I'm seeing still some issues when combining ZIP/7Z drive mounts with an overlay folder (for writing changes, like save games). So, more debugging is probably advised.

I played tested a few games yesterday all using an overlay and had no issues personally. If I come across one I'll update this reply. I'm sure there will be a couple of games in my collection that access drives in a weird and wonderful manner.

aetchris commented 2 years ago

Seems to work very well! Thanks a lot for the fix. I will test some games to see if i can find issues.

NebularNerd commented 2 years ago

I think I've found another zip related issue. Z by the Bitmap Bros. Installing from an .iso of my own CD and running it from a mounted folder is fine, however the moment you try to run it from a .zip your get file missing errors on load.

I've also tested this using a copy found at Z @ myabandonware it runs just a treat in their browser dosbox version (it unpacks to a directory) and if you download/unpack it to a folder on your own system, however try running their .zip as a mounted drive again gives files errors.

grapeli commented 2 years ago

@NebularNerd I confirm. Attempting to run this game "Z 1996" directly from the archive fails. Only the reason is completely different from the one in this original bug report.

Although this is a regression. Works fine to version dosbox-x 0.83.13. The game stopped starting exactly with this code change dc798b56a.

edit: After restoring an unnecessarily deleted semicolon from drive_physfs.cpp, the game starts correctly from the archive.

NebularNerd commented 2 years ago

@grapeli

Ok cool, so we need a fix to 'fix' https://github.com/joncampbell123/dosbox-x/commit/dc798b56a1bc2cb25c90e4b057815a22822cd1e2

tbr commented 2 years ago

@grapeli @NebularNerd

I believe the line 2883 should state: case DOS_SEEK_END:mypos = PHYSFS_fileLength(fhandle)-mypos; break;

The semicolon fix was correct (or at least makes sense), but the += needs to be =. The fileseek from end calculates position as "file_length" minus the provided "mypos".

grapeli commented 2 years ago

The semicolon fix was correct (or at least makes sense)...

It was certainly not correct. Without this semicolon, this game would not start. Z_DOS_EN.zip

dosbox-x -c "mount c Z_DOS_EN.zip" -c "c:" -c "cd z" -c "z.bat"

After undoing this unfortunate commit dc798b5, starts without error.

tbr commented 2 years ago

@grapeli I tried your Z_DOS_EN.zip with the provided command. On my system, it fails (with or without the unfortunate commit).

NebularNerd commented 2 years ago

Found another couple that have .zip issues (again likely different from the others). From #2577 the two Holiday Hare versions inside Elkjøp (2).zip both produce the same error.

Unpack to a folder, run setup (I used Soundblaster 16 for audio while testing) and both work as they should. Run from the .zip and you get Setup Error: Can't initialize file manager then it will start the game and appear to work just fine.

command_001 avi_20220907_155428 430

When you exit the game you may get a Protected Mode Error across the top of the screen is shown where the normal This is not shareware message should be. Seems to happen very rarely.

command_002 avi_20220907_155817 503

There is a write overlay set which works as I can save a config file by adjusting the volume via the ingame menu.

grapeli commented 2 years ago

@tbr I don't know what your cause might be. I checked with an incorrect commit dc798b5 undone as well as with your patch. In both cases, it runs flawlessly.

grapeli commented 2 years ago

@NebularNerd It would be ideal if each packed archive in combination with the overlay worked flawlessly. Unfortunately, this is not the case yet. You have to prepare yourself a properly configured archive of this kind. Is there a bug here? I don't think so.

This version works flawlessly. Elkjop.3.zip

dosbox-x -c "mount c Elkjop.3.zip" -c "c:" -c "cd JAZZHH94" -c "jazz"

NebularNerd commented 2 years ago

@NebularNerd It would be ideal if each packed archive in combination with the overlay worked flawlessly. Unfortunately, this is not the case yet. You have to prepare yourself a properly configured archive of this kind. Is there a bug here? I don't think so.

This version works flawlessly. Elkjop.3.zip

dosbox-x -c "mount c Elkjop.3.zip" -c "c:" -c "cd JAZZHH94" -c "jazz"

@grapeli Most strange, that seems to behave the same for me (although setup worked in Elkjøp (2).zip here it just fails totally, mounted just the zip (no overlay) and still sends out the error, then starts the game as before. I'm still on the dosbox-x-vsbuild-win64-20220901051309 if that makes any odds.

command_000 avi_20220907_183633 162

I've been playing heavily with .zip mounting (from way back in the early days when it was announced on VOGONS) so I know there is always going to be some weirdness with a pretend filesystem. As you say it would be awesome if everything just worked, but then there would be no fun in it.

grapeli commented 2 years ago

@NebularNerd You expect too much. The files in the packed archive are read-only. To be able to use it, you have to go through the configuration stage and repack again. Then you can count that such a prepared and connected archive will work in dosbox-x (of course in many cases it will also be impossible).

You can't really count on using an overlay as this layer is not entirely physfs compatible. I recommend that you read this #3355 bug report carefully.

NebularNerd commented 2 years ago

@NebularNerd You expect too much. The files in the packed archive are read-only. To be able to use it, you have to go through the configuration stage and repack again. Then you can count that such a prepared and connected archive will work in dosbox-x (of course in many cases it will also be impossible).

You can't really count on using an overlay as this layer is not entirely physfs compatible. I recommend that you read this #3355 bug report carefully.

@grapeli Far from it, I simply copied your command as you printed it to test under the same conditions and found that it produced the issues as posted. I'm fully aware of what you can and cannot do with .zip files. I wish my coding skills were better and I could more actively help debug and submit patches like yourself and @tbr, but beyond some basic stuff I'm no expert, however I'm good at testing/breaking things. 😁

I use the same method to test each game/program. Download & unpack to a folder / Install from images of my original media, setup, test and patch as needed. Once it's all running as it should, repack to a .7z or .zip then run from a bat file with the following script.

%~dp0DOSBox-X\dosbox-x.exe -conf %~dp0DOSBox-X\PLAYGAME.conf -defaultdir %~dp0DOSBox-X -c "mount c %~dpnx1" -c "mount c %~dp0~~write\d-games -t overlay"

This mounts the .zip plus an overlay for write access, test again, if all works, excellent, the game runs from the .zip and saves/additional files live in the overlay. Where it does not I'm mentioning them here where those with the skills to identify the issues may wish to investigate. As @tbr mentioned, some things do not behave as they should even after the recent commit. As we see in the case of Z, another commit lays behind that issue rather than the initial one that this thread was about.

Testing the following using the original files, we see that a change occurred in the code between JAZZ.zip (from #2577) and the later Holiday Hare versions in the Elkjop.zip's that makes them .zip unfriendly. I did not bother running the game in each case, as mentioned further up the jazz.exe's will run and save a config file to the overlay if you tweak the audio volume via the ingame menu, this shows the overlay is setup correctly in all instances, however the setup's in the HH versions produce the errors as shown here and simply do not run. Unlike the Z issue, the jazz.exe's are still playable so it's more of a 'that's weird' rather than a 'it don't work at all'. If you perform the setup then rezip as you state, the game will play with your preferred settings albeit displaying the errors. Personally I will still run from the .zip versions and just ignore the error messages.

JAZZ.zip

https://user-images.githubusercontent.com/8470449/189087590-f6c1b419-7adf-4d52-9852-52adef8c4d15.mp4

Elkjop.2.zip

https://user-images.githubusercontent.com/8470449/189087829-6fdb5f17-5430-42f3-bf3a-bbfedc4aec7b.mp4

Elkjop.3.zip

https://user-images.githubusercontent.com/8470449/189088146-fd3637a3-3308-4318-a606-08648278e0cf.mp4

Regarding #3355, while this is true it should not prevent the setup.exe from running. The only issue in the case of using these .zip's 'as is' should be that running setup would either give a write error (with no overlay, although in the case of JAZZ.zip if you delete SOUNDCRD.INF from the .zip then mount without the overlay it simply exits without any error) OR the overlay copy of the file is ignored until the copy inside the .zip is replaced or deleted. Ideally the overlay should take preference as @thesnable has raised in #3355 and here icculus/physfs#18, I have also asked about it in the plans for PhyFS 4.0 here icculus/physfs#41

grapeli commented 2 years ago

@NebularNerd As mentioned, I undone this commit dc798b5. For me, setup.exe starts from the Elkjop.3.zip zip archive.

https://user-images.githubusercontent.com/452325/189121309-aca398a9-4d5a-4e87-9b8d-6914c362697b.mp4

NebularNerd commented 2 years ago

@NebularNerd As mentioned, I undone this commit dc798b5. For me, setup.exe starts from the Elkjop.3.zip zip archive.

dosbox-x.zip.mount.mp4

Cool so it's the same bug that prevents Z running inside a .zip, I wonder what else is out there it fixes...

NebularNerd commented 2 years ago

I'll mention this here as well as in #3355, the most awesome @icculus has offered a solution to the contents of overlays not superseding the original zip contents here: icculus/physfs#18

https://github.com/joncampbell123/dosbox-x/blob/3b002fa724a67b3cf26d5030391cbf3d5dea90a0/src/dos/drive_physfs.cpp#L371

Make that 1 a 0 so the overlay gets checked for files before the thing it overlays.

This should then let mounted .zip/overlay combo's work as we expect them to (maybe in conjunction with an unmount/remount or rescan depending on how it behaves), saving the need to delete files inside the .zip just to experiment with a mod or sound configuration.

grapeli commented 2 years ago

@NebularNerd Great. Really simple. It works.

https://user-images.githubusercontent.com/452325/189322932-cefca9c8-00ef-42e3-9fdf-df613b2136df.mp4

Thanks for bringing this issue up.

NebularNerd commented 2 years ago

@NebularNerd Great. Really simple. It works.

dosbox-x.mount.zip.with.overlay.mp4 Thanks for bringing this issue up.

@grapeli Awesome, the JAZZHH94 directory has a SOUNDCRD.INF file in the zip (from the uploader setting it for GUS) so we see that it's definitely updated in order to run the game. 😁🎉

I'll try and figure out how the whole pull request thing works, unless anyone else is happy to add it.

grapeli commented 2 years ago

@NebularNerd This particular example (Elkjop.2.zip) only works correctly with an undone commit dc798b5, like the previous one.

NebularNerd commented 2 years ago

@grapeli

True, this will work for others games like the JAZZ.zip we have been using which is .zip friendly. If we get https://github.com/joncampbell123/dosbox-x/commit/dc798b56a1bc2cb25c90e4b057815a22822cd1e2 resolved as well then that should open up a lot more games being .zip'able

grapeli commented 2 years ago

There is also a significant error here (I noticed it much earlier).

If you try again, the configuration changes are saved using overlay, the segmentation fault follows (Elkjop.2.zip). I am posting backtraces, although without hoping for a fix from dosbox-x developers (their bugs are not really interested in them), but maybe someone else will find a remedy for it. backtrace.log

NebularNerd commented 2 years ago

Which change causes the segfault? Commit https://github.com/joncampbell123/dosbox-x/commit/dc798b56a1bc2cb25c90e4b057815a22822cd1e2 or the overlay fix?

grapeli commented 2 years ago

@NebularNerd Certainly not an overlay fix. Without undoing this unfortunate commit, as you know, setup.exe will not run at all. For my non-developer feel, both have no effect on it.

Maybe this is a bug in physfs. I do not know. However, I see that the code of this physfs library is updated from time to time, but the one included with dosbox-x sources is not modified (synchronized changes) at all. It is certainly not appropriate.

NebularNerd commented 2 years ago

@grapeli @tbr

I've thrown a quick build together and can confirm that:

I could not see the segfaults in the logging window, are these shown elsewhere? I've added the semicolon into my PR, it works with everything I tested so seems to make sense to restore it.

icculus commented 2 years ago

PHYSFS_fileLength(fhandle) THEN -mypos is required as two separate steps.

Just to dip my toes in the pool for a second, since I'm CC'd on this thread: -mypos by itself is a C expression without side effects: it doesn't change mypos, it just calculates what the negative version of it is and then throws it away, so there's no situation where this is required...it's the same as just deleting the -mypos; part from the file.

icculus commented 2 years ago

(A brief glance at that code suggests removing the semicolon is the right thing to do, and the bug is somewhere else, even if the semicolon's existence appeared to be mitigating the bug in some way.)

NebularNerd commented 2 years ago

Thanks @icculus I cc'ed you in to give credit for helping resolve the overlay issue. Thanks for the clarification on the -mypos, now we need to hunt down where the other bug is that the ; bodges back into life 🤣

grapeli commented 2 years ago

If with this semicolon both tested cases work Z_DOS_EN.zip and Elkjop.2.zip correctly, whether it is fully legitimate or not. Until someone finds a better solution, let them stay where they were.

https://user-images.githubusercontent.com/452325/189395442-82662f87-a366-4aae-b8e1-f9bc559654fa.mp4

NebularNerd commented 2 years ago

Ah that's interesting, Z_DOS_EN.zip (with no cutscenes) works fine with both patches, my copy with cutscenes breaks still 🤔 more playing/testing needed. EDIT: My original copy with cutscenes works now, had to layout the .zip in an odd way to make it work.

I agree about the semicolon fix/bodge, that it works is great, that it's a bodge not so much. I've left it in the PR with a note for @joncampbell123 to decide to include or not as seen fit.

grapeli commented 2 years ago

An identical segmentation fault occurs when trying to run a Barney Splat. BarneySplat (1993).zip

dosbox-x -c 'mount c "./BarneySplat (1993).zip"' -c "mount c /tmp/dos -t overlay" -c "c:" -c "cd barneysp" -c ansi -c "@autotype -w .4 -p .2 w , enter , 2 , 4 , enter , enter , enter , enter" -c bs

backtrace.log

grapeli commented 2 years ago

@NebularNerd Another case that doesn't work properly due to a change in the dc798b5 code. After the semicolon is restored, it works fine.

Circuit Racer (1997).zip dosbox-x -c 'mount c "./Circuit Racer (1997).zip"' -c "c:" -c "cd cirracer" -c "cr -novbl"

I suspect that there may be many more such "cases".

edit: Next "case" found.

Claas - Der Erntespezialist (1996).zip dosbox-x -c 'mount c "./Claas - Der Erntespezialist (1996).zip"' -c "c:" -c "cd claasder" -c "claas.bat"

NebularNerd commented 2 years ago

Can confirm the Circuit Racer and Claas behave as above.

Where do I need to look for the seg faults? Also BarneySplat is a very disturbed 'text adventure'

Found one that refuses to play even with the patches, cyberboxdlx.tar.gz from #3090. Unpacked it runs fine, repacked into a .zip, it complains about CURSR-01.DAT missing and bombs out to the dos prompt.

grapeli commented 2 years ago

Where do I need to look for the seg faults? Also BarneySplat is a very disturbed 'text adventure'

For me, it follows this command that I pasted. Only the first time writing to the overlay directory. Not the next time you run it. Without the fix on mount overlay does not occur. Although she herself does not seem to have any influence on it.

Found one that refuses to play even with the patches, cyberboxdlx.tar.gz from #3090. Unpacked it runs fine, repacked into a .zip, it complains about CURSR-01.DAT missing and bombs out to the dos prompt.

I can confirm. edit: At startup it tries to change (set) attributes on several files (maybe mount overlay can't do that).

NebularNerd commented 2 years ago

I tried placing the CURSR-01.DAT in the overlay to see if that would do it and no luck, tried removing it from the zip just to doubly make sure and still no luck. Whatever it's doing it's doing it in a weird way.

grapeli commented 2 years ago

@NebularNerd Unfortunately, the cyberbox does not work when used with mount zip.

This program runs without any problems if you mount the directory with this game in read-only mode and use the overlay mount additionally. Then the attributes of the files in the overlay directory are properly changed.

.DBOVERLAY_ATR_BOXES-01.DAT 
.DBOVERLAY_ATR_CA-CYBER.CFG
.DBOVERLAY_ATR_CA-CYBER.TXT
.DBOVERLAY_ATR_CURSR-01.DAT
.DBOVERLAY_ATR_V-EDC-01.DAT
.DBOVERLAY_ATR_V-EDC-01.SOL
BOXES-01.DAT 
CA-CYBER.CFG
CA-CYBER.TXT
CURSR-01.DAT
V-EDC-01.DAT

The game starts. I tested it like this. systemd-run -G --user -p PrivateUsers=true -p BindReadOnlyPaths="/tmp/test/cyber:/tmp/cyber" ./dosbox-x -defaultdir /tmp/dosbox-x/src

https://user-images.githubusercontent.com/452325/189619673-24658c5c-37b0-4a78-8680-288272b23948.mp4

NebularNerd commented 2 years ago

Interesting, so that would point to PhyFS being the issue as it works with your test. I copied your method to test WordPerfect 5.0 as that has some weird issues where you cannot save a file when inside a .zip and it produces similar results. Inside a .zip it starts writing a mix of files when you try to save a document but you just get write errors. With your method the file saves just fine.

grapeli commented 2 years ago

Interesting, so that would point to PhyFS being the issue as it works with your test.

Exactly. You can do this test yourself. The mounted directory from Cyberbox must be read-only. More you can't change the file permissions on it. Most conveniently, if the owner of this directory with files is another user (even the administrator), you will only have read access.

grapeli commented 2 years ago

DOOM 2D is a very interesting case. DOOM 2D (1996).zip

Well, straight from the zip archive it does not start (also with overlay). Even if it started correctly, it would be dramatically slow that this way of starting it is disqualifying. When loading the DOOM2D.WAD file, it takes about 60 seconds in my case, the whole CPU load is generated by tinfl_decompress from src/libs/physfs/physfs_miniz.h. Then all sorts of things happen. After 5 minutes, I usually stopped. Sometimes. command_000

Sometimes dosbox-x exits with a message - E_Exit: IRET:Illegal descriptor type 0.

Surprisingly, with the 7z (lzma2) archive, doom2d boots up and it runs quite smoothly. DOOM 2D (1996).7z.txt

Unpacked and mounted directory in read-only mode does not start. command_001

With the help of an additional overlay directory - starts up.

NebularNerd commented 2 years ago

The .zip copy really has some weird performance issues, I tried using dos32a doom2d.exe and that got the game running everytime against the sporadic loading without (still slow). It throws up an error after you exit but it runs...

command_001 This happens on the .7z copy as well.

I also tried repacking the .zip to see if that would help but still the same fault, repacking to .7z (even my crazy solid archive setting) does cure the issue. I also managed to figure the setup options and set to Soundblaster from Adlib, just need to figure out the keys bindings as I can't seem to trigger the door switches.

I tested with my own test build and the new autobuild from my PR #3723 (64bit Visual Studio)

EDIT This is not just an issue with DOOM-2D, other games perform differently depending on where they are. The XCar Demo mentioned in #1285 runs fine in a regular .zip, but hates being in a heavily compressed solid .7z despite only being 6mb in size it more or less locks up after a slow load to the title screen.