Thanks for opening this PR! We'll review it soon.

I'm a bot that checks for some common problems. I'll update this comment with anything I find.

Here are some minor issues, that might need fixing:

• Head's up, libraries/Translation/ is a subtree. Please ensure you're pushing these changes upstream.
• Head's up, libraries/ZMusic/ is a subtree. Please ensure you're pushing these changes upstream.
• Head's up, libraries/ZWidget/ is a subtree. Please ensure you're pushing these changes upstream.
• Head's up, libraries/ZVulkan/ is a subtree. Please ensure you're pushing these changes upstream.

Not a good idea, subtrees were added due to the problems submodules were causing. Besides submodules were very limited and i ended up removing the widepix submodule even before than gzdoom and that created a duplicate folder xD. Y wenas.

subtrees were added due to the problems submodules were causing

Does that imply that there are other subtrees in the repo that we don't know about? The ones that are being removed are fairly new, and were replacing copypasted code (not submodules).

Besides submodules were very limited

Fair. They both have their limitations. Seeing as we pull in changes frequently from these repo, and the code basically only moves in one direction, these are perfect candidates for submodules.

removing the widepix submodule

That's a good example of what probably shouldn't be a submodule. Honestly, they should just be copypasted assets, since they basically never change and just get moved around.

The reason for this pr is that the subtrees are causing problems, and have branching limitation that is a bit of a blocker. I believe the benefits of using submodules well outway the annoyance of having to use git pull --recurse-submodules instead of just git pull

and hello :D

I'll be keeping an eye on this; I'm also a little weary of the ttf pr adding 850,000 lines because of external libraries :P

I mean the subtrees were added instead of submodules AFAIK. Just my two cents nobody asked for but submodules were a mess xD. BTW wasn't this discussed on discord? I think i remember reading about adding the subtrees mentioning submodules.

I mean the subtrees were added instead of submodules AFAIK

This is correct. I suggested submodules, but was told the man with veto power hated them, so I suggested subtrees as a compromise

BTW wasn't this discussed on discord

It was, but months ago. It was specifically discussed when adding zmusic to gzdoom. I was told that there was absolutely no way that submodules would be added. Now it's a different project, and submodules are a more appropriate tool, so I figured I'd pitch it again.

They both have their pros and cons, I'll edit this comment later with as many as I can think of.

GZDoom originally used submodules, but nobody liked them because they were a constant pain point when checking out or updating the repo. You had to remember to manually initialize the submodules (it doesn't happen automatically when cloning a repo), and know to update them (it doesn't happen automatically when rebasing/resetting/merging/switching branches/etc), and even if you did know to, it wasn't always a smooth experience. Like drfrag666 says, we switched from submodules to subtrees to avoid those problems, and I can't say I'd like to go back.

Pretty late but a lot of the extra work that submodules introduce can be addressed by adding a CMake option to automatically run the relevant git commands during building. I personally think this would be the best of both worlds.

(The diff's hard to check from the site so not sure if this branch already does this)

Looking at the commit history, there was exactly one submodule that was kept around for a couple months. That's really not enough time for people to learn how to work with anything. I feel the benefits of using the correct tool for a job, vastly outweighs any annoyance you might have from using submodules. Submodules are a well-established git feature used in countless projects. If you have trouble remembering to update them, I feel that is more of a tooling issue on the user's end. I'm pretty suspicious of any claim that a feature should outright not be used. That being said there are plenty of programmers that would not have any reservations about using submodules. I honestly think that this is one of those vocal minority things, where people are resistant to change. But I'm only one voice here, so ultimately this needs to be a group decision. I'm not too worried about it either way.

Pretty late but a lot of the extra work that submodules introduce can be addressed by adding a CMake option to automatically run the relevant git commands during building. I personally think this would be the best of both worlds.

(The diff's hard to check from the site so not sure if this branch already does this)

That's a great idea, and no it does not. I've never set up cmake to do that. What does this look like for windows users that only ever run cmake once to generate the vs project?

Looking at the commit history, there was exactly one submodule that was kept around for a couple months. That's really not enough time for people to learn how to work with anything.

Submodules aren't new. They've existed in Git for a long time so people have learned how they work, how to use them and the issues that arise. They're pretty widely maligned in general, not just in GZDoom's codebase.

Pretty late but a lot of the extra work that submodules introduce can be addressed by adding a CMake option to automatically run the relevant git commands during building.

CMake is the build system, which is orthogonal to Git being the version control system. CMake shouldn't handle VCS functionality like that, e.g. if I want to switch branches, I'd run git checkout, not some custom cmake command to do a checkout and update submodules. CMake wouldn't know what (if any) git submodule commands need to be run on its own, since a checkout may or may not need to update submodules depending on the checked out branch. Similar for updating a submodule target in a given branch. So you'd end up still needing to manually run things, but using less familiar custom cmake targets, instead of the documented git commands.

That's a great idea, and no it does not. I've never set up cmake to do that. What does this look like for windows users that only ever run cmake once to generate the vs project?

(for the record, i agree wtih kcat, i don't think we should switch to submodules + cmake either, but)

Visual Studio itself runs cmake on every compilation, so if it's part of the build step it should Just Work™

Submodules aren't new. They've existed in Git for a long time so people have learned how they work, how to use them and the issues that arise.

You missed the part where I said "Submodules are a well-established git feature used in countless projects."

What I am saying is that the gzdoom devs did not have time to get used to it. I know you use Linux. How many people have decided Linux is bad, because they used it for a tiny bit, and made up there mind that they don't want to change their workflow?

They're pretty widely maligned in general, not just in GZDoom's codebase.

I can only find a handful of rant-y blogposts and a couple forum threads where people say "submodules are bad", and other people promptly tell them it's a skill issue. Submodules are not for splitting up a monorepo, they're for managing dependancies where you are not modifying the code. This is precisely our use-case

CMake is the build system, which is orthogonal to Git being the version control system. CMake shouldn't handle VCS functionality like that, e.g. if I want to switch branches, I'd run git checkout, not some custom cmake command to do a checkout and update submodules. CMake wouldn't know what (if any) git submodule commands need to be run on its own, since a checkout may or may not need to update submodules depending on the checked out branch. Similar for updating a submodule target in a given branch. So you'd end up still needing to manually run things, but using less familiar custom cmake targets, instead of the documented git commands.

I mean, in my head it would just be a message "yo, your submodules are out of date". We already run plenty of git commands during the build process.

But again, I'm not too worried about things either way. What I see in the short discussion here is 3 devs saying "yep, this is a useful tool", and 3 other devs passionately arguing against it. Unless I'm missing something, the arguments against them boil down to "we don't like them". I'm not trying to be rude or dismissive, it's just that the only issues brought up are that remembering to update them is annoying. I'm pretty sure whatever git client you use can be configured to do that automagically, since I know the my system can.

• We currently have 2 PRs (Feature: use fonts generated from TTF Files #879, New Launcher Prototype #904) open that pull in millions of lines of code for a dependencies, they would be much better served as submodules.
• We currently have 4 components as subtrees, since they get updates sometimes.
  • We require a linear commit history, which forces rebases instead of merges. It's (afaik) impossible to rebase a a subtree update. Every time I attempt it, there are countless conflicts due to the rebase happening in the root of the repo, causing common files (like .gitignore) to have horrid conflicts, since they are literally different files.
  • Every time the subtrees require an update, they get squashed. This loses any file history, making it really hard to pinpoint where an issue arises , since bisecting is impossible (see: the recent zwidget update that causes ctd on windows). In addition to this, our branch protection rules need to be temporarily disabled every time a subtree gets merged, due to that squash.
  • We had to add a bot (that some people find annoying), to remind people to not touch the subtrees, since we get a lot of people doing exactly that. When asking a contributor what a better solution was, they said that we should just be using submodules instead.
• We currently have countless libraries that are copied wholesale, but are slowly diverging from upstream, since we have no way of updating them. I have not taken inventory of how many still maintained projects we just copied in, but one that comes to mind is asmjit. We are now using an ancient version, with no clear route to getting fixes in. I don't think anyone here really wants to put in the legwork to get that done.

If you have a solution for the above issues, I'm very open to discussing. I also find submodules annoying (I've used them a lot in professional contexts. I'm pretty used to the issues they can cause.), but in my opinion, they are the correct solution in this case.

What I am saying is that the gzdoom devs did not have time to get used to it.

Sure, but that's presuming the only time we've had to get used to them is with GZDoom. I remember there being a rather immediate negative response to GZDoom starting to use them, since some of us already had negative experiences with them in other projects, with the response being "let's just try it for a bit". We did that, with the expected problems cropping up, resulting in us ditching them.

I can only find a handful of rant-y blogposts and a couple forum threads where people say "submodules are bad", and other people promptly tell them it's a skill issue. Submodules are not for splitting up a monorepo, they're for managing dependancies where you are not modifying the code. This is precisely our use-case

Conceptually, the idea of submodules isn't bad. They do seem useful, which is why they were added and people started using them. But in practice, people find them difficult to use, being more trouble than they're worth, hence the rant-y blogposts. I wouldn't put much stock in the responses of "skill issue"/"get gud" since these aren't unseasoned developers that dislike like them.

I mean, in my head it would just be a message "yo, your submodules are out of date".

I don't think there's a way to detect submodules being "out of date". If it was easy to know when submodules need to be updated or to detect issues from them being set up wrong, I'd have expected them to improve, or at least a clearer understanding of how to properly manage them to emerge for projects that want to use them. Instead, we get features like subtrees as an alternative to submodules, suggesting that's just how they are and you need something different if you want to address the issues people have with them.

Sure, but that's presuming the only time we've had to get used to them is with GZDoom. I remember there being a rather immediate negative response to GZDoom starting to use them, since some of us already had negative experiences with them in other projects, with the response being "let's just try it for a bit". We did that, with the expected problems cropping up, resulting in us ditching them.

Sure, I'll give you that. Thing is, the "we" then is a fairly different group of people than the "we" now.

I wouldn't put much stock in the responses of "skill issue"/"get gud" since these aren't unseasoned developers that dislike like them.

That's a strange thing to say, since you're asking me to blindly trust that submodules are bad and should just not be used. I, as someone who has used them for most of my time as a developer, think they're fine.

I don't think there's a way to detect submodules being "out of date". If it was easy to know when submodules need to be updated or to detect issues from them being set up wrong, I'd have expected them to improve, or at least a clearer understanding of how to properly manage them to emerge for projects that want to use them. Instead, we get features like subtrees as an alternative to submodules, suggesting that's just how they are and you need something different if you want to address the issues people have with them.

This is why I have issues with discussions like this. You're just telling me you've never checked. It's git submodule status, which has been around since the day submodules were added as a mainline feature almost 20 years ago. It returns a list of submodules and their status. It's easily machine-readable, so it's appropriate for a script.

You have way more experience than me as a developer, and I trust you know what you're talking about. Just this specific topic, I'm having trouble taking what you're saying in good faith. The last reply you sent were two nuh-uh's followed by something false. I'm not really sure what to do with that.

If you're against submodules, what alternative do you suggest that fixes the issues I've outlined?

That's a strange thing to say, since you're asking me to blindly trust that submodules are bad and should just not be used. I, as someone who has used them for most of my time as a developer, think they're fine.

I'm not sure I'd say they're wholly "bad" per-se, or else they would've been removed or deprecated, just that they're more of a maintenance and usage burden than you may expect or want, such that subtrees are probably a better fit for more use-cases.

You have way more experience than me as a developer, and I trust you know what you're talking about. Just this specific topic, I'm having trouble taking what you're saying in good faith.

To be fair, I haven't used submodules that much. I have used them in a couple other projects though, and I do remember having issues where builds would just start failing because the submodules got out of sync, and it wasn't clear that that's what the issue was. I also remember a point when GZDoom had them, and I thought I was handling them correctly from my experience in past projects, but I would still get unexplained build errors and not know how to properly fix it. It could be that I personally was just "skill issue"ing it, but I also think it's fair to ask, if enough people are having trouble with them like this, how much is really the user's fault for not being good enough, vs it being a less-well-designed feature?

There's a couple other points worth bringing up. First, like FetchContent a submodule does require pulling the dependency from an online source to do the build. It's not done as part of cmake like FetchContent, but it is still something that needs to be done to build. If that project's online repo ever moves or goes away, the commits that rely on it can't build anymore without some fiddling to manually replace it.

Second, scripting anything through cmake is really only useful when running cmake. I can only speak for myself, but I don't actually run cmake directly that often. I'll use git to change branches or merge my local changes, but I'll run make or ninja to do the build and let the build re-run cmake only as needed (whenever CMakeLists.txt or other dependent file changes). I only really run cmake when restarting with a completely fresh build tree, so cmake won't always be able to check and notify on submodules being out of sync. Maybe there's a way to make the build itself check and report the submodule status (I think there may be, but it causes certain IDEs to report the target as always needing a rebuild, so not necessarily ideal), but it's easy for general status messages to get flooded out by other build messages.

Regarding your points:

We currently have 2 PRs (#879, #904) open that pull in millions of lines of code for a dependencies, they would be much better served as submodules.

Developers need to pull those lines regardless. Whether its part of the UZDoom repo, or with submodules, those sources need to get on the system for the build. Submodules just add an extra step to getting the UZDoom source tree into a buildable state.

We currently have 4 components as subtrees, since they get updates sometimes. [...]

For my part, I don't even bother with subtrees in my projects. For OpenAL Soft, I just pull in a raw copy of fmtlib, for example, and add_subdirectory it to use it. This also lets me make custom changes as required (name changes to avoid ODR violations/multiple definition conflicts if something else also uses fmtlib in the project, fixing the build to compile in a compatible manner, etc), something submodules doesn't allow. I get this isn't always relevant, and I can't say I have much experience with large repositories like UZDoom (aside from (G/U)ZDoom), but this is my perspective.

We currently have countless libraries that are copied wholesale, but are slowly diverging from upstream, since we have no way of updating them. I have not taken inventory of how many still maintained projects we just copied in, but one that comes to mind is asmjit. We are now using an ancient version, with no clear route to getting fixes in. I don't think anyone here really wants to put in the legwork to get that done.

Would submodules really change anything? If you can't update asmjit because it requires too many changes to the code using it, that would mean you equally couldn't update its submodule target to a newer version without that same work. Or do you mean we can't update asmjit because of custom changes that would need to be applied to an updated version, and there's not enough manpower to do it? In that case, that really highlights a previously mentioned issue with submodules; those changes couldn't have been made in the first place, making it harder or impossible to use.

I honestly don't know what the ideal solution would be. I don't think any programming language or development environment has licked the issue of dependency management, as everything comes with its own problems and trade-offs. Keeping local copies of dependencies helps ensure everything can be locally managed, that there's nothing extra beyond the development environment itself that developers need and there's no unnecessary reliance or trust put on external repositories that are out of our control.

also, submodules don't stop people from modifying them, it just excludes the modifications from the PR

though if i had to say one, i think the only thing that'd really benefit from submodules as opposed to subtrees, it would be the translations (but NOT zmusic/zwidget/zvulkan)

though if i had to say one, i think the only thing that'd really benefit from submodules as opposed to subtrees, it would be the translations (but NOT zmusic/zwidget/zvulkan)

that way the submodule can always just point at trunk, not at one particular commit