Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ZMusic as a submodule #1354

Closed
nikitalita opened this issue Mar 25, 2021 · 7 comments
Closed

Add ZMusic as a submodule #1354

nikitalita opened this issue Mar 25, 2021 · 7 comments

Comments

@nikitalita
Copy link

Now that GZDoom is using submodules in its repo, I'd like to see ZMusic be added as a submodule (with an optional flag to turn it off and use a system/custom installation) to make this a bit easier to compile from a fresh clone.

@Cacodemon345
Copy link
Contributor

If that's to be done it should be done in such a way that it doesn't inconvenience users doing a simple git clone.

@madame-rachelle
Copy link
Collaborator

ZMusic is probably far from the only thing that should be added as a submodule. But not everyone likes submodules in git, and unfortunately, that is for good reason. I think it mostly comes down to determining when the pros outweigh the cons of doing so it is time to do it.

@nikitalita
Copy link
Author

ZMusic is probably far from the only thing that should be added as a submodule. But not everyone likes submodules in git, and unfortunately, that is for good reason. I think it mostly comes down to determining when the pros outweigh the cons of doing so it is time to do it.

Understandable, but I think in this case it's a good candidate. It's a build requirement, it's not available in most system repositories or vcpkg, and the CI already requires it be pinned to a certain version.

@madame-rachelle
Copy link
Collaborator

It's a build requirement, it's not available in most system repositories or vcpkg, and the CI already requires it be pinned to a certain version.

This is exactly why submodules are hated in git.

Some people easily understand you have to check out submodules automatically when cloning and fetching a repository. It's not obvious to a Git noob though, and when their compile doesn't work one can spend hours trying to figure out the problem before finally understanding that they need to use submodules to get it. At least for the widescreen project it was fairly harmless (you just don't get an optional archive), but imagine how much hassle it would be if you didn't know you needed a submodule for ZMusic or how to activate it.

I'm not against the idea, I'm just saying submodules come with baggage. And I really hate the way Git forces you to activate them in order to use them.

@nikitalita
Copy link
Author

Some people easily understand you have to check out submodules automatically when cloning and fetching a repository. It's not obvious to a Git noob though, and when their compile doesn't work one can spend hours trying to figure out the problem before finally understanding that they need to use submodules to get it. At least for the widescreen project it was fairly harmless (you just don't get an optional archive), but imagine how much hassle it would be if you didn't know you needed a submodule for ZMusic or how to activate it.

I mean... people already get confused by the ZMusic requirement. It's not mentioned in the wiki article that you have to build and install that first and then point to it for the GZDoom build via a CMake param. I had to look at the CI scripts to figure out how to do that. Even if the article were updated to tell you how to do that, I would think that saying "Clone this repo with git clone --recurse-submodules https://github.com/coelckers/gzdoom" or "Run git submodule update --init after cloning" in the README or something would be far less confusing and less of a hassle?

@madame-rachelle
Copy link
Collaborator

Again - I am not against your idea, I just don't think it's the perfect "silver bullet". And because of that, I think it should be approached more carefully.

Someone had the idea of remaking the CMake project completely, and I would prefer that to be done first, before we decide what to submodule and what not to, in terms of libraries. The system we have right now is not good, I know that, but it's better to make a good decision when you have all the cards on the table.

@Blzut3
Copy link
Member

Blzut3 commented May 31, 2021

Done in pull request #1401.

@coelckers coelckers closed this as not planned Won't fix, can't repro, duplicate, stale May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants