MM-21722 - Repository synchronization tool

[tasdomas]

Summary

This PR adds a proof-of-concept implementation of a tool for synchronizing mattermost plugin repositories with the template repository (mattermost-plugin-starter-template).

Ticket Link

See https://mattermost.atlassian.net/browse/MM-21722

[tasdomas]

Running against mattermost-plugin-demo:

➜  mattermost-plugin-starter-template git:(sync) go run ./build/sync/main.go ./build/sync/plan.yml ../mattermost-plugin-demo
FAILED  .circleci/config.yml: check failed, file "/home/d/src/mattermost-plugin-demo/.circleci/config.yml" has been altered
UPDATED .editorconfig
FAILED  Makefile: check failed, file "/home/d/src/mattermost-plugin-demo/Makefile" has been altered
UPDATED build
FAILED  go.mod: check failed, file "/home/d/src/mattermost-plugin-demo/go.mod" has been altered
FAILED  public/hello.html: check failed, file "/home/d/src/mattermost-plugin-demo/public/hello.html" has been altered
FAILED  server/configuration.go: check failed, file "/home/d/src/mattermost-plugin-demo/server/configuration.go" has been altered
FAILED  server/plugin.go: check failed, file "/home/d/src/mattermost-plugin-demo/server/plugin.go" has been altered
FAILED  server/plugin_test.go: check failed, path "server/plugin_test.go" does not exist
FAILED  webapp/.babelrc: check failed, path "webapp/.babelrc" does not exist
UPDATED webapp/.eslintrc.json
FAILED  webapp/i18n/en.json: check failed, file "/home/d/src/mattermost-plugin-demo/webapp/i18n/en.json" has been altered
FAILED  webapp/package.json: check failed, file "/home/d/src/mattermost-plugin-demo/webapp/package.json" has been altered
FAILED  webapp/src/index.js: check failed, file "/home/d/src/mattermost-plugin-demo/webapp/src/index.js" has been altered
FAILED  webapp/tsconfig.json: check failed, path "webapp/tsconfig.json" does not exist
FAILED  webapp/webpack.config.js: check failed, file "/home/d/src/mattermost-plugin-demo/webapp/webpack.config.js" has been altered

[tasdomas]

Followups for this:

• implement format-specific merge actions (e.g. for go.mod)
• implement more intelligent non-existant file handling (a file could have been deleted on the target repository or it may have never been part of it, if it was introduced on the template repository recently)
• add a make sync target to checkout the template repository and run sync against the plugin
• investigate update dependencies (updating ./build may require updating go.mod)
• store which commit of the template repository a plugin has been sync'ed with

To make a long story short - proper, developer-friendly synchronization is complicated.

[mattermod]

This issue has been automatically labelled "stale" because it hasn't had recent activity. A core team member will check in on the status of the PR to help with questions. Thank you for your contribution!

/cc @jasonblais @hanzei

[mickmister]

Hi @tasdomas, thanks for all of the great work in this PR. Do you plan on continuing the project? If not, I would like to continue the work in a separate PR.

[tasdomas]

@mickmister - sorry for the late reply. I was sort of expecting reviews for the PR.

Give me a day or so to at least clean up the PR and address the existing comments. Then you can take over.

[mickmister]

@mickmister - sorry for the late reply. I was sort of expecting reviews for the PR.

@tasdomas No problem at all. I wrongly assumed you did not intend to continue with the PR, but looking at the history it is absolutely due for reviews. There is a ton of work here that has not been reviewed for quite some time. It is up to you whether you choose to continue with this PR. I can give this a review sometime this week, please let me know what you think.

Thank you for all you've done here. This is an unprecedented tool that will make a huge difference.

[tasdomas]

@mickmister - I've updated my pull request, addressing most of the review comments.

I wouldn't mind continuing work on this feature, but my time is limited and I'm worried that the PR is already large and would need to grow even more if the goal is to introduce a production-ready feature with one pull request. It seems to me that either a feature branch or adding improvements in follow-up PRs would work better for this.

[tasdomas]

@mickmister - thanks for the review!

The config-driven approach was mainly inspired by past experience - it's usually easier to edit config files than code.

[mickmister]

@tasdomas Of the followup tasks you mentioned:

• implement format-specific merge actions (e.g. for go.mod)
• implement more intelligent non-existant file handling (a file could have been deleted on the target repository or it may have never been part of it, if it was introduced on the template repository recently)
• add a make sync target to checkout the template repository and run sync against the plugin
• investigate update dependencies (updating ./build may require updating go.mod)
• store which commit of the template repository a plugin has been sync'ed with

I think syncing dependencies is not something we would expect the tool to do. Each plugin has varying dependencies on purpose. For example, a plugin that does not require new functionality from mattermost-server we will compile it with the earliest version of server-compatible code, so we can make it as backwards compatible as possible. We do dependency updates fairly manually anyway.

I would say items one, two and three seem very practical.

For the first item, keeping track of config/source files would be very useful, even if just for analysis. An easily accessible git diff of the two files would be very useful to start out.

For item two, the scenario of "it was introduced on the template repository recently" is definitely the most common out of them. If we want to add a new config file and propagate it, I think your code already supports this. It's not common that a repo would have deleted a file given from the template.

A file that gets updated often is the Makefile for the repo. What's challenging is that no Makefiles will remain in sync, because like the dependencies, they have things specific to the repo that won't be used in other repos. If we want to add new commands to the Makefile for all repos, what steps do you think we would take, possibly with future improvements of the tool with that in mind?

Can you provide some command line usage of the program?

[tasdomas]

@mickmister , for the Makefile, I'd probably say that the best approach would include splitting the makefile into a base part, which would not be modified in forks and a Makefile.local, which would be included in the base Makefile and contain fork-specific make commands. In such a setup the sync tool could update the Makefile.

To run the sync tool, in the starter template repository root run

go run ./build/sync/main.go ./build/sync/plan.yml [path to plugin repository]

[tasdomas]

@mickmister - I've also added some documentation for the plan.yml file format.

[mickmister]

@tasdomas For the point store which commit of the template repository a plugin has been sync'ed with, how do you think we would distribute this state to other teammates? Some sort of revision history in its own repo? I think this item is very useful in that we know which commit be left off at, so we don't have false positives for changes etc.

@mickmister - I've also added some documentation for the plan.yml file format.

Excellent! I see that you already have the command line usage in that file. Docs look good :+1:

[mickmister]

@lieut-data Do you have any comments on this piece regarding keeping Makefiles in sync across the plugin repos? I don't know enough about the details of how Makefiles work to make an opinion on this:

for the Makefile, I'd probably say that the best approach would include splitting the makefile into a base part, which would not be modified in forks and a Makefile.local, which would be included in the base Makefile and contain fork-specific make commands. In such a setup the sync tool could update the Makefile.

[tasdomas]

@tasdomas For the point store which commit of the template repository a plugin has been sync'ed with, how do you think we would distribute this state to other teammates? Some sort of revision history in its own repo? I think this item is very useful in that we know which commit be left off at, so we don't have false positives for changes etc.

I was thinking of something like a dotfile (e.g. .starter-template-rev) to store the hash of the template repository.

[lieut-data]

the best approach would include splitting the makefile into a base part, which would not be modified in forks

@mickmister & @tasdomas, fully agree with this approach, though I note we currently have this approach in build/custom.mk where we've started locating things specific to the plugin in question (e.g. make mocks). I'd love to be able to just maintain the known hashes of the historical Makefile and be able to replace them wholesale.

[hanzei]

@tasdomas I would love to hear you thoughts on my comment about the schema changes in plan.yml. Once these discussions are resolved, I'm happy to merge the PR and we can start iterating on the exact content of plan.yml.

[tasdomas]

@Ben, I'm currently away on vacation. The suggestions look sensible, but I would like to take a closer look start of next week.

[hanzei]

@tasdomas Enjoy your vacation. Looking forward to your feedback.

[tasdomas]

@hanzei, @mickmister, @jfrerich - I've updated the PR with your suggestions.

Also, I've noticed that the root go.mod file references go 1.12, but ci is using 1.14.

[mickmister]

Would it be possible to support an exclude field in plan.yml near paths, in order to exclude certain files/folders? build/sync for example should only be in the starter-template, and not in the other plugin repos. So when the build folder is sync'd, the build/sync folder comes along too.

[tasdomas]

Would it be possible to support an exclude field in plan.yml near paths, in order to exclude certain files/folders? build/sync for example should only be in the starter-template, and not in the other plugin repos. So when the build folder is sync'd, the build/sync folder comes along too.

Sure. I'd like to handle that in a follow-up though.

[mickmister]

@tasdomas Do you mind putting the go.mod file in the root dir of the build folder? I think we want to keep all the build deps in one file. Other than that, I think this is good to merge as I'm making followup tickets for the other portions.

Repo sync tool: Support excludes field in actions #121 Repo sync tool: Partial equality checks/merges #122 Repo sync tool: Script to check if a target repo needs to be sync'd #123

@hanzei What do you think about merging this, given these followup tickets?

[tasdomas]

@mickmister - I've updated the PR.

[hanzei]

@mickmister Absolutely. Let's get this merged.

[tasdomas]

@hanzei, @mickmister, @ben, @jfrerich thank you for reviewing this!

Time for :champagne: :)

[hanzei]

Heads up: I've posted some follow up thoughts in https://community-release.mattermost.com/core/pl/tkyb38n9hib3jby3t1g64w1xia