SvarDOS / edrdos

Enhanced DR-DOS kernel and command interpreter ported to JWasm and OpenWatcom C
http://svardos.org/
Other
38 stars 4 forks source link

DIR /2 file/directory display misalignment #69

Closed ecm-pushbx closed 3 months ago

ecm-pushbx commented 3 months ago

With the update from https://github.com/SvarDOS/edrdos/commit/2a20b1ae4d2a038a6ca3ae212c104d6259d3620f

Picked into https://hg.pushbx.org/ecm/edrdos/rev/f54f5656a8c8

Look at this image of one of the FreeDOS test .iso CDs:

Screenshot_20240722-103649

boeckmann commented 3 months ago

Interesting, the build provided via the action does not show this behaviour. I made a diff between your comint.c and mine. I had to reformat them with clang-format to get a meaningful diff. After reformatting the sources are exactly the same. Wondering what is causing the issue.

Bildschirmfoto 2024-07-22 um 12 01 48
ecm-pushbx commented 3 months ago

I uploaded my build to https://pushbx.org/ecm/test/20240722/command.com

Did you try it on a shsucdx-redirected drive? My test is from the same file system as described in https://github.com/SvarDOS/edrdos/issues/63

ecm-pushbx commented 3 months ago

Not just on a CD though, the FAT FS C: drive also shows this: Screenshot_20240722-125140

boeckmann commented 3 months ago

Ouch, compared the same files instead your hg version with mine, no wonder they were the same...

There seem to be some significant changes between your version and mine. When reformatting your file with clang-format, it breaks the file, so that it does not compile anymore. Makes comparing the files not exactly easy...

I'll try with another clang-format version, but under MacOS I currently only have version 18.1.8 available.

However, I think I got your version to work by doing the following change:

--- a/command/comint.c  Mon Jul 22 09:54:06 2024 +0200
+++ b/command/comint.c  Mon Jul 22 12:51:47 2024 +0200
@@ -1134,9 +1134,9 @@
                  total_size += fsize;
                  if ( OPT( DIR_2COLS ) ) {
                    /* printf ("%9lu", search.fsize);*/
-                   printf( "%10s", format_number( fsize, 1 ) );
+                   printf( "%10s ", format_number( fsize, 1 ) );
                  } else {
-                   printf( "%10s", format_number( fsize, 1 ) );
+                   printf( "%10s ", format_number( fsize, 1 ) );
                  }
                }
ecm-pushbx commented 3 months ago

That seems to fix it, yes. Any idea as to the root of this difference? (I know I didn't pick your reformat commits several months ago but you mentioned there's non-formatting differences as well.)

ecm-pushbx commented 3 months ago

By the way, when the date and time are zero (as is the default for my bootimg script) then no stamp is displayed. But that also seems to misalign DIR /2:

Screenshot_20240722-130507

boeckmann commented 3 months ago

For example I stripped out all the CDOS ifdefs and removed support Concurrent DOS. If there are functional differences I have to look more sharply.

boeckmann commented 3 months ago

That with the no timestamp display will probably also affect my version. I first have to find out how to create a file with the timestamp set to zero.

boeckmann commented 3 months ago

Any idea as to the root of this difference?

The blame shows it was part of the following commit by me:

https://github.com/SvarDOS/edrdos/blame/1925c5fed3644352de29ee0a61a841c4514c20ca/command/comint.c#L1038

boeckmann commented 3 months ago

May it be that you incorporated this by hand and missed the trailing space inside the strings?

ecm-pushbx commented 3 months ago

Oh, looking at the commit's diff in https://github.com/SvarDOS/edrdos/commit/2a20b1ae4d2a038a6ca3ae212c104d6259d3620f you did insert the blanks. I just missed that bit.

As for how to create a file with zero date/time you can use my NASM script bootimg (also requires the lmacros collection), which defaults to zeroes for the date and time. Here's an example scriptlet using it:

nasm -D_MEDIAID=0F8h -D_SPF=2000 ../bootimg/bootimg.asm -I ../bootimg/ -I ../lmacros/ -D_MBR -D_BPE=32 -D_MBR_PART_TYPE=fat32 -o big32.img -D_PAYLOADFILE=::fill,'(512 * 76000),26h,fill.dat,::chdir,testdir,::fill,32,32,test.dat' -D_SPI=256000 && nasm ../bootimg/bootimg.asm -I ../kernel/boot/ -I ../edrdos/repo/ -I ../edrdos/repo/bin/ -I ../edrdos/pack/ -I ../lmacros/ -I ../edrdos/edrsys/ -I ../edrdos/repo/drbio/bin/ -I ../edrdos/repo/drdos/bin/ -I ../edrdos/repo/command/bin/ -I ../ldebug/bin/ -I ../ldebug/test/ -I ../ldosboot/ -o diskedr.img -D_PAYLOADFILE=::rename,drbio.sys.unpacked,drbio.sys,drdos.sys,fdsys.com,instsect.com,boot32,boot32lb,fdkold32.bin,edrdos.com,edrpack.com,edrdos.sys,edrpack.sys,ii13.sld,../edrdos/repo/command/bin/command.com,dconfig.sys,xcdrom/XCDROM.SYS,shsucdx/shsucdx.com,drautoex.bat,ldebug.sld,himemx.exe,ldebug.com,::rename,ldebug.sld,lcdebug.sld,lcdebug.com,SYS.COM,quit.com,quit.eld,dpb.eld,int3.com,int19.com,callver.com,more.exe,extlib.eld,ldebugx.com,lcdebugx.com,HDPMI32.EXE,dpmimini.com,testcdxu.bat,amiscmd.eld,amisoth.eld,amismsg.eld,menu.sld,menu2.sld && "$DOSEMU" -I "floppy { device diskedr.img }" -E "a:sys A: /bootonly /oem:fd /k ldebug.com" -dumb -td -kt -quiet && qemu-system-i386 -fda diskedr.img -boot order=a -fdb ~/test/20240102/disk1.img -display curses -chardev serial,id=serial2,path=/tmp/vptty-dos -serial null -serial chardev:serial2 -hda big32.img -cdrom T2407LGCY.iso; stty sane

ecm-pushbx commented 3 months ago

May it be that you incorporated this by hand and missed the trailing space inside the strings?

Yes, I applied this earlier today but that hunk didn't apply cleanly. So I manually did the same change, or so I thought. Fixed in https://hg.pushbx.org/ecm/edrdos/rev/f184213d988f

ecm-pushbx commented 3 months ago

The -D_MEDIAID=0F8h seems to be required for EDR-DOS to accept a hdd BPB.

boeckmann commented 3 months ago

Thanks, this worked. I simply used bootimg to create a floppy with a few files on it. Two-column mode should now align correct 23aa2246. I did not test test the display of long filenames yet. According to the source, there seems to be some support for it...

rofl0r commented 3 months ago

please excuse my interference, but i can't help myself wondering why each of you works on his own copy of edr-dos, instead of joining forces and work together on the same repo ? from a user PoV its kinda confusing - "which one should i use, which one is better?", and having to backport patches all the times seems to be an awkward situation as well.

ecm-pushbx commented 3 months ago

Thanks, this worked. I simply used bootimg to create a floppy with a few files on it. Two-column mode should now align correct 23aa224. I did not test test the display of long filenames yet. According to the source, there seems to be some support for it...

Yes, the shell supports it but you need doslfn or a redirector with LFN support to use it. I just tested in dosemu2 (FreeDOS kernel) and it seems to be fine with this update. I'll pick it later today for my repo.

boeckmann commented 3 months ago

please excuse my interference, but i can't help myself wondering why each of you works on his own copy of edr-dos, instead of joining forces and work together on the same repo ? from a user PoV its kinda confusing - "which one should i use, which one is better?", and having to backport patches all the times seems to be an awkward situation as well.

Hello @rofl0r, no interference at all :)

I can understand that from a user perspective it is a little confusing. In large parts we are working together, albeit there are two separate repositories. We have for example some kind of double-checking. Changes that go into this repository are regularly checked by @ecm-pushbx before importing it into her repo, and this is also largely true for the other way around.

While there should not be much user-visible differences apart from @ecm-pushbx providing single-file versions of the kernel, the projects differ in having completely different build systems, and they are utilizing different version control systems. While this repo is based on Git, the ecm one is based on Mercurial. Having this repo gives us a public issue tracker and some "range" by being on Github. The Mercurial repo seems to be tightly integrated into ecm's workflow, and the commit log is much more accurate than the one for this repo (I am a bit sloppy in this respect).

mateuszviste commented 3 months ago

While there should not be much user-visible differences apart from @ecm-pushbx providing single-file versions of the kernel

I hope I won't sound nagging, but would there be any chance for this single-compressed-file version to land in your repo here? That's the one thing I am looking forward to before integrating the EDR kernel in the SvarDOS core. :) BTW I'm very happy to see this ongoing work & cooperation between you & ECM. Thank you both for the outstanding work you're putting into the EDR kernel!

boeckmann commented 3 months ago

I hope I won't sound nagging, but would there be any chance for this single-compressed-file version to land in your repo here? That's the one thing I am looking forward to before integrating the EDR kernel in the SvarDOS core. :) BTW I'm very happy to see this ongoing work & cooperation between you & ECM. Thank you both for the outstanding work you're putting into the EDR kernel!

Yes, if you eagerly await it I will prioritize this. If I understand it correct, I can not simply import it from @ecm-pushbx repo, because this kernel would not be bootable by the bootsector provided with the SYS command. But I will have a look at it shortly...

ecm-pushbx commented 3 months ago

Replying to @rofl0r:

please excuse my interference, but i can't help myself wondering why each of you works on his own copy of edr-dos,

*his or her own copy

instead of joining forces and work together on the same repo ? from a user PoV its kinda confusing - "which one should i use, which one is better?", and having to backport patches all the times seems to be an awkward situation as well.

Well, @boeckmann already replied to parts of this. We do review one another's contributions for the most part, except things like the build system that I do not wish to carry over and single-file load which hasn't landed in the SvarDOS repo (this repo) yet.

I recently replied to someone else per email (had to get home to access my sent mail), which also describes the situation some:

I just checked and Bernd Böckmann's SvarDOS fork of EDR-DOS did increment the version number, to 7.01.09 actually [3]. But it also includes the date alongside the version number. We're on friendly terms, sharing most but not all of the changes done in one another's repos. (A would-be single repo is hindered by the choice of hg (Mercurial) vs git as the DSCM of choice and by the changes that we do not wish to share.)


Replying to @boeckmann:

please excuse my interference, but i can't help myself wondering why each of you works on his own copy of edr-dos, instead of joining forces and work together on the same repo ? from a user PoV its kinda confusing - "which one should i use, which one is better?", and having to backport patches all the times seems to be an awkward situation as well.

Hello @rofl0r, no interference at all :)

I can understand that from a user perspective it is a little confusing. In large parts we are working together, albeit there are two separate repositories. We have for example some kind of double-checking. Changes that go into this repository are regularly checked by @ecm-pushbx before importing it into her repo, and this is also largely true for the other way around.

Exactly, we do work together to some degree, despite the two repos.

While there should not be much user-visible differences apart from @ecm-pushbx providing single-file versions of the kernel, the projects differ in having completely different build systems,

Yes, I'm still using a variant of the original build scripts which run in DOS as batch files, except for the kernel post-processing to create the single-file executables and the kernel's trace listing files.

and they are utilizing different version control systems. While this repo is based on Git, the ecm one is based on Mercurial. Having this repo gives us a public issue tracker and some "range" by being on Github. The Mercurial repo seems to be tightly integrated into ecm's workflow,

I don't know about this; I do use the hg changeset revisions hashes and "amount of ancestors" in some places (eg lDebug ?build, the Source Control Revision ID appendix in some manuals, and the new ident86 version header), but this is not impossible to do for git either. I think I just prefer to use hg over git because I learned it first and consider it more user-friendly.

and the commit log is much more accurate than the one for this repo (I am a bit sloppy in this respect).

That much is true! Much like for FreeDOS Debug I also imported the old version history into the EDR-DOS repo, to be able to reference the earlier versions, back up to the original OpenDOS 7.01 Machine Readable Source release. (I think there was more fine-grained WIP patches available which I didn't all import. But it is something.)

ecm-pushbx commented 3 months ago

I hope I won't sound nagging, but would there be any chance for this single-compressed-file version to land in your repo here? That's the one thing I am looking forward to before integrating the EDR kernel in the SvarDOS core. :) BTW I'm very happy to see this ongoing work & cooperation between you & ECM. Thank you both for the outstanding work you're putting into the EDR kernel!

It's nice to be appreciated =)

Yes, if you eagerly await it I will prioritize this. If I understand it correct, I can not simply import it from @ecm-pushbx repo, because this kernel would not be bootable by the bootsector provided with the SYS command. But I will have a look at it shortly...

This is not true actually, the EDR-DOS load protocol and (its basis) the FreeDOS load protocol are both able to load all four variants of the single-file kernel that I provide. The differences between the use of the files are grouped into whether the files use lDOS iniload (*.com named files) or lDOS drload (*.sys named files). The only case in which the EDR-DOS/FreeDOS load cannot be used is if arbitrary data (eg a .zip archive) is appended to the file so that it becomes larger than 128 KiB.

I have documented the FreeDOS load in the lDOS boot manual. Do note the assumptions section as well.

The part on the EDR-DOS load protocol describes the traditional differences from FreeDOS load. The dl versus bl question is doubly obsolete, as lDOS (both iniload and drload) does not care, and recent FreeDOS boot loaders do provide both dl and bl. The ds:bp versus ss:bp is relevant, but there is little reason for any loader not to provide ss:bp. The differing load and entrypoint addresses are of little concern for lDOS drload, and can be safely ignored for lDOS iniload if the kernel file is at least 32 KiB in size.

The difference between iniload and drload is that iniload can be loaded as a lot of different kernel formats, whereas drload requires being loaded as a FreeDOS or EDR-DOS kernel. All of my single-file EDR-DOS kernels may be loaded using the FreeDOS or EDR-DOS load protocols; the iniload (*.com named files) may also be loaded as different formats.

ecm-pushbx commented 3 months ago

Thanks, this worked. I simply used bootimg to create a floppy with a few files on it. Two-column mode should now align correct 23aa224. I did not test test the display of long filenames yet. According to the source, there seems to be some support for it...

Tested and picked in https://hg.pushbx.org/ecm/edrdos/rev/c3a7e99c10bb - thanks!

ecm-pushbx commented 3 months ago

That much said on the single-file load, you will have to reference or import at least some files from lmacros, ldosboot, kernwrap, and for the compression also the inicomp stage plus at least one method's packer. scanptab and bootimg may also be needed, as do dosemu2 or qemu + DOS files to run the decompression test (required to gather the amount of dots for the progress display if it is enabled).

ecm-pushbx commented 3 months ago

For the qemu test run mtools is needed as well, I think.

boeckmann commented 3 months ago

For the start I relocated the zero-uncompression code into the deblocking buffer, to compress the whole DRBIO. If you may have a look at it. Shrinks the BIO by around 11K. My goal is to have this running at least in the zero-compression only case (no exomizer et. al.) without depending on kernwrap etc.

boeckmann commented 3 months ago

@mateuszviste thanks for the appreciation. Would it be ok for the combined file to initially be around 50K in size? This would be without "sophisticated" compression utilized.

mateuszviste commented 3 months ago

@mateuszviste thanks for the appreciation. Would it be ok for the combined file to initially be around 50K in size? This would be without "sophisticated" compression utilized.

It would be cool, yes. My primary goal is not to use more disk space than what we have with the FD Kernel. The UPXed FD kernel is smaller (45K), but 5K feels like an acceptable cost for a beta-test run ;-) I assume that after a "real" compression is applied (Exomizer or what not), the EDR kernel will be at least as small as the FD kernel.

ecm-pushbx commented 3 months ago

@mateuszviste thanks for the appreciation. Would it be ok for the combined file to initially be around 50K in size? This would be without "sophisticated" compression utilized.

It would be cool, yes. My primary goal is not to use more disk space than what we have with the FD Kernel. The UPXed FD kernel is smaller (45K), but 5K feels like an acceptable cost for a beta-test run ;-) I assume that after a "real" compression is applied (Exomizer or what not), the EDR kernel will be at least as small as the FD kernel.

46 kB with LZMA-lzip and the drload entry:

unzip -l wwwecm/download/edrdos.zip bin/*.com bin/*.sys
Archive:  wwwecm/download/edrdos.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
    80896  2024-07-22 23:53   bin/edrdos.com
    76304  2024-07-22 23:53   bin/edrdos.sys
    50688  2024-07-22 23:53   bin/edrpack.com
    46128  2024-07-22 23:53   bin/edrpack.sys
---------                     -------
   254016                     4 files
mateuszviste commented 3 months ago

46 kB with LZMA-lzip and the drload entry:

That's interesting! Any idea why it's slightly bigger than the UPXed FD kernel?

image

Does this EDR kernel build come with some extra features, like LFN or such, or is the C-generated FreeDOS kernel machine code simply more compact than EDR's hand-crafted assembly?

Also, do you think such LZMA-packed kernel likely to noticeably inflate the boot time on a 8086-era PC?

ecm-pushbx commented 3 months ago

46 kB with LZMA-lzip and the drload entry:

That's interesting! Any idea why it's slightly bigger than the UPXed FD kernel?

image

Does this EDR kernel build come with some extra features, like LFN or such, or is the C-generated FreeDOS kernel machine code simply more compact than EDR's hand-crafted assembly?

Not sure. The EDR-DOS kernel supports some nominally "LFN" functions but only actually with SFNs; the functions are implemented to allow returning 38-bit file sizes for its FAT+ support. That does eat some memory.

My depacker is likely not as optimised as UPX's. It needs about 3 kB for the inicomp stage.

Also, do you think such LZMA-packed kernel likely to noticeably inflate the boot time on a 8086-era PC?

Yes, the depacking may need several minutes. This is a reason for the depack progress indicator lest the user thinks the machine hangs.

In a blog post I tested depacking LZMA-lzip packed lDebug on the HP 95LX, which runs a NEC V20 at 5.37 MHz: https://pushbx.org/ecm/dokuwiki/blog/pushbx/2023/0321_cpu_performance_comparison#the_hp_95lx

The corresponding ldebug.com file (includes lDOS iniload) is 70 kB, the compressed payload is 62 kB, and the uncompressed .big image is 108 kB:

    62855  2023-03-08 18:21   SOURCE/LDEBUG/ldebug/tmp/lz/debug.lz
    70656  2023-03-08 18:21   SOURCE/LDEBUG/ldebug/tmp/lz/ldebug.com
   108528  2023-03-08 18:21   SOURCE/LDEBUG/ldebug/tmp/debug.big

So the 3 minutes estimate is too high for depacking EDR-DOS because the kernel is smaller than lDebug r5, but it is too low because an 8088 is slower than the NEC V20.

mateuszviste commented 3 months ago

The EDR-DOS kernel supports some nominally "LFN" functions but only actually with SFNs; the functions are implemented to allow returning 38-bit file sizes for its FAT+ support. That does eat some memory.

I am probably committing blasphemy by asking this... but is is possible to disable FAT+ support at build time, or is it hardwired into the overall kernel design? My understanding is that FAT+ is an improved FAT32 that supports files of up to 256G. I wonder if anyone really uses that.

Yes, the depacking may need several minutes.

Ah, so it's similar to my experience of using upx --lzma... At some point I was happy to compress my programs that way, until I realized that it added a runtime overhead of between 30s and a minute on my 8086. :/

That being said, the lzma decompressor is relatively large, so given the modest size of the kernel, perhaps a weaker compression algorithm with a more compact decompressor would be actually better.. the "no zeroes" version of the kernel being less than 64K, maybe is there a more trivial (de)compression method better fitted for tiny (16-bit addressable) contents?

ecm-pushbx commented 3 months ago

The EDR-DOS kernel supports some nominally "LFN" functions but only actually with SFNs; the functions are implemented to allow returning 38-bit file sizes for its FAT+ support. That does eat some memory.

I am probably committing blasphemy by asking this... but is is possible to disable FAT+ support at build time, or is it hardwired into the overall kernel design? My understanding is that FAT+ is an improved FAT32 that supports files of up to 256G. I wonder if anyone really uses that.

FAT16+ technically exists too, but needs large cluster sizes (supported by EDR-DOS itself, FreeDOS, and lDOS fork of MS-DOS v4).

I'd rather not make FAT+ optional but it can be done.

Yes, the depacking may need several minutes.

Ah, so it's similar to my experience of using upx --lzma... At some point I was happy to compress my programs that way, until I realized that it added a runtime overhead of between 30s and a minute on my 8086. :/

In fact UPX with --lzma is probably faster than my LZMA depacker as well, at least I wouldn't put it past them.

That being said, the lzma decompressor is relatively large, so given the modest size of the kernel, perhaps a weaker compression algorithm with a more compact decompressor would be actually better.. the "no zeroes" version of the kernel being less than 64K, maybe is there a more trivial (de)compression method better fitted for tiny (16-bit addressable) contents?

My depackers all need >1024 Bytes for their inicomp stages although these are capable of both compressed and decompressed sizes beyond 64 KiB. So it is possible to optimise them some if you do not use them for large files.

Feel free to poke around the source texts!

boeckmann commented 3 months ago

The issue went a little off-topic. The original bug seems to be fixed? Then I could close this.

mateuszviste commented 3 months ago

The issue went a little off-topic. The original bug seems to be fixed? Then I could close this.

just out of curiosity -- is there any practical reason to use EDR-COMMAND instead of SvarCOM? Like better compatibility, smaller footprint, some features, etc - or is it more a matter of personal preference?

boeckmann commented 3 months ago

Looks like EDR-COMMAND contains a ton of compatibility stuff, some even originating from Digital Researches Concurrent etc. I maintain this mainly for documentation purposes, and use EDR COMMAND only as the default shell for the Github action disk images, so I can test if it still works. I will not put much effort in improving it apart from fixing bugs reported.

Honestly, under DOS I prefer 4DOS.