TASEmulators / pcem-old

https://pcem-emulator.co.uk/
GNU General Public License v2.0
3 stars 1 forks source link

Meta discussion #1

Open vadosnaprimer opened 5 years ago

vadosnaprimer commented 5 years ago

There's this patch we need to be applied: https://github.com/clementgallet/libTAS/issues/208#issuecomment-483568526

@clementgallet do you have push access?

clementgallet commented 5 years ago

Not on this repo. I was given access to fs-uae, but without any information. Am I supposed to apply deterministic patches to those repos?

vadosnaprimer commented 5 years ago

Yes please! We don't know if the fs-uae patch ever gets accepted, and regardless of the result we might want to have a fork for tasvideos purposes. Same with PCem (except with it we don't even intend to send a deterministic PR as discussed earlier). So let's keep all the determinism work in these forks, and do the tests using them. Also hi @slamotas!

clementgallet commented 5 years ago

For some reason, I cannot run my win95 image (or the FreeDOS one provided by slamotas) with the current master. I'm getting stuck during the boot sequence: PCem boot softlock

I tried git bisect to identify the relevant commit, but I cannot compile most of the commits between v14 and master due to some autoconf/automake issues, or a compile error regarding -march flag.

So for now I pushed the deterministic patch here, but I couldn't test it.

vadosnaprimer commented 5 years ago

Ouch! But it was working against the original repo's master branch?

Also I forgot to mention it on time (sorry), but maintaining such patches is easier in a separate branch, so that master could be pulled from upstream whenever we need, and then merged into the patched branch if it's compatible. I'll manage this within a few days.

clementgallet commented 5 years ago

No, it doesn't work on the original repo's master branch either.

Oh yeah, it totally makes sense to patch inside a branch.

vadosnaprimer commented 5 years ago

Okay, I created the tasvideos branch with your commits, I guess I'll need to force-push to clean-up the master before it's too late, and move the readme info to repo description. Is the latest release fine regarding the patch?

clementgallet commented 5 years ago

Yes, v14 release is fine regarding the patch. They are working on a v15 release this month I think, so the issue I'm having may be fixed by then.

vadosnaprimer commented 5 years ago

Master branch has been hard-reset to upstream, please pull. I'm going to create the tags now, dunno if they can be fetched during HG conversion.

EDIT: Tags are done, I had to create them by looking at the commits, the original repo doesn't seem to have them. Haven't compared with released tarballs cuz it's tedious, but if something doesn't build, one should try some nearby commit.

EDIT: And I cleaned-up fs-uae as well, in the same way: master is for upstream updates, tasvideos is for us.

clementgallet commented 5 years ago

About the issue posted above, it appears between commits 7454684deee5f52978846a80aa985d9358d72e7f and 20e38f6134c13f223e93d2d0f52b65c911cf6510 (I can't compile commits in between). I'll post on PCem forum about it.

clementgallet commented 5 years ago

v15 was released, but didn't fix my problem. Also, I wasn't given access to the forum, so I guess I'll have to debug myself :/

slamotas commented 5 years ago

I'm not sure what the problem is. I compiled and ran v15 (unpatched) with my images from v14 without any issues. It could be a BIOS setting causing your softlock, but I'm not sure which setting would cause it to happen at that particular point. Does it work if you make a new setup with a new hard drive in v15?

As far as the patch goes, I tried applying it and there are a few incompatibilies. Here's what I got when I tried to patch it: https://pastebin.com/RyWgTJCL

clementgallet commented 5 years ago

Ok, I'll have to install a new setup then. I already applied the patch in the tasvideos branch here, could you test it then, please?

Edit: Also, maybe we could update the upstream repo to v15, so we can have a stable patched version here?

slamotas commented 5 years ago

For me, the branch gets further along than the screen you posted. DOS stuff seems to work fine, but my Windows 98 image crashes during the loading screen (it either segfaults or closes with no message). Switching to an unpatched GPU and turning off sound doesn't make a difference. From what I recall from reading the forums, PCem v15 had some similar issues during the testing phase. The official v15 works nicely for me so it's worth updating this.

vadosnaprimer commented 5 years ago

I imported the current PCem source using fast-export, and commit hashes don't match what natt got in his repo (which we're forking).

@nattthebear does it seem to depend on anything like using msys/mingw/linux/windows? Oddly enough, I'm getting consistent hashes every time I re-clone/re-import, they are just different from yours...

nattthebear commented 5 years ago

Smells like file EOL stuff. Are the files themselves byte-identical to mine? Git EOL modes? I can boot up the VM I used to do it this weekend and investigate those things as well.

vadosnaprimer commented 5 years ago

This is absurd! Commit hashes start deviating after 719524123eacabc1d97dd126235747c136002bcc, but the first commit that has different hashes for you and me has identical file hash for the only file that was changed! And later on file hashes match, but commit hashes don't. WTF.

EDIT:

Found it.

Revision: 2b885e681816e04f7e1219268b5f4b6ba80e0fc9 Author: Salvador Pardiñas

Revision: fa703e0694739b7f31f539f4aa35fc5fdde83536 Author: Salvador Pardi?as

The best part is I can't find on google how to hg-clone with unicode names properly shown (Win7 with Russian locale, though both hg and msys are configured to work with utf-8, and it doesn't help).

vadosnaprimer commented 5 years ago

Ran in on xubuntu vm and got it to work, updates pushed to master. Added v15 tag matching what was released as v15 (since upstream doesn't have tags).

@clementgallet please see if master builds/works correctly, then I'll rebase the patch.

clementgallet commented 5 years ago

Thanks! It would be easier to just merge master into tasvideos, because I already changed a few things from the original patch (and some parts of the patch are obsolete with the updated code). If it's ok for you, I'll work on it.

vadosnaprimer commented 5 years ago

I didn't mention it earlier, but since we want this fork to be set up as nicely as possible, we have some notes on well organized forking here: http://tasvideos.org/LawsOfTAS/OnVersionControl.html#Forking

It is advised to keep updates (when upstream is pulled) in separate branches, and patches on top of them. That way we won't have to inherit old patches that might be incompatible with updated codebase, because no patch would be old: they would all be applied on top of target branches.

Does this approach look feasible? Especially given that some parts of the old patch are now obsolete.

As a result, we would have master, versions of master corresponding to upstream pulling, tasvideos branch, and versions of tasvideos branch corresponding to merged upstream pulls (or to our own releases).

clementgallet commented 5 years ago

Ok, I see, it makes more sense indeed. You can do the rebase then.

vadosnaprimer commented 5 years ago

So after rebasing you would tweak the resulting patch as needed for v15?

clementgallet commented 5 years ago

Yes, there shouldn't be much to do, there's almost no intersection between the v15 updates and the patch.

vadosnaprimer commented 5 years ago

This issue seems to be getting all-in-one really. While manipulating this on windows I discovered git recorded executable flags for 4 files. For future reference, this should be fought by running git config core.fileMode false.

vadosnaprimer commented 5 years ago

Branch v15_235273a committed (named after tag + upstream tip). Feel free to tweak it as needed. This approach might make tasvideos branch kinda obsolete but whatever.

I was unsure what to do with readme commits. Maybe the best solution will be squashing them and merging to v15*. But it's easy to do either way now that it's in a separate branch.

clementgallet commented 5 years ago

I updated branch v15_235273a so now it works fine. I managed to fix the issue I was having by updating gcc (I often forget that running a Debian testing means that you may get broken packages). So @slamotas you could do tests on this version.

slamotas commented 5 years ago

Looks good! My Windows 98 setup loads fine now. I tried running my test TAS from v14 and it looks like I'll have to do a little resyncing, but everything is still deterministic.

vadosnaprimer commented 4 years ago

PCem moved to git! No conversion needed anymore. But the tree is now different, so we'd have to fork from scratch and reapply the patches.

https://github.com/sarah-walker-pcem/pcem

The reason: https://pcem-emulator.co.uk/phpBB3/viewtopic.php?f=3&t=3351

We can rename this repo to pcem-old.

clementgallet commented 4 years ago

Nice. Could you do the fork and give me access? For some reason we can't compile this anymore (#4), but I could compile the new github pcem repo.

vadosnaprimer commented 4 years ago

Done and invited.