A proposal for better patch isolation

Developer's Forum, for discussion of bugs, code, and other developmental aspects of DOSBox.

A proposal for better patch isolation

Postby Ant_222 » 2017-6-22 @ 20:13

It seems that Automake's .am files comprise the bottleneck in the keeping of patches orthogonal, or independent of each other. If some two patches add new source files to the same directory, they must needs modify the same .am file located in that directory. Whereas the latter contains a list of source files, with many filenames specified on each line, conflicts are almost inevitable, as the conflict between the Munt and the Pixel-Perfect patch shows.

A reader of the Automake mailing list has written that arranging such file lists with one file per line and keeping them alphabetically sorted will considerably reduce the likeness of the said conflicts. May I ask the developers to effect this modification in all the file lists inside .am files? Although this will break all patches that modify .am files, the benefit will greatly exceed the little trouble of fixing the patches.
Ant_222
Member
 
Posts: 375
Joined: 2010-7-24 @ 21:29

Re: A proposal for better patch isolation

Postby Yesterplay80 » 2017-6-23 @ 08:07

It would certainly be nice to have patches, that don't depend of each other, rearranging the .am files would be almost useless for that case, though. There are other files, especially dosbox.cpp, that almost every patch messes with even more, so you'd still have to edit the patches to be able to apply more of them to the same source.
My full-featured DOSBox SVN builds (without debugger) for Windows: Vanilla DOSBox and DOSBox ECE (Enhanced Community Edition)
User avatar
Yesterplay80
Member
 
Posts: 338
Joined: 2016-2-23 @ 11:02
Location: Germany

Re: A proposal for better patch isolation

Postby Ant_222 » 2017-6-23 @ 08:45

Source files being large, they may accept many independent changes to different sections of code. Not so with .am files, where changed regions often share the same context (via contextual diff).
Ant_222
Member
 
Posts: 375
Joined: 2010-7-24 @ 21:29

Re: A proposal for better patch isolation

Postby Dominus » 2017-6-23 @ 11:27

The am patches are the easiest to fix as they change so little. I doubt the devs would change that just to accommodate patches they don't want to apply in the first place...
User avatar
Dominus
DOSBox Moderator
 
Posts: 7644
Joined: 2002-10-03 @ 09:54
Location: Ludwigsburg

Re: A proposal for better patch isolation

Postby Ant_222 » 2017-6-23 @ 12:09

Dominus wrote:The am patches are the easiest to fix as they change so little.
Yet such trivial conflicts sometimes prevent the maintainers of custom builds from incorporating otherwise perfectly compatible patches.
I doubt the devs would change that just to accommodate patches they don't want to apply in the first place...
True, but they could do it as an act of good will for all developers, for it would affect all patches that add new source files, not only the two mentioned. With their permission, I can make such a patch myself. I belive this modification will not harm DOSBox in any way, so there is no compromise or downside.
Ant_222
Member
 
Posts: 375
Joined: 2010-7-24 @ 21:29

Re: A proposal for better patch isolation

Postby Dominus » 2017-6-23 @ 12:19

Yes, a patch for this is the first logical step of course.
User avatar
Dominus
DOSBox Moderator
 
Posts: 7644
Joined: 2002-10-03 @ 09:54
Location: Ludwigsburg

Re: A proposal for better patch isolation

Postby Ant_222 » 2017-6-23 @ 12:27

I hope you understood that I mean a patch that rewrites all file lists in .am files as one file per line in alphabetical order. Unless it is admitted to the official upstream SVN, it will be useless and even harmful.
Ant_222
Member
 
Posts: 375
Joined: 2010-7-24 @ 21:29

Re: A proposal for better patch isolation

Postby Dominus » 2017-6-23 @ 13:58

Yes, I understand
User avatar
Dominus
DOSBox Moderator
 
Posts: 7644
Joined: 2002-10-03 @ 09:54
Location: Ludwigsburg

Re: A proposal for better patch isolation

Postby Serious Callers Only » 2017-6-27 @ 04:57

Good stuff. Looks like i might use your new graphics scaling patch after all Ant.
Serious Callers Only
Member
 
Posts: 371
Joined: 2003-4-26 @ 21:34

Re: A proposal for better patch isolation

Postby Ant_222 » 2017-6-27 @ 11:50

Serious Callers Only wrote:Good stuff. Looks like i might use your new graphics scaling patch after all Ant.
Very well then—I hope you have already read my last comment about it in the patch's thread. Thanks.
Ant_222
Member
 
Posts: 375
Joined: 2010-7-24 @ 21:29

Re: A proposal for better patch isolation

Postby Serious Callers Only » 2017-6-27 @ 18:16

Ehh, i'll do it, once and if dosbox upstream commits your patch. If we don't encourage this practice upstream it would increase fragmentation even more, between those patches that use the upstream file and those that don't. Then, might as well say 'screw upstream' then and baptize one of the forks and the new 'standard' one, which i don't want to do at all.
Serious Callers Only
Member
 
Posts: 371
Joined: 2003-4-26 @ 21:34

Re: A proposal for better patch isolation

Postby Ant_222 » 2017-6-28 @ 12:49

It is a bold stance that you take! The current indifferent policy of the core DOSBox developers (God bless them!) regarding patches annoys me as well. I cannot talk about individual patches, because I don't use any except mine, but in general I should expect the following approach:
  • May reject the patch at will if it does not improve DOSBox as an emulator of MS-DOS games.
  • If the patch has commendable intentions but compromises existing functionality or introduces poorly structured code, set well-defined goals for the patch developer about the correcting of the former and the refactoring of the latter, so that the patch might be untimately accepted.
As to the Pixel-perfect patch, I think that emulation of MS-DOS games without lossless scaling makes no sense. It is noteworthy that people are asking videocard developers to implement pixel-perfect modes: here and here.
Ant_222
Member
 
Posts: 375
Joined: 2010-7-24 @ 21:29

Re: A proposal for better patch isolation

Postby DosFreak » 2017-6-28 @ 13:41

If you want something changed then email or PM qbix or harekiet. Don't complain about it on a post on a forum and assume that it will be read.

As for major changes to DOSBox that don't have anything to do with improving DOS game compatibility those are usually introduced in private builds and then released to the public in the next version of DOSBox (yeah yeah I know). Should that change? Mabye, mabye not but that's the way it's been.
User avatar
DosFreak
l33t++
 
Posts: 9833
Joined: 2002-6-30 @ 16:35
Location: Your Head

Re: A proposal for better patch isolation

Postby Qbix » 2017-6-28 @ 17:07

I don't know the exact names of the files in the munt patch, but munt and pixel_perfect are pretty close in the alphabet, so your proposal might not work for this specific case that you are proposing it for.
Water flows down the stream
How to ask questions the smart way!
User avatar
Qbix
DOSBox Author
 
Posts: 10652
Joined: 2002-11-27 @ 14:50
Location: Fryslan

Re: A proposal for better patch isolation

Postby Qbix » 2017-6-28 @ 18:15

Serious Callers Only wrote:Ehh, i'll do it, once and if dosbox upstream commits your patch. If we don't encourage this practice upstream it would increase fragmentation even more, between those patches that use the upstream file and those that don't. Then, might as well say 'screw upstream' then and baptize one of the forks and the new 'standard' one, which i don't want to do at all.


Aren't you being a bit extreme here ? We are talking about the easiest editable files of DOSBox, which can be edited without knowing anything about programming in C/C++.
Water flows down the stream
How to ask questions the smart way!
User avatar
Qbix
DOSBox Author
 
Posts: 10652
Joined: 2002-11-27 @ 14:50
Location: Fryslan

Re: A proposal for better patch isolation

Postby Yesterplay80 » 2017-6-29 @ 05:55

Just to be sure: We're talking about this section of a makefile, right?

Code: Select all
libhardware_a_SOURCES = adlib.cpp dma.cpp gameblaster.cpp hardware.cpp iohandler.cpp joystick.cpp keyboard.cpp \
                        memory.cpp mixer.cpp pcspeaker.cpp pci_bus.cpp pic.cpp sblaster.cpp tandy_sound.cpp timer.cpp \
         vga.cpp vga_attr.cpp vga_crtc.cpp vga_dac.cpp vga_draw.cpp vga_gfx.cpp vga_other.cpp \
         vga_memory.cpp vga_misc.cpp vga_seq.cpp vga_xga.cpp vga_s3.cpp vga_tseng.cpp vga_paradise.cpp \
         cmos.cpp disney.cpp gus.cpp mpu401.cpp ipx.cpp ipxserver.cpp dbopl.cpp


If so, what would listing them each per line achieve? Still the order and line numbers would change with every patch that affects files in the current directory. As far as i know, you can't just add additional lines at the end of a file using the "patch" utility, but you always have to specify a line number and the number of lines you're changing, right? So, if you're adding multiple patches, you'd run into the same trouble you'd run into now: You wouldn't know which lines to change and you'd still have to adjust every patch to the others applied before it.
My full-featured DOSBox SVN builds (without debugger) for Windows: Vanilla DOSBox and DOSBox ECE (Enhanced Community Edition)
User avatar
Yesterplay80
Member
 
Posts: 338
Joined: 2016-2-23 @ 11:02
Location: Germany

Re: A proposal for better patch isolation

Postby Dominus » 2017-6-29 @ 06:28

It would work a bit better as patch is not as dependent on line number but on "context", what is before and after the patched lines. But as Qbix pointed out, if the patches are adding files too close to each other, patch will fail anyway.
As it is trivial to change how about you (Ant_) take it for a test drive and see if it actually helps or whether multiple patches break it anyway.

@serious callers only: you are blowing this out of proportion. But yeah, no one is stopping you from forking. I know from Exult that you tend to have good ideas and get annoyed by the unwillingness of upstream to apply your patches ...
User avatar
Dominus
DOSBox Moderator
 
Posts: 7644
Joined: 2002-10-03 @ 09:54
Location: Ludwigsburg

Re: A proposal for better patch isolation

Postby Serious Callers Only » 2017-6-29 @ 19:10

It's more i don't want to fork. Dosbox ecosystem is in a bad shape with multiple forks already, and i'm not the kind of guy that wants to wade into a c compiler when patches inevitably stop working if they aren't modular.

As a aside, this makefile thing makes me think that if there was a way to collect these sources at compile time instead of being explict on the file (ie something like a dir where those cpp files above would be dumped and 'libhardware_a_SOURCES = hardware/*' ) that this small source of conflict could go away. I'm not sure make allows that or anyone involved is willing to upheave their methodology / update their patches to accommodate others.
Serious Callers Only
Member
 
Posts: 371
Joined: 2003-4-26 @ 21:34


Return to DOSBox Development

Who is online

Users browsing this forum: No registered users and 1 guest