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

Please do not make nested go.mod modules #2271

Open
litecomb opened this issue Nov 2, 2024 · 3 comments
Open

Please do not make nested go.mod modules #2271

litecomb opened this issue Nov 2, 2024 · 3 comments

Comments

@litecomb
Copy link

litecomb commented Nov 2, 2024

Rookies have been active, creating unnecessary nested go modules. These are notoriously hard to get right, cause part of the project depend on entirely different versions of itself, and even though creating nested modules appears (on the surface only) to cause no problem, the opposite is true. Whats worse, once the problem is spotted, deleting the nested modules is near impossible.

The attached patch in gist attempts to clean the mess caused by other contributors:

https://gist.github.com/litecomb/ea736ccb1573ee468f40518065f59329

Note that this assumes that beyond that point the btcec package will share its version with the rest of the repo. I have no idea if that is desired or necessary at all.

Unfortunately since deleting the nested modules is near impossible, a full fork of the repo may be necessary to properly flush external sites such as pkg.go.dev and the infamous google mitm source code proxy which backs go get.

@guggero
Copy link
Collaborator

guggero commented Nov 4, 2024

I agree that having nested modules that are more than one level deep (e.g. btcd -> btcutil -> psbt) isn't ideal.
And there's an old PR that tries to clean that up somewhat, you're welcome to review that.

But not having any submodules isn't an option either. Do you really want to pull in all of btcd into your project just because you want to use the elliptic curve library (btcec) or want to be able to create a BTC address (btcutil) in Golang?
Moving those libraries into other repositories doesn't really help a lot either, as it makes development way harder (dependencies across multiple projects). In fact, btcutil used to be its own project but was moved here to avoid exactly those development dependency issues.

@litecomb
Copy link
Author

litecomb commented Nov 4, 2024

I appreciate your perspective on the use of multiple go.mod files, but I’d like to share why I believe that having a single go.mod at the root is the more sensible choice.

First and foremost, the developer experience should be the priority in any project. When working with a single go.mod, the workflow becomes straightforward: you can clone the repository, develop your code, build, test, and commit without the confusion that comes from managing nested modules. This approach eliminates unnecessary steps, like having to delete extra go.mod files created by others, which many developers often have to do.

In contrast, when multiple go.mod files are present, it can lead to significant misunderstandings. For instance, a developer might start coding without realizing that their changes won't take effect, as they may be inadvertently referencing outdated dependencies. This can lead to split pull requests and the creation of additional go.mod files, complicating the project further.

By having a single go.mod, both new and experienced developers can converge on a simpler process:

  1. Clone the repo.
  2. Develop the necessary features.
  3. Build, test, and commit their changes.
  4. Move on without worrying about module complexities.

Additionally, maintaining one go.mod file simplifies the release process. It ensures that there’s a single source for dependency management, reducing the risk of mismatched versions and potential security vulnerabilities. This makes it easier for release managers, who won’t need to worry about updating multiple go.mod and go.sum files or potentially overlooking one.

As for the argument regarding importing nested modules, in many real-world scenarios, dependencies are interconnected. Thus, the expectation that using nested modules improves modularity often doesn’t hold up. Most dependencies will still pull in various parts of the project, regardless of the nested structure.

Finally, while you mentioned that certain modules, like btcutil, were moved to simplify dependency issues, it's clear that the underlying problems can persist if it's go.mod file is not properly managed. Keeping everything within a single go.mod file eliminates this potential for oversight.

In summary, I believe that adopting a single go.mod file enhances collaboration, simplifies the development process, and reduces the risks associated with dependency management. Thank you for considering this perspective!

If there is anything else I can help with please let me know

@guggero
Copy link
Collaborator

guggero commented Nov 5, 2024

Oh we definitely understand the developer pain that comes with submodules. We've been using them in all our projects and it's definitely not easy.

But, especially with btcd, many of the packages within some projects are mostly seen as a library first.
So being able to pull in just those library packages through the use of submodules is important.
Especially if the final build size is a concern as is with mobile and WASM builds.

So I'm definitely not saying your concerns aren't valid. I'm just saying that the chances of this project changing its approach are very slim in my opinion.
We have many consumers (especially on the btcec library) and almost no reviewers. So a massive code change as you're suggesting will likely not get much review attention (just take a look at my PR linked above that actually fixes issues).

But of course you're welcome to try, I'd just recommend to manage expectations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants