How to vendor scripts in pathogen repos?

[jameshadfield]

This issue was originally written as an overview of git subtree, but later repurposed into a discussion of how to vendor scripts, specifically choosing between git subtree and git subrepo. In the end, we settled on git subrepo noting that this is a small implementation detail that can vary by pathogen repo, and be changed in the future.

Original issue

This is a summary of how I used git subtree as part of #2. Note that different pipelines can choose different methods of vendoring scripts from this repo, but git subtree is particularly nice as it requires no knowledge of its existence from a user of the pipeline.

Helpful reading: Git Subtree basics

The script was added to this repo (nextstrain/ingest) from within nextstrain/hepatitisB using ingest as a subtree. Specifically, from the hepB repo:

# use a branch (in hepB)
git checkout -b 'vendored-scripts'

# add the ingest repo as a subtre, using the 'apply-geolocation-rules' branch
git remote add ingest-remote git@github.com:nextstrain/ingest.git
git subtree add --prefix ingest/vendored ingest-remote apply-geolocation-rules --squash
# Adds a merge commit with one parent the previous host repo HEAD commit,
#     and the other a squashed commit of the 'ingest' repo

# move the script to the subtree repo (ingest/vendored), modify the snakemake
# rules accordingly and commit changes (to the hepB repo)

# push the changes up to the subtree repo (ingest) on branch apply-geolocation-rules
git subtree push --prefix ingest/vendored ingest-remote apply-geolocation-rules
# The commit message was identical, but only the changes to ingest/vendored
# were part of the subtree commit (probably obvious!)

It was tested in monkeypox by pulling in (this branch of) the ingest repo as a subtree, and updating the transform rule accordingly.

Reflections

This approach is pretty straightforward but changing the branch of a subtree seems to pollute the git history a bit. An alternative approach would be to simply have a subtree of ingest at the main branch, push any changes to the subtree up to a branch of the subtree, merge that branch via GitHub (with code review etc), then pull down the changes once they're on the main branch (of the subtree repo).

Given a script with differences in multiple repos, the most straightforward may be to simply to create a to-be-vendored version of a script locally, copy it into each repo to test, and when you are satisfied create a PR in this ingest repo without using subtrees at all. Once it's in main, it is straightforward to pull it into each pathogen repo using git subtree pull ....

Comments / improvements welcome. At the very least this may give others a quick start guide!

[victorlin]

What's the use case of changing the branch of a subtree? Always pulling from nextstrain/ingest@main seems like a straightforward approach, and mimics how things work with other shared scripts in augur curate.

[joverlee521]

I briefly explored git-subrepo as a potential alternative to git subtree.

Some things that are nice compared to git subtree:

• It's obvious that a directory is a subrepo because it contains an auto-generated .gitrepo file
• The .gitrepo file contains info on the remote source that the current subrepo is using.
• Switching branches in the subrepo is painless and creates a single commit with message that summarizes the switch

I tested this out by creating a subrepo for nextstrain/ingest in my personal testing repo.
These two commits include the initial clone of the subrepo and a switch to the apply-geolocation-rules branch.

[jameshadfield]

What's the use case of changing the branch of a subtree? Always pulling from nextstrain/ingest@main seems like a straightforward approach, and mimics how things work with other shared scripts in augur curate.

One use case is when a script needs modifications and you want to push them, however I think my subsequently suggested way to do this outside of subtree (or subrepo) is better. Hopefully we don't have too many occurrences of these.

git-subrepo looks good too. I'll give it a try. It shares the very nice feature that a user of the pipeline (and oftentimes the developer) doesn't need to know anything about it!

[victorlin]

I tried this myself for the same apply-geolocation-rules script. See the following PRs:

• Copy apply-geolocation-rules from monkeypox repo #4
• Copy edits to apply-geolocation-rules from hepatitisB repo #5
• Use nextstrain/ingest (git subtree add) hbv#3
• Use centralized apply-geolocation-rules (git subtree pull) hbv#4

Notes:

• I did not use git subtree push. A summary of my approach can be found at nextstrain/hbv@c8c94af.
• I did not use --squash since it can be useful to see the commit messages from nextstrain/ingest in pathogen repos.

[victorlin]

Example PRs using git-subrepo:

• Use nextstrain/ingest hbv#5
• Use centralized apply-geolocation-rules hbv#6

[victorlin]

Thoughts on subtree vs. subrepo after the experiments above:

• subrepo does not provide a way to preserve commit messages from the other repo - it sees that as a problem rather than feature. This makes it comparable to using subtree with --squash. Between those two, subrepo has cleaner commit history:
  • subrepo makes 1 squash commit based on the history of the pathogen repo
  • subtree --squash makes 1 squash commit based on external history + 1 merge commit to bring it back into the pathogen repo's history
• subrepo requires extra installation for anyone who wants to set up usage of centralized scripts using subrepo clone or pull new changes using subrepo pull.
• With a pull-only approach, there is flexibility: individual pathogen repo maintainers can choose between subtree or subrepo. Of course, we can provide suggestions here.
• subrepo's README lists several benefits compared to the built-in git commands. It's not yet clear to me which of these could be downsides of sticking with subtree, but it seems most of them can be avoided by the pull-only approach.

[joverlee521]

We are going with git subtree, based on internal team discussion on Slack.

Added vendoring section to the README in 87ca1ed.

[victorlin]

From #3 (comment):

I did not use --squash since it can be useful to see the commit messages from nextstrain/ingest in pathogen repos.

I just realized that these commit messages aren't visible when looking at the log of a particular file, but they are in the blame. Examples:

• subtree log
• subtree blame
• subrepo log
• subrepo blame

Having the blame might be useful, at the cost of a more verbose git history. I think the only way to truly understand the value here is to use it in practice for a bit!

[joverlee521]

Oh, just figured out it's possible to see the commit history of a subrepo (without all of the git history pollution):

git subrepo fetch ingest/vendored
git log refs/subrepo/ingest/vendored/fetch 

You can also see commit history of a single file if you provide the filepath from the ingest repo:

git log refs/subrepo/ingest/vendored/fetch -- notify-slack

[victorlin]

I'm looking back at the Slack conversation and it seems like we were on the fence about subrepo vs. subtree and I chose one mostly to have a decision to move forward.

Since we've adopted git subrepo, we've ran into some issues such as #21, and recently @tsibley made a "bad suggestion" (which I don't think is that bad) to use git subtree. I think it can be reconsidered. If other people think so too, I'll make PRs to switch to git subtree in the pathogen repos, and we can see how it looks.

Things to consider:

• We've already spent some time with git subrepo, and it works well enough. git subtree might come with new hurdles that we haven't discovered yet.
• It's meant to be a temporarily solution, though external usage is likely with Add tutorials for ingest pipelines docs.nextstrain.org#179.
• In the end, the difference for external usage is just an extra installation step for git subrepo which is straightforward.

[victorlin]

Closing with status quo of git subrepo