Use submodules for dependencies

[qaisjp]

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 --recursive or git pull --recursive --shallow-submodules (the latter doing --depth=1 on 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.

[qaisjp]

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 status and diff display 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: so git status shows 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, use git 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, use git clone --recursive. This will pull mtasa-blue along with any dependencies.
• ❗️If you have accidentally done git clone or have an existing repository, use git submodule update --init to checkout submodules.

Bringing your local repository up to date

• ❗️Generally do not use git pull, use git pull --recurse-submodules.
• ❗️If there is a submodule update and you have used git pull, do git submodule update --init to checkout the correct submodule trees. (This is the "forgot command" mentioned in the list of caveats below)
• ❗️git submodule sync --recursive is 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
  

  • .gitmodules should now look like this:

  cat .gitmodules
  [submodule "vendor/curl"]
    path = vendor/curl
    url = https://github.com/multitheftauto/curl
  

  • If you cd vendor/curl, then do git 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-blue repository and not in the curl repo (cd ../..)
• A regular git push can 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/curl
• git fetch
• git log --oneline origin/master -10 and verify the updates you want are there.
• git checkout origin/master to update our submodule to the latest version
  • swap origin/master to a valid tag or hash, if necessary
  • for forked dependencies, origin/master is 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)
• cd ../../ to get to the root of the parent mtasa-blue repository
• 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 push can 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.

[qaisjp]

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 svn to git we 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 build mtasa-net) and delivery infrastructure (to deliver installers).
  • This requires some syncing trickery if we need to rebuild mtasa-net for every push, as mtasa-net builds need have to happen remotely. I am not sure if mtasa-net is currently rebuilt for every mtasa-blue build. It is probably not required (as only mtasa-net build artifacts are needed).
  • Generally an mtasa-net build would happen asynchronously to changes on mtasa-blue and a successful mtasa-net build could then trigger an mtasa-blue build on Azure Pipelines.
• 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.

[qaisjp]

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.)

[qaisjp]

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.)

[sbx320]

I think that offset was chosen back when we moved to Github, to ensure that we get properly incrementing build revisions.

[qaisjp]

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.