mtasa-blue
mtasa-blue copied to clipboard
Use submodules for dependencies
I've briefly discussed this on the development chat and I've created this issue so that we can properly evaluate whether we want to use submodules. Please share your thoughts.
Is your feature request related to a problem? Please describe. Updating and verifying vendor code is fairly manual work.
Describe the solution you'd like For "vendored" code we haven't modified we can use git submodules.
This would make it easier to update and verify dependencies as we no longer need to fiddle about with copying code and figuring out which files to keep. We would just need to update the submodule reference.
Some things to be aware of:
- We need to remind people to use
git pull --recursiveorgit pull --recursive --shallow-submodules(the latter doing--depth=1on submodules) and there are a couple other caveats - Most importantly, it must work with the build server (uses svn)
Additional context
- @sbx320 highlighted a potential problem with clone times
- @patrikjuvonen highlighted that we can fork dependencies that we modify and then submodule our forks
- This is a really good article about submodules - if you are not fluent with submodules I recommend you to download the repo in that article and follow along.
This is a cut down version of the guide above and the Git book. The below information is intended to be added to a developer guide.
This content is not yet validated against mtasa-blue, it is content converted from output of a bare-ish test repository (test and test-2).
Important things to remember
- Every time you add a submodule, change its remote’s URL, or change the referenced commit for it, you demand a manual update by every collaborator.
- Forgetting this explicit update can result in silent regressions of the submodule’s referenced commit.
- Commands such as
statusanddiffdisplay precious little info about submodules by default. - Because lifecycles are separate, updating a submodule inside its container project requires two commits and two pushes (one in the dependency project, and one in mtasa-blue)
- Submodule heads are generally detached, so any local update requires various preparatory actions to avoid creating a lost commit. Do not perform local submodule updates, ever.
- Removing a submodule requires several commands and tweaks, some of which are manual and unassisted.
Recommended pre-configuration
git config --global status.submoduleSummary true: sogit statusshows submodule reference changes (if necessary)git config --global diff.submodule log: shows clearer container diffs when submodule references change
Other recommendations:
git config --global pull.rebase true: ensures that pulling does not cause a merge commit (you do not always want to use rebase, but more often than not, you do. in those special cases, usegit pull --no-rebase.)git config --global fetch.recurseSubmodules on-demand: automatically fetch updates to submodule references when necessary. note: this does not auto-update submodules. (This setting is a default in Git 1.7.5. It is encouraged to keep your version of Git up-to-date.)
Working alongside submodules (changes to the current workflow)
Cloning mtasa-blue
- ❗️Do not use
git clone, usegit clone --recursive. This will pullmtasa-bluealong with any dependencies. - ❗️If you have accidentally done
git cloneor have an existing repository, usegit submodule update --initto checkout submodules.
Bringing your local repository up to date
- ❗️Generally do not use
git pull, usegit pull --recurse-submodules. - ❗️If there is a submodule update and you have used
git pull, dogit submodule update --initto checkout the correct submodule trees. (This is the "forgot command" mentioned in the list of caveats below) - ❗️
git submodule sync --recursiveis sometimes needed to update submodules' remote URL configuration setting to the value specified in .gitmodules. This is necessary if we fork a dependency which was previously unforked. (❓do you need to re-update after syncing)
Caveats if you do git pull:
- There is no problem if submodules have not been touched. This is likely as dependency updates are infrequent.
- Older versions of Git will also cause these submodules to not actually be updated. Make sure you have followed the recommended configuration if you do not want to update your version of Git.
- This will not affect the repository state if you push back (❓needs verifying).
- ❗️This may affect the codebase if you push back code that relies on an outdated submodule
- ❕You might not notice it, as your local copy of the submodule will contain an old version of the code.
- On the current version of Git (v2.20.1) you still need to the "forgot command" above to bring in new submodules. This is obvious as there will not be any code checked out. New dependencies are infrequent so this is not a common problem.
BUGS Using --recurse-submodules can only fetch new commits in already checked out submodules right now. When e.g. upstream added a new submodule in the just fetched commits of the superproject the submodule itself can not be fetched, making it impossible to check out that submodule later without having to do a fetch again. This is expected to be fixed in a future Git version.
Working with submodules directly
Adding a dependency
git submodule add https://github.com/multitheftauto/curl vendor/curl- This added some settings in our local configuration (
cat .git/config)
[submodule "vendor/curl"] url = https://github.com/multitheftauto/curl active = true- This also staged two files, see
git status:
new file: .gitmodules new file: vendor/curl.gitmodulesshould now look like this:
cat .gitmodules [submodule "vendor/curl"] path = vendor/curl url = https://github.com/multitheftauto/curl- If you
cd vendor/curl, then dogit status, you will notice that you are in a different repository — this is the curl repository, not mtasa-blue. Remember to never modify a submodule repository unless you know what you are doing. - Make sure you are back in the
mtasa-bluerepository and not in thecurlrepo (cd ../..)
- This added some settings in our local configuration (
- A regular
git pushcan be used to push all these changes up.
Updating a dependency
This section only covers updating mtasa-blue to a new commit of a remote dependency. It does not cover updating a forked dependency.
We follow the below instructions:
Therefore, I recommend splitting the process manually: first git fetch to get all new data from the remote in local cache, then log to verify what you have and checkout on the desired SHA1.
cd vendor/curlgit fetchgit log --oneline origin/master -10and verify the updates you want are there.git checkout origin/masterto update our submodule to the latest version- swap
origin/masterto a valid tag or hash, if necessary - for forked dependencies,
origin/masteris usually all you will need. In some obscure cases you may need a hash. - for unforked dependencies, stick to tags (we only ever want released versions of dependencies, not bleeding edge versions)
- swap
cd ../../to get to the root of the parentmtasa-bluerepository- Observe
git status:modified: vendor/curl (new commits) Submodules changed but not updated: ... - Commit your changes
git commit -am "Update curl to v3".- Forked dependencies will still have a version number, even if the fork diverges from the official repository. Make sure to include that version number.
- A regular
git pushcan be used to push all these changes up.
Removing a dependency
This rarely happens, and it's complicated!
Refer to the relevant section "Permanently removing a submodule" in "Mastering Git submodules".
What about submodule merge conflicts?
From "Merging Submodule Changes", 7.11 - Git Tools - Submodules:
If you change a submodule reference at the same time as someone else, you may run into some problems. That is, if the submodule histories have diverged and are committed to diverging branches in a superproject, it may take a bit of effort for you to fix.
If one of the commits is a direct ancestor of the other (a fast-forward merge), then Git will simply choose the latter for the merge, so that works fine.
Because submodules in mtasa-blue will (should) only contain consecutive commits on the master branch, you theoretically should never have to deal with this problem. Hopefully this problem never happens to you.
Dependency on build infrastructure
This issue requires us to upgrade the build system to use git instead of svn.
See output below (some lines converted to [...] for brevity):
➜ svn checkout https://github.com/qaisjp/test.git
A test.git/branches
[...]
A test.git/branches/test-draft-pr
A test.git/branches/test-draft-pr/README.md
A test.git/trunk
[...]
A test.git/trunk/.gitmodules
A test.git/trunk/vendor
Checked out revision 31.
➜ cd test.git/trunk/vendor
➜ ls
[nothing here]
The output for ls should look like this:
➜ ls
test-2
Upgrading the build infrastructure comes with a plethora of its own issues (in both senses of the word), and detailed discussion should probably be kept to a different issue. Some quick notes on this anyway:
- If we chose to move our build system from using
svntogitwe could look towards using CI/CD services like Azure Pipelines. With the use of secure tokens we can still use the existing build infrastructure (to buildmtasa-net) and delivery infrastructure (to deliver installers).- This requires some syncing trickery if we need to rebuild
mtasa-netfor every push, asmtasa-netbuilds need have to happen remotely. I am not sure ifmtasa-netis currently rebuilt for everymtasa-bluebuild. It is probably not required (as onlymtasa-netbuild artifacts are needed). - Generally an
mtasa-netbuild would happen asynchronously to changes onmtasa-blueand a successfulmtasa-netbuild could then trigger anmtasa-bluebuild on Azure Pipelines.
- This requires some syncing trickery if we need to rebuild
- We'd also need to generate our own revision numbers (
git rev-list --count HEADyields7017, which is incorrect)... or still rely onsvn checkoutto generate it for us.
Playing devil's advocate...
if there are so many caveats, it's probably not worth doing this.
I'm inclined to agree.
I would however say that this isn't really a problem for _new (prospective) contributors as excellent first-time developer documentation here isn't available anyway. If we used submodules we would be forced to to write a comprehensive developer guide. (Good!)
These developer guides can then be referred back to later, so it's useful for seasoned contributors as well.
The developer guide could also include git hooks that automate some of this.
it's still too complex!
Yeah... that's true. But it's cleaner and decreases cost of dependency maintenance. Just remember to use the recursive flag, you lazy git!
no, but really. it'll be a pain.
That's why I want your feedback. Please give me feedback! (Even if you're in support.)
We'd also need to generate our own revision numbers (git rev-list --count HEAD yields 7017, which is incorrect)... or still rely on svn checkout to generate it for us.
Actually, svn checkout currently generates this revision number: 11833. Which doesn't match up with MTA's r16616. So one of of these generated numbers also have another constant added to it.
(I recall @Cazomino05 saying this as well when we moved back to Git.)
I think that offset was chosen back when we moved to Github, to ensure that we get properly incrementing build revisions.
The build server uses git now (courtesy of ccw), and has been confirmed to pull in submodules! ~~I am not sure if it clones recursively, but according to the internet it does.~~ After testing, we can confirm that our build server does pull in submodules!
What's next?
This change seems to have approval from @patrikjuvonen & @saml1er (via 👍) and an implied approval by @botder (via milestoning).
We still need to evaluate clone times & get approval from @sbx320 (in response to their feedback).
Once clone times have been evaluated and confirmed to not be a blocker, I will be happy with the "approval level". Feedback or blessing from more of the dev team would be great.