orbitersim / orbiter

Open-source repository of Orbiter Space Flight Simulator
MIT License
1.62k stars 220 forks source link

Revert Orbiter to use Fetch mechanism instead of submodules #474

Closed DarkWanderer closed 2 months ago

DarkWanderer commented 4 months ago

Reasons

Change

Use Fetch_MakeAvailable instead of .gitmodules to avoid unnecessary complexity. Lua modules are out of scope for now

TheGondos commented 4 months ago

I plead guilty for adding plain source external dependencies, because adding submodules in pull requests seems a mess to synchronize between different repositories, especially when you do modifications to the original code.

If there's a simple way to do it then I'd be happy to hear it.

DarkWanderer commented 4 months ago

I plead guilty for adding plain source external dependencies, because adding submodules in pull requests seems a mess to synchronize between different repositories, especially when you do modifications to the original code.

If there's a simple way to do it then I'd be happy to hear it.

What is the use case for the changes in original code? Can it be covered by 'glue' code instead?

TheGondos commented 4 months ago

The ldoc module is generating CSS that looks terrible in htmlhelp, I patched some hard coded CSS values to work around it. There's probably a cleaner way to do it but I was running out of steam doing the documentation work and forgot to investigate it further.

DarkWanderer commented 3 months ago

That's not the best way to handle it generally - as it will make updating the modified library a sisyphean task later. It seems like time to fix that exceeds the time I have currently though, so I have limited the scope of the PR to elimination of submodules. Ready for review

TheGondos commented 3 months ago

A couple of remarks :

Build All failed.

DarkWanderer commented 3 months ago

I don't know if the Lua github is the official one but it's not listing the latest 5.1.5 release. I tried to use

Yes, the GitHub repo doesn't have 5.1.5 for some reason. Do we need that version specifically? Can we live with 5.1.1?

If I comment the #if(ORBITER_MAKE_DOC) to generate the Lua doc, the build fails :

Sounds like we need documents build enabled in CI... I'll take a look

TheGondos commented 3 months ago

Can we live with 5.1.1?

Yes it should be good enough for the time being

Regarding the documentation, it's not producing a pdf but still using htmlhelp to generate a .chm file.

DarkWanderer commented 3 months ago

Prereq to continue working on this: #480 ✅

DarkWanderer commented 2 months ago

This PR is ready for merge