Provide source tarball reproducibility guidance, add test to detect breaks

[dagood]

In 1.26.0-1, we changed our infrastructure to add the MICROSOFT_REVISION file to the go directory just before running distpack. We've been doing this with a fake VERSION file in the microsoft/main branch for a while, but 1.26 is the first time this is happening in a release branch.

Our project doesn't make it clear if the output source tarball is intended to be reproducible, and how to reproduce it. The file copy step doesn't happen within distpack, it's in our eng/_util module, so it's not necessarily obvious.

In release branches up until 1.26.0-1, all changes come from the patch files. 1.26.0-1 changes that, so anyone who has that expectation will be broken. A script that keeps the Microsoft build of Go up to date in Dalec has an automated process with that expectation.

---

I think that it would make sense for us to at least:

• Create a reproducibility test for the source tarball that doesn't involve running eng/_util.
  • At this point, breaks would be fine, but they give us an opportunity to let partners know or consider alternatives to the break.
• Have clear guidance on how to reproduce the source tarball.
  • I think that ideally, the test code is clear enough that it itself can be the bulk of the guidance.

We need to consider:

• Should we decide to not alter the source tree using eng/_util?
  • Copying in MICROSOFT_REVISION using an automatically maintained patch file would limit the ways that the source tarball can be changed, making it easier to reproduce. But: that's a decent chunk of infra that has to play nice with the otherwise manually maintained patch files--and play nice with the maintainers themselves.
  • It's much simpler for us to maintain the copy in eng/_util.
• Should we move the MICROSOFT_REVISION file into the patches/ directory?
  • Maybe patches/insert/ to treat it as something like a file insertion system we could use for other things?
  • The idea is that the file would be more obvious, leading to the question "how do I treat this file like a patch?" However, if we provide easy to find repro guidance, this move doesn't seem justified.

[cpuguy83]

Thanks for opening.
Definitely don't want to put undue burden on the team for this.

As long as we can check if/when there are discrepancies what changed in a human-friendly format this would be good enough.

In this particular case I did have to special case MICROSOFT_REVISION by checking if its in the source tar and if it is then we copy it in on our side before distpack so as to make sure it works with older versions as well.
I suppose this does make any sort of similar future changes difficult as well but manageable, at least for now.

---

One suggestion for automating this is to do something like a go:generate to automatically generate a patch file to include.
Changes to the file requires go generate (or other automation) on commit with CI to validate that it happened.

[dagood]

I suppose this does make any sort of similar future changes difficult as well but manageable, at least for now.

To make sure I understand, do you mean difficult as in "each affected file would need a similar handful of lines of workaround code", or is there some steeper difficulty you have in mind that would come up?

---

Something else I'm wondering is if at some point, this kind of add/replacement might be considered to simply go too far. Let's say we change how we update src/vendor--instead of a patch file, we copy it in from somewhere else. In theory, a special case on your side would work: overwrite the vendor dir in the upstream Go repo with the one from our source tarball. This seems like it might be on the border line: src/vendor is machine-generated and reproducible from the go.mod, but it does contain a lot of code. (Maybe that would be a better special case, re-vendor from scratch?)

Modifying random go files in the stdlib seems to me like it might cross the line because there's no easily associated explanation of the code change. But... in eng/_util somewhere, we would have an explanation, and our reproducibility instructions would point in the right direction.

I guess what I'm getting at is that if we don't set the line at "no changes that aren't concrete in a patch file", I'm not sure where to set the next line.

---

For auto-updating a patch file, currently the infra that updates MICROSOFT_REVISION is at go-infra, so the main change would be creating/updating the patch file around there. The patch file could take advantage of a patch number command to avoid some conflict with manual maintenance.

Then, yeah, adding a go generate + go test combo for manual maintenance would also make sense.

[cpuguy83]

To make sure I understand, do you mean difficult as in "each affected file would need a similar handful of lines of workaround code", or is there some steeper difficulty you have in mind that would come up?

Yes, just special casing each file.

I think it would all be fine if we just use the eng/_util tool to reproduce the tar here.

[dagood]

Hmm, using eng/_util requires Go. I see now that that particular script does already require it, but I wonder if we should aim to avoid a Go dependency. (Of course you already need a Go to build the next Go, but maybe someone has specific infra requirements between "setup" and "build" phases, etc.)